[Animation Worklet] Ensure dispatcher's done callback always runs

Previously it was possible for the
AnimationWorkletMutatorDispatcherImpl::MutateSynchronously  to wait
indefinitely and hang if the worklet task runner didn't execute the posted
mutation task. (I suspect this would happen if task is posted during the
worklet thread shutdown process)


In fact, this condition would occur occasionally when running our tests
and causing the content_shell to hang and test to timeout which lead
to the test flakiness.

This fix ensures the done closure is guaranteed to run even when destination
task runner drops it. This is achieved by using base::ScopedClosureRunner
which runs the closure on destruction if not run already.

TEST: Locally running the tests no longer produces the hang.
TEST: AnimationWorkletMutatorDispatcherImplTest.DispatcherShouldNotHangWhenMutatorGoesAway

Bug: 930462
Change-Id: Ie35ab9c2072d71c301d304b10e0cf15c3d6616da
Reviewed-on: https://chromium-review.googlesource.com/c/1477960
Commit-Queue: Majid Valipour <majidvp@chromium.org>
Reviewed-by: Jeremy Roman <jbroman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#634334}

third_party/blink/renderer/platform/graphics/DEPS

@@ -9,6 +9,7 @@ include_rules = [
     "+base/threading/sequenced_task_runner_handle.h",
     "+base/threading/thread_restrictions.h",
     "+base/barrier_closure.h",
+    "+base/callback_helpers.h",
     "+cc",
     "+components/viz/client",
     "+components/viz/common",

third_party/blink/renderer/platform/graphics/animation_worklet_mutator_dispatcher_impl.cc

@@ -5,6 +5,7 @@
 #include "third_party/blink/renderer/platform/graphics/animation_worklet_mutator_dispatcher_impl.h"
 
 #include "base/barrier_closure.h"
+#include "base/callback_helpers.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/synchronization/waitable_event.h"
 #include "base/timer/elapsed_timer.h"
@@ -266,23 +267,30 @@ void AnimationWorkletMutatorDispatcherImpl::RequestMutations(
     scoped_refptr<base::SingleThreadTaskRunner> worklet_queue = pair.value;
     int worklet_id = mutator->GetWorkletId();
     DCHECK(!worklet_queue->BelongsToCurrentThread());
+
+    // Wrap the barrier closure in a ScopedClosureRunner to guarantee it runs
+    // even if the posted task does not run.
+    auto on_done_runner =
+        std::make_unique<base::ScopedClosureRunner>(on_mutator_done);
+
     auto it = mutator_input_map_.find(worklet_id);
     if (it == mutator_input_map_.end()) {
-      // No input to process.
-      on_mutator_done.Run();
+      // Here the on_done_runner goes out of scope which causes the barrier
+      // closure to run.
       continue;
     }
+
     PostCrossThreadTask(
         *worklet_queue, FROM_HERE,
         CrossThreadBind(
             [](AnimationWorkletMutator* mutator,
                std::unique_ptr<AnimationWorkletInput> input,
                scoped_refptr<OutputVectorRef> outputs, int index,
-               WTF::CrossThreadClosure on_mutator_done) {
+               std::unique_ptr<base::ScopedClosureRunner> on_done_runner) {
               std::unique_ptr<AnimationWorkletOutput> output =
                   mutator ? mutator->Mutate(std::move(input)) : nullptr;
               outputs->get()[index] = std::move(output);
-              on_mutator_done.Run();
+              on_done_runner->RunAndReset();
             },
             // The mutator is created and destroyed on the worklet thread.
             WrapCrossThreadWeakPersistent(mutator),
@@ -292,7 +300,7 @@ void AnimationWorkletMutatorDispatcherImpl::RequestMutations(
             // on the host thread. It can outlive the dispatcher during shutdown
             // of a process with a running animation.
             outputs_, next_request_index++,
-            WTF::Passed(WTF::CrossThreadClosure(on_mutator_done))));
+            WTF::Passed(std::move(on_done_runner))));
   }
 }
 

third_party/blink/renderer/platform/graphics/animation_worklet_mutator_dispatcher_impl_test.cc

@@ -323,6 +323,28 @@ TEST_F(
   Mock::VerifyAndClearExpectations(client_.get());
 }
 
+TEST_F(AnimationWorkletMutatorDispatcherImplTest,
+       DispatcherShouldNotHangWhenMutatorGoesAway) {
+  // Create a thread to run mutator tasks.
+  std::unique_ptr<Thread> first_thread = CreateThread("FirstAnimationThread");
+  MockAnimationWorkletMutator* first_mutator =
+      MakeGarbageCollected<MockAnimationWorkletMutator>(
+          first_thread->GetTaskRunner());
+
+  mutator_->RegisterAnimationWorkletMutator(first_mutator,
+                                            first_thread->GetTaskRunner());
+
+  EXPECT_CALL(*first_mutator, GetWorkletId()).WillRepeatedly(Return(11));
+  EXPECT_CALL(*client_, SetMutationUpdateRef(_)).Times(0);
+
+  // Shutdown the thread so its task runner no longer executes tasks.
+  first_thread.reset();
+
+  mutator_->MutateSynchronously(CreateTestMutatorInput());
+
+  Mock::VerifyAndClearExpectations(client_.get());
+}
+
 // -----------------------------------------------------------------------
 // Asynchronous version of tests.