Destroy textures on raster thread (#427)

* Destroy textures on raster thread

Signed-off-by: Bari Rao <[email protected]>

* Fix format

Signed-off-by: Bari Rao <[email protected]>

---------

Signed-off-by: Bari Rao <[email protected]>

src/flutter/fml/closure.h

@@ -0,0 +1,77 @@
+// 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 FLUTTER_FML_CLOSURE_H_
+#define FLUTTER_FML_CLOSURE_H_
+
+#include <functional>
+
+#include "flutter/fml/macros.h"
+
+namespace fml {
+
+using closure = std::function<void()>;
+
+//------------------------------------------------------------------------------
+/// @brief      Wraps a closure that is invoked in the destructor unless
+///             released by the caller.
+///
+///             This is especially useful in dealing with APIs that return a
+///             resource by accepting ownership of a sub-resource and a closure
+///             that releases that resource. When such APIs are chained, each
+///             link in the chain must check that the next member in the chain
+///             has accepted the resource. If not, it must invoke the closure
+///             eagerly. Not doing this results in a resource leak in the
+///             erroneous case. Using this wrapper, the closure can be released
+///             once the next call in the chain has successfully accepted
+///             ownership of the resource. If not, the closure gets invoked
+///             automatically at the end of the scope. This covers the cases
+///             where there are early returns as well.
+///
+class ScopedCleanupClosure final {
+ public:
+  ScopedCleanupClosure() = default;
+
+  ScopedCleanupClosure(ScopedCleanupClosure&& other) {
+    closure_ = other.Release();
+  }
+
+  ScopedCleanupClosure& operator=(ScopedCleanupClosure&& other) {
+    closure_ = other.Release();
+    return *this;
+  }
+
+  explicit ScopedCleanupClosure(const fml::closure& closure)
+      : closure_(closure) {}
+
+  ~ScopedCleanupClosure() { Reset(); }
+
+  fml::closure SetClosure(const fml::closure& closure) {
+    auto old_closure = closure_;
+    closure_ = closure;
+    return old_closure;
+  }
+
+  fml::closure Release() {
+    fml::closure closure = closure_;
+    closure_ = nullptr;
+    return closure;
+  }
+
+  void Reset() {
+    if (closure_) {
+      closure_();
+      closure_ = nullptr;
+    }
+  }
+
+ private:
+  fml::closure closure_;
+
+  FML_DISALLOW_COPY_AND_ASSIGN(ScopedCleanupClosure);
+};
+
+}  // namespace fml
+
+#endif  // FLUTTER_FML_CLOSURE_H_

src/flutter/fml/macros.h

@@ -0,0 +1,44 @@
+// 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 FLUTTER_FML_MACROS_H_
+#define FLUTTER_FML_MACROS_H_
+
+#ifndef FML_USED_ON_EMBEDDER
+
+#define FML_EMBEDDER_ONLY [[deprecated]]
+
+#else  // FML_USED_ON_EMBEDDER
+
+#define FML_EMBEDDER_ONLY
+
+#endif  // FML_USED_ON_EMBEDDER
+
+#define FML_DISALLOW_COPY(TypeName) TypeName(const TypeName&) = delete
+
+#define FML_DISALLOW_ASSIGN(TypeName) \
+  TypeName& operator=(const TypeName&) = delete
+
+#define FML_DISALLOW_MOVE(TypeName) \
+  TypeName(TypeName&&) = delete;    \
+  TypeName& operator=(TypeName&&) = delete
+
+#define FML_DISALLOW_COPY_AND_ASSIGN(TypeName) \
+  TypeName(const TypeName&) = delete;          \
+  TypeName& operator=(const TypeName&) = delete
+
+#define FML_DISALLOW_COPY_ASSIGN_AND_MOVE(TypeName) \
+  TypeName(const TypeName&) = delete;               \
+  TypeName(TypeName&&) = delete;                    \
+  TypeName& operator=(const TypeName&) = delete;    \
+  TypeName& operator=(TypeName&&) = delete
+
+#define FML_DISALLOW_IMPLICIT_CONSTRUCTORS(TypeName) \
+  TypeName() = delete;                               \
+  FML_DISALLOW_COPY_ASSIGN_AND_MOVE(TypeName)
+
+#define FML_FRIEND_TEST(test_case_name, test_name) \
+  friend class test_case_name##_##test_name##_Test
+
+#endif  // FLUTTER_FML_MACROS_H_

src/flutter/shell/platform/common/client_wrapper/core_implementations.cc

@@ -210,9 +210,32 @@ bool TextureRegistrarImpl::MarkTextureFrameAvailable(int64_t texture_id) {
       texture_registrar_ref_, texture_id);
 }
 
