Commit 50cb1c9e authored by Hiroshige Hayashizaki's avatar Hiroshige Hayashizaki Committed by Commit Bot

Remove DCHECK()s in ExecuteScriptsWaitingForLoad()

Previously, HTMLParserScriptRunner::ExecuteScriptsWaitingForLoad()
required the given PendingScript is still the valid
ParserBlockingScript().
However, this isn't the case when
HTMLParserScriptRunner::ExecuteParsingBlockingScripts()
is called after PendingScriptFinished() before NotifyScriptLoaded(),
because the PendingScript is already evaluated and
is no longer parser blocking.

This CL allows ExecuteScriptsWaitingForLoad() to be called
even when the finished PendingScript doesn't match with
ParserBlockingScript() or even when there are no longer
ParserBlockingScript(),
by removing DCHECK()s and |pending_script| argument.

This is safe because:
- All necessary conditions for evaluation
  is checked in ExecuteParsingBlockingScripts(),
  and extra stale calls to ExecuteParsingBlockingScripts()
  (i.e. called when parser blocking scripts are not ready)
  are ignored.
- |pending_script| hasn't been used other than for DCHECK()s.

Bug: 1093051
Change-Id: I921f8d8f1fc261547fa214822f75341de03c8cba
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2239770
Commit-Queue: Hiroshige Hayashizaki <hiroshige@chromium.org>
Reviewed-by: default avatarKouhei Ueno <kouhei@chromium.org>
Cr-Commit-Position: refs/heads/master@{#789864}
parent e8811741
...@@ -1394,7 +1394,7 @@ void HTMLDocumentParser::AppendCurrentInputStreamToPreloadScannerAndScan() { ...@@ -1394,7 +1394,7 @@ void HTMLDocumentParser::AppendCurrentInputStreamToPreloadScannerAndScan() {
ScanAndPreload(preload_scanner_.get()); ScanAndPreload(preload_scanner_.get());
} }
void HTMLDocumentParser::NotifyScriptLoaded(PendingScript* pending_script) { void HTMLDocumentParser::NotifyScriptLoaded() {
TRACE_EVENT1("blink", "HTMLDocumentParser::NotifyScriptLoaded", "parser", TRACE_EVENT1("blink", "HTMLDocumentParser::NotifyScriptLoaded", "parser",
(void*)this); (void*)this);
DCHECK(script_runner_); DCHECK(script_runner_);
...@@ -1412,7 +1412,7 @@ void HTMLDocumentParser::NotifyScriptLoaded(PendingScript* pending_script) { ...@@ -1412,7 +1412,7 @@ void HTMLDocumentParser::NotifyScriptLoaded(PendingScript* pending_script) {
return; return;
} }
script_runner_->ExecuteScriptsWaitingForLoad(pending_script); script_runner_->ExecuteScriptsWaitingForLoad();
if (!IsPaused()) if (!IsPaused())
ResumeParsingAfterPause(); ResumeParsingAfterPause();
} }
......
...@@ -168,7 +168,7 @@ class CORE_EXPORT HTMLDocumentParser : public ScriptableDocumentParser, ...@@ -168,7 +168,7 @@ class CORE_EXPORT HTMLDocumentParser : public ScriptableDocumentParser,
void DocumentElementAvailable() override; void DocumentElementAvailable() override;
// HTMLParserScriptRunnerHost // HTMLParserScriptRunnerHost
void NotifyScriptLoaded(PendingScript*) final; void NotifyScriptLoaded() final;
HTMLInputStream& InputStream() final { return input_; } HTMLInputStream& InputStream() final { return input_; }
bool HasPreloadScanner() const final { bool HasPreloadScanner() const final {
return preload_scanner_.get() && !CanParseAsynchronously(); return preload_scanner_.get() && !CanParseAsynchronously();
......
...@@ -311,8 +311,7 @@ void HTMLParserScriptRunner::PendingScriptFinished( ...@@ -311,8 +311,7 @@ void HTMLParserScriptRunner::PendingScriptFinished(
document_->GetTaskRunner(TaskType::kInternalContinueScriptLoading) document_->GetTaskRunner(TaskType::kInternalContinueScriptLoading)
->PostTask(FROM_HERE, ->PostTask(FROM_HERE,
WTF::Bind(&HTMLParserScriptRunnerHost::NotifyScriptLoaded, WTF::Bind(&HTMLParserScriptRunnerHost::NotifyScriptLoaded,
WrapPersistent(host_.Get()), WrapPersistent(host_.Get())));
WrapPersistent(pending_script)));
} }
// <specdef href="https://html.spec.whatwg.org/C/#scriptEndTag"> // <specdef href="https://html.spec.whatwg.org/C/#scriptEndTag">
...@@ -380,11 +379,20 @@ void HTMLParserScriptRunner::ExecuteParsingBlockingScripts() { ...@@ -380,11 +379,20 @@ void HTMLParserScriptRunner::ExecuteParsingBlockingScripts() {
// that is blocking scripts and the script's "ready to be parser-executed" // that is blocking scripts and the script's "ready to be parser-executed"
// flag is set.</spec> // flag is set.</spec>
// //
// These conditions correspond to isParserBlockingScriptReady() and // These conditions correspond to IsParserBlockingScriptReady().
// if it is false, executeParsingBlockingScripts() will be called later // If it is false at the time of #prepare-a-script,
// when isParserBlockingScriptReady() becomes true: // ExecuteParsingBlockingScripts() will be called later
// (1) from HTMLParserScriptRunner::executeScriptsWaitingForResources(), or // when IsParserBlockingScriptReady() might become true:
// (2) from HTMLParserScriptRunner::executeScriptsWaitingForLoad(). // - Called from HTMLParserScriptRunner::ExecuteScriptsWaitingForResources()
// when the parser's Document has no style sheet that is blocking scripts,
// - Called from HTMLParserScriptRunner::ExecuteScriptsWaitingForLoad()
// when the script's "ready to be parser-executed" flag is set, or
// - Other cases where any of the conditions isn't met or even when there are
// no longer parser blocking scripts at all.
// (For example, see the comment in ExecuteScriptsWaitingForLoad())
//
// Because we check the conditions below and do nothing if the conditions
// aren't met, it's safe to have extra ExecuteParsingBlockingScripts() calls.
while (HasParserBlockingScript() && IsParserBlockingScriptReady()) { while (HasParserBlockingScript() && IsParserBlockingScriptReady()) {
DCHECK(document_); DCHECK(document_);
DCHECK(!IsExecutingScript()); DCHECK(!IsExecutingScript());
...@@ -406,13 +414,17 @@ void HTMLParserScriptRunner::ExecuteParsingBlockingScripts() { ...@@ -406,13 +414,17 @@ void HTMLParserScriptRunner::ExecuteParsingBlockingScripts() {
} }
} }
void HTMLParserScriptRunner::ExecuteScriptsWaitingForLoad( void HTMLParserScriptRunner::ExecuteScriptsWaitingForLoad() {
PendingScript* pending_script) { // Note(https://crbug.com/1093051): ExecuteScriptsWaitingForLoad() is
// triggered asynchronously from PendingScriptFinished(pending_script), but
// the |pending_script| might be no longer the ParserBlockginScript() here,
// because it might have been evaluated or disposed after
// PendingScriptFinished() before ExecuteScriptsWaitingForLoad(). Anyway we
// call ExecuteParsingBlockingScripts(), because necessary conditions for
// evaluation are checked safely there.
TRACE_EVENT0("blink", "HTMLParserScriptRunner::executeScriptsWaitingForLoad"); TRACE_EVENT0("blink", "HTMLParserScriptRunner::executeScriptsWaitingForLoad");
DCHECK(!IsExecutingScript()); DCHECK(!IsExecutingScript());
DCHECK(HasParserBlockingScript());
DCHECK_EQ(pending_script, ParserBlockingScript());
DCHECK(ParserBlockingScript()->IsReady());
ExecuteParsingBlockingScripts(); ExecuteParsingBlockingScripts();
} }
......
...@@ -86,7 +86,7 @@ class HTMLParserScriptRunner final ...@@ -86,7 +86,7 @@ class HTMLParserScriptRunner final
// Invoked when the parsing-blocking script resource has loaded, to execute // Invoked when the parsing-blocking script resource has loaded, to execute
// parsing-blocking scripts. // parsing-blocking scripts.
void ExecuteScriptsWaitingForLoad(PendingScript*); void ExecuteScriptsWaitingForLoad();
// Invoked when all script-blocking resources (e.g., stylesheets) have loaded, // Invoked when all script-blocking resources (e.g., stylesheets) have loaded,
// to execute parsing-blocking scripts. // to execute parsing-blocking scripts.
......
...@@ -32,14 +32,13 @@ ...@@ -32,14 +32,13 @@
namespace blink { namespace blink {
class HTMLInputStream; class HTMLInputStream;
class PendingScript;
class CORE_EXPORT HTMLParserScriptRunnerHost : public GarbageCollectedMixin { class CORE_EXPORT HTMLParserScriptRunnerHost : public GarbageCollectedMixin {
public: public:
virtual ~HTMLParserScriptRunnerHost() = default; virtual ~HTMLParserScriptRunnerHost() = default;
void Trace(Visitor* visitor) const override {} void Trace(Visitor* visitor) const override {}
virtual void NotifyScriptLoaded(PendingScript*) = 0; virtual void NotifyScriptLoaded() = 0;
virtual HTMLInputStream& InputStream() = 0; virtual HTMLInputStream& InputStream() = 0;
virtual bool HasPreloadScanner() const = 0; virtual bool HasPreloadScanner() const = 0;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment