Add a new test to verify that the resource context is destroyed in the task runner's thread

Author: ColdPaleLight

ci/licenses_golden/licenses_flutter

@@ -170,7 +170,6 @@ FILE: ../../../flutter/flow/raster_cache_unittests.cc
 FILE: ../../../flutter/flow/rtree.cc
 FILE: ../../../flutter/flow/rtree.h
 FILE: ../../../flutter/flow/rtree_unittests.cc
-FILE: ../../../flutter/flow/skia_gpu_object.cc
 FILE: ../../../flutter/flow/skia_gpu_object.h
 FILE: ../../../flutter/flow/skia_gpu_object_unittests.cc
 FILE: ../../../flutter/flow/surface.cc

flow/BUILD.gn

@@ -64,7 +64,6 @@ source_set("flow") {
     "raster_cache_key.h",
     "rtree.cc",
     "rtree.h",
-    "skia_gpu_object.cc",
     "skia_gpu_object.h",
     "surface.cc",
     "surface.h",

flow/skia_gpu_object.cc

@@ -11,25 +11,46 @@
 #include "flutter/fml/memory/ref_counted.h"
 #include "flutter/fml/memory/weak_ptr.h"
 #include "flutter/fml/task_runner.h"
+#include "flutter/fml/trace_event.h"
 #include "third_party/skia/include/core/SkRefCnt.h"
 #include "third_party/skia/include/gpu/GrDirectContext.h"
 
 namespace flutter {
 
 // A queue that holds Skia objects that must be destructed on the given task
 // runner.
-class SkiaUnrefQueue : public fml::RefCountedThreadSafe<SkiaUnrefQueue> {
+template <class T>
+class UnrefQueue : public fml::RefCountedThreadSafe<UnrefQueue<T>> {
  public:
-  void Unref(SkRefCnt* object);
+  using ResourceContext = T;
+
+  void Unref(SkRefCnt* object) {
+    std::scoped_lock lock(mutex_);
+    objects_.push_back(object);
+    if (!drain_pending_) {
+      drain_pending_ = true;
+      task_runner_->PostDelayedTask(
+          [strong = fml::Ref(this)]() { strong->Drain(); }, drain_delay_);
+    }
+  }
 
   // Usually, the drain is called automatically. However, during IO manager
   // shutdown (when the platform side reference to the OpenGL context is about
   // to go away), we may need to pre-emptively drain the unref queue. It is the
   // responsibility of the caller to ensure that no further unrefs are queued
   // after this call.
-  void Drain();
+  void Drain() {
+    TRACE_EVENT0("flutter", "SkiaUnrefQueue::Drain");
+    std::deque<SkRefCnt*> skia_objects;
+    {
+      std::scoped_lock lock(mutex_);
+      objects_.swap(skia_objects);
+      drain_pending_ = false;
+    }
+    DoDrain(skia_objects, context_);
+  }
 
-  void UpdateResourceContext(sk_sp<GrDirectContext> context) {
+  void UpdateResourceContext(sk_sp<ResourceContext> context) {
     context_ = context;
   }
 
@@ -39,25 +60,47 @@ class SkiaUnrefQueue : public fml::RefCountedThreadSafe<SkiaUnrefQueue> {
   std::mutex mutex_;
   std::deque<SkRefCnt*> objects_;
   bool drain_pending_;
-  sk_sp<GrDirectContext> context_;
+  sk_sp<ResourceContext> context_;
 
   // The `GrDirectContext* context` is only used for signaling Skia to
   // performDeferredCleanup. It can be nullptr when such signaling is not needed
   // (e.g., in unit tests).
-  SkiaUnrefQueue(fml::RefPtr<fml::TaskRunner> task_runner,
-                 fml::TimeDelta delay,
-                 sk_sp<GrDirectContext> context = nullptr);
-
-  ~SkiaUnrefQueue();
+  UnrefQueue(fml::RefPtr<fml::TaskRunner> task_runner,
+             fml::TimeDelta delay,
+             sk_sp<ResourceContext> context = nullptr)
+      : task_runner_(std::move(task_runner)),
+        drain_delay_(delay),
+        drain_pending_(false),
+        context_(context) {}
+
+  ~UnrefQueue() {
+    fml::TaskRunner::RunNowOrPostTask(
+        task_runner_, [objects = std::move(objects_),
+                       context = std::move(context_)]() mutable {
+          DoDrain(objects, context);
+          context.reset();
+        });
+  }
 
+  // static
   static void DoDrain(const std::deque<SkRefCnt*>& skia_objects,
-                      sk_sp<GrDirectContext> context);
+                      sk_sp<ResourceContext> context) {
+    for (SkRefCnt* skia_object : skia_objects) {
+      skia_object->unref();
+    }
+
+    if (context && skia_objects.size() > 0) {
+      context->performDeferredCleanup(std::chrono::milliseconds(0));
+    }
+  }
 
-  FML_FRIEND_REF_COUNTED_THREAD_SAFE(SkiaUnrefQueue);
-  FML_FRIEND_MAKE_REF_COUNTED(SkiaUnrefQueue);
-  FML_DISALLOW_COPY_AND_ASSIGN(SkiaUnrefQueue);
+  FML_FRIEND_REF_COUNTED_THREAD_SAFE(UnrefQueue);
+  FML_FRIEND_MAKE_REF_COUNTED(UnrefQueue);
+  FML_DISALLOW_COPY_AND_ASSIGN(UnrefQueue);
 };
 
+using SkiaUnrefQueue = UnrefQueue<GrDirectContext>;
+
 /// An object whose deallocation needs to be performed on an specific unref
 /// queue. The template argument U need to have a call operator that returns
 /// that unref queue.

flow/skia_gpu_object.h

@@ -22,7 +22,7 @@ class TestSkObject : public SkRefCnt {
                fml::TaskQueueId* dtor_task_queue_id)
       : latch_(latch), dtor_task_queue_id_(dtor_task_queue_id) {}
 
-  ~TestSkObject() {
+  virtual ~TestSkObject() {
     if (dtor_task_queue_id_) {
       *dtor_task_queue_id_ = fml::MessageLoop::GetCurrentTaskQueueId();
     }
@@ -34,6 +34,15 @@ class TestSkObject : public SkRefCnt {
   fml::TaskQueueId* dtor_task_queue_id_;
 };
 
+class TestResourceContext : public TestSkObject {
+ public:
+  TestResourceContext(std::shared_ptr<fml::AutoResetWaitableEvent> latch,
+                      fml::TaskQueueId* dtor_task_queue_id)
+      : TestSkObject(latch, dtor_task_queue_id) {}
+  ~TestResourceContext() = default;
+  void performDeferredCleanup(std::chrono::milliseconds msNotUsed) {}
+};
+
 class SkiaGpuObjectTest : public ThreadTest {
  public:
   SkiaGpuObjectTest()
@@ -127,5 +136,27 @@ TEST_F(SkiaGpuObjectTest, ObjectResetTwice) {
   ASSERT_EQ(dtor_task_queue_id, unref_task_runner()->GetTaskQueueId());
 }
 
+TEST_F(SkiaGpuObjectTest, UnrefResourceContextInTaskRunnerThread) {
+  std::shared_ptr<fml::AutoResetWaitableEvent> latch =
+      std::make_shared<fml::AutoResetWaitableEvent>();
+  fml::RefPtr<UnrefQueue<TestResourceContext>> unref_queue;
+  fml::TaskQueueId dtor_task_queue_id(0);
+  unref_task_runner()->PostTask([&]() mutable {
+    auto resource_context =
+        sk_make_sp<TestResourceContext>(latch, &dtor_task_queue_id);
+    unref_queue = fml::MakeRefCounted<UnrefQueue<TestResourceContext>>(
+        unref_task_runner(), fml::TimeDelta::FromSeconds(0), resource_context);
+    latch->Signal();
+  });
+  latch->Wait();
+
+  // Delete the unref queue, it will schedule a task to unref the resource
+  // context in the task runner's thread.
+  unref_queue = nullptr;
+  latch->Wait();
+  // Verify that the resource context was destroyed in the task runner's thread.
+  ASSERT_EQ(dtor_task_queue_id, unref_task_runner()->GetTaskQueueId());
+}
+
 }  // namespace testing
 }  // namespace flutter