+void TextureRegistrarImpl::UnregisterTexture(int64_t texture_id,
+                                             std::function<void()> callback) {
+  if (callback == nullptr) {
+    FlutterDesktopTextureRegistrarUnregisterExternalTexture(
+        texture_registrar_ref_, texture_id, nullptr, nullptr);
+    return;
+  }
+
+  struct Captures {
+    std::function<void()> callback;
+  };
+  auto captures = new Captures();
+  captures->callback = std::move(callback);
+  FlutterDesktopTextureRegistrarUnregisterExternalTexture(
+      texture_registrar_ref_, texture_id,
+      [](void* opaque) {
+        auto captures = reinterpret_cast<Captures*>(opaque);
+        captures->callback();
+        delete captures;
+      },
+      captures);
+}
+
 bool TextureRegistrarImpl::UnregisterTexture(int64_t texture_id) {
-  return FlutterDesktopTextureRegistrarUnregisterExternalTexture(
-      texture_registrar_ref_, texture_id);
+  UnregisterTexture(texture_id, nullptr);
+  return true;
 }
 
 }  // namespace flutter

src/flutter/shell/platform/common/client_wrapper/include/flutter/texture_registrar.h

@@ -128,8 +128,13 @@ class TextureRegistrar {
   // the callback that was provided upon creating the texture.
   virtual bool MarkTextureFrameAvailable(int64_t texture_id) = 0;
 
-  // Unregisters an existing Texture object.
-  // Textures must not be unregistered while they're in use.
+  // Asynchronously unregisters an existing texture object.
+  // Upon completion, the optional |callback| gets invoked.
+  virtual void UnregisterTexture(int64_t texture_id,
+                                 std::function<void()> callback) = 0;
+
+  // Unregisters an existing texture object.
+  // DEPRECATED: Use UnregisterTexture(texture_id, optional_callback) instead.
   virtual bool UnregisterTexture(int64_t texture_id) = 0;
 };
 

src/flutter/shell/platform/common/client_wrapper/texture_registrar_impl.h

