Bug 1539742 - Notify of appends to the right document when parsing into an already-adopted node. r=bzbarsky
authorEmilio Cobos Álvarez <emilio@crisal.io>
Fri, 29 Mar 2019 09:55:10 +0000
changeset 466734 cf6cfe33476622c202fcbc34ef12f8ca1c039b8a
parent 466733 4055d335fa50d7b14e9875c4f843d09aa003fd1f
child 466735 387e71baf2efedfb90a287a378dc2ed45a1161cd
push id35780
push useropoprus@mozilla.com
push dateFri, 29 Mar 2019 21:53:01 +0000
treeherdermozilla-central@414f37afbe07 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbzbarsky
bugs1539742
milestone68.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1539742 - Notify of appends to the right document when parsing into an already-adopted node. r=bzbarsky nsContentSink used to decide that it was fine to not notify of silent appends to a document from the parser if the node was not on our document already. That's not ok, since if styling or layout have happened already on the document we're getting inserted into nobody notices them, which is wrong. Differential Revision: https://phabricator.services.mozilla.com/D25300
dom/base/nsContentSink.cpp
dom/base/nsContentSink.h
dom/html/nsHTMLContentSink.cpp
dom/xml/nsXMLContentSink.cpp
testing/web-platform/tests/xhtml/adopt-while-parsing-001-ref.html
testing/web-platform/tests/xhtml/adopt-while-parsing-001.html
testing/web-platform/tests/xhtml/adopt-while-parsing.xhtml
--- a/dom/base/nsContentSink.cpp
+++ b/dom/base/nsContentSink.cpp
@@ -94,17 +94,16 @@ NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN(
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mCSSLoader)
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mNodeInfoManager)
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mScriptLoader)
 NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END
 
 nsContentSink::nsContentSink()
     : mBackoffCount(0),
       mLastNotificationTime(0),
-      mBeganUpdate(0),
       mLayoutStarted(0),
       mDynamicLowerValue(0),
       mParsing(0),
       mDroppedTimer(0),
       mDeferredLayoutStart(0),
       mDeferredFlushTags(0),
       mIsDocumentObserver(0),
       mRunsToCompletion(0),
@@ -1206,27 +1205,23 @@ void nsContentSink::StartLayout(bool aIg
 
   // If the document we are loading has a reference or it is a
   // frameset document, disable the scroll bars on the views.
 
   mDocument->SetScrollToRef(mDocument->GetDocumentURI());
 }
 
 void nsContentSink::NotifyAppend(nsIContent* aContainer, uint32_t aStartIndex) {
-  if (aContainer->GetUncomposedDoc() != mDocument) {
-    // aContainer is not actually in our document anymore.... Just bail out of
-    // here; notifying on our document for this append would be wrong.
-    return;
-  }
-
   mInNotification++;
 
   {
     // Scope so we call EndUpdate before we decrease mInNotification
-    MOZ_AUTO_DOC_UPDATE(mDocument, !mBeganUpdate);
+    //
+    // Note that aContainer->OwnerDoc() may not be mDocument.
+    MOZ_AUTO_DOC_UPDATE(aContainer->OwnerDoc(), true);
     nsNodeUtils::ContentAppended(
         aContainer, aContainer->GetChildAt_Deprecated(aStartIndex));
     mLastNotificationTime = PR_Now();
   }
 
   mInNotification--;
 }
 
--- a/dom/base/nsContentSink.h
+++ b/dom/base/nsContentSink.h
@@ -283,18 +283,16 @@ class nsContentSink : public nsICSSLoade
 
   // Time of last notification
   // Note: mLastNotificationTime is only valid once mLayoutStarted is true.
   PRTime mLastNotificationTime;
 
   // Timer used for notification
   nsCOMPtr<nsITimer> mNotificationTimer;
 
-  // Have we already called BeginUpdate for this set of content changes?
-  uint8_t mBeganUpdate : 1;
   uint8_t mLayoutStarted : 1;
   uint8_t mDynamicLowerValue : 1;
   uint8_t mParsing : 1;
   uint8_t mDroppedTimer : 1;
   // If true, we deferred starting layout until sheets load
   uint8_t mDeferredLayoutStart : 1;
   // If true, we deferred notifications until sheets load
   uint8_t mDeferredFlushTags : 1;
