wph migration

Author: dcharkes

lib/ui/painting/image_decoder.cc

@@ -223,12 +223,13 @@ void ImageDecoder::Decode(fml::RefPtr<ImageDescriptor> descriptor,
   FML_DCHECK(callback);
   FML_DCHECK(runners_.GetUITaskRunner()->RunsTasksOnCurrentThread());
 
-  // Always service the callback on the UI thread.
-  auto result = [callback, ui_runner = runners_.GetUITaskRunner()](
+  // Always service the callback (and cleanup the descriptor) on the UI thread.
+  auto result = [callback, descriptor, ui_runner = runners_.GetUITaskRunner()](
                     SkiaGPUObject<SkImage> image,
                     fml::tracing::TraceFlow flow) {
-    ui_runner->PostTask(fml::MakeCopyable(
-        [callback, image = std::move(image), flow = std::move(flow)]() mutable {
+    ui_runner->PostTask(
+        fml::MakeCopyable([callback, descriptor, image = std::move(image),
+                           flow = std::move(flow)]() mutable {
           // We are going to terminate the trace flow here. Flows cannot
           // terminate without a base trace. Add one explicitly.
           TRACE_EVENT0("flutter", "ImageDecodeCallback");

lib/ui/painting/image_encoding.cc

@@ -35,9 +35,7 @@ enum ImageByteFormat {
   kPNG,
 };
 
-void FinalizeSkData(void* isolate_callback_data,
-                    Dart_WeakPersistentHandle handle,
-                    void* peer) {
+void FinalizeSkData(void* isolate_callback_data, void* peer) {
   SkData* buffer = reinterpret_cast<SkData*>(peer);
   buffer->unref();
 }

runtime/dart_isolate.cc

@@ -868,6 +868,11 @@ void DartIsolate::AddIsolateShutdownCallback(const fml::closure& closure) {
 }
 
 void DartIsolate::OnShutdownCallback() {
+  tonic::DartState* state = tonic::DartState::Current();
+  if (state != nullptr) {
+    state->SetIsShuttingDown();
+  }
+
   {
     tonic::DartApiScope api_scope;
     Dart_Handle sticky_error = Dart_GetStickyError();

shell/platform/embedder/embedder.cc

@@ -1883,8 +1883,7 @@ FlutterEngineResult FlutterEnginePostDartObject(
         dart_object.value.as_external_typed_data.data = buffer;
         dart_object.value.as_external_typed_data.peer = peer;
         dart_object.value.as_external_typed_data.callback =
-            +[](void* unused_isolate_callback_data,
-                Dart_WeakPersistentHandle unused_handle, void* peer) {
+            +[](void* unused_isolate_callback_data, void* peer) {
               auto typed_peer = reinterpret_cast<ExternalTypedDataPeer*>(peer);
               typed_peer->trampoline(typed_peer->user_data);
               delete typed_peer;

shell/platform/fuchsia/dart_runner/dart_runner.cc

@@ -92,6 +92,10 @@ void IsolateShutdownCallback(void* isolate_group_data, void* isolate_data) {
     tonic::DartMicrotaskQueue::GetForCurrentThread()->Destroy();
     async_loop_quit(loop);
   }
+
+  auto state =
+      static_cast<std::shared_ptr<tonic::DartState>*>(isolate_group_data);
+  state->get()->SetIsShuttingDown();
 }
 
 void IsolateGroupCleanupCallback(void* isolate_group_data) {

third_party/tonic/BUILD.gn

@@ -29,6 +29,8 @@ source_set("tonic") {
     "dart_persistent_value.h",
     "dart_state.cc",
     "dart_state.h",
+    "dart_weak_persistent_value.cc",
+    "dart_weak_persistent_value.h",
     "dart_wrappable.cc",
     "dart_wrappable.h",
     "dart_wrapper_info.h",

third_party/tonic/dart_state.cc

@@ -27,7 +27,8 @@ DartState::DartState(int dirfd,
       message_handler_(new DartMessageHandler()),
       file_loader_(new FileLoader(dirfd)),
       message_epilogue_(message_epilogue),
-      has_set_return_code_(false) {}
+      has_set_return_code_(false),
+      is_shutting_down_(false) {}
 
 DartState::~DartState() {}
 

third_party/tonic/dart_state.h

@@ -68,6 +68,9 @@ class DartState : public std::enable_shared_from_this<DartState> {
   void SetReturnCodeCallback(std::function<void(uint32_t)> callback);
   bool has_set_return_code() const { return has_set_return_code_; }
 
+  void SetIsShuttingDown() { is_shutting_down_ = true; }
+  bool IsShuttingDown() { return is_shutting_down_; }
+
   virtual void DidSetIsolate();
 
   static Dart_Handle HandleLibraryTag(Dart_LibraryTag tag,
@@ -83,6 +86,7 @@ class DartState : public std::enable_shared_from_this<DartState> {
   std::function<void(Dart_Handle)> message_epilogue_;
   std::function<void(uint32_t)> set_return_code_callback_;
   bool has_set_return_code_;
+  std::atomic<bool> is_shutting_down_;
 
  protected:
   TONIC_DISALLOW_COPY_AND_ASSIGN(DartState);

third_party/tonic/dart_weak_persistent_value.cc

@@ -0,0 +1,65 @@
+// Copyright 2013 The Flutter Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "tonic/dart_weak_persistent_value.h"
+
+#include "tonic/dart_state.h"
+#include "tonic/scopes/dart_isolate_scope.h"
+
+namespace tonic {
+
+DartWeakPersistentValue::DartWeakPersistentValue() : handle_(nullptr) {}
+
+DartWeakPersistentValue::~DartWeakPersistentValue() {
+  Clear();
+}
+
+void DartWeakPersistentValue::Set(DartState* dart_state,
+                                  Dart_Handle object,
+                                  void* peer,
+                                  intptr_t external_allocation_size,
+                                  Dart_HandleFinalizer callback) {
+  TONIC_DCHECK(is_empty());
+  dart_state_ = dart_state->GetWeakPtr();
+  handle_ = Dart_NewWeakPersistentHandle(object, peer, external_allocation_size,
+                                         callback);
+}
+
+void DartWeakPersistentValue::Clear() {
+  if (!handle_) {
+    return;
+  }
+
+  auto dart_state = dart_state_.lock();
+  if (!dart_state) {
+    return;
+  }
+
+  if (!dart_state->IsShuttingDown()) {
+    if (Dart_CurrentIsolateGroup()) {
+      Dart_DeleteWeakPersistentHandle(handle_);
+    } else {
+      // If we are not on the mutator thread, this will fail. The caller must
+      // ensure to be on the mutator thread.
+      DartIsolateScope scope(dart_state->isolate());
+      Dart_DeleteWeakPersistentHandle(handle_);
+    }
+  }
+  // If it's shutting down, the handle will be deleted already.
+
+  dart_state_.reset();
+  handle_ = nullptr;
+}
+
+Dart_Handle DartWeakPersistentValue::Get() {
+  auto dart_state = dart_state_.lock();
+  TONIC_DCHECK(dart_state);
+  TONIC_DCHECK(!dart_state->IsShuttingDown());
+  if (!handle_) {
+    return nullptr;
+  }
+  return Dart_HandleFromWeakPersistent(handle_);
+}
+
+}  // namespace tonic

third_party/tonic/dart_weak_persistent_value.h

@@ -0,0 +1,48 @@
+// Copyright 2013 The Flutter Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef LIB_TONIC_DART_WEAK_PERSISTENT_VALUE_H_
+#define LIB_TONIC_DART_WEAK_PERSISTENT_VALUE_H_
+
+#include <memory>
+
+#include "third_party/dart/runtime/include/dart_api.h"
+#include "tonic/common/macros.h"
+
+namespace tonic {
+class DartState;
+
+// DartWeakPersistentValue is a bookkeeping class to help pair calls to
+// Dart_NewWeakPersistentHandle with Dart_DeleteWeakPersistentHandle even in
+// the case if IsolateGroup shutdown. Consider using this class instead of
+// holding a Dart_PersistentHandle directly so that you don't leak the
+// Dart_WeakPersistentHandle.
+class DartWeakPersistentValue {
+ public:
+  DartWeakPersistentValue();
+  ~DartWeakPersistentValue();
+
+  Dart_WeakPersistentHandle value() const { return handle_; }
+  bool is_empty() const { return handle_ == nullptr; }
+
+  void Set(DartState* dart_state,
+           Dart_Handle object,
+           void* peer,
+           intptr_t external_allocation_size,
+           Dart_HandleFinalizer callback);
+  void Clear();
+  Dart_Handle Get();
+
+  const std::weak_ptr<DartState>& dart_state() const { return dart_state_; }
+
+ private:
+  std::weak_ptr<DartState> dart_state_;
+  Dart_WeakPersistentHandle handle_;
+
+  TONIC_DISALLOW_COPY_AND_ASSIGN(DartWeakPersistentValue);
+};
+
+}  // namespace tonic
+
+#endif  // LIB_TONIC_DART_WEAK_PERSISTENT_VALUE_H_