[ES6 modules] ModuleMap::GetFetchedModuleScript to return nullptr when entry not found / "fetching"

Before this CL, we asserted that ModuleMap::GetFetchedModuleScript always queried load completed module scripts, but it was not always the case. This CL changes the member function to return nullptr if the entry doesn't exist or the entry is being fetched. This CL fixes a crash that happens when we early-exit module script graph fetch by a sub-graph instantiation failed, but there are still partial subgraph in flight. BUG=718442,594639 Review-Url: https://codereview.chromium.org/2860993002 Cr-Commit-Position: refs/heads/master@{#472192}

[ES6 modules] ModuleMap::GetFetchedModuleScript to return nullptr when entry not found / "fetching"
Before this CL, we asserted that ModuleMap::GetFetchedModuleScript always queried load completed module scripts, but it was not always the case. This CL changes the member function to return nullptr if the entry doesn't exist or the entry is being fetched. This CL fixes a crash that happens when we early-exit module script graph fetch by a sub-graph instantiation failed, but there are still partial subgraph in flight. BUG=718442,594639 Review-Url: https://codereview.chromium.org/2860993002 Cr-Commit-Position: refs/heads/master@{#472192}
1f92cfbd · kouhei · Commit bot · 883ea6f5 · 1f92cfbd · 1f92cfbd
Commit 1f92cfbd authored May 16, 2017 by kouhei Committed by Commit bot May 16, 2017
5 changed files
--- a/third_party/WebKit/LayoutTests/external/wpt/html/semantics/scripting-1/the-script-element/module/import-subgraph-404.html
+++ b/third_party/WebKit/LayoutTests/external/wpt/html/semantics/scripting-1/the-script-element/module/import-subgraph-404.html
+<!DOCTYPE html>
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<script type="module">
+import { delayedLoaded }  from "./resources/delayed-modulescript.py";
+import { A } from "./404.js";
+window.loadSuccess = delayedLoaded;
+</script>
+<script type="module">
+test(function () {
+    assert_equals(window.loadSuccess, undefined,
+      "module tree w/ its sub graph 404 should fail to load without crashing");
+}, "Import a module graph w/ sub-graph 404.");
+</script>
--- a/third_party/WebKit/LayoutTests/external/wpt/html/semantics/scripting-1/the-script-element/module/resources/delayed-modulescript.py
+++ b/third_party/WebKit/LayoutTests/external/wpt/html/semantics/scripting-1/the-script-element/module/resources/delayed-modulescript.py
+import time
+
+def main(request, response):
+    delay = float(request.GET.first("ms", 500))
+    time.sleep(delay / 1E3);
+
+    return [("Content-type", "text/javascript")], "export let delayedLoaded = true;"
--- a/third_party/WebKit/Source/core/dom/Modulator.h
+++ b/third_party/WebKit/Source/core/dom/Modulator.h
@@ -92,7 +92,8 @@ class CORE_EXPORT Modulator : public GarbageCollectedFinalized<Modulator>,

  // Synchronously retrieves a single module script from existing module map
  // entry.
-  // Note: returns nullptr if the module map entry is still "fetching".
+  // Note: returns nullptr if the module map entry doesn't exist, or
+  // is still "fetching".
  virtual ModuleScript* GetFetchedModuleScript(const KURL&) = 0;

  // https://html.spec.whatwg.org/#resolve-a-module-specifier

--- a/third_party/WebKit/Source/core/dom/ModuleMap.cpp
+++ b/third_party/WebKit/Source/core/dom/ModuleMap.cpp
@@ -96,7 +96,6 @@ void ModuleMap::Entry::NotifyNewSingleModuleFinished(
 }

 ModuleScript* ModuleMap::Entry::GetModuleScript() const {
-  DCHECK(!is_fetching_);
  return module_script_.Get();
 }

@@ -146,7 +145,8 @@ void ModuleMap::FetchSingleModuleScript(const ModuleScriptFetchRequest& request,

 ModuleScript* ModuleMap::GetFetchedModuleScript(const KURL& url) const {
  MapImpl::const_iterator it = map_.find(url);
-  CHECK_NE(it, map_.end());
+  if (it == map_.end())
+    return nullptr;
  return it->value->GetModuleScript();
 }


--- a/third_party/WebKit/Source/core/dom/ModuleMap.h
+++ b/third_party/WebKit/Source/core/dom/ModuleMap.h
@@ -41,8 +41,8 @@ class CORE_EXPORT ModuleMap final : public GarbageCollected<ModuleMap>,
                               SingleModuleClient*);

  // Synchronously get the ModuleScript for a given URL.
-  // Note: fetchSingleModuleScript of the ModuleScript must be complete before
-  // calling this.
+  // If the URL wasn't fetched, or is currently being fetched, this returns a
+  // nullptr.
  ModuleScript* GetFetchedModuleScript(const KURL&) const;

  Modulator* GetModulator() { return modulator_; }