Enable module scripts in ScriptLoader

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

 // Copyright 2017 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "bindings/core/v8/ScriptModule.h"
 
 #include "bindings/core/v8/V8Binding.h"
 #include "core/dom/Modulator.h"
+#include "core/dom/ModuleScript.h"
 #include "core/dom/ScriptModuleResolver.h"
 
 namespace blink {
 
 ScriptModule::ScriptModule(v8::Isolate* isolate, v8::Local<v8::Module> module)
     : module_(SharedPersistent<v8::Module>::Create(module, isolate)),
       identity_hash_(static_cast<unsigned>(module->GetIdentityHash())) {
   DCHECK(!module_->IsEmpty());
 }
 
@@ skipping 29 matching lines @@
   bool success = module_->NewLocal(script_state->GetIsolate())
                      ->Instantiate(context, &ResolveModuleCallback);
   if (!success) {
     DCHECK(try_catch.HasCaught());
     return ScriptValue(script_state, try_catch.Exception());
   }
   DCHECK(!try_catch.HasCaught());
   return ScriptValue();
 }
 
-void ScriptModule::Evaluate(ScriptState* script_state) {
+// https://html.spec.whatwg.org/#run-a-module-script
+void ScriptModule::Evaluate(ScriptState* script_state,
+                            const ModuleScript* moduleScript) {
   v8::Isolate* isolate = script_state->GetIsolate();
   v8::TryCatch try_catch(isolate);
   try_catch.SetVerbose(true);
   v8::Local<v8::Value> result;
+
+  // 3. "If s's instantiation state is "errored", then report the exception

[hiroshige, 2017/04/12 19:57:05]

Where should this logic be placed?
(Note that this logic is not tested and thus significant changes might be done
later, but I'd like to start the discussion of the place of this logic)

I temporarily put this logic here, because
- this step require |isolate|, and I uses |isolate| at Line 62 here.
- In the classic script case, ClassicScript::RunScript() is implemented totally
in bindings/core/v8, so having this logic here in bindings/core/v8 might be
consistent.

An alternative is to place this in ModuleScript, as we already get ScriptState
there via ToScriptStateForMainWorld(frame), and we can get isolate.

But this makes me think about more general question:
Which script state and isolate should we use in module scripts?
I expect it is Modulator.
So I expect ModuleScript should have a direct reference to Modulator, removing
the following comments in ModuleScript.h:
// Note: A "module script"'s "setttings object" is ommitted, as we currently
// always have access to the corresponding Modulator when operating on a
// ModuleScript instance.

(also related to https://github.com/whatwg/html/issues/2469)

[kouhei (in TOK), 2017/04/13 02:28:36]

I'd put this logic in ModulatorImpl
This path is also fine for me. I don't have a strong opinion here and trust your
judgement :)

[hiroshige, 2017/04/13 23:53:15]

Splitted the ModulatorImpl change into
https://codereview.chromium.org/2819733002, and making ModuleScript to have
Modulator reference in https://codereview.chromium.org/2823483002.

+  //     given by s's instantiation error for s and abort these steps."
+  if (moduleScript->InstantiationState() ==
+      ModuleInstantiationState::kErrored) {
+    V8ThrowException::ThrowException(
+        isolate, moduleScript->CreateInstantiationError(isolate));
+    return;
+  }
+
+  // 4. "Assert: s's instantiation state is "instantiated" (and thus its
+  //     module record is not null)."
+  DCHECK_EQ(moduleScript->InstantiationState(),
+            ModuleInstantiationState::kInstantiated);
+
+  // 5. "Let record be s's module record."
+  const ScriptModule& record = moduleScript->Record();
+  DCHECK(!record.IsNull());
+
+  // 6.--9.?
   if (!V8Call(
-          V8ScriptRunner::EvaluateModule(module_->NewLocal(isolate),
+          V8ScriptRunner::EvaluateModule(record.module_->NewLocal(isolate),
                                          script_state->GetContext(), isolate),
           result, try_catch)) {
     // TODO(adamk): report error
   }
 }
 
 Vector<String> ScriptModule::ModuleRequests(ScriptState* script_state) {
   if (IsNull())
     return Vector<String>();
 
@@ skipping 26 matching lines @@
   if (resolved.IsNull()) {
     DCHECK(exception_state.HadException());
     return v8::MaybeLocal<v8::Module>();
   }
 
   DCHECK(!exception_state.HadException());
   return v8::MaybeLocal<v8::Module>(resolved.module_->NewLocal(isolate));
 }
 
 }  // namespace blink