Windows: Texture Registrar: Destroy textures on raster thread

shell/platform/common/client_wrapper/core_implementations.cc

@@ -195,9 +195,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 = std::make_unique<Captures>();
+  captures->callback = callback;
+  FlutterDesktopTextureRegistrarUnregisterExternalTexture(
+      texture_registrar_ref_, texture_id,
+      [](void* opaque) {
+        auto captures = reinterpret_cast<Captures*>(opaque);
+        captures->callback();
+        delete captures;
+      },
+      captures.release());
+}
+
 bool TextureRegistrarImpl::UnregisterTexture(int64_t texture_id) {
-  return FlutterDesktopTextureRegistrarUnregisterExternalTexture(
-      texture_registrar_ref_, texture_id);
+  UnregisterTexture(texture_id, nullptr);
+  return true;
 }
 
 }  // namespace flutter

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

@@ -95,9 +95,12 @@ 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.
-  virtual bool UnregisterTexture(int64_t texture_id) = 0;
+  // Asynchronously unregisters an existing texture object.
+  virtual void UnregisterTexture(int64_t texture_id,
+                                 std::function<void()> callback) = 0;
+
+  // Unregisters an existing texture object.
+  [[deprecated]] virtual bool UnregisterTexture(int64_t texture_id) = 0;
 };
 
 }  // namespace flutter

shell/platform/common/client_wrapper/testing/stub_flutter_api.cc

@@ -109,15 +109,17 @@ int64_t FlutterDesktopTextureRegistrarRegisterExternalTexture(
   return result;
 }
 
