Worklet: Reject addModule() with script's error to rethrow if available

This CL makes Worklet#addModule() fail with a more specific error object
compared to before.

Before this change, Worklet::addModule() was rejected with AbortError regardless
of an actual error reason, and that made it difficult for developers to debug
worklets. This behavior was defined in the Worklets spec, but it was changed
recently (see the links below).

After this change, Worklet::addModule() is rejected with script's error to
rethrow when it is available as the new spec defines.

Spec changes:
- https://github.com/w3c/css-houdini-drafts/pull/958
- https://github.com/w3c/css-houdini-drafts/pull/967

Chromestatus:
- https://www.chromestatus.com/feature/5116796497559552

Bug: 782066
Change-Id: Iabd30eff28ed1bffed9219898649a89b8abebbb2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1258785Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Reviewed-by: Hiroshige Hayashizaki <hiroshige@chromium.org>
Commit-Queue: Hiroki Nakagawa <nhiroki@chromium.org>
Cr-Commit-Position: refs/heads/master@{#703606}

third_party/blink/renderer/core/workers/worklet_global_scope.cc

@@ -228,13 +228,11 @@ void WorkletGlobalScope::FetchAndInvokeScript(
   // moduleURLRecord, moduleResponsesMap, credentialOptions, outsideSettings,
   // and insideSettings when it asynchronously completes."
 
-  Modulator* modulator = Modulator::From(ScriptController()->GetScriptState());
-
   // Step 3 to 5 are implemented in
   // WorkletModuleTreeClient::NotifyModuleTreeLoadFinished.
-  WorkletModuleTreeClient* client =
-      MakeGarbageCollected<WorkletModuleTreeClient>(
-          modulator, std::move(outside_settings_task_runner), pending_tasks);
+  auto* client = MakeGarbageCollected<WorkletModuleTreeClient>(
+      ScriptController()->GetScriptState(),
+      std::move(outside_settings_task_runner), pending_tasks);
 
   // TODO(nhiroki): Pass an appropriate destination defined in each worklet
   // spec (e.g., "paint worklet", "audio worklet") (https://crbug.com/843980,

third_party/blink/renderer/core/workers/worklet_module_tree_client.cc

@@ -5,6 +5,7 @@
 #include "third_party/blink/renderer/core/workers/worklet_module_tree_client.h"
 
 #include "third_party/blink/public/platform/task_type.h"
+#include "third_party/blink/renderer/bindings/core/v8/serialization/serialized_script_value.h"
 #include "third_party/blink/renderer/core/script/module_script.h"
 #include "third_party/blink/renderer/core/workers/worker_reporting_proxy.h"
 #include "third_party/blink/renderer/core/workers/worklet_global_scope.h"
@@ -14,10 +15,10 @@
 namespace blink {
 
 WorkletModuleTreeClient::WorkletModuleTreeClient(
-    Modulator* modulator,
+    ScriptState* script_state,
     scoped_refptr<base::SingleThreadTaskRunner> outside_settings_task_runner,
     WorkletPendingTasks* pending_tasks)
-    : modulator_(modulator),
+    : script_state_(script_state),
       outside_settings_task_runner_(std::move(outside_settings_task_runner)),
       pending_tasks_(pending_tasks) {}
 
@@ -29,45 +30,57 @@ void WorkletModuleTreeClient::NotifyModuleTreeLoadFinished(
   // TODO(nhiroki): Call reporting proxy functions appropriately (e.g.,
   // DidFailToFetchModuleScript(), WillEvaluateModuleScript()).
 
+  // "Note: Specifically, if a script fails to parse or fails to load over the
+  // network, it will reject the promise. If the script throws an error while
+  // first evaluating the promise it will resolve as classes may have been
+  // registered correctly."
+  // https://drafts.css-houdini.org/worklets/#fetch-a-worklet-script
+  //
+  // When a network failure happens, |module_script| should be nullptr, and the
+  // case will be handled by the step 3.
+  // When a parse failure happens, |module_script| has an error to rethrow, and
+  // the case will be handled by the step 4.
+
+  // Step 3: "If script is null, then queue a task on outsideSettings's
+  // responsible event loop to run these steps:"
   if (!module_script) {
-    // Step 3: "If script is null, then queue a task on outsideSettings's
-    // responsible event loop to run these steps:"
-    // The steps are implemented in WorkletPendingTasks::Abort().
+    // Null |error_to_rethrow| will be replaced with AbortError.
     PostCrossThreadTask(
         *outside_settings_task_runner_, FROM_HERE,
         CrossThreadBindOnce(&WorkletPendingTasks::Abort,
-                            WrapCrossThreadPersistent(pending_tasks_.Get())));
+                            WrapCrossThreadPersistent(pending_tasks_.Get()),
+                            /*error_to_rethrow=*/nullptr));
     return;
   }
 
-  // "Note: Specifically, if a script fails to parse or fails to load over the
-  // network, it will reject the promise. If the script throws an error while
-  // first evaluating the promise it will resolve as classes may have been
-  // registered correctly."
-  // https://drafts.css-houdini.org/worklets/#fetch-a-worklet-script
-  //
-  // When a network failure happens, |module_script| should be nullptr and the
-  // case should already be handled above.
-  //
-  // Check whether a syntax error happens.
+  // Step 4: "If script's error to rethrow is not null, then queue a task on
+  // outsideSettings's responsible event loop given script's error to rethrow to
+  // run these steps:
   if (module_script->HasErrorToRethrow()) {
+    ScriptState::Scope scope(script_state_);
     PostCrossThreadTask(
         *outside_settings_task_runner_, FROM_HERE,
-        CrossThreadBindOnce(&WorkletPendingTasks::Abort,
-                            WrapCrossThreadPersistent(pending_tasks_.Get())));
+        CrossThreadBindOnce(
+            &WorkletPendingTasks::Abort,
+            WrapCrossThreadPersistent(pending_tasks_.Get()),
+            SerializedScriptValue::SerializeAndSwallowExceptions(
+                script_state_->GetIsolate(),
+                module_script->CreateErrorToRethrow().V8Value())));
     return;
   }
 
-  // Step 4: "Run a module script given script."
-  ScriptValue error = modulator_->ExecuteModule(
-      module_script, Modulator::CaptureEvalErrorFlag::kReport);
+  // Step 5: "Run a module script given script."
+  ScriptValue error =
+      Modulator::From(script_state_)
+          ->ExecuteModule(module_script,
+                          Modulator::CaptureEvalErrorFlag::kReport);
 
-  WorkletGlobalScope* global_scope = To<WorkletGlobalScope>(
-      ExecutionContext::From(modulator_->GetScriptState()));
+  auto* global_scope =
+      To<WorkletGlobalScope>(ExecutionContext::From(script_state_));
 
   global_scope->ReportingProxy().DidEvaluateModuleScript(error.IsEmpty());
 
-  // Step 5: "Queue a task on outsideSettings's responsible event loop to run
+  // Step 6: "Queue a task on outsideSettings's responsible event loop to run
   // these steps:"
   // The steps are implemented in WorkletPendingTasks::DecrementCounter().
   PostCrossThreadTask(
@@ -77,7 +90,7 @@ void WorkletModuleTreeClient::NotifyModuleTreeLoadFinished(
 }
 
 void WorkletModuleTreeClient::Trace(blink::Visitor* visitor) {
-  visitor->Trace(modulator_);
+  visitor->Trace(script_state_);
   ModuleTreeClient::Trace(visitor);
 }
 

third_party/blink/renderer/core/workers/worklet_module_tree_client.h

@@ -13,12 +13,13 @@
 namespace blink {
 
 class ModuleScript;
+class ScriptState;
 
 // A ModuleTreeClient that lives on the worklet context's thread.
 class WorkletModuleTreeClient final : public ModuleTreeClient {
  public:
   WorkletModuleTreeClient(
-      Modulator*,
+      ScriptState*,
       scoped_refptr<base::SingleThreadTaskRunner> outside_settings_task_runner,
       WorkletPendingTasks*);
 
@@ -28,7 +29,7 @@ class WorkletModuleTreeClient final : public ModuleTreeClient {
   void Trace(blink::Visitor*) override;
 
  private:
-  Member<Modulator> modulator_;
+  Member<ScriptState> script_state_;
   scoped_refptr<base::SingleThreadTaskRunner> outside_settings_task_runner_;
   CrossThreadPersistent<WorkletPendingTasks> pending_tasks_;
 };

third_party/blink/renderer/core/workers/worklet_pending_tasks.cc

@@ -4,6 +4,7 @@
 
 #include "third_party/blink/renderer/core/workers/worklet_pending_tasks.h"
 
+#include "third_party/blink/renderer/bindings/core/v8/serialization/serialized_script_value.h"
 #include "third_party/blink/renderer/core/dom/dom_exception.h"
 #include "third_party/blink/renderer/core/workers/worklet.h"
 #include "third_party/blink/renderer/platform/heap/heap.h"
@@ -22,18 +23,35 @@ void WorkletPendingTasks::InitializeCounter(int counter) {
   counter_ = counter;
 }
 
-void WorkletPendingTasks::Abort() {
+void WorkletPendingTasks::Abort(
+    scoped_refptr<SerializedScriptValue> error_to_rethrow) {
   DCHECK(IsMainThread());
+  // This function can be called from the following steps. See
+  // WorkletModuleTreeClient::NotifyModuleTreeLoadFinished().
+  //
   // Step 3: "If script is null, then queue a task on outsideSettings's
   // responsible event loop to run these steps:"
   //   1: "If pendingTaskStruct's counter is not -1, then run these steps:"
   //     1: "Set pendingTaskStruct's counter to -1."
   //     2: "Reject promise with an "AbortError" DOMException."
+  //
+  // Step 4: "If script's error to rethrow is not null, then queue a task on
+  // outsideSettings's responsible event loop given script's error to rethrow to
+  // run these steps:
+  //   1: "If pendingTaskStruct's counter is not -1, then run these steps:"
+  //     1: "Set pendingTaskStruct's counter to -1."
+  //     2: "Reject promise with error to rethrow."
   if (counter_ != -1) {
     counter_ = -1;
     worklet_->FinishPendingTasks(this);
-    resolver_->Reject(
-        MakeGarbageCollected<DOMException>(DOMExceptionCode::kAbortError));
+    if (error_to_rethrow) {
+      ScriptState::Scope scope(resolver_->GetScriptState());
+      resolver_->Reject(error_to_rethrow->Deserialize(
+          resolver_->GetScriptState()->GetIsolate()));
+    } else {
+      resolver_->Reject(
+          MakeGarbageCollected<DOMException>(DOMExceptionCode::kAbortError));
+    }
   }
 }
 

third_party/blink/renderer/core/workers/worklet_pending_tasks.h

@@ -11,6 +11,7 @@
 
 namespace blink {
 
+class SerializedScriptValue;
 class Worklet;
 
 // Implementation of the "pending tasks struct":
@@ -31,7 +32,7 @@ class CORE_EXPORT WorkletPendingTasks final
   void InitializeCounter(int counter);
 
   // Sets |counter_| to -1 and rejects the promise.
-  void Abort();
+  void Abort(scoped_refptr<SerializedScriptValue> error_to_rethrow);
 
   // Decrements |counter_| and resolves the promise if the counter becomes 0.
   void DecrementCounter();

third_party/blink/web_tests/external/wpt/worklets/resources/import-tests.js

@@ -139,14 +139,14 @@ function runImportTests(worklet_type) {
 
     promise_test(t => {
         const kScriptURL = 'resources/syntax-error-worklet-script.js';
-        return promise_rejects(t, new DOMException('', 'AbortError'),
+        return promise_rejects(t, new DOMException('', 'SyntaxError'),
                                worklet.addModule(kScriptURL));
     }, 'Importing a script that has a syntax error should reject the given ' +
        'promise.');
 
     promise_test(t => {
         const kScriptURL = 'resources/import-syntax-error-worklet-script.js';
-        return promise_rejects(t, new DOMException('', 'AbortError'),
+        return promise_rejects(t, new DOMException('', 'SyntaxError'),
                                worklet.addModule(kScriptURL));
     }, 'Importing a nested script that has a syntax error should reject the ' +
        'given promise.');
@@ -155,7 +155,7 @@ function runImportTests(worklet_type) {
         const kBlob = new Blob(["import 'invalid-specifier.js';"],
                                {type: 'text/javascript'});
         const kBlobURL = URL.createObjectURL(kBlob);
-        return promise_rejects(t, new DOMException('', 'AbortError'),
+        return promise_rejects(t, new DOMException('', 'TypeError'),
                                worklet.addModule(kBlobURL));
     }, 'Importing a script that imports an invalid identifier should reject ' +
        'the given promise.');