[ES6 modules] Return previous error when an instantiation is reattempt.

third_party/WebKit/Source/core/loader/modulescript/ModuleTreeLinkerTest.cpp

Index: third_party/WebKit/Source/core/loader/modulescript/ModuleTreeLinkerTest.cpp
diff --git a/third_party/WebKit/Source/core/loader/modulescript/ModuleTreeLinkerTest.cpp b/third_party/WebKit/Source/core/loader/modulescript/ModuleTreeLinkerTest.cpp
index 17187ee5de030d910f707eaa6f5f9fcd13bccbf8..c203ce5d3738105a73b24bf075ee050bc5e98954 100644
--- a/third_party/WebKit/Source/core/loader/modulescript/ModuleTreeLinkerTest.cpp
+++ b/third_party/WebKit/Source/core/loader/modulescript/ModuleTreeLinkerTest.cpp
@@ -62,7 +62,8 @@ class ModuleTreeLinkerTestModulator final : public DummyModulator {
   // Resolve last |Modulator::FetchSingle()| call.
   ModuleScript* ResolveSingleModuleScriptFetch(
       const KURL& url,
-      const Vector<String>& dependency_module_requests) {
+      const Vector<String>& dependency_module_requests,
+      ModuleInstantiationState state) {
     ScriptState::Scope scope(script_state_.Get());
 
     StringBuilder source_text;
@@ -85,6 +86,13 @@ class ModuleTreeLinkerTestModulator final : public DummyModulator {
     auto result_map = module_map_.insert(url, module_script);
     EXPECT_TRUE(result_map.is_new_entry);
 
+    if (state == ModuleInstantiationState::kErrored) {
+      v8::Local<v8::Value> error = V8ThrowException::CreateError(
+          script_state_->GetIsolate(), "Instantiation failure.");
+      module_script->SetInstantiationErrorAndClearRecord(
+          ScriptValue(script_state_.Get(), error));
+    }
+
     EXPECT_EQ(url, pending_request_url_);

[hiroshige, 2017/04/27 22:56:40]

EXPECT_EQ(state, module_script->InstantiationState()) around here?

[kouhei (in TOK), 2017/04/28 00:55:09]

Done.

     EXPECT_TRUE(pending_client_);
     pending_client_->NotifyModuleLoadFinished(module_script);
@@ -176,8 +184,18 @@ class ModuleTreeLinkerTestModulator final : public DummyModulator {
     return ScriptValue();
   }
 
+  ScriptValue GetInstantiationError(const ModuleScript*) override {
+    ScriptState::Scope scope(script_state_.Get());

[hiroshige, 2017/04/27 22:56:40]

Can we make this just the same as ModulatorImpl's one?

[kouhei (in TOK), 2017/04/28 00:55:09]

Done.

+    v8::Local<v8::Value> error = V8ThrowException::CreateError(
+        script_state_->GetIsolate(), "Previous instantiation failure.");
+    return ScriptValue(script_state_.Get(), error);
+  }
+
   Vector<String> ModuleRequestsFromScriptModule(
       ScriptModule script_module) override {
+    if (script_module.IsNull())
+      return Vector<String>();
+
     const auto& it = dependency_module_requests_map_.Find(script_module);
     if (it == dependency_module_requests_map_.end())
       return Vector<String>();
@@ -240,7 +258,8 @@ TEST_F(ModuleTreeLinkerTest, FetchTreeNoDeps) {
       << "ModuleTreeLinker should always finish asynchronously.";
   EXPECT_FALSE(client->GetModuleScript());
 
-  GetModulator()->ResolveSingleModuleScriptFetch(url, {});
+  GetModulator()->ResolveSingleModuleScriptFetch(
+      url, {}, ModuleInstantiationState::kUninstantiated);
   EXPECT_TRUE(client->WasNotifyFinished());
   ASSERT_TRUE(client->GetModuleScript());
   EXPECT_EQ(client->GetModuleScript()->InstantiationState(),
@@ -264,13 +283,37 @@ TEST_F(ModuleTreeLinkerTest, FetchTreeInstantiationFailure) {
       << "ModuleTreeLinker should always finish asynchronously.";
   EXPECT_FALSE(client->GetModuleScript());
 
-  GetModulator()->ResolveSingleModuleScriptFetch(url, {});
+  GetModulator()->ResolveSingleModuleScriptFetch(
+      url, {}, ModuleInstantiationState::kUninstantiated);
   EXPECT_TRUE(client->WasNotifyFinished());
   ASSERT_TRUE(client->GetModuleScript());
   EXPECT_EQ(client->GetModuleScript()->InstantiationState(),
             ModuleInstantiationState::kErrored);
 }
 
+TEST_F(ModuleTreeLinkerTest, FetchTreePreviousInstantiationFailure) {

[hiroshige, 2017/04/27 22:56:40]

This looks almost the same as FetchTreeInstantiationFailure.
Probably we should do an additional Fetch() call at the end of this test.

[kouhei (in TOK), 2017/04/28 00:55:09]

It is similar, but I'd like to keep them separate.
- ModuleTreeLinkerModulator doesn't have a "real" module map to dedupe
previously resolved module fetch.
- This test is expected to fail without Modulator::InstantiateModule().

[hiroshige, 2017/04/28 01:28:35]

Oh, understood.
So
Emulates "previous instantiation failure", and
this tests the case where Fetch() results in kErrored when *previous*
instantiation is failed.

And FetchTreeInstantiationFailure tests the case where
Fetch() results in kErrored when *current* instantiation is failed
emulated by SetInstantiateShouldFail(true);

Probably adding some comments might be helpful.

+  ModuleTreeLinkerRegistry* registry = ModuleTreeLinkerRegistry::Create();
+
+  KURL url(kParsedURLString, "http://example.com/root.js");
+  ModuleScriptFetchRequest module_request(
+      url, String(), kParserInserted, WebURLRequest::kFetchCredentialsModeOmit);
+  TestModuleTreeClient* client = new TestModuleTreeClient;
+  registry->Fetch(module_request, AncestorList(),
+                  ModuleGraphLevel::kTopLevelModuleFetch, GetModulator(),
+                  client);
+
+  EXPECT_FALSE(client->WasNotifyFinished())
+      << "ModuleTreeLinker should always finish asynchronously.";
+  EXPECT_FALSE(client->GetModuleScript());
+
+  GetModulator()->ResolveSingleModuleScriptFetch(
+      url, {}, ModuleInstantiationState::kErrored);
+  EXPECT_TRUE(client->WasNotifyFinished());
+  ASSERT_TRUE(client->GetModuleScript());
+  EXPECT_EQ(ModuleInstantiationState::kErrored,
+            client->GetModuleScript()->InstantiationState());
+}
+
 TEST_F(ModuleTreeLinkerTest, FetchTreeWithSingleDependency) {
   ModuleTreeLinkerRegistry* registry = ModuleTreeLinkerRegistry::Create();
 
@@ -286,7 +329,8 @@ TEST_F(ModuleTreeLinkerTest, FetchTreeWithSingleDependency) {
       << "ModuleTreeLinker should always finish asynchronously.";
   EXPECT_FALSE(client->GetModuleScript());
 
-  GetModulator()->ResolveSingleModuleScriptFetch(url, {"./dep1.js"});
+  GetModulator()->ResolveSingleModuleScriptFetch(
+      url, {"./dep1.js"}, ModuleInstantiationState::kUninstantiated);
   EXPECT_FALSE(client->WasNotifyFinished());
 
   KURL url_dep1(kParsedURLString, "http://example.com/dep1.js");
@@ -320,7 +364,8 @@ TEST_F(ModuleTreeLinkerTest, FetchTreeWith3Deps) {
   EXPECT_FALSE(client->GetModuleScript());
 
   GetModulator()->ResolveSingleModuleScriptFetch(
-      url, {"./dep1.js", "./dep2.js", "./dep3.js"});
+      url, {"./dep1.js", "./dep2.js", "./dep3.js"},
+      ModuleInstantiationState::kUninstantiated);
   EXPECT_FALSE(client->WasNotifyFinished());
 
   Vector<KURL> url_deps;
@@ -370,7 +415,8 @@ TEST_F(ModuleTreeLinkerTest, FetchTreeWith3Deps1Fail) {
   EXPECT_FALSE(client->GetModuleScript());
 
   GetModulator()->ResolveSingleModuleScriptFetch(
-      url, {"./dep1.js", "./dep2.js", "./dep3.js"});
+      url, {"./dep1.js", "./dep2.js", "./dep3.js"},
+      ModuleInstantiationState::kUninstantiated);
   EXPECT_FALSE(client->WasNotifyFinished());
 
   Vector<KURL> url_deps;
@@ -429,7 +475,8 @@ TEST_F(ModuleTreeLinkerTest, FetchDependencyTree) {
       << "ModuleTreeLinker should always finish asynchronously.";
   EXPECT_FALSE(client->GetModuleScript());
 
-  GetModulator()->ResolveSingleModuleScriptFetch(url, {"./depth2.js"});
+  GetModulator()->ResolveSingleModuleScriptFetch(
+      url, {"./depth2.js"}, ModuleInstantiationState::kUninstantiated);
 
   KURL url_dep2(kParsedURLString, "http://example.com/depth2.js");
   auto ancestor_list = GetModulator()->GetAncestorListForTreeFetch(url_dep2);