Don't expect errors in ResolveModuleCallback.

This CL updates ResolveModuleCallback to reflect the HTML specification
changes at whatwg/html#2674.  Concretely, it
replaces certain error handling cases with assertions that these errors
can (no longer) happen.

Bug: 
Change-Id: I66490625652c556b0e26218d9fc1015b694c680f
Reviewed-on: https://chromium-review.googlesource.com/595973
Commit-Queue: Georg Neis <neis@chromium.org>
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Cr-Commit-Position: refs/heads/master@{#492286}

Author: GeorgNeis

third_party/WebKit/Source/bindings/core/v8/ScriptModule.cpp

@@ -17,7 +17,7 @@ const char* ScriptModuleStateToString(ScriptModuleState state) {
     case ScriptModuleState::kUninstantiated:
       return "uninstantiated";
     case ScriptModuleState::kInstantiating:
-      return "instatinating";
+      return "instantinating";
     case ScriptModuleState::kInstantiated:
       return "instantiated";
     case ScriptModuleState::kEvaluating:
@@ -202,13 +202,9 @@ v8::MaybeLocal<v8::Module> ScriptModule::ResolveModuleCallback(
                                  "ScriptModule", "resolveModuleCallback");
   ScriptModule resolved = modulator->GetScriptModuleResolver()->Resolve(
       ToCoreStringWithNullCheck(specifier), referrer_record, exception_state);
-  if (resolved.IsNull()) {
-    DCHECK(exception_state.HadException());
-    return v8::MaybeLocal<v8::Module>();
-  }
-
+  DCHECK(!resolved.IsNull());
   DCHECK(!exception_state.HadException());
-  return v8::MaybeLocal<v8::Module>(resolved.module_->NewLocal(isolate));
+  return resolved.module_->NewLocal(isolate);
 }
 
 }  // namespace blink

third_party/WebKit/Source/core/dom/ScriptModuleResolverImpl.cpp

