Move ScriptState::Scope etc. around V8ScriptRunner::EvaluateModule()

Before this CL, for ModuleScript::RunScript(), we entered:

- v8::HandleScope in ModuleScript::RunScript(),
- v8::Context in ModuleScript::RunScript(),
- v8::EscapableHandleScope in V8ScriptRunner::EvaluateModule(), and
- v8::Context in V8ScriptRunner::EvaluateModule().

After this CL, we enter:

- v8::HandleScope in ModuleScript::RunScript(), and
- v8::Context in V8ScriptRunner::EvaluateModule().

This is to remove duplicated scopes and thus simplify
the semantics: callers of V8ScriptRunner::EvaluateModule()
should enter v8::HandleScope while EvaluateModule() enters
v8::Context.

Bug: 1111134, 1151165
Change-Id: I84be5b89038fa5fb9f20a36ac9e4996c0e3c7510
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2476693
Commit-Queue: Hiroshige Hayashizaki <hiroshige@chromium.org>
Reviewed-by: Yuki Shiino <yukishiino@chromium.org>
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Cr-Commit-Position: refs/heads/master@{#829556}

third_party/blink/renderer/bindings/core/v8/script_evaluation_result.cc

@@ -81,12 +81,6 @@ ScriptEvaluationResult ScriptEvaluationResult::FromModuleAborted() {
                                 ResultType::kAborted, {});
 }
 
-ScriptEvaluationResult& ScriptEvaluationResult::Escape(
-    ScriptState::EscapableScope* scope) {
-  value_ = scope->Escape(value_);
-  return *this;
-}
-
 v8::Local<v8::Value> ScriptEvaluationResult::GetSuccessValue() const {
   DCHECK_EQ(result_type_, ResultType::kSuccess);
   DCHECK(!value_.IsEmpty());

third_party/blink/renderer/bindings/core/v8/script_evaluation_result.h

@@ -96,8 +96,6 @@ class CORE_EXPORT ScriptEvaluationResult final {
       v8::Local<v8::Value> exception);
   static ScriptEvaluationResult FromModuleAborted();
 
-  ScriptEvaluationResult& Escape(ScriptState::EscapableScope* scope);
-
   ResultType GetResultType() const { return result_type_; }
 
   // Can be called only when GetResultType() == kSuccess.

third_party/blink/renderer/bindings/core/v8/v8_script_runner.cc

@@ -671,6 +671,10 @@ ScriptEvaluationResult V8ScriptRunner::EvaluateModule(
   ExecutionContext* execution_context = ExecutionContext::From(script_state);
   v8::Isolate* isolate = script_state->GetIsolate();
 
+  // TODO(crbug.com/1151165): Ideally v8::Context should be entered before
+  // CanExecuteScripts().
+  v8::Context::Scope scope(script_state->GetContext());
+
   // <spec step="3">Check if we can run script with settings. If this returns
   // "do not run" then return NormalCompletion(empty).</spec>
   if (!execution_context->CanExecuteScripts(kAboutToExecuteScript)) {
@@ -683,7 +687,6 @@ ScriptEvaluationResult V8ScriptRunner::EvaluateModule(
   v8::MicrotasksScope microtasks_scope(isolate,
                                        ToMicrotaskQueue(execution_context),
                                        v8::MicrotasksScope::kRunMicrotasks);
-  ScriptState::EscapableScope scope(script_state);
 
   // Without TLA: <spec step="5">Let evaluationStatus be null.</spec>
   ScriptEvaluationResult result = ScriptEvaluationResult::FromModuleNotRun();
@@ -784,9 +787,9 @@ ScriptEvaluationResult V8ScriptRunner::EvaluateModule(
   }
 
   // <spec step="8">Clean up after running script with settings.</spec>
-  // - Partially implement in MicrotaskScope destructor and the
-  // - ScriptState::EscapableScope destructor.
-  return result.Escape(&scope);
+  // Partially implemented in MicrotaskScope destructor and the
+  // v8::Context::Scope destructor.
+  return result;
 }
 
 void V8ScriptRunner::ReportException(v8::Isolate* isolate,

third_party/blink/renderer/core/script/module_script.cc

@@ -106,7 +106,7 @@ void ModuleScript::Trace(Visitor* visitor) const {
 void ModuleScript::RunScript(LocalDOMWindow*) {
   // We need a HandleScope for the `ScriptEvaluationResult` returned from
   // `RunScriptAndReturnValue`.
-  ScriptState::Scope scope(SettingsObject()->GetScriptState());
+  v8::HandleScope scope(SettingsObject()->GetScriptState()->GetIsolate());
   DVLOG(1) << *this << "::RunScript()";
   ignore_result(RunScriptAndReturnValue());
 }
@@ -115,7 +115,7 @@ bool ModuleScript::RunScriptOnWorkerOrWorklet(
     WorkerOrWorkletGlobalScope& global_scope) {
   // We need a HandleScope for the `ScriptEvaluationResult` returned from
   // `RunScriptAndReturnValue`.
-  ScriptState::Scope scope(SettingsObject()->GetScriptState());
+  v8::HandleScope scope(SettingsObject()->GetScriptState()->GetIsolate());
   DCHECK(global_scope.IsContextThread());
 
   // TODO(nhiroki): Catch an error when an evaluation error happens.