Hold a reference to the Skia unref queue in UIDartState (#13239)

Obtaining the SkiaUnrefQueue through the IOManager is unsafe because
UIDartState has a weak pointer to the IOManager that can not be dereferenced
on the UI thread.

lib/ui/ui_dart_state.cc

@@ -18,6 +18,7 @@ UIDartState::UIDartState(
     TaskObserverAdd add_callback,
     TaskObserverRemove remove_callback,
     fml::WeakPtr<IOManager> io_manager,
+    fml::RefPtr<SkiaUnrefQueue> skia_unref_queue,
     fml::WeakPtr<ImageDecoder> image_decoder,
     std::string advisory_script_uri,
     std::string advisory_script_entrypoint,
@@ -28,6 +29,7 @@ UIDartState::UIDartState(
       add_callback_(std::move(add_callback)),
       remove_callback_(std::move(remove_callback)),
       io_manager_(std::move(io_manager)),
+      skia_unref_queue_(std::move(skia_unref_queue)),
       image_decoder_(std::move(image_decoder)),
       advisory_script_uri_(std::move(advisory_script_uri)),
       advisory_script_entrypoint_(std::move(advisory_script_entrypoint)),
@@ -83,16 +85,7 @@ fml::WeakPtr<IOManager> UIDartState::GetIOManager() const {
 }
 
 fml::RefPtr<flutter::SkiaUnrefQueue> UIDartState::GetSkiaUnrefQueue() const {
-  // TODO(gw280): The WeakPtr here asserts that we are derefing it on the
-  // same thread as it was created on. As we can't guarantee that currently
-  // we're being called from the IO thread (construction thread), we need
-  // to use getUnsafe() here to avoid failing the assertion.
-  //
-  // https://github.com/flutter/flutter/issues/42946
-  if (!io_manager_.getUnsafe()) {
-    return nullptr;
-  }
-  return io_manager_.getUnsafe()->GetSkiaUnrefQueue();
+  return skia_unref_queue_;
 }
 
 void UIDartState::ScheduleMicrotask(Dart_Handle closure) {

lib/ui/ui_dart_state.h

@@ -77,6 +77,7 @@ class UIDartState : public tonic::DartState {
               TaskObserverAdd add_callback,
               TaskObserverRemove remove_callback,
               fml::WeakPtr<IOManager> io_manager,
+              fml::RefPtr<SkiaUnrefQueue> skia_unref_queue,
               fml::WeakPtr<ImageDecoder> image_decoder,
               std::string advisory_script_uri,
               std::string advisory_script_entrypoint,
@@ -99,6 +100,7 @@ class UIDartState : public tonic::DartState {
   const TaskObserverAdd add_callback_;
   const TaskObserverRemove remove_callback_;
   fml::WeakPtr<IOManager> io_manager_;
+  fml::RefPtr<SkiaUnrefQueue> skia_unref_queue_;
   fml::WeakPtr<ImageDecoder> image_decoder_;
   const std::string advisory_script_uri_;
   const std::string advisory_script_entrypoint_;

runtime/dart_isolate.cc

@@ -36,6 +36,7 @@ std::weak_ptr<DartIsolate> DartIsolate::CreateRootIsolate(
     TaskRunners task_runners,
     std::unique_ptr<Window> window,
     fml::WeakPtr<IOManager> io_manager,
+    fml::RefPtr<SkiaUnrefQueue> unref_queue,
     fml::WeakPtr<ImageDecoder> image_decoder,
     std::string advisory_script_uri,
     std::string advisory_script_entrypoint,
@@ -61,6 +62,7 @@ std::weak_ptr<DartIsolate> DartIsolate::CreateRootIsolate(
           std::move(shared_snapshot),   // shared snapshot
           task_runners,                 // task runners
           std::move(io_manager),        // IO manager
+          std::move(unref_queue),       // Skia unref queue
           std::move(image_decoder),     // Image Decoder
           advisory_script_uri,          // advisory URI
           advisory_script_entrypoint,   // advisory entrypoint
@@ -105,6 +107,7 @@ DartIsolate::DartIsolate(const Settings& settings,
                          fml::RefPtr<const DartSnapshot> shared_snapshot,
                          TaskRunners task_runners,
                          fml::WeakPtr<IOManager> io_manager,
+                         fml::RefPtr<SkiaUnrefQueue> unref_queue,
                          fml::WeakPtr<ImageDecoder> image_decoder,
                          std::string advisory_script_uri,
                          std::string advisory_script_entrypoint,
@@ -115,6 +118,7 @@ DartIsolate::DartIsolate(const Settings& settings,
                   settings.task_observer_add,
                   settings.task_observer_remove,
                   std::move(io_manager),
+                  std::move(unref_queue),
                   std::move(image_decoder),
                   advisory_script_uri,
                   advisory_script_entrypoint,
@@ -596,6 +600,7 @@ Dart_Isolate DartIsolate::DartCreateAndStartServiceIsolate(
           null_task_runners,              // task runners
           nullptr,                        // window
           {},                             // IO Manager
+          {},                             // Skia unref queue
           {},                             // Image Decoder
           DART_VM_SERVICE_ISOLATE_NAME,   // script uri
           DART_VM_SERVICE_ISOLATE_NAME,   // script entrypoint
@@ -708,7 +713,8 @@ DartIsolate::CreateDartVMAndEmbedderObjectPair(
             (*raw_embedder_isolate)->GetSharedSnapshot(),   // shared_snapshot
             null_task_runners,                              // task_runners
             fml::WeakPtr<IOManager>{},                      // io_manager
-            fml::WeakPtr<ImageDecoder>{},                   // io_manager
+            fml::RefPtr<SkiaUnrefQueue>{},                  // unref_queue
+            fml::WeakPtr<ImageDecoder>{},                   // image_decoder
             advisory_script_uri,         // advisory_script_uri
             advisory_script_entrypoint,  // advisory_script_entrypoint
             (*raw_embedder_isolate)->child_isolate_preparer_,    // preparer

runtime/dart_isolate.h

@@ -156,6 +156,7 @@ class DartIsolate : public UIDartState {
   /// @param[in]  window                      The weak pointer to the window
   ///                                         associated with this root isolate.
   /// @param[in]  io_manager                  The i/o manager.
+  /// @param[in]  unref_queue                 The Skia unref queue.
   /// @param[in]  image_decoder               The image decoder.
   /// @param[in]  advisory_script_uri         The advisory script uri. This is
   ///                                         only used in instrumentation.
@@ -196,6 +197,7 @@ class DartIsolate : public UIDartState {
       TaskRunners task_runners,
       std::unique_ptr<Window> window,
       fml::WeakPtr<IOManager> io_manager,
+      fml::RefPtr<SkiaUnrefQueue> skia_unref_queue,
       fml::WeakPtr<ImageDecoder> image_decoder,
       std::string advisory_script_uri,
       std::string advisory_script_entrypoint,
@@ -441,6 +443,7 @@ class DartIsolate : public UIDartState {
               fml::RefPtr<const DartSnapshot> shared_snapshot,
               TaskRunners task_runners,
               fml::WeakPtr<IOManager> io_manager,
+              fml::RefPtr<SkiaUnrefQueue> unref_queue,
               fml::WeakPtr<ImageDecoder> image_decoder,
               std::string advisory_script_uri,
               std::string advisory_script_entrypoint,

runtime/dart_isolate_unittests.cc

@@ -42,6 +42,7 @@ TEST_F(DartIsolateTest, RootIsolateCreationAndShutdown) {
       std::move(task_runners),            // task runners
       nullptr,                            // window
       {},                                 // io manager
+      {},                                 // unref queue
       {},                                 // image decoder
       "main.dart",                        // advisory uri
       "main",                             // advisory entrypoint,
@@ -75,6 +76,7 @@ TEST_F(DartIsolateTest, IsolateShutdownCallbackIsInIsolateScope) {
       std::move(task_runners),            // task runners
       nullptr,                            // window
       {},                                 // io manager
+      {},                                 // unref queue
       {},                                 // image decoder
       "main.dart",                        // advisory uri
       "main",                             // advisory entrypoint
@@ -185,6 +187,7 @@ static void RunDartCodeInIsolate(DartVMRef& vm_ref,
       std::move(task_runners),            // task runners
       nullptr,                            // window
       {},                                 // io manager
+      {},                                 // unref queue
       {},                                 // image decoder
       "main.dart",                        // advisory uri
       "main",                             // advisory entrypoint

runtime/dart_lifecycle_unittests.cc

@@ -57,6 +57,7 @@ static std::shared_ptr<DartIsolate> CreateAndRunRootIsolate(
       runners,                            // task_runners
       {},                                 // window
       {},                                 // io_manager
+      {},                                 // unref_queue
       {},                                 // image_decoder
       "main.dart",                        // advisory_script_uri
       entrypoint.c_str(),                 // advisory_script_entrypoint

runtime/runtime_controller.cc

@@ -21,6 +21,7 @@ RuntimeController::RuntimeController(
     fml::RefPtr<const DartSnapshot> p_shared_snapshot,
     TaskRunners p_task_runners,
     fml::WeakPtr<IOManager> p_io_manager,
+    fml::RefPtr<SkiaUnrefQueue> p_unref_queue,
     fml::WeakPtr<ImageDecoder> p_image_decoder,
     std::string p_advisory_script_uri,
     std::string p_advisory_script_entrypoint,
@@ -34,6 +35,7 @@ RuntimeController::RuntimeController(
                         std::move(p_shared_snapshot),
                         std::move(p_task_runners),
                         std::move(p_io_manager),
+                        std::move(p_unref_queue),
                         std::move(p_image_decoder),
                         std::move(p_advisory_script_uri),
                         std::move(p_advisory_script_entrypoint),
@@ -50,6 +52,7 @@ RuntimeController::RuntimeController(
     fml::RefPtr<const DartSnapshot> p_shared_snapshot,
     TaskRunners p_task_runners,
     fml::WeakPtr<IOManager> p_io_manager,
+    fml::RefPtr<SkiaUnrefQueue> p_unref_queue,
     fml::WeakPtr<ImageDecoder> p_image_decoder,
     std::string p_advisory_script_uri,
     std::string p_advisory_script_entrypoint,
@@ -64,6 +67,7 @@ RuntimeController::RuntimeController(
       shared_snapshot_(std::move(p_shared_snapshot)),
       task_runners_(p_task_runners),
       io_manager_(p_io_manager),
+      unref_queue_(p_unref_queue),
       image_decoder_(p_image_decoder),
       advisory_script_uri_(p_advisory_script_uri),
       advisory_script_entrypoint_(p_advisory_script_entrypoint),
@@ -82,6 +86,7 @@ RuntimeController::RuntimeController(
                                      task_runners_,                    //
                                      std::make_unique<Window>(this),   //
                                      io_manager_,                      //
+                                     unref_queue_,                     //
                                      image_decoder_,                   //
                                      p_advisory_script_uri,            //
                                      p_advisory_script_entrypoint,     //
@@ -142,6 +147,7 @@ std::unique_ptr<RuntimeController> RuntimeController::Clone() const {
       shared_snapshot_,             //
       task_runners_,                //
       io_manager_,                  //
+      unref_queue_,                 //
       image_decoder_,               //
       advisory_script_uri_,         //
       advisory_script_entrypoint_,  //

runtime/runtime_controller.h

@@ -35,7 +35,8 @@ class RuntimeController final : public WindowClient {
       fml::RefPtr<const DartSnapshot> shared_snapshot,
       TaskRunners task_runners,
       fml::WeakPtr<IOManager> io_manager,
-      fml::WeakPtr<ImageDecoder> iamge_decoder,
+      fml::RefPtr<SkiaUnrefQueue> unref_queue,
+      fml::WeakPtr<ImageDecoder> image_decoder,
       std::string advisory_script_uri,
       std::string advisory_script_entrypoint,
       std::function<void(int64_t)> idle_notification_callback,
@@ -131,6 +132,7 @@ class RuntimeController final : public WindowClient {
   fml::RefPtr<const DartSnapshot> shared_snapshot_;
   TaskRunners task_runners_;
   fml::WeakPtr<IOManager> io_manager_;
+  fml::RefPtr<SkiaUnrefQueue> unref_queue_;
   fml::WeakPtr<ImageDecoder> image_decoder_;
   std::string advisory_script_uri_;
   std::string advisory_script_entrypoint_;
@@ -149,6 +151,7 @@ class RuntimeController final : public WindowClient {
       fml::RefPtr<const DartSnapshot> shared_snapshot,
       TaskRunners task_runners,
       fml::WeakPtr<IOManager> io_manager,
+      fml::RefPtr<SkiaUnrefQueue> unref_queue,
       fml::WeakPtr<ImageDecoder> image_decoder,
       std::string advisory_script_uri,
       std::string advisory_script_entrypoint,

shell/common/engine.cc

@@ -43,7 +43,8 @@ Engine::Engine(Delegate& delegate,
                TaskRunners task_runners,
                Settings settings,
                std::unique_ptr<Animator> animator,
-               fml::WeakPtr<IOManager> io_manager)
+               fml::WeakPtr<IOManager> io_manager,
+               fml::RefPtr<SkiaUnrefQueue> unref_queue)
     : delegate_(delegate),
       settings_(std::move(settings)),
       animator_(std::move(animator)),
@@ -64,6 +65,7 @@ Engine::Engine(Delegate& delegate,
       std::move(shared_snapshot),            // shared snapshot
       task_runners_,                         // task runners
       std::move(io_manager),                 // io manager
+      std::move(unref_queue),                // Skia unref queue
       image_decoder_.GetWeakPtr(),           // image decoder
       settings_.advisory_script_uri,         // advisory script uri
       settings_.advisory_script_entrypoint,  // advisory script entrypoint

shell/common/engine.h

@@ -280,7 +280,8 @@ class Engine final : public RuntimeDelegate, PointerDataDispatcher::Delegate {
          TaskRunners task_runners,
          Settings settings,
          std::unique_ptr<Animator> animator,
-         fml::WeakPtr<IOManager> io_manager);
+         fml::WeakPtr<IOManager> io_manager,
+         fml::RefPtr<SkiaUnrefQueue> unref_queue);
 
   //----------------------------------------------------------------------------
   /// @brief      Destroys the engine engine. Called by the shell on the UI task

shell/common/shell.cc

@@ -89,6 +89,8 @@ std::unique_ptr<Shell> Shell::CreateShellOnPlatformThread(
   auto io_manager_future = io_manager_promise.get_future();
   std::promise<fml::WeakPtr<ShellIOManager>> weak_io_manager_promise;
   auto weak_io_manager_future = weak_io_manager_promise.get_future();
+  std::promise<fml::RefPtr<SkiaUnrefQueue>> unref_queue_promise;
+  auto unref_queue_future = unref_queue_promise.get_future();
   auto io_task_runner = shell->GetTaskRunners().GetIOTaskRunner();
 
   // TODO(gw280): The WeakPtr here asserts that we are derefing it on the
@@ -101,6 +103,7 @@ std::unique_ptr<Shell> Shell::CreateShellOnPlatformThread(
       io_task_runner,
       [&io_manager_promise,                          //
        &weak_io_manager_promise,                     //
+       &unref_queue_promise,                         //
        platform_view = platform_view->GetWeakPtr(),  //
        io_task_runner                                //
   ]() {
@@ -108,6 +111,7 @@ std::unique_ptr<Shell> Shell::CreateShellOnPlatformThread(
         auto io_manager = std::make_unique<ShellIOManager>(
             platform_view.getUnsafe()->CreateResourceContext(), io_task_runner);
         weak_io_manager_promise.set_value(io_manager->GetWeakPtr());
+        unref_queue_promise.set_value(io_manager->GetSkiaUnrefQueue());
         io_manager_promise.set_value(std::move(io_manager));
       });
 
@@ -126,7 +130,8 @@ std::unique_ptr<Shell> Shell::CreateShellOnPlatformThread(
                          isolate_snapshot = std::move(isolate_snapshot),  //
                          shared_snapshot = std::move(shared_snapshot),    //
                          vsync_waiter = std::move(vsync_waiter),          //
-                         &weak_io_manager_future                          //
+                         &weak_io_manager_future,                         //
+                         &unref_queue_future                              //
   ]() mutable {
         TRACE_EVENT0("flutter", "ShellSetupUISubsystem");
         const auto& task_runners = shell->GetTaskRunners();
@@ -137,15 +142,16 @@ std::unique_ptr<Shell> Shell::CreateShellOnPlatformThread(
                                                    std::move(vsync_waiter));
 
         engine_promise.set_value(std::make_unique<Engine>(
-            *shell,                       //
-            dispatcher_maker,             //
-            *shell->GetDartVM(),          //
-            std::move(isolate_snapshot),  //
-            std::move(shared_snapshot),   //
-            task_runners,                 //
-            shell->GetSettings(),         //
-            std::move(animator),          //
-            weak_io_manager_future.get()  //
+            *shell,                        //
+            dispatcher_maker,              //
+            *shell->GetDartVM(),           //
+            std::move(isolate_snapshot),   //
+            std::move(shared_snapshot),    //
+            task_runners,                  //
+            shell->GetSettings(),          //
+            std::move(animator),           //
+            weak_io_manager_future.get(),  //
+            unref_queue_future.get()       //
             ));
       }));