Bug 1364399 - Call DidBuildModel() when not flushing. r=smaug.
authorHenri Sivonen <hsivonen@hsivonen.fi>
Mon, 30 Oct 2017 17:15:19 +0200
changeset 389549 388ffddeb4623b932ed67554f2ddc434b1fed8fb
parent 389548 3660e2cf9fdbb05e7890f70e9a35572a0d714694
child 389550 a137a0d96fb663b8f90af9f1ec985534c11d6e40
push id96870
push userhsivonen@mozilla.com
push dateWed, 01 Nov 2017 11:17:44 +0000
treeherdermozilla-inbound@388ffddeb462 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs1364399
milestone58.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 1364399 - Call DidBuildModel() when not flushing. r=smaug. MozReview-Commit-ID: CtYHqN8myb1
parser/html/nsHtml5TreeOpExecutor.cpp
parser/html/nsHtml5TreeOperation.cpp
parser/html/nsHtml5TreeOperation.h
--- a/parser/html/nsHtml5TreeOpExecutor.cpp
+++ b/parser/html/nsHtml5TreeOpExecutor.cpp
@@ -151,34 +151,22 @@ nsHtml5TreeOpExecutor::WillBuildModel(ns
   return NS_OK;
 }
 
 
 // This is called when the tree construction has ended
 NS_IMETHODIMP
 nsHtml5TreeOpExecutor::DidBuildModel(bool aTerminated)
 {
-  if (!aTerminated) {
-    // This is needed to avoid unblocking loads too many times on one hand
-    // and on the other hand to avoid destroying the frame constructor from
-    // within an update batch. See bug 537683.
-    EndDocUpdate();
-    
-    // If the above caused a call to nsIParser::Terminate(), let that call
-    // win.
-    if (!mParser) {
-      return NS_OK;
-    }
-  }
-  
   if (mRunsToCompletion) {
     return NS_OK;
   }
 
-  GetParser()->DropStreamParser();
+  MOZ_RELEASE_ASSERT(!IsInDocUpdate(),
+    "DidBuildModel from inside a doc update.");
 
   // This comes from nsXMLContentSink and nsHTMLContentSink
   // If this parser has been marked as broken, treat the end of parse as
   // forced termination.
   DidBuildModelImpl(aTerminated || NS_FAILED(IsBroken()));
 
   if (!mLayoutStarted) {
     // We never saw the body, and layout never got started. Force
@@ -206,16 +194,18 @@ nsHtml5TreeOpExecutor::DidBuildModel(boo
     return NS_OK;
   }
 
   // We may not have called BeginLoad() if loading is terminated before
   // OnStartRequest call.
   if (mStarted) {
     mDocument->EndLoad();
   }
+
+  GetParser()->DropStreamParser();
   DropParserAndPerfHint();
 #ifdef GATHER_DOCWRITE_STATISTICS
   printf("UNSAFE SCRIPTS: %d\n", sUnsafeDocWrites);
   printf("TOKENIZER-SAFE SCRIPTS: %d\n", sTokenSafeDocWrites);
   printf("TREEBUILDER-SAFE SCRIPTS: %d\n", sTreeSafeDocWrites);
 #endif
 #ifdef DEBUG_NS_HTML5_TREE_OP_EXECUTOR_FLUSH
   printf("MAX NOTIFICATION BATCH LEN: %d\n", sAppendBatchMaxSize);
@@ -477,33 +467,34 @@ nsHtml5TreeOpExecutor::RunFlushLoop()
       // Avoid bothering the rest of the engine with a doc update if there's 
       // nothing to do.
       return;
     }
 
 
     nsIContent* scriptElement = nullptr;
     bool interrupted = false;
+    bool streamEnded = false;
 
     {
       // autoFlush clears mOpQueue in its destructor unless
       // SetNumberOfOpsToRemove is called first, in which case only
       // some ops from the start of the queue are cleared.
       nsHtml5AutoFlush autoFlush(this);
 
       nsHtml5TreeOperation* first = mOpQueue.Elements();
       nsHtml5TreeOperation* last = first + mOpQueue.Length() - 1;
       for (nsHtml5TreeOperation* iter = first;; ++iter) {
         if (MOZ_UNLIKELY(!mParser)) {
           // The previous tree op caused a call to nsIParser::Terminate().
           return;
         }
         MOZ_ASSERT(IsInDocUpdate(),
                    "Tried to perform tree op outside update batch.");
-        nsresult rv = iter->Perform(this, &scriptElement, &interrupted);
+        nsresult rv = iter->Perform(this, &scriptElement, &interrupted, &streamEnded);
         if (NS_FAILED(rv)) {
           MarkAsBroken(rv);
           break;
         }
 
         // Be sure not to check the deadline if the last op was just performed.
         if (MOZ_UNLIKELY(iter == last)) {
           break;
@@ -520,17 +511,28 @@ nsHtml5TreeOpExecutor::RunFlushLoop()
 
     } // end autoFlush
 
     if (MOZ_UNLIKELY(!mParser)) {
       // The parse ended already.
       return;
     }
 
-    if (scriptElement) {
+    if (streamEnded) {
+      DidBuildModel(false);
+#ifdef DEBUG
+      if (scriptElement) {
+        nsCOMPtr<nsIScriptElement> sele = do_QueryInterface(scriptElement);
+        if (!sele) {
+          MOZ_ASSERT(nsNameSpaceManager::GetInstance()->mSVGDisabled, "Node didn't QI to script, but SVG wasn't disabled.");
+        }
+        MOZ_ASSERT(sele->IsMalformed(), "Script wasn't marked as malformed.");
+      }
+#endif
+    } else if (scriptElement) {
       // must be tail call when mFlushState is eNotFlushing
       RunScript(scriptElement);
       
       // Always check the clock in nsContentSink right after a script
       StopDeflecting();
       if (nsContentSink::DidProcessATokenImpl() == 
           NS_ERROR_HTMLPARSER_INTERRUPTED) {
         #ifdef DEBUG_NS_HTML5_TREE_OP_EXECUTOR_FLUSH
@@ -573,16 +575,17 @@ nsHtml5TreeOpExecutor::FlushDocumentWrit
                      "Got doc write flush when reading from stage");
 
 #ifdef DEBUG
   mStage.AssertEmpty();
 #endif
   
   nsIContent* scriptElement = nullptr;
   bool interrupted = false;
+  bool streamEnded = false;
 
   {
     // autoFlush clears mOpQueue in its destructor.
     nsHtml5AutoFlush autoFlush(this);
 
 
   nsHtml5TreeOperation* start = mOpQueue.Elements();
   nsHtml5TreeOperation* end = start + mOpQueue.Length();
@@ -590,31 +593,42 @@ nsHtml5TreeOpExecutor::FlushDocumentWrit
        iter < end;
        ++iter) {
       if (MOZ_UNLIKELY(!mParser)) {
         // The previous tree op caused a call to nsIParser::Terminate().
         return rv;
       }
       NS_ASSERTION(IsInDocUpdate(),
                    "Tried to perform tree op outside update batch.");
-      rv = iter->Perform(this, &scriptElement, &interrupted);
+      rv = iter->Perform(this, &scriptElement, &interrupted, &streamEnded);
       if (NS_FAILED(rv)) {
         MarkAsBroken(rv);
         break;
       }
     }
 
   } // autoFlush
 
   if (MOZ_UNLIKELY(!mParser)) {
     // Ending the doc update caused a call to nsIParser::Terminate().
     return rv;
   }
 
-  if (scriptElement) {
+  if (streamEnded) {
+    DidBuildModel(false);
+#ifdef DEBUG
+    if (scriptElement) {
+      nsCOMPtr<nsIScriptElement> sele = do_QueryInterface(scriptElement);
+      if (!sele) {
+        MOZ_ASSERT(nsNameSpaceManager::GetInstance()->mSVGDisabled, "Node didn't QI to script, but SVG wasn't disabled.");
+      }
+      MOZ_ASSERT(sele->IsMalformed(), "Script wasn't marked as malformed.");
+    }
+#endif
+  } else if (scriptElement) {
     // must be tail call when mFlushState is eNotFlushing
     RunScript(scriptElement);
   }
   return rv;
 }
 
 // copied from HTML content sink
 bool
@@ -682,31 +696,24 @@ nsHtml5TreeOpExecutor::RunScript(nsICont
 {
   if (mRunsToCompletion) {
     // We are in createContextualFragment() or in the upcoming document.parse().
     // Do nothing. Let's not even mark scripts malformed here, because that
     // could cause serialization weirdness later.
     return;
   }
 
-  NS_ASSERTION(aScriptElement, "No script to run");
+  MOZ_ASSERT(mParser, "Trying to run script with a terminated parser.");
+  MOZ_ASSERT(aScriptElement, "No script to run");
   nsCOMPtr<nsIScriptElement> sele = do_QueryInterface(aScriptElement);
   if (!sele) {
     MOZ_ASSERT(nsNameSpaceManager::GetInstance()->mSVGDisabled, "Node didn't QI to script, but SVG wasn't disabled.");
     return;
   }
   
-  if (!mParser) {
-    NS_ASSERTION(sele->IsMalformed(), "Script wasn't marked as malformed.");
-    // We got here not because of an end tag but because the tree builder
-    // popped an incomplete script element on EOF. Returning here to avoid
-    // calling back into mParser anymore.
-    return;
-  }
-  
   if (sele->GetScriptDeferred() || sele->GetScriptAsync()) {
     DebugOnly<bool> block = sele->AttemptToExecute();
     NS_ASSERTION(!block, "Defer or async script tried to block.");
     return;
   }
 
   MOZ_RELEASE_ASSERT(mFlushState == eNotFlushing,
                      "Tried to run script while flushing.");
--- a/parser/html/nsHtml5TreeOperation.cpp
+++ b/parser/html/nsHtml5TreeOperation.cpp
@@ -806,17 +806,18 @@ nsHtml5TreeOperation::MarkMalformedIfScr
     // Make sure to serialize this script correctly, for nice round tripping.
     sele->SetIsMalformed();
   }
 }
 
 nsresult
 nsHtml5TreeOperation::Perform(nsHtml5TreeOpExecutor* aBuilder,
                               nsIContent** aScriptElement,
-                              bool* aInterrupted)
+                              bool* aInterrupted,
+                              bool* aStreamEnded)
 {
   switch(mOpCode) {
     case eTreeOpUninitialized: {
       MOZ_CRASH("eTreeOpUninitialized");
     }
     case eTreeOpAppend: {
       nsIContent* node = *(mOne.node);
       nsIContent* parent = *(mTwo.node);
@@ -1029,17 +1030,17 @@ nsHtml5TreeOperation::Perform(nsHtml5Tre
       return NS_OK;
     }
     case eTreeOpMarkMalformedIfScript: {
       nsIContent* node = *(mOne.node);
       MarkMalformedIfScript(node);
       return NS_OK;
     }
     case eTreeOpStreamEnded: {
-      aBuilder->DidBuildModel(false); // this causes a notifications flush anyway
+      *aStreamEnded = true;
       return NS_OK;
     }
     case eTreeOpSetStyleLineNumber: {
       nsIContent* node = *(mOne.node);
       nsCOMPtr<nsIStyleSheetLinkingElement> ssle = do_QueryInterface(node);
       if (ssle) {
         ssle->SetLineNumber(mFour.integer);
       } else {
--- a/parser/html/nsHtml5TreeOperation.h
+++ b/parser/html/nsHtml5TreeOperation.h
@@ -541,17 +541,18 @@ class nsHtml5TreeOperation final {
         "Setting a snapshot for a tree operation other than eTreeOpRunScript!");
       NS_PRECONDITION(aSnapshot, "Initialized tree op with null snapshot.");
       mTwo.state = aSnapshot;
       mFour.integer = aLine;
     }
 
     nsresult Perform(nsHtml5TreeOpExecutor* aBuilder,
                      nsIContent** aScriptElement,
-                     bool* aInterrupted);
+                     bool* aInterrupted,
+                     bool* aStreamEnded);
 
   private:
     nsHtml5TreeOperation(const nsHtml5TreeOperation&) = delete;
     nsHtml5TreeOperation& operator=(const nsHtml5TreeOperation&) = delete;
 
     // possible optimization:
     // Make the queue take items the size of pointer and make the op code
     // decide how many operands it dequeues after it.