From 6c948ba2485ea2c812b5e01414118508d8df1bc1 Mon Sep 17 00:00:00 2001
From: Jason Simmons <jason-simmons@users.noreply.github.com>
Date: Thu, 28 Mar 2019 12:37:18 -0700
Subject: [PATCH] Ensure that Picture::RasterizeToImage destroys Dart
 persistent values on the UI thread (#8182)

The DartPersistentValue used to hold the image callback is tied to a
Dart isolate.  Destructing the DartPersistentValue requires entering
the isolate and must be done on the UI thread.

Fixes https://github.com/flutter/flutter/issues/29379
---
 lib/ui/painting/picture.cc | 81 +++++++++++++++++---------------------
 1 file changed, 36 insertions(+), 45 deletions(-)

diff --git a/lib/ui/painting/picture.cc b/lib/ui/painting/picture.cc
index ac132d05a9..f7cdd78ca1 100644
--- a/lib/ui/painting/picture.cc
+++ b/lib/ui/painting/picture.cc
@@ -70,8 +70,8 @@ Dart_Handle Picture::RasterizeToImage(sk_sp<SkPicture> picture,
   }
 
   auto* dart_state = UIDartState::Current();
-  auto image_callback = std::make_unique<tonic::DartPersistentValue>(
-      dart_state, raw_image_callback);
+  tonic::DartPersistentValue* image_callback =
+      new tonic::DartPersistentValue(dart_state, raw_image_callback);
   auto unref_queue = dart_state->GetSkiaUnrefQueue();
   auto ui_task_runner = dart_state->GetTaskRunners().GetUITaskRunner();
   auto gpu_task_runner = dart_state->GetTaskRunners().GetGPUTaskRunner();
@@ -84,52 +84,43 @@ Dart_Handle Picture::RasterizeToImage(sk_sp<SkPicture> picture,
 
   auto picture_bounds = SkISize::Make(width, height);
 
-  auto ui_task = fml::MakeCopyable(
-      [ui_task_runner, image_callback = std::move(image_callback),
-       unref_queue](sk_sp<SkImage> raster_image) mutable {
-        // Send the raster image back to the UI thread for submission to the
-        // framework.
-        fml::TaskRunner::RunNowOrPostTask(
-            ui_task_runner,
-            fml::MakeCopyable([raster_image,
-                               image_callback = std::move(image_callback),
-                               unref_queue]() mutable {
-              auto dart_state = image_callback->dart_state().lock();
-              if (!dart_state) {
-                // The root isolate could have died in the meantime.
-                return;
-              }
-              tonic::DartState::Scope scope(dart_state);
-
-              if (!raster_image) {
-                tonic::DartInvoke(image_callback->Get(), {Dart_Null()});
-                return;
-              }
-
-              auto dart_image = CanvasImage::Create();
-              dart_image->set_image(
-                  {std::move(raster_image), std::move(unref_queue)});
-              auto* raw_dart_image = tonic::ToDart(std::move(dart_image));
-
-              // All done!
-              tonic::DartInvoke(image_callback->Get(), {raw_dart_image});
-            }));
-      });
-
-  auto gpu_task = fml::MakeCopyable([gpu_task_runner, picture, picture_bounds,
-                                     snapshot_delegate, ui_task]() {
-    fml::TaskRunner::RunNowOrPostTask(gpu_task_runner, [snapshot_delegate,
-                                                        picture, picture_bounds,
-                                                        ui_task]() {
-      // Snapshot the picture on the GPU thread. This thread has access to the
-      // GPU contexts that may contain the sole references to texture backed
-      // images in the picture.
-      ui_task(snapshot_delegate->MakeRasterSnapshot(picture, picture_bounds));
-    });
+  auto ui_task = fml::MakeCopyable([image_callback, unref_queue](
+                                       sk_sp<SkImage> raster_image) mutable {
+    auto dart_state = image_callback->dart_state().lock();
+    if (!dart_state) {
+      // The root isolate could have died in the meantime.
+      return;
+    }
+    tonic::DartState::Scope scope(dart_state);
+
+    if (!raster_image) {
+      tonic::DartInvoke(image_callback->Get(), {Dart_Null()});
+      return;
+    }
+
+    auto dart_image = CanvasImage::Create();
+    dart_image->set_image({std::move(raster_image), std::move(unref_queue)});
+    auto* raw_dart_image = tonic::ToDart(std::move(dart_image));
+
+    // All done!
+    tonic::DartInvoke(image_callback->Get(), {raw_dart_image});
+
+    // image_callback is associated with the Dart isolate and must be deleted
+    // on the UI thread
+    delete image_callback;
   });
 
   // Kick things off on the GPU.
-  gpu_task();
+  fml::TaskRunner::RunNowOrPostTask(
+      gpu_task_runner,
+      [ui_task_runner, snapshot_delegate, picture, picture_bounds, ui_task] {
+        sk_sp<SkImage> raster_image =
+            snapshot_delegate->MakeRasterSnapshot(picture, picture_bounds);
+
+        fml::TaskRunner::RunNowOrPostTask(
+            ui_task_runner,
+            [ui_task, raster_image]() { ui_task(raster_image); });
+      });
 
   return Dart_Null();
 }
-- 
GitLab