@@ -27,6 +27,10 @@ class TextureRegistrarImpl : public TextureRegistrar {
   // |flutter::TextureRegistrar|
   bool MarkTextureFrameAvailable(int64_t texture_id) override;
 
+  // |flutter::TextureRegistrar|
+  void UnregisterTexture(int64_t texture_id,
+                         std::function<void()> callback) override;
+
   // |flutter::TextureRegistrar|
   bool UnregisterTexture(int64_t texture_id) override;
 

src/flutter/shell/platform/common/public/flutter_texture_registrar.h

@@ -195,13 +195,15 @@ FLUTTER_EXPORT int64_t FlutterDesktopTextureRegistrarRegisterExternalTexture(
     FlutterDesktopTextureRegistrarRef texture_registrar,
     const FlutterDesktopTextureInfo* info);
 
-// Unregisters an existing texture from the Flutter engine for a |texture_id|.
-// Returns true on success or false if the specified texture doesn't exist.
+// Asynchronously unregisters the texture identified by |texture_id| from the
+// Flutter engine.
+// An optional |callback| gets invoked upon completion.
 // This function can be called from any thread.
-// However, textures must not be unregistered while they're in use.
-FLUTTER_EXPORT bool FlutterDesktopTextureRegistrarUnregisterExternalTexture(
+FLUTTER_EXPORT void FlutterDesktopTextureRegistrarUnregisterExternalTexture(
     FlutterDesktopTextureRegistrarRef texture_registrar,
-    int64_t texture_id);
+    int64_t texture_id,
+    void (*callback)(void* user_data),
+    void* user_data);
 
 // Marks that a new texture frame is available for a given |texture_id|.
 // Returns true on success or false if the specified texture doesn't exist.

src/flutter/shell/platform/linux_embedded/flutter_elinux.cc

@@ -284,11 +284,18 @@ int64_t FlutterDesktopTextureRegistrarRegisterExternalTexture(
       ->RegisterTexture(texture_info);
 }
 
-bool FlutterDesktopTextureRegistrarUnregisterExternalTexture(
+void FlutterDesktopTextureRegistrarUnregisterExternalTexture(
     FlutterDesktopTextureRegistrarRef texture_registrar,
-    int64_t texture_id) {
-  return TextureRegistrarFromHandle(texture_registrar)
-      ->UnregisterTexture(texture_id);
+    int64_t texture_id,
+    void (*callback)(void* user_data),
+    void* user_data) {
+  auto registrar = TextureRegistrarFromHandle(texture_registrar);
+  if (callback) {
+    registrar->UnregisterTexture(
+        texture_id, [callback, user_data]() { callback(user_data); });
+    return;
+  }
+  registrar->UnregisterTexture(texture_id);
 }
 
 bool FlutterDesktopTextureRegistrarMarkExternalTextureFrameAvailable(

src/flutter/shell/platform/linux_embedded/flutter_elinux_engine.cc

@@ -425,6 +425,26 @@ bool FlutterELinuxEngine::MarkExternalTextureFrameAvailable(
               engine_, texture_id) == kSuccess);
 }
 
+bool FlutterELinuxEngine::PostRasterThreadTask(fml::closure callback) {
+  struct Captures {
+    fml::closure callback;
+  };
+  auto captures = new Captures();
+  captures->callback = std::move(callback);
+  if (embedder_api_.PostRenderThreadTask(
+          engine_,
+          [](void* opaque) {
+            auto captures = reinterpret_cast<Captures*>(opaque);
+            captures->callback();
+            delete captures;
+          },
+          captures) == kSuccess) {
+    return true;
+  }
+  delete captures;
+  return false;
+}
+
 void FlutterELinuxEngine::OnVsync(uint64_t last_frame_time_nanos,
                                   uint64_t vsync_interval_time_nanos) {
   uint64_t current_time_nanos = embedder_api_.GetCurrentTime();

src/flutter/shell/platform/linux_embedded/flutter_elinux_engine.h

@@ -120,6 +120,9 @@ class FlutterELinuxEngine {
   // given |texture_id|.
   bool MarkExternalTextureFrameAvailable(int64_t texture_id);
 
+  // Posts the given callback onto the raster thread.
+  bool PostRasterThreadTask(fml::closure callback);
+
   // Notifies the engine about the vsync event.
   void OnVsync(uint64_t last_frame_time_nanos,
                uint64_t vsync_interval_time_nanos);

src/flutter/shell/platform/linux_embedded/flutter_elinux_texture_registrar.cc

@@ -72,20 +72,28 @@ int64_t FlutterELinuxTextureRegistrar::EmplaceTexture(
   return texture_id;
 }
 
-bool FlutterELinuxTextureRegistrar::UnregisterTexture(int64_t texture_id) {
-  {
-    std::lock_guard<std::mutex> lock(map_mutex_);
-    auto it = textures_.find(texture_id);
-    if (it == textures_.end()) {
-      return false;
-    }
-    textures_.erase(it);
-  }
-
+void FlutterELinuxTextureRegistrar::UnregisterTexture(int64_t texture_id,
+                                                      fml::closure callback) {
   engine_->task_runner()->RunNowOrPostTask([engine = engine_, texture_id]() {
     engine->UnregisterExternalTexture(texture_id);
   });
-  return true;
+
+  bool posted = engine_->PostRasterThreadTask([this, texture_id, callback]() {
+    {
+      std::lock_guard<std::mutex> lock(map_mutex_);
+      auto it = textures_.find(texture_id);
+      if (it != textures_.end()) {
+        textures_.erase(it);
+      }
+    }
+    if (callback) {
+      callback();
+    }
+  });
+
+  if (!posted && callback) {
+    callback();
+  }
 }
 
 bool FlutterELinuxTextureRegistrar::MarkTextureFrameAvailable(

src/flutter/shell/platform/linux_embedded/flutter_elinux_texture_registrar.h

@@ -9,6 +9,7 @@
 #include <mutex>
 #include <unordered_map>
 
+#include "flutter/fml/closure.h"
 #include "flutter/shell/platform/common/public/flutter_texture_registrar.h"
 #include "flutter/shell/platform/linux_embedded/external_texture.h"
 
@@ -28,8 +29,7 @@ class FlutterELinuxTextureRegistrar {
   int64_t RegisterTexture(const FlutterDesktopTextureInfo* texture_info);
 
   // Attempts to unregister the texture identified by |texture_id|.
-  // Returns true if the texture was successfully unregistered.
-  bool UnregisterTexture(int64_t texture_id);
+  void UnregisterTexture(int64_t texture_id, fml::closure callback = nullptr);
 
   // Notifies the engine about a new frame being available.
   // Returns true on success.