Pause HTML parser for external stylesheets in the body

third_party/WebKit/Source/core/html/parser/HTMLDocumentParser.cpp

Index: third_party/WebKit/Source/core/html/parser/HTMLDocumentParser.cpp
diff --git a/third_party/WebKit/Source/core/html/parser/HTMLDocumentParser.cpp b/third_party/WebKit/Source/core/html/parser/HTMLDocumentParser.cpp
index 1a41b81856c6b42adf792ccf44b7f99d5035fc58..fedefa9df368322bdae3ee3b951eb1e3881554db 100644
--- a/third_party/WebKit/Source/core/html/parser/HTMLDocumentParser.cpp
+++ b/third_party/WebKit/Source/core/html/parser/HTMLDocumentParser.cpp
@@ -151,7 +151,9 @@ HTMLDocumentParser::HTMLDocumentParser(Document& document,
       m_pumpSessionNestingLevel(0),
       m_pumpSpeculationsSessionNestingLevel(0),
       m_isParsingAtLineNumber(false),
-      m_triedLoadingLinkHeaders(false) {
+      m_triedLoadingLinkHeaders(false),
+      m_addedPendingStylesheetInBody(false),
+      m_isWaitingForStylesheets(false) {
   ASSERT(shouldUseThreading() || (m_token && m_tokenizer));
   // Threading is not allowed in prefetch mode.
   DCHECK(!document.isPrefetchOnly() || !shouldUseThreading());
@@ -256,7 +258,7 @@ bool HTMLDocumentParser::isParsingFragment() const {
 }
 
 void HTMLDocumentParser::pumpTokenizerIfPossible() {
-  if (isStopped() || isWaitingForScripts())
+  if (isStopped() || isWaitingForScripts() || m_isWaitingForStylesheets)

[kouhei (in TOK), 2017/01/06 03:32:43]

Would you mind cleaning up the HTMLDocumentParser states?
Option A (less work): Create a helper method to consolidate boolean expressions.
eg.) bool isBlocked() const { return isWaitingForScripts ||
m_isWaitingForStylesheets; }

Option B (more work, preferred): Create an explicit state machine enum + state
transition checks.
Something like:
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/platform/heap/...

[Pat Meenan, 2017/01/09 16:37:05]

For Option B, the state is currently tracked in the grandparent DocumentParser
for isStopped() and in the HTML parser for script/stylesheet/resume.  Would you
want a single state machine to handle all of the states or just the
HTML-parser-specific states?

https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/dom/Docum...

For the most part the script/stylesheet states are always checked as a pair so
collapsing those into a helper and checking the combined state would be easy
enough but checking all of the other states would likely look similar to how it
does now (just tracking an enum for the state instead of the 2-3 bool's).

     return;
 
   pumpTokenizer();
@@ -293,7 +295,7 @@ bool HTMLDocumentParser::canTakeNextToken() {
   // continuing.
   if (m_treeBuilder->hasParserBlockingScript())
     runScriptsForPausedTreeBuilder();
-  if (isStopped() || isWaitingForScripts())
+  if (isStopped() || isWaitingForScripts() || m_isWaitingForStylesheets)
     return false;
 
   // FIXME: It's wrong for the HTMLDocumentParser to reach back to the
@@ -381,7 +383,8 @@ void HTMLDocumentParser::notifyPendingTokenizedChunks() {
   for (auto& chunk : pendingChunks)
     m_speculations.append(std::move(chunk));
 
-  if (!isWaitingForScripts() && !isScheduledForResume()) {
+  if (!isWaitingForScripts() && !isScheduledForResume() &&
+      !m_isWaitingForStylesheets) {
     if (m_tasksWereSuspended)
       m_parserScheduler->forceResumeAfterYield();
     else
@@ -399,19 +402,19 @@ void HTMLDocumentParser::validateSpeculations(
   ASSERT(chunk);
   // TODO(kouhei): We should simplify codepath here by disallowing
   // validateSpeculations
-  // while isWaitingForScripts, and m_lastChunkBeforeScript can simply be
+  // while isWaitingForScripts, and m_lastChunkBeforePause can simply be
   // pushed to m_speculations.
-  if (isWaitingForScripts()) {
+  if (isWaitingForScripts() || m_isWaitingForStylesheets) {
     // We're waiting on a network script, just save the chunk, we'll get a
     // second validateSpeculations call after the script completes. This call
     // should have been made immediately after runScriptsForPausedTreeBuilder
     // which may have started a network load and left us waiting.
-    ASSERT(!m_lastChunkBeforeScript);
-    m_lastChunkBeforeScript = std::move(chunk);
+    DCHECK(!m_lastChunkBeforePause);
+    m_lastChunkBeforePause = std::move(chunk);
     return;
   }
 
-  ASSERT(!m_lastChunkBeforeScript);
+  DCHECK(!m_lastChunkBeforePause);
   std::unique_ptr<HTMLTokenizer> tokenizer = std::move(m_tokenizer);
   std::unique_ptr<HTMLToken> token = std::move(m_token);
 
@@ -493,7 +496,8 @@ size_t HTMLDocumentParser::processTokenizedChunkFromBackgroundParser(
   ASSERT(shouldUseThreading());
   ASSERT(!m_tokenizer);
   ASSERT(!m_token);
-  ASSERT(!m_lastChunkBeforeScript);
+  DCHECK(!m_lastChunkBeforePause);
+  DCHECK(!m_isWaitingForStylesheets);
 
   std::unique_ptr<TokenizedChunk> chunk(std::move(popChunk));
   std::unique_ptr<CompactHTMLTokenStream> tokens = std::move(chunk->tokens);
@@ -558,6 +562,11 @@ size_t HTMLDocumentParser::processTokenizedChunkFromBackgroundParser(
       break;
     }
 
+    if (m_isWaitingForStylesheets && it + 1 == tokens->end()) {
+      validateSpeculations(std::move(chunk));
+      break;
+    }
+
     if (it->type() == HTMLToken::EndOfFile) {
       // The EOF is assumed to be the last token of this bunch.
       ASSERT(it + 1 == tokens->end());
@@ -586,11 +595,12 @@ void HTMLDocumentParser::pumpPendingSpeculations() {
   // m_tokenizer and m_token don't have state that invalidates m_speculations.
   ASSERT(!m_tokenizer);
   ASSERT(!m_token);
-  ASSERT(!m_lastChunkBeforeScript);
+  DCHECK(!m_lastChunkBeforePause);
   ASSERT(!isWaitingForScripts());
   ASSERT(!isStopped());
   ASSERT(!isScheduledForResume());
   ASSERT(!inPumpSession());
+  DCHECK(!m_isWaitingForStylesheets);
 
   // FIXME: Here should never be reached when there is a blocking script,
   // but it happens in unknown scenarios. See https://crbug.com/440901
@@ -621,7 +631,8 @@ void HTMLDocumentParser::pumpPendingSpeculations() {
     // isScheduledForResume() may be set here as a result of
     // processTokenizedChunkFromBackgroundParser running arbitrary javascript
     // which invokes nested event loops. (e.g. inspector breakpoints)
-    if (!isParsing() || isWaitingForScripts() || isScheduledForResume())
+    if (!isParsing() || isWaitingForScripts() || isScheduledForResume() ||
+        m_isWaitingForStylesheets)
       break;
 
     if (m_speculations.isEmpty() ||
@@ -709,14 +720,14 @@ void HTMLDocumentParser::pumpTokenizer() {
   m_treeBuilder->flush(FlushAlways);
   RELEASE_ASSERT(!isStopped());
 
-  if (isWaitingForScripts()) {
+  if (isWaitingForScripts() || m_isWaitingForStylesheets) {
     ASSERT(m_tokenizer->getState() == HTMLTokenizer::DataState);
 
     ASSERT(m_preloader);
     // TODO(kouhei): m_preloader should be always available for synchronous
     // parsing case, adding paranoia if for speculative crash fix for
     // crbug.com/465478
-    if (m_preloader) {
+    if (m_preloader && document()->url().isValid()) {
       if (!m_preloadScanner) {
         m_preloadScanner = createPreloadScanner();
         m_preloadScanner->appendToEnd(m_input.current());
@@ -748,6 +759,9 @@ void HTMLDocumentParser::constructTreeFromHTMLToken() {
 
   m_treeBuilder->constructTree(&atomicToken);
 
+  if (m_addedPendingStylesheetInBody)
+    m_isWaitingForStylesheets = true;
+
   // FIXME: constructTree may synchronously cause Document to be detached.
   if (!m_token)
     return;
@@ -762,6 +776,8 @@ void HTMLDocumentParser::constructTreeFromCompactHTMLToken(
     const CompactHTMLToken& compactToken) {
   AtomicHTMLToken token(compactToken);
   m_treeBuilder->constructTree(&token);
+  if (m_addedPendingStylesheetInBody)
+    m_isWaitingForStylesheets = true;
 }
 
 bool HTMLDocumentParser::hasInsertionPoint() {
@@ -792,13 +808,15 @@ void HTMLDocumentParser::insert(const SegmentedString& source) {
   m_input.insertAtCurrentInsertionPoint(excludedLineNumberSource);
   pumpTokenizerIfPossible();
 
-  if (isWaitingForScripts()) {
-    // Check the document.write() output with a separate preload scanner as
-    // the main scanner can't deal with insertions.
-    if (!m_insertionPreloadScanner)
-      m_insertionPreloadScanner = createPreloadScanner();
-    m_insertionPreloadScanner->appendToEnd(source);
-    scanAndPreload(m_insertionPreloadScanner.get());
+  if (isWaitingForScripts() || m_isWaitingForStylesheets) {
+    if (document()->url().isValid()) {
+      // Check the document.write() output with a separate preload scanner as
+      // the main scanner can't deal with insertions.
+      if (!m_insertionPreloadScanner)
+        m_insertionPreloadScanner = createPreloadScanner();
+      m_insertionPreloadScanner->appendToEnd(source);
+      scanAndPreload(m_insertionPreloadScanner.get());
+    }
   }
 
   endIfDelayed();
@@ -888,25 +906,28 @@ void HTMLDocumentParser::append(const String& inputSource) {
   const SegmentedString source(inputSource);
 
   if (document()->isPrefetchOnly()) {
-    if (!m_preloadScanner)
-      m_preloadScanner = createPreloadScanner();
+    if (document()->url().isValid()) {
+      if (!m_preloadScanner)
+        m_preloadScanner = createPreloadScanner();
 
-    m_preloadScanner->appendToEnd(source);
-    scanAndPreload(m_preloadScanner.get());
+      m_preloadScanner->appendToEnd(source);
+      scanAndPreload(m_preloadScanner.get());
+    }
 
     // Return after the preload scanner, do not actually parse the document.
     return;
   }
 
   if (m_preloadScanner) {
-    if (m_input.current().isEmpty() && !isWaitingForScripts()) {
+    if (m_input.current().isEmpty() && !isWaitingForScripts() &&
+        !m_isWaitingForStylesheets) {
       // We have parsed until the end of the current input and so are now moving
       // ahead of the preload scanner. Clear the scanner so we know to scan
       // starting from the current input point if we block again.
       m_preloadScanner.reset();
     } else {
       m_preloadScanner->appendToEnd(source);
-      if (isWaitingForScripts())
+      if (isWaitingForScripts() || m_isWaitingForStylesheets)
         scanAndPreload(m_preloadScanner.get());
     }
   }
@@ -1057,14 +1078,15 @@ bool HTMLDocumentParser::isWaitingForScripts() const {
          m_reentryPermit->parserPauseFlag();
 }
 
-void HTMLDocumentParser::resumeParsingAfterScriptExecution() {
+void HTMLDocumentParser::resumeParsingAfterPause() {
   ASSERT(!isExecutingScript());
   ASSERT(!isWaitingForScripts());
+  DCHECK(!m_isWaitingForStylesheets);
 
   if (m_haveBackgroundParser) {
-    if (m_lastChunkBeforeScript) {
-      validateSpeculations(std::move(m_lastChunkBeforeScript));
-      DCHECK(!m_lastChunkBeforeScript);
+    if (m_lastChunkBeforePause) {
+      validateSpeculations(std::move(m_lastChunkBeforePause));
+      DCHECK(!m_lastChunkBeforePause);
       pumpPendingSpeculations();
     }
     return;
@@ -1097,8 +1119,8 @@ void HTMLDocumentParser::notifyScriptLoaded(PendingScript* pendingScript) {
   }
 
   m_scriptRunner->executeScriptsWaitingForLoad(pendingScript);
-  if (!isWaitingForScripts())
-    resumeParsingAfterScriptExecution();
+  if (!isWaitingForScripts() && !m_isWaitingForStylesheets)
+    resumeParsingAfterPause();
 }
 
 void HTMLDocumentParser::executeScriptsWaitingForResources() {
@@ -1108,8 +1130,36 @@ void HTMLDocumentParser::executeScriptsWaitingForResources() {
   // will not be called in the DocumentFragment case.
   DCHECK(m_scriptRunner);
   m_scriptRunner->executeScriptsWaitingForResources();
-  if (!isWaitingForScripts())
-    resumeParsingAfterScriptExecution();
+  if (!isWaitingForScripts() && !m_isWaitingForStylesheets)
+    resumeParsingAfterPause();
+}
+
+void HTMLDocumentParser::didAddPendingStylesheetInBody() {
+  // When in-body CSS doesn't block painting, the parser needs to pause so that
+  // the DOM doesn't include any elements that may depend on the CSS for style.
+  // The stylesheet can be added and removed during the parsing of a single
+  // token so don't actually set the bit to block parsing here, just track
+  // the state of the added sheet in case it does persist beyond a single
+  // token.
+  if (RuntimeEnabledFeatures::cssInBodyDoesNotBlockPaintEnabled())
+    m_addedPendingStylesheetInBody = true;
+}
+
+void HTMLDocumentParser::didLoadAllStylesheets() {
+  m_addedPendingStylesheetInBody = false;
+  if (m_isWaitingForStylesheets) {
+    m_isWaitingForStylesheets = false;
+    if (!isWaitingForScripts()) {
+      // Post an async task to resume parsing since this can (and often is)
+      // called while already parsing.
+      m_resumeParsingTaskHandle =
+          TaskRunnerHelper::get(TaskType::Networking, document())
+              ->postCancellableTask(
+                  BLINK_FROM_HERE,
+                  WTF::bind(&HTMLDocumentParser::resumeParsingAfterPause,
+                            wrapWeakPersistent(this)));
+    }
+  }
 }
 
 void HTMLDocumentParser::parseDocumentFragment(
@@ -1257,9 +1307,11 @@ void HTMLDocumentParser::evaluateAndPreloadScriptForDocumentWrite(
 
   int currentPreloadCount = document()->loader()->fetcher()->countPreloads();
 
-  std::unique_ptr<HTMLPreloadScanner> scanner = createPreloadScanner();
-  scanner->appendToEnd(SegmentedString(writtenSource));
-  scanAndPreload(scanner.get());
+  if (document()->url().isValid()) {

[kouhei (in TOK), 2017/01/06 03:32:43]

Would you describe your change here?

[Pat Meenan, 2017/01/09 16:37:05]

Sorry, left-over from some earlier iterations and isn't needed any longer.  Will
be cleaned up when the state machine/blocking check logic is implemented (later
today).

+    std::unique_ptr<HTMLPreloadScanner> scanner = createPreloadScanner();
+    scanner->appendToEnd(SegmentedString(writtenSource));
+    scanAndPreload(scanner.get());
+  }
 
   int numPreloads =
       document()->loader()->fetcher()->countPreloads() - currentPreloadCount;