Revert of Script blocking resources tracking should be only done by...

Revert of Script blocking resources tracking should be only done by Document::isScriptExecutionReady (patchset #3 id:40001 of https://codereview.chromium.org/2536753003/ )

Reason for revert:
Speculative revert for crbug.com/669435.

BUG=669435

Original issue's description:
> Script blocking resources tracking should be only done by Document::isScriptExecutionReady
>
> Before this CL, we had a duplicated tracking flag
> HTMLScriptRunner::m_hasScriptsWaitingForResources.
>
> This CL removes the flag in HTMLScriptRunner and changes code so it checks
> Document::isScriptExecutionReady() state instead.
>
> This CL also removes a comment referring to HTMLDocumentParser::executeScriptsWaitingForResources
> being called when it hasScriptsWaitingForResources as it should never happen
> and replaces it with a DCHECK
>
> BUG=None
>
> Committed: https://crrev.com/74341a4386cbe0c0b94eba0535fbbbf8d1aff8ea
> Cr-Commit-Position: refs/heads/master@{#434935}

TBR=dominicc@chromium.org,yhirano@chromium.org,kouhei@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=None

Review-Url: https://codereview.chromium.org/2540653002
Cr-Commit-Position: refs/heads/master@{#434960}

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

@@ -398,10 +398,6 @@ void HTMLDocumentParser::didReceiveEncodingDataFromBackgroundParser(
 void HTMLDocumentParser::validateSpeculations(
     std::unique_ptr<TokenizedChunk> chunk) {
   ASSERT(chunk);
-  // TODO(kouhei): We should simplify codepath here by disallowing
-  // validateSpeculations
-  // while isWaitingForScripts, and m_lastChunkBeforeScript can simply be
-  // pushed to m_speculations.
   if (isWaitingForScripts()) {
     // We're waiting on a network script, just save the chunk, we'll get a
     // second validateSpeculations call after the script completes. This call
@@ -1065,11 +1061,9 @@ void HTMLDocumentParser::resumeParsingAfterScriptExecution() {
   ASSERT(!isWaitingForScripts());
 
   if (m_haveBackgroundParser) {
-    if (m_lastChunkBeforeScript) {
-      validateSpeculations(std::move(m_lastChunkBeforeScript));
-      DCHECK(!m_lastChunkBeforeScript);
-      pumpPendingSpeculations();
-    }
+    validateSpeculations(std::move(m_lastChunkBeforeScript));
+    ASSERT(!m_lastChunkBeforeScript);
+    pumpPendingSpeculations();
     return;
   }
 
@@ -1104,11 +1098,14 @@ void HTMLDocumentParser::notifyScriptLoaded(Resource* cachedResource) {
 }
 
 void HTMLDocumentParser::executeScriptsWaitingForResources() {
-  DCHECK(document()->isScriptExecutionReady());
-
   // Document only calls this when the Document owns the DocumentParser so this
   // will not be called in the DocumentFragment case.
-  DCHECK(m_scriptRunner);
+  ASSERT(m_scriptRunner);
+  // Ignore calls unless we have a script blocking the parser waiting on a
+  // stylesheet load.  Otherwise we are currently parsing and this is a
+  // re-entrant call from encountering a </ style> tag.
+  if (!m_scriptRunner->hasScriptsWaitingForResources())
+    return;
   m_scriptRunner->executeScriptsWaitingForResources();
   if (!isWaitingForScripts())
     resumeParsingAfterScriptExecution();

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

@@ -154,7 +154,8 @@ HTMLScriptRunner::HTMLScriptRunner(HTMLParserReentryPermit* reentryPermit,
     : m_reentryPermit(reentryPermit),
       m_document(document),
       m_host(host),
-      m_parserBlockingScript(PendingScript::create(nullptr, nullptr)) {
+      m_parserBlockingScript(PendingScript::create(nullptr, nullptr)),
+      m_hasScriptsWaitingForResources(false) {
   ASSERT(m_host);
   ThreadState::current()->registerPreFinalizer(this);
 }
@@ -180,17 +181,18 @@ void HTMLScriptRunner::detach() {
   // detached.
 }
 
-bool HTMLScriptRunner::isPendingScriptReady() {
-  if (!m_document->isScriptExecutionReady())
+bool HTMLScriptRunner::isPendingScriptReady(const PendingScript* script) {
+  m_hasScriptsWaitingForResources = !m_document->isScriptExecutionReady();
+  if (m_hasScriptsWaitingForResources)
     return false;
-  return m_parserBlockingScript->isReady();
+  return script->isReady();
 }
 
 void HTMLScriptRunner::executeParsingBlockingScript() {
-  DCHECK(m_document);
-  DCHECK(!isExecutingScript());
-  DCHECK(m_document->isScriptExecutionReady());
-  DCHECK(isPendingScriptReady());
+  ASSERT(m_document);
+  ASSERT(!isExecutingScript());
+  ASSERT(m_document->isScriptExecutionReady());
+  ASSERT(isPendingScriptReady(m_parserBlockingScript.get()));
 
   InsertionPointRecord insertionPointRecord(m_host->inputStream());
   executePendingScriptAndDispatchEvent(m_parserBlockingScript.get(),
@@ -213,9 +215,10 @@ void HTMLScriptRunner::executePendingScriptAndDispatchEvent(
   if (!isExecutingScript()) {
     Microtask::performCheckpoint(V8PerIsolateData::mainThreadIsolate());
     if (pendingScriptType == ScriptStreamer::ParsingBlocking) {
+      m_hasScriptsWaitingForResources = !m_document->isScriptExecutionReady();
       // The parser cannot be unblocked as a microtask requested another
       // resource
-      if (!m_document->isScriptExecutionReady())
+      if (m_hasScriptsWaitingForResources)
         return;
     }
   }
@@ -370,7 +373,8 @@ bool HTMLScriptRunner::hasParserBlockingScript() const {
 }
 
 void HTMLScriptRunner::executeParsingBlockingScripts() {
-  while (hasParserBlockingScript() && isPendingScriptReady())
+  while (hasParserBlockingScript() &&
+         isPendingScriptReady(m_parserBlockingScript.get()))
     executeParsingBlockingScript();
 }
 
@@ -386,6 +390,9 @@ void HTMLScriptRunner::executeScriptsWaitingForLoad(Resource* resource) {
 void HTMLScriptRunner::executeScriptsWaitingForResources() {
   TRACE_EVENT0("blink", "HTMLScriptRunner::executeScriptsWaitingForResources");
   ASSERT(m_document);
+  // Callers should check hasScriptsWaitingForResources() before calling
+  // to prevent parser or script re-entry during </style> parsing.
+  ASSERT(hasScriptsWaitingForResources());
   ASSERT(!isExecutingScript());
   ASSERT(m_document->isScriptExecutionReady());
   executeParsingBlockingScripts();

third_party/WebKit/Source/core/html/parser/HTMLScriptRunner.h

@@ -64,6 +64,9 @@ class HTMLScriptRunner final
                const TextPosition& scriptStartPosition);
 
   void executeScriptsWaitingForLoad(Resource*);
+  bool hasScriptsWaitingForResources() const {
+    return m_hasScriptsWaitingForResources;
+  }
   void executeScriptsWaitingForResources();
   bool executeScriptsWaitingForParsing();
 
@@ -92,7 +95,7 @@ class HTMLScriptRunner final
 
   void runScript(Element*, const TextPosition& scriptStartPosition);
 
-  bool isPendingScriptReady();
+  bool isPendingScriptReady(const PendingScript*);
 
   void stopWatchingResourceForLoad(Resource*);
 
@@ -104,6 +107,12 @@ class HTMLScriptRunner final
   Member<PendingScript> m_parserBlockingScript;
   // http://www.whatwg.org/specs/web-apps/current-work/#list-of-scripts-that-will-execute-when-the-document-has-finished-parsing
   HeapDeque<Member<PendingScript>> m_scriptsToExecuteAfterParsing;
+
+  // We only want stylesheet loads to trigger script execution if script
+  // execution is currently stopped due to stylesheet loads, otherwise we'd
+  // cause nested script execution when parsing <style> tags since </style>
+  // tags can cause Document to call executeScriptsWaitingForResources.
+  bool m_hasScriptsWaitingForResources;
 };
 
 }  // namespace blink