-bool FlutterDesktopTextureRegistrarUnregisterExternalTexture(
+void FlutterDesktopTextureRegistrarUnregisterExternalTexture(
     FlutterDesktopTextureRegistrarRef texture_registrar,
-    int64_t texture_id) {
-  bool result = false;
+    int64_t texture_id,
+    void (*callback)(void* user_data),
+    void* user_data) {
   if (s_stub_implementation) {
-    result = s_stub_implementation->TextureRegistrarUnregisterExternalTexture(
-        texture_id);
+    s_stub_implementation->TextureRegistrarUnregisterExternalTexture(
+        texture_id, callback, user_data);
+  } else if (callback) {
+    callback(user_data);
   }
-  return result;
 }
 
 bool FlutterDesktopTextureRegistrarMarkExternalTextureFrameAvailable(

shell/platform/common/client_wrapper/testing/stub_flutter_api.h

@@ -65,18 +65,19 @@ class StubFlutterApi {
                                     FlutterDesktopMessageCallback callback,
                                     void* user_data) {}
 
-  // Called for FlutterDesktopRegisterExternalTexture.
+  // Called for FlutterDesktopTextureRegistrarRegisterExternalTexture.
   virtual int64_t TextureRegistrarRegisterExternalTexture(
       const FlutterDesktopTextureInfo* info) {
     return -1;
   }
 
-  // Called for FlutterDesktopUnregisterExternalTexture.
-  virtual bool TextureRegistrarUnregisterExternalTexture(int64_t texture_id) {
-    return false;
-  }
+  // Called for FlutterDesktopTextureRegistrarUnregisterExternalTexture.
+  virtual void TextureRegistrarUnregisterExternalTexture(
+      int64_t texture_id,
+      void (*callback)(void* user_data),
+      void* user_data) {}
 
-  // Called for FlutterDesktopMarkExternalTextureFrameAvailable.
+  // Called for FlutterDesktopTextureRegistrarMarkExternalTextureFrameAvailable.
   virtual bool TextureRegistrarMarkTextureFrameAvailable(int64_t texture_id) {
     return false;
   }

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;
 

shell/platform/common/client_wrapper/texture_registrar_unittests.cc

@@ -8,6 +8,7 @@
 #include <memory>
 #include <vector>
 
+#include "flutter/fml/synchronization/waitable_event.h"
 #include "flutter/shell/platform/common/client_wrapper/include/flutter/plugin_registrar.h"
 #include "flutter/shell/platform/common/client_wrapper/testing/stub_flutter_api.h"
 #include "gtest/gtest.h"
@@ -41,13 +42,17 @@ class TestApi : public testing::StubFlutterApi {
     return last_texture_id_;
   }
 
-  bool TextureRegistrarUnregisterExternalTexture(int64_t texture_id) override {
+  void TextureRegistrarUnregisterExternalTexture(
+      int64_t texture_id,
+      void (*callback)(void* user_data),
+      void* user_data) override {
     auto it = textures_.find(texture_id);
     if (it != textures_.end()) {
       textures_.erase(it);
-      return true;
     }
-    return false;
+    if (callback) {
+      callback(user_data);
+    }
   }
 
   bool TextureRegistrarMarkTextureFrameAvailable(int64_t texture_id) override {
@@ -110,24 +115,27 @@ TEST(TextureRegistrarTest, RegisterUnregisterTexture) {
   EXPECT_TRUE(success);
   EXPECT_EQ(texture->mark_count, 3);
 
-  success = textures->UnregisterTexture(texture_id);
-  EXPECT_TRUE(success);
+  fml::AutoResetWaitableEvent unregister_latch;
+  textures->UnregisterTexture(texture_id, [&]() { unregister_latch.Signal(); });
+  unregister_latch.Wait();
 
   texture = test_api->GetFakeTexture(texture_id);
   EXPECT_EQ(texture, nullptr);
   EXPECT_EQ(test_api->textures_size(), static_cast<size_t>(0));
 }
 
-// Tests that unregistering a texture with an unknown id returns false.
+// Tests that the unregister callback gets also invoked when attempting to
+// unregister a texture with an unknown id.
 TEST(TextureRegistrarTest, UnregisterInvalidTexture) {
   auto dummy_registrar_handle =
       reinterpret_cast<FlutterDesktopPluginRegistrarRef>(1);
   PluginRegistrar registrar(dummy_registrar_handle);
 
   TextureRegistrar* textures = registrar.texture_registrar();
 
-  bool success = textures->UnregisterTexture(42);
-  EXPECT_FALSE(success);
+  fml::AutoResetWaitableEvent latch;
+  textures->UnregisterTexture(42, [&]() { latch.Signal(); });
+  latch.Wait();
 }
 
 // Tests that claiming a new frame being available for an unknown texture

shell/platform/common/public/flutter_texture_registrar.h

@@ -162,13 +162,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.

shell/platform/glfw/flutter_glfw.cc

@@ -728,10 +728,9 @@ static void SetUpLocales(FlutterDesktopEngineState* state) {
   // Convert the locale list to the locale pointer list that must be provided.
   std::vector<const FlutterLocale*> flutter_locale_list;
   flutter_locale_list.reserve(flutter_locales.size());
-  std::transform(
-      flutter_locales.begin(), flutter_locales.end(),
-      std::back_inserter(flutter_locale_list),
-      [](const auto& arg) -> const auto* { return &arg; });
+  std::transform(flutter_locales.begin(), flutter_locales.end(),
+                 std::back_inserter(flutter_locale_list),
+                 [](const auto& arg) -> const auto* { return &arg; });
   FlutterEngineResult result = FlutterEngineUpdateLocales(
       state->flutter_engine, flutter_locale_list.data(),
       flutter_locale_list.size());
@@ -1114,11 +1113,12 @@ int64_t FlutterDesktopTextureRegistrarRegisterExternalTexture(
   return -1;
 }
 
-bool FlutterDesktopTextureRegistrarUnregisterExternalTexture(
+void FlutterDesktopTextureRegistrarUnregisterExternalTexture(
     FlutterDesktopTextureRegistrarRef texture_registrar,
-    int64_t texture_id) {
+    int64_t texture_id,
+    void (*callback)(void* user_data),
+    void* user_data) {
   std::cerr << "GLFW Texture support is not implemented yet." << std::endl;
-  return false;
 }
 
 bool FlutterDesktopTextureRegistrarMarkExternalTextureFrameAvailable(

shell/platform/windows/flutter_windows.cc

@@ -302,11 +302,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(

shell/platform/windows/flutter_windows_engine.cc

@@ -557,6 +557,27 @@ bool FlutterWindowsEngine::MarkExternalTextureFrameAvailable(
               engine_, texture_id) == kSuccess);
 }
 
+bool FlutterWindowsEngine::PostRasterThreadTask(
+    std::function<void()> callback) {
+  struct Captures {
+    std::function<void()> callback;
+  };
+  auto captures = std::make_unique<Captures>();
+  captures->callback = callback;
+  if (embedder_api_.PostRenderThreadTask(
+          engine_,
+          [](void* opaque) {
+            auto captures = reinterpret_cast<Captures*>(opaque);
+            captures->callback();
+            delete captures;
+          },
+          captures.get()) == kSuccess) {
+    captures.release();
+    return true;
+  }
+  return false;
+}
+
 bool FlutterWindowsEngine::DispatchSemanticsAction(
     uint64_t target,
     FlutterSemanticsAction action,

shell/platform/windows/flutter_windows_engine.h

@@ -190,6 +190,9 @@ class FlutterWindowsEngine {
   // given |texture_id|.
   bool MarkExternalTextureFrameAvailable(int64_t texture_id);
 
+  // Posts the given callback onto the raster thread.
+  bool PostRasterThreadTask(std::function<void()> callback);
+
   // Invoke on the embedder's vsync callback to schedule a frame.
   void OnVsync(intptr_t baton);
 

shell/platform/windows/flutter_windows_engine_unittests.cc

@@ -446,5 +446,21 @@ TEST(FlutterWindowsEngine, UpdateHighContrastFeature) {
   EXPECT_FALSE(engine->high_contrast_enabled());
 }
 
+TEST(FlutterWindowsEngine, PostRasterThreadTask) {
+  std::unique_ptr<FlutterWindowsEngine> engine = GetTestEngine();
+  EngineModifier modifier(engine.get());
+
+  modifier.embedder_api().PostRenderThreadTask = MOCK_ENGINE_PROC(
+      PostRenderThreadTask, ([](auto engine, auto callback, auto context) {
+        callback(context);
+        return kSuccess;
+      }));
+
+  bool called = false;
+  engine->PostRasterThreadTask([&called]() { called = true; });
+
+  EXPECT_TRUE(called);
+}
+
 }  // namespace testing
 }  // namespace flutter

shell/platform/windows/flutter_windows_texture_registrar.cc

@@ -77,20 +77,26 @@ int64_t FlutterWindowsTextureRegistrar::EmplaceTexture(
   return texture_id;
 }
 
-bool FlutterWindowsTextureRegistrar::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 FlutterWindowsTextureRegistrar::UnregisterTexture(
+    int64_t texture_id,
+    std::function<void()> callback) {
   engine_->task_runner()->RunNowOrPostTask([engine = engine_, texture_id]() {
     engine->UnregisterExternalTexture(texture_id);
   });
-  return true;
+
+  engine_->PostRasterThreadTask(
+      [this, texture_id, callback = std::move(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();
+        }
+      });
 }
 
 bool FlutterWindowsTextureRegistrar::MarkTextureFrameAvailable(

shell/platform/windows/flutter_windows_texture_registrar.h

@@ -5,6 +5,7 @@
 #ifndef FLUTTER_SHELL_PLATFORM_WINDOWS_FLUTTER_WINDOWS_TEXTURE_REGISTRAR_H_
 #define FLUTTER_SHELL_PLATFORM_WINDOWS_FLUTTER_WINDOWS_TEXTURE_REGISTRAR_H_
 
+#include <functional>
 #include <memory>
 #include <mutex>
 #include <unordered_map>
@@ -28,8 +29,8 @@ class FlutterWindowsTextureRegistrar {
   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,
+                         std::function<void()> callback = nullptr);
 
   // Notifies the engine about a new frame being available.
   // Returns true on success.

shell/platform/windows/flutter_windows_texture_registrar_unittests.cc

@@ -4,6 +4,7 @@
 
 #include <iostream>
 
+#include "flutter/fml/synchronization/waitable_event.h"
 #include "flutter/shell/platform/embedder/test_utils/proc_table_replacement.h"
 #include "flutter/shell/platform/windows/flutter_windows_engine.h"
 #include "flutter/shell/platform/windows/flutter_windows_texture_registrar.h"
@@ -113,6 +114,13 @@ TEST(FlutterWindowsTextureRegistrarTest, RegisterUnregisterTexture) {
                          return kSuccess;
                        }));
 
+  modifier.embedder_api().PostRenderThreadTask =
+      MOCK_ENGINE_PROC(PostRenderThreadTask,
+                       [](auto engine, auto callback, void* callback_data) {
+                         callback(callback_data);
+                         return kSuccess;
+                       });
+
   auto texture_id = registrar.RegisterTexture(&texture_info);
   EXPECT_TRUE(register_called);
   EXPECT_NE(texture_id, -1);
@@ -121,8 +129,10 @@ TEST(FlutterWindowsTextureRegistrarTest, RegisterUnregisterTexture) {
   EXPECT_TRUE(registrar.MarkTextureFrameAvailable(texture_id));
   EXPECT_TRUE(mark_frame_available_called);
 
-  EXPECT_TRUE(registrar.UnregisterTexture(texture_id));
-  EXPECT_TRUE(unregister_called);
+  fml::AutoResetWaitableEvent latch;
+  registrar.UnregisterTexture(texture_id, [&]() { latch.Signal(); });
+  latch.Wait();
+  ASSERT_TRUE(unregister_called);
 }
 
 TEST(FlutterWindowsTextureRegistrarTest, RegisterUnknownTextureType) {