Windows: Texture Registrar: Destroy textures on raster thread (#33688)

jnschulze · web-flow · commit 90815e5f75de · 2022-09-09T16:22:40.000+02:00
diff --git a/shell/platform/common/client_wrapper/core_implementations.cc b/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 = 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
diff --git a/shell/platform/common/client_wrapper/include/flutter/texture_registrar.h b/shell/platform/common/client_wrapper/include/flutter/texture_registrar.h
@@ -95,8 +95,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;
 };
 
diff --git a/shell/platform/common/client_wrapper/testing/stub_flutter_api.cc b/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(
diff --git a/shell/platform/common/client_wrapper/testing/stub_flutter_api.h b/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;
   }
diff --git a/shell/platform/common/client_wrapper/texture_registrar_impl.h b/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;
 
diff --git a/shell/platform/common/client_wrapper/texture_registrar_unittests.cc b/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
diff --git a/shell/platform/common/public/flutter_texture_registrar.h b/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.
diff --git a/shell/platform/glfw/flutter_glfw.cc b/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(
diff --git a/shell/platform/windows/angle_surface_manager.cc b/shell/platform/windows/angle_surface_manager.cc
@@ -301,8 +301,6 @@ EGLSurface AngleSurfaceManager::CreateSurfaceFromHandle(
 }
 
 bool AngleSurfaceManager::GetDevice(ID3D11Device** device) {
-  using Microsoft::WRL::ComPtr;
-
   if (!resolved_device_) {
     PFNEGLQUERYDISPLAYATTRIBEXTPROC egl_query_display_attrib_EXT =
         reinterpret_cast<PFNEGLQUERYDISPLAYATTRIBEXTPROC>(
diff --git a/shell/platform/windows/flutter_windows.cc b/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(
diff --git a/shell/platform/windows/flutter_windows_engine.cc b/shell/platform/windows/flutter_windows_engine.cc
@@ -557,6 +557,26 @@ bool FlutterWindowsEngine::MarkExternalTextureFrameAvailable(
               engine_, texture_id) == kSuccess);
 }
 
+bool FlutterWindowsEngine::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;
+}
+
 bool FlutterWindowsEngine::DispatchSemanticsAction(
     uint64_t target,
     FlutterSemanticsAction action,
diff --git a/shell/platform/windows/flutter_windows_engine.h b/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(fml::closure callback);
+
   // Invoke on the embedder's vsync callback to schedule a frame.
   void OnVsync(intptr_t baton);
 
diff --git a/shell/platform/windows/flutter_windows_engine_unittests.cc b/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
diff --git a/shell/platform/windows/flutter_windows_texture_registrar.cc b/shell/platform/windows/flutter_windows_texture_registrar.cc
@@ -77,20 +77,28 @@ 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,
+                                                       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 FlutterWindowsTextureRegistrar::MarkTextureFrameAvailable(
diff --git a/shell/platform/windows/flutter_windows_texture_registrar.h b/shell/platform/windows/flutter_windows_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/windows/external_texture.h"
 
@@ -28,8 +29,7 @@ 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, fml::closure callback = nullptr);
 
   // Notifies the engine about a new frame being available.
   // Returns true on success.
diff --git a/shell/platform/windows/flutter_windows_texture_registrar_unittests.cc b/shell/platform/windows/flutter_windows_texture_registrar_unittests.cc

Original file line number	Diff line number	Diff line change
`@@ -301,8 +301,6 @@ EGLSurface AngleSurfaceManager::CreateSurfaceFromHandle(`
`301`	`301`	`}`
`302`	`302`
`303`	`303`	`bool AngleSurfaceManager::GetDevice(ID3D11Device** device) {`
`304`		`- using Microsoft::WRL::ComPtr;`
`305`		`-`
`306`	`304`	`if (!resolved_device_) {`
`307`	`305`	`PFNEGLQUERYDISPLAYATTRIBEXTPROC egl_query_display_attrib_EXT =`
`308`	`306`	`reinterpret_cast<PFNEGLQUERYDISPLAYATTRIBEXTPROC>(`