TaskWorklet: always return a promise from task.result.

Previously task.result sometimes returned the result value directly. Making
it always return a promise makes it easily to handle in JS.

Bug: 879306
Change-Id: I6c0168d828d8234875a20e1128b14f1ab6603d4f
Reviewed-on: https://chromium-review.googlesource.com/c/1303100Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org>
Commit-Queue: Nate Chapin <japhet@chromium.org>
Cr-Commit-Position: refs/heads/master@{#603596}

third_party/blink/renderer/core/workers/experimental/task.cc

@@ -17,37 +17,38 @@
 namespace blink {
 
 ThreadPoolTask::ThreadPoolTask(ThreadPoolThreadProvider* thread_provider,
-                               v8::Isolate* isolate,
+                               ScriptState* script_state,
                                const ScriptValue& function,
                                const Vector<ScriptValue>& arguments,
                                TaskType task_type)
     : ThreadPoolTask(thread_provider,
-                     isolate,
+                     script_state,
                      function,
                      String(),
                      arguments,
                      task_type) {}
 
 ThreadPoolTask::ThreadPoolTask(ThreadPoolThreadProvider* thread_provider,
-                               v8::Isolate* isolate,
+                               ScriptState* script_state,
                                const String& function_name,
                                const Vector<ScriptValue>& arguments,
                                TaskType task_type)
     : ThreadPoolTask(thread_provider,
-                     isolate,
+                     script_state,
                      ScriptValue(),
                      function_name,
                      arguments,
                      task_type) {}
 
 ThreadPoolTask::ThreadPoolTask(ThreadPoolThreadProvider* thread_provider,
-                               v8::Isolate* isolate,
+                               ScriptState* script_state,
                                const ScriptValue& function,
                                const String& function_name,
                                const Vector<ScriptValue>& arguments,
                                TaskType task_type)
     : task_type_(task_type),
       self_keep_alive_(base::AdoptRef(this)),
+      resolver_(ScriptPromiseResolver::Create(script_state)),
       function_name_(function_name.IsolatedCopy()),
       arguments_(arguments.size()),
       weak_factory_(this) {
@@ -55,6 +56,7 @@ ThreadPoolTask::ThreadPoolTask(ThreadPoolThreadProvider* thread_provider,
   DCHECK_EQ(!function.IsEmpty(), function_name.IsNull());
   DCHECK(task_type_ == TaskType::kUserInteraction ||
          task_type_ == TaskType::kIdleTask);
+  v8::Isolate* isolate = script_state->GetIsolate();
 
   // TODO(japhet): Handle serialization failures
   if (!function.IsEmpty()) {
@@ -167,7 +169,6 @@ void ThreadPoolTask::RegisterDependencies(
 
 ThreadPoolTask::~ThreadPoolTask() {
   DCHECK(IsMainThread());
-  DCHECK(!resolver_);
   DCHECK(HasFinished());
   DCHECK(!function_);
   DCHECK(arguments_.IsEmpty());
@@ -340,37 +341,28 @@ void ThreadPoolTask::TaskCompleted() {
     DCHECK(HasFinished());
     rejected = state_ == State::kFailed;
   }
-  if (resolver_ && resolver_->GetScriptState()->ContextIsValid()) {
-    ScriptState::Scope scope(resolver_->GetScriptState());
-    ScriptValue value = GetResult(resolver_->GetScriptState());
+
+  ScriptState* script_state = resolver_->GetScriptState();
+  if (script_state->ContextIsValid()) {
+    ScriptState::Scope scope(script_state);
+    v8::Local<v8::Value> value;
+    {
+      MutexLocker lock(mutex_);
+      value = serialized_result_->Deserialize(script_state->GetIsolate());
+    }
     if (rejected)
-      resolver_->Reject(v8::Exception::Error(value.V8Value().As<v8::String>()));
+      resolver_->Reject(v8::Exception::Error(value.As<v8::String>()));
     else
       resolver_->Resolve(value);
   }
-  resolver_ = nullptr;
   worker_thread_->DecrementTasksInProgressCount();
   self_keep_alive_.reset();
   // |this| may be deleted here.
 }
 
-ScriptValue ThreadPoolTask::GetResult(ScriptState* script_state) {
+ScriptPromise ThreadPoolTask::GetResult() {
   DCHECK(IsMainThread());
-  MutexLocker lock(mutex_);
-  if (!HasFinished()) {
-    DCHECK(!serialized_result_);
-    DCHECK(deserialized_result_.IsEmpty());
-    if (!resolver_)
-      resolver_ = ScriptPromiseResolver::Create(script_state);
-    return resolver_->Promise().GetScriptValue();
-  }
-  if (deserialized_result_.IsEmpty()) {
-    ScriptState::Scope scope(script_state);
-    deserialized_result_ = ScriptValue(
-        script_state,
-        serialized_result_->Deserialize(script_state->GetIsolate()));
-  }
-  return deserialized_result_;
+  return resolver_->Promise();
 }
 
 void ThreadPoolTask::Cancel() {

third_party/blink/renderer/core/workers/experimental/task.h

@@ -26,19 +26,18 @@ class ThreadPoolTask final : public RefCounted<ThreadPoolTask> {
  public:
   // Called on main thread
   ThreadPoolTask(ThreadPoolThreadProvider*,
-                 v8::Isolate*,
+                 ScriptState*,
                  const ScriptValue& function,
                  const Vector<ScriptValue>& arguments,
                  TaskType);
   ThreadPoolTask(ThreadPoolThreadProvider*,
-                 v8::Isolate*,
+                 ScriptState*,
                  const String& function_name,
                  const Vector<ScriptValue>& arguments,
                  TaskType);
   ~ThreadPoolTask();
-  // Returns the result of this task, or a promise that will be resolved with
-  // the result when it completes.
-  ScriptValue GetResult(ScriptState*) LOCKS_EXCLUDED(mutex_);
+  // Returns a promise that will be resolved with the result when it completes.
+  ScriptPromise GetResult();
   void Cancel() LOCKS_EXCLUDED(mutex_);
 
   base::WeakPtr<ThreadPoolTask> GetWeakPtr() {
@@ -49,7 +48,7 @@ class ThreadPoolTask final : public RefCounted<ThreadPoolTask> {
   enum class State { kPending, kStarted, kCancelPending, kCompleted, kFailed };
 
   ThreadPoolTask(ThreadPoolThreadProvider*,
-                 v8::Isolate*,
+                 ScriptState*,
                  const ScriptValue& function,
                  const String& function_name,
                  const Vector<ScriptValue>& arguments,
@@ -85,7 +84,6 @@ class ThreadPoolTask final : public RefCounted<ThreadPoolTask> {
 
   // Main thread only
   scoped_refptr<ThreadPoolTask> self_keep_alive_;
-  ScriptValue deserialized_result_;
   Persistent<ScriptPromiseResolver> resolver_;
 
   // Created in constructor on the main thread, consumed and cleared on
@@ -143,9 +141,7 @@ class Task : public ScriptWrappable {
       : thread_pool_task_(thread_pool_task) {}
   ~Task() override = default;
 
-  ScriptValue result(ScriptState* script_state) {
-    return thread_pool_task_->GetResult(script_state);
-  }
+  ScriptPromise result() { return thread_pool_task_->GetResult(); }
   void cancel() { thread_pool_task_->Cancel(); }
 
   ThreadPoolTask* GetThreadPoolTask() const { return thread_pool_task_.get(); }

third_party/blink/renderer/core/workers/experimental/task.idl

@@ -6,6 +6,6 @@
     Exposed=Window,
     RuntimeEnabled=WorkerTaskQueue
 ] interface Task {
-    [CallWith=ScriptState] readonly attribute any result;
+    readonly attribute Promise<any> result;
     void cancel();
 };

third_party/blink/renderer/core/workers/experimental/task_worklet.cc

@@ -55,18 +55,16 @@ Task* TaskWorklet::postTask(ScriptState* script_state,
   // TODO(japhet): Here and below: it's unclear what task type should be used,
   // and whether the API should allow it to be configured. Using kIdleTask as a
   // placeholder for now.
-  ThreadPoolTask* thread_pool_task =
-      new ThreadPoolTask(this, script_state->GetIsolate(), function, arguments,
-                         TaskType::kIdleTask);
+  ThreadPoolTask* thread_pool_task = new ThreadPoolTask(
+      this, script_state, function, arguments, TaskType::kIdleTask);
   return new Task(thread_pool_task);
 }
 
 Task* TaskWorklet::postTask(ScriptState* script_state,
                             const String& function_name,
                             const Vector<ScriptValue>& arguments) {
-  ThreadPoolTask* thread_pool_task =
-      new ThreadPoolTask(this, script_state->GetIsolate(), function_name,
-                         arguments, TaskType::kIdleTask);
+  ThreadPoolTask* thread_pool_task = new ThreadPoolTask(
+      this, script_state, function_name, arguments, TaskType::kIdleTask);
   return new Task(thread_pool_task);
 }
 

third_party/blink/renderer/core/workers/experimental/worker_task_queue.cc

@@ -46,16 +46,12 @@ ScriptPromise WorkerTaskQueue::postFunction(
   DCHECK(task.IsFunction());
 
   ThreadPoolTask* thread_pool_task = new ThreadPoolTask(
-      ThreadPool::From(*document_), script_state->GetIsolate(), task, arguments,
-      task_type_);
+      ThreadPool::From(*document_), script_state, task, arguments, task_type_);
   if (signal) {
     signal->AddAlgorithm(
         WTF::Bind(&ThreadPoolTask::Cancel, thread_pool_task->GetWeakPtr()));
   }
-  ScriptValue value = thread_pool_task->GetResult(script_state);
-  DCHECK(value.V8Value()->IsPromise());
-
-  return ScriptPromise(script_state, value.V8Value());
+  return thread_pool_task->GetResult();
 }
 
 Task* WorkerTaskQueue::postTask(ScriptState* script_state,
@@ -64,9 +60,9 @@ Task* WorkerTaskQueue::postTask(ScriptState* script_state,
   DCHECK(document_->IsContextThread());
   DCHECK(function.IsFunction());
 
-  ThreadPoolTask* thread_pool_task = new ThreadPoolTask(
-      ThreadPool::From(*document_), script_state->GetIsolate(), function,
-      arguments, task_type_);
+  ThreadPoolTask* thread_pool_task =
+      new ThreadPoolTask(ThreadPool::From(*document_), script_state, function,
+                         arguments, task_type_);
   return new Task(thread_pool_task);
 }