--- a/dom/html/nsHTMLContentSink.cpp
+++ b/dom/html/nsHTMLContentSink.cpp
@@ -452,25 +452,23 @@ nsresult SinkContext::GrowStack() {
  * Flush all elements that have been seen so far such that
  * they are visible in the tree. Specifically, make sure
  * that they are all added to their respective parents.
  * Also, do notification at the top for all content that
  * has been newly added so that the frame tree is complete.
  */
 nsresult SinkContext::FlushTags() {
   mSink->mDeferredFlushTags = false;
-  bool oldBeganUpdate = mSink->mBeganUpdate;
   uint32_t oldUpdates = mSink->mUpdatesInNotification;
 
   ++(mSink->mInNotification);
   mSink->mUpdatesInNotification = 0;
   {
     // Scope so we call EndUpdate before we decrease mInNotification
     mozAutoDocUpdate updateBatch(mSink->mDocument, true);
-    mSink->mBeganUpdate = true;
 
     // Start from the base of the stack (growing downward) and do
     // a notification from the node that is closest to the root of
     // tree for any content that has been added.
 
     // Note that we can start at stackPos == 0 here, because it's the caller's
     // responsibility to handle flushing interactions between contexts (see
     // HTMLContentSink::BeginContext).
@@ -510,17 +508,16 @@ nsresult SinkContext::FlushTags() {
   }
   --(mSink->mInNotification);
 
   if (mSink->mUpdatesInNotification > 1) {
     UpdateChildCounts();
   }
 
   mSink->mUpdatesInNotification = oldUpdates;
-  mSink->mBeganUpdate = oldBeganUpdate;
 
   return NS_OK;
 }
 
 /**
  * NOTE!! Forked into nsXMLContentSink. Please keep in sync.
  */
 void SinkContext::UpdateChildCounts() {
@@ -865,27 +862,23 @@ void HTMLContentSink::CloseHeadContext()
     uint32_t n = mContextStack.Length() - 1;
     mCurrentContext = mContextStack.ElementAt(n);
     mContextStack.RemoveElementAt(n);
   }
 }
 
 void HTMLContentSink::NotifyInsert(nsIContent* aContent,
                                    nsIContent* aChildContent) {
-  if (aContent && aContent->GetUncomposedDoc() != mDocument) {
-    // aContent is not actually in our document anymore.... Just bail out of
-    // here; notifying on our document for this insert would be wrong.
-    return;
-  }
-
   mInNotification++;
 
   {
     // Scope so we call EndUpdate before we decrease mInNotification
-    MOZ_AUTO_DOC_UPDATE(mDocument, !mBeganUpdate);
+    // Note that aContent->OwnerDoc() may be different to mDocument already.
+    MOZ_AUTO_DOC_UPDATE(aContent ? aContent->OwnerDoc() : mDocument.get(),
+                        true);
     nsNodeUtils::ContentInserted(NODE_FROM(aContent, mDocument), aChildContent);
     mLastNotificationTime = PR_Now();
   }
 
   mInNotification--;
 }
 
 void HTMLContentSink::NotifyRootInsertion() {
--- a/dom/xml/nsXMLContentSink.cpp
+++ b/dom/xml/nsXMLContentSink.cpp
@@ -1440,25 +1440,23 @@ void nsXMLContentSink::FlushPendingNotif
  * Flush all elements that have been seen so far such that
  * they are visible in the tree. Specifically, make sure
  * that they are all added to their respective parents.
  * Also, do notification at the top for all content that
  * has been newly added so that the frame tree is complete.
  */
 nsresult nsXMLContentSink::FlushTags() {
   mDeferredFlushTags = false;
-  bool oldBeganUpdate = mBeganUpdate;
   uint32_t oldUpdates = mUpdatesInNotification;
 
   mUpdatesInNotification = 0;
   ++mInNotification;
   {
     // Scope so we call EndUpdate before we decrease mInNotification
     mozAutoDocUpdate updateBatch(mDocument, true);
-    mBeganUpdate = true;
 
     // Don't release last text node in case we need to add to it again
     FlushText(false);
 
     // Start from the base of the stack (growing downward) and do
     // a notification from the node that is closest to the root of
     // tree for any content that has been added.
 
@@ -1483,18 +1481,16 @@ nsresult nsXMLContentSink::FlushTags() {
   }
   --mInNotification;
 
   if (mUpdatesInNotification > 1) {
     UpdateChildCounts();
   }
 
   mUpdatesInNotification = oldUpdates;
-  mBeganUpdate = oldBeganUpdate;
-
   return NS_OK;
 }
 
 /**
  * NOTE!! Forked from SinkContext. Please keep in sync.
  */
 void nsXMLContentSink::UpdateChildCounts() {
   // Start from the top of the stack (growing upwards) and see if any
new file mode 100644
--- /dev/null
+++ b/testing/web-platform/tests/xhtml/adopt-while-parsing-001-ref.html
@@ -0,0 +1,9 @@
+<!doctype html>
+<title>Test reference</title>
+<style>
+  html, body { margin: 0 }
+</style>
+<iframe src="about:blank"></iframe>
+<div>
+  PASS
+</div>
new file mode 100644
--- /dev/null
+++ b/testing/web-platform/tests/xhtml/adopt-while-parsing-001.html
@@ -0,0 +1,19 @@
+<!doctype html>
+<title>Appending from the parser after adopting in an XML document doesn't miss notifications</title>
+<link rel="match" href="adopt-while-parsing-001-ref.html">
+<link rel="help" href="https://bugzilla.mozilla.org/show_bug.cgi?id=1511329">
+<link rel="author" title="Emilio Cobos Álvarez" href="mailto:emilio@crisal.io">
+<link rel="author" title="Mozilla" href="https://mozilla.org">
+<style>
+  html, body { margin: 0 }
+</style>
+<script>
+  // If we don't get notified of the <div> insertion, the PASS text will never appear.
+  function parsingInterrupted() {
+    let frameDoc = document.querySelector("iframe").contentDocument;
+    let root = frameDoc.documentElement;
+    document.documentElement.appendChild(root);
+    root.offsetTop;
+  }
+</script>
+<iframe src="adopt-while-parsing.xhtml"></iframe>
new file mode 100644
--- /dev/null
+++ b/testing/web-platform/tests/xhtml/adopt-while-parsing.xhtml
@@ -0,0 +1,11 @@
+<!DOCTYPE html>
+<html xmlns="http://www.w3.org/1999/xhtml">
+<body>
+  <script>
+    window.parent.parsingInterrupted();
+  </script>
+  <div>
+    PASS
+  </div>
+</body>
+</html>