Commit 87dc11bd authored by kouhei's avatar kouhei Committed by Commit bot

[ES6 modules] ModuleTreeLinker::Instantiate shouldn't proceed on invalid context

Before this CL, ModuleTreeLinker::Instantiate assumed that it is only called for
modulator with a valid context. However, asynchronous module graph node load
completion may be triggered after the context was destroyed.

This CL fixes the issue by making ModuleTreeLinker::Instantiate fail without crashing
if the context is invalid.

BUG=594639,716935

Review-Url: https://codereview.chromium.org/2886593002
Cr-Commit-Position: refs/heads/master@{#472246}
parent 5d609a1b
<!DOCTYPE html>
<title>Module script graph fetch in flight when document is destroyed shouldn't crash.</title>
<script>
// This test expects the testrunner terminates the test without waiting for onload.
if (window.testRunner)
testRunner.dumpAsText();
</script>
<script type="module">
import { notfound } from "./404.js";
import { slow } from "./../../resources/slow-script.pl?delay=1000";
</script>
...@@ -100,6 +100,8 @@ class CORE_EXPORT Modulator : public GarbageCollectedFinalized<Modulator>, ...@@ -100,6 +100,8 @@ class CORE_EXPORT Modulator : public GarbageCollectedFinalized<Modulator>,
static KURL ResolveModuleSpecifier(const String& module_request, static KURL ResolveModuleSpecifier(const String& module_request,
const KURL& base_url); const KURL& base_url);
virtual bool HasValidContext() = 0;
virtual ScriptModule CompileModule(const String& script, virtual ScriptModule CompileModule(const String& script,
const String& url_str, const String& url_str,
AccessControlStatus) = 0; AccessControlStatus) = 0;
......
...@@ -111,6 +111,10 @@ ModuleScript* ModulatorImpl::GetFetchedModuleScript(const KURL& url) { ...@@ -111,6 +111,10 @@ ModuleScript* ModulatorImpl::GetFetchedModuleScript(const KURL& url) {
return map_->GetFetchedModuleScript(url); return map_->GetFetchedModuleScript(url);
} }
bool ModulatorImpl::HasValidContext() {
return script_state_->ContextIsValid();
}
ScriptModule ModulatorImpl::CompileModule( ScriptModule ModulatorImpl::CompileModule(
const String& provided_source, const String& provided_source,
const String& url_str, const String& url_str,
......
...@@ -55,6 +55,7 @@ class ModulatorImpl final : public Modulator { ...@@ -55,6 +55,7 @@ class ModulatorImpl final : public Modulator {
ModuleGraphLevel, ModuleGraphLevel,
SingleModuleClient*) override; SingleModuleClient*) override;
ModuleScript* GetFetchedModuleScript(const KURL&) override; ModuleScript* GetFetchedModuleScript(const KURL&) override;
bool HasValidContext() override;
void FetchNewSingleModule(const ModuleScriptFetchRequest&, void FetchNewSingleModule(const ModuleScriptFetchRequest&,
ModuleGraphLevel, ModuleGraphLevel,
ModuleScriptLoaderClient*) override; ModuleScriptLoaderClient*) override;
......
...@@ -376,6 +376,13 @@ void ModuleTreeLinker::Instantiate() { ...@@ -376,6 +376,13 @@ void ModuleTreeLinker::Instantiate() {
// https://html.spec.whatwg.org/multipage/webappapis.html#internal-module-script-graph-fetching-procedure // https://html.spec.whatwg.org/multipage/webappapis.html#internal-module-script-graph-fetching-procedure
// [nospec] Abort the steps if the browsing context is discarded.
if (!modulator_->HasValidContext()) {
descendants_module_script_ = nullptr;
AdvanceState(State::kFinished);
return;
}
// Step 5. Let instantiationStatus be null. // Step 5. Let instantiationStatus be null.
// Note: The |error| variable corresponds to spec variable // Note: The |error| variable corresponds to spec variable
// "instantiationStatus". If |error| is empty, it indicates successful // "instantiationStatus". If |error| is empty, it indicates successful
......
...@@ -91,6 +91,10 @@ void DummyModulator::FetchNewSingleModule(const ModuleScriptFetchRequest&, ...@@ -91,6 +91,10 @@ void DummyModulator::FetchNewSingleModule(const ModuleScriptFetchRequest&,
NOTREACHED(); NOTREACHED();
} }
bool DummyModulator::HasValidContext() {
return true;
}
ScriptModule DummyModulator::CompileModule(const String& script, ScriptModule DummyModulator::CompileModule(const String& script,
const String& url_str, const String& url_str,
AccessControlStatus) { AccessControlStatus) {
......
...@@ -50,6 +50,7 @@ class DummyModulator : public Modulator { ...@@ -50,6 +50,7 @@ class DummyModulator : public Modulator {
void FetchNewSingleModule(const ModuleScriptFetchRequest&, void FetchNewSingleModule(const ModuleScriptFetchRequest&,
ModuleGraphLevel, ModuleGraphLevel,
ModuleScriptLoaderClient*) override; ModuleScriptLoaderClient*) override;
bool HasValidContext() override;
ScriptModule CompileModule(const String& script, ScriptModule CompileModule(const String& script,
const String& url_str, const String& url_str,
AccessControlStatus) override; AccessControlStatus) override;
......
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