@@ -44,7 +44,7 @@ ScriptModule ScriptModuleResolverImpl::Resolve(
     const String& specifier,
     const ScriptModule& referrer,
     ExceptionState& exception_state) {
-  // https://html.spec.whatwg.org/commit-snapshots/f8e75a974ed9185e5b462bc5b2dfb32034bd1145#hostresolveimportedmodule(referencingmodule,-specifier)
+  // https://html.spec.whatwg.org/multipage/webappapis.html#hostresolveimportedmodule(referencingmodule,-specifier)
   DVLOG(1) << "ScriptModuleResolverImpl::resolve(specifier=\"" << specifier
            << ", referrer.hash=" << ScriptModuleHash::GetHash(referrer) << ")";
 
@@ -59,42 +59,26 @@ ScriptModule ScriptModuleResolverImpl::Resolve(
   // (Modulator) from context where HostResolveImportedModule was called.
 
   // Step 3. Let url be the result of resolving a module specifier given
-  // referencing module script and specifier. If the result is failure, then
-  // throw a TypeError exception and abort these steps.
+  // referencing module script and specifier.
   KURL url =
       Modulator::ResolveModuleSpecifier(specifier, referrer_module->BaseURL());
-  if (!url.IsValid()) {
-    exception_state.ThrowTypeError("Failed to resolve module specifier '" +
-                                   specifier + "'");
-    return ScriptModule();
-  }
-
-  // Step 4. Let resolved module script be moduleMap[url]. If no such entry
-  // exists, or if resolved module script is null or "fetching", then throw a
-  // TypeError exception and abort these steps.
+
+  // Step 4. Assert: url is never failure, because resolving a module specifier
+  // must have been previously successful with these same two arguments.
+  DCHECK(url.IsValid());
+
+  // Step 5. Let resolved module script be moduleMap[url]. (This entry must
+  // exist for us to have gotten to this point.)
   ModuleScript* module_script = modulator_->GetFetchedModuleScript(url);
-  if (!module_script) {
-    exception_state.ThrowTypeError(
-        "Failed to load module script for module specifier '" + specifier +
-        "'");
-    return ScriptModule();
-  }
-
-  // Step 5. If resolved module script's instantiation state is "errored", then
-  // throw resolved module script's instantiation error.
-  // TODO(kouhei): Update spec references.
-  if (module_script->IsErrored()) {
-    ScriptValue error = modulator_->GetError(module_script);
-    exception_state.RethrowV8Exception(error.V8Value());
-    return ScriptModule();
-  }
-
-  // Step 6. Assert: resolved module script's module record is not null.
-  ScriptModule record = module_script->Record();
-  CHECK(!record.IsNull());
-
-  // Step 7. Return resolved module script's module record.
-  return record;
+
+  // Step 6. Assert: resolved module script is a module script (i.e., is not
+  // null or "fetching").
+  // Step 7. Assert: resolved module script is not errored.
+  DCHECK(module_script);
+  CHECK(!module_script->IsErrored());
+
+  // Step 8. Return resolved module script's module record.
+  return module_script->Record();
 }
 
 void ScriptModuleResolverImpl::ContextDestroyed(ExecutionContext*) {

third_party/WebKit/Source/core/dom/ScriptModuleResolverImplTest.cpp

@@ -153,78 +153,4 @@ TEST_F(ScriptModuleResolverImplTest, RegisterResolveSuccess) {
       << "Unexpectedly fetched URL: " << modulator_->FetchedUrl().GetString();
 }
 
-TEST_F(ScriptModuleResolverImplTest, ResolveInvalidModuleSpecifier) {
-  V8TestingScope scope;
-  ScriptModuleResolver* resolver = ScriptModuleResolverImpl::Create(
-      Modulator(), scope.GetExecutionContext());
-  Modulator()->SetScriptState(scope.GetScriptState());
-
-  ModuleScript* referrer_module_script =
-      CreateReferrerModuleScript(modulator_, scope);
-  resolver->RegisterModuleScript(referrer_module_script);
-
-  ModuleScript* target_module_script =
-      CreateTargetModuleScript(modulator_, scope);
-  Modulator()->SetModuleScript(target_module_script);
-
-  ScriptModule resolved = resolver->Resolve(
-      "invalid", referrer_module_script->Record(), scope.GetExceptionState());
-  EXPECT_TRUE(scope.GetExceptionState().HadException());
-  EXPECT_EQ(kV8TypeError, scope.GetExceptionState().Code());
-  EXPECT_TRUE(resolved.IsNull());
-  EXPECT_EQ(0, modulator_->GetFetchedModuleScriptCalled());
-}
-
-TEST_F(ScriptModuleResolverImplTest, ResolveLoadFailedModule) {
-  V8TestingScope scope;
-  ScriptModuleResolver* resolver = ScriptModuleResolverImpl::Create(
-      Modulator(), scope.GetExecutionContext());
-  Modulator()->SetScriptState(scope.GetScriptState());
-
-  ModuleScript* referrer_module_script =
-      CreateReferrerModuleScript(modulator_, scope);
-  resolver->RegisterModuleScript(referrer_module_script);
-
-  ModuleScript* target_module_script =
-      CreateTargetModuleScript(modulator_, scope);
-  // Set Modulator::getFetchedModuleScript to return nullptr, which represents
-  // that the target module failed to load.
-  Modulator()->SetModuleScript(nullptr);
-
-  ScriptModule resolved =
-      resolver->Resolve("./target.js", referrer_module_script->Record(),
-                        scope.GetExceptionState());
-  EXPECT_TRUE(scope.GetExceptionState().HadException());
-  EXPECT_EQ(kV8TypeError, scope.GetExceptionState().Code());
-  EXPECT_TRUE(resolved.IsNull());
-  EXPECT_EQ(1, modulator_->GetFetchedModuleScriptCalled());
-  EXPECT_EQ(modulator_->FetchedUrl(), target_module_script->BaseURL())
-      << "Unexpectedly fetched URL: " << modulator_->FetchedUrl().GetString();
-}
-
-TEST_F(ScriptModuleResolverImplTest, ResolveInstantiationFailedModule) {
-  V8TestingScope scope;
-  ScriptModuleResolver* resolver = ScriptModuleResolverImpl::Create(
-      Modulator(), scope.GetExecutionContext());
-  Modulator()->SetScriptState(scope.GetScriptState());
-
-  ModuleScript* referrer_module_script =
-      CreateReferrerModuleScript(modulator_, scope);
-  resolver->RegisterModuleScript(referrer_module_script);
-
-  ModuleScript* target_module_script =
-      CreateTargetModuleScript(modulator_, scope, ScriptModuleState::kErrored);
-  Modulator()->SetModuleScript(target_module_script);
-
-  ScriptModule resolved =
-      resolver->Resolve("./target.js", referrer_module_script->Record(),
-                        scope.GetExceptionState());
-  EXPECT_TRUE(scope.GetExceptionState().HadException());
-  EXPECT_EQ(kUnknownError, scope.GetExceptionState().Code());
-  EXPECT_TRUE(resolved.IsNull());
-  EXPECT_EQ(1, modulator_->GetFetchedModuleScriptCalled());
-  EXPECT_EQ(modulator_->FetchedUrl(), target_module_script->BaseURL())
-      << "Unexpectedly fetched URL: " << modulator_->FetchedUrl().GetString();
-}
-
 }  // namespace blink