Consider module script evaluation with an exception as a failure

Previously, when an exception is thrown,
ModulatorImplBase::ExecuteModule() returns
ModuleEvaluationResult::Empty(),
when CaptureEvalErrorFlag::kReport is used and
top-level await is not enabled.
This is considered as success, but this is inconsistent with

- ModuleRecord::Evaluate() that returns
  ModuleEvaluationResult::FromException()
  (that is considered as non-success), and
- Classic script evaluation.

This CL makes ModulatorImplBase::ExecuteModule() return
ModuleEvaluationResult with an exception in such cases.

Behavior change:
This changes WorkerReportingProxy::DidEvaluateTopLevelScript()'s
|is_success| value from true to false when a module script
have parse/evaluation/etc. errors.
This makes serviceworker registration fails for such scripts,
fixing crbug.com/1129795.

This CL fixes content/test/data/service_worker/worker.js
to use `globalThis` instead of `this`, because on module scripts
`this` can't be used to get the global object.
(Previously, the test using this script
ServiceWorkerVersionBrowserTest.StartInstalledModuleScriptWhileOffline
unexpectedly threw an exception during evaluation of `worker.js`
as a module service worker top-level script, but didn't fail
due to this issue)

Bug: 1129795, 1129743, 1111134
Change-Id: I5443539c201e1ab220fd17f9291b307432853450
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2419054
Commit-Queue: Hiroshige Hayashizaki <hiroshige@chromium.org>
Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Reviewed-by: Dominic Farolino <dom@chromium.org>
Cr-Commit-Position: refs/heads/master@{#810141}

content/test/data/service_worker/worker.js

@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-this.oninstall = function(event) {};
+globalThis.oninstall = function(event) {};
 
 // So sorry about this waste of bytes:
 // Filler comment, to trigger code caching heuristic (script > 1K.)

third_party/blink/renderer/core/script/modulator.h

@@ -203,10 +203,11 @@ class CORE_EXPORT Modulator : public GarbageCollected<Modulator>,
   // - When "rethrow errors" is to be set, use kCapture for EvaluateModule().
   // Then EvaluateModule() wraps exceptions in a ModuleEvaluationResult instead
   // of throwing it and the caller should rethrow the exception.
-  // - When "rethrow errors" is not to be set, use kReport. EvaluateModule()
-  // "report the error" inside it (if any), and returns either a
-  // ModuleEvaluationResult that is empty or contains the successful
-  // evaluation result.
+  // - When "rethrow errors" is not to be set, use kReport. If there is an error
+  // to throw, EvaluateModule() "report the error" inside it, and returns
+  // ModuleEvaluationResult wrapping the error. Otherwise, it returns either a
+  // ModuleEvaluationResult that is empty or contains the successful evaluation
+  // result.
   virtual ModuleEvaluationResult ExecuteModule(ModuleScript*,
                                                CaptureEvalErrorFlag) = 0;
 

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

@@ -336,6 +336,7 @@ void ModulatorImplBase::ProduceCacheModuleTree(
 }
 
 // <specdef href="https://html.spec.whatwg.org/C/#run-a-module-script">
+// Spec with TLA: https://github.com/whatwg/html/pull/4352
 ModuleEvaluationResult ModulatorImplBase::ExecuteModule(
     ModuleScript* module_script,
     CaptureEvalErrorFlag capture_error) {
@@ -409,24 +410,23 @@ ModuleEvaluationResult ModulatorImplBase::ExecuteModule(
       result.GetPromise(script_state_)
           .Then(v8::Local<v8::Function>(), callback_failure);
     }
-    return result.Escape(&scope);
   } else {
     // <spec step="8">If evaluationStatus is an abrupt completion, then:</spec>
     if (result.IsException()) {
       // <spec step="8.1">If rethrow errors is true, rethrow the exception given
       // by evaluationStatus.[[Value]].</spec>
-      if (capture_error == CaptureEvalErrorFlag::kCapture)
-        return result.Escape(&scope);
-
-      // <spec step="8.2">Otherwise, report the exception given by
-      // evaluationStatus.[[Value]] for script.</spec>
-      ModuleRecord::ReportException(script_state_, result.GetException());
+      if (capture_error == CaptureEvalErrorFlag::kReport) {
+        // <spec step="8.2">Otherwise, report the exception given by
+        // evaluationStatus.[[Value]] for script.</spec>
+        ModuleRecord::ReportException(script_state_, result.GetException());
+      }
     }
-    // <spec step="8">Clean up after running script with settings.</spec>
-    // - Partially implement in MicrotaskScope destructor and the
-    // - ScriptState::EscapableScope destructor.
-    return ModuleEvaluationResult::Empty();
   }
+
+  // <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);
 }
 
 void ModulatorImplBase::Trace(Visitor* visitor) const {

third_party/blink/web_tests/external/wpt/service-workers/service-worker/registration-script-module.https-expected.txt

-This is a testharness.js-based test.
-PASS Registering invalid chunked encoding script
-PASS Registering invalid chunked encoding script with flush
-FAIL Registering script including parse error assert_unreached: Should have rejected: Registration of script including parse error should fail. Reached unreachable code
-FAIL Registering script including undefined error assert_unreached: Should have rejected: Registration of script including undefined error should fail. Reached unreachable code
-FAIL Registering script including uncaught exception assert_unreached: Should have rejected: Registration of script including uncaught exception should fail. Reached unreachable code
-PASS Registering non-existent script
-PASS Registering script including caught exception
-Harness: the test ran to completion.
-