From f44f50f3c58785807936b12b8ed6bec42a135f84 Mon Sep 17 00:00:00 2001
From: Chinmay Garde <chinmaygarde@google.com>
Date: Mon, 3 Feb 2020 19:57:43 -0800
Subject: [PATCH] Fix race in SkiaGPUObject unit-tests. (#16351)

There are two issues in the test as written:
* There is a race on the first check to dtor_task_queue_id which might be
  encountered if the calling thread is de-scheduled and the unref queue manages
  to collect the object before the end of the scope.
* Two threads were owning a shared object but we relied on the object to be
  collected on the unref queue.
---
 flow/skia_gpu_object_unittests.cc | 30 ++++--------------------------
 1 file changed, 4 insertions(+), 26 deletions(-)

diff --git a/flow/skia_gpu_object_unittests.cc b/flow/skia_gpu_object_unittests.cc
index 35737708ac..86eaffb1ae 100644
--- a/flow/skia_gpu_object_unittests.cc
+++ b/flow/skia_gpu_object_unittests.cc
@@ -88,12 +88,10 @@ TEST_F(SkiaGpuObjectTest, ObjectDestructor) {
   std::shared_ptr<fml::AutoResetWaitableEvent> latch =
       std::make_shared<fml::AutoResetWaitableEvent>();
   fml::TaskQueueId dtor_task_queue_id(0);
-
+  auto object = sk_make_sp<TestSkObject>(latch, &dtor_task_queue_id);
   {
-    auto object = sk_make_sp<TestSkObject>(latch, &dtor_task_queue_id);
-    SkiaGPUObject<TestSkObject> sk_object(object, unref_queue());
-    ASSERT_EQ(sk_object.get(), object);
-    ASSERT_EQ(dtor_task_queue_id, 0);
+    SkiaGPUObject<TestSkObject> sk_object(std::move(object), unref_queue());
+    // Verify that the default SkiaGPUObject dtor queues and unref.
   }
 
   latch->Wait();
@@ -106,29 +104,9 @@ TEST_F(SkiaGpuObjectTest, ObjectReset) {
   fml::TaskQueueId dtor_task_queue_id(0);
   SkiaGPUObject<TestSkObject> sk_object(
       sk_make_sp<TestSkObject>(latch, &dtor_task_queue_id), unref_queue());
-
+  // Verify that explicitly resetting the GPU object queues and unref.
   sk_object.reset();
   ASSERT_EQ(sk_object.get(), nullptr);
-
-  latch->Wait();
-  ASSERT_EQ(dtor_task_queue_id, unref_task_runner()->GetTaskQueueId());
-}
-
-TEST_F(SkiaGpuObjectTest, ObjectResetBeforeDestructor) {
-  std::shared_ptr<fml::AutoResetWaitableEvent> latch =
-      std::make_shared<fml::AutoResetWaitableEvent>();
-  fml::TaskQueueId dtor_task_queue_id(0);
-
-  {
-    auto object = sk_make_sp<TestSkObject>(latch, &dtor_task_queue_id);
-    SkiaGPUObject<TestSkObject> sk_object(object, unref_queue());
-    ASSERT_EQ(sk_object.get(), object);
-    ASSERT_EQ(dtor_task_queue_id, 0);
-
-    sk_object.reset();
-    ASSERT_EQ(sk_object.get(), nullptr);
-  }
-
   latch->Wait();
   ASSERT_EQ(dtor_task_queue_id, unref_task_runner()->GetTaskQueueId());
 }
-- 
GitLab