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

This reverts commit 90815e5.

Author: XilaiZhang

shell/platform/common/client_wrapper/core_implementations.cc

@@ -195,32 +195,9 @@ 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) {
-  UnregisterTexture(texture_id, nullptr);
-  return true;
+  return FlutterDesktopTextureRegistrarUnregisterExternalTexture(
+      texture_registrar_ref_, texture_id);
 }
 
 }  // namespace flutter

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

@@ -95,13 +95,8 @@ class TextureRegistrar {
   // the callback that was provided upon creating the texture.
   virtual bool MarkTextureFrameAvailable(int64_t texture_id) = 0;
 
-  // 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.
+  // Unregisters an existing Texture object.
+  // Textures must not be unregistered while they're in use.
   virtual bool UnregisterTexture(int64_t texture_id) = 0;
 };
 

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

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

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

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

shell/platform/common/client_wrapper/texture_registrar_impl.h

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

shell/platform/common/public/flutter_texture_registrar.h

@@ -162,15 +162,13 @@ FLUTTER_EXPORT int64_t FlutterDesktopTextureRegistrarRegisterExternalTexture(
     FlutterDesktopTextureRegistrarRef texture_registrar,
     const FlutterDesktopTextureInfo* info);
 
-// Asynchronously unregisters the texture identified by |texture_id| from the
-// Flutter engine.
-// An optional |callback| gets invoked upon completion.
+// 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.
 // This function can be called from any thread.
-FLUTTER_EXPORT void FlutterDesktopTextureRegistrarUnregisterExternalTexture(
+// However, textures must not be unregistered while they're in use.
+FLUTTER_EXPORT bool FlutterDesktopTextureRegistrarUnregisterExternalTexture(
     FlutterDesktopTextureRegistrarRef texture_registrar,
-    int64_t texture_id,
-    void (*callback)(void* user_data),
-    void* user_data);
+    int64_t texture_id);
 
 // 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,9 +728,10 @@ 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());
@@ -1113,12 +1114,11 @@ int64_t FlutterDesktopTextureRegistrarRegisterExternalTexture(
   return -1;
 }
 
-void FlutterDesktopTextureRegistrarUnregisterExternalTexture(
+bool FlutterDesktopTextureRegistrarUnregisterExternalTexture(
     FlutterDesktopTextureRegistrarRef texture_registrar,
-    int64_t texture_id,
-    void (*callback)(void* user_data),
-    void* user_data) {
+    int64_t texture_id) {
   std::cerr << "GLFW Texture support is not implemented yet." << std::endl;
+  return false;
 }
 
 bool FlutterDesktopTextureRegistrarMarkExternalTextureFrameAvailable(

shell/platform/windows/angle_surface_manager.cc

@@ -301,6 +301,8 @@ EGLSurface AngleSurfaceManager::CreateSurfaceFromHandle(
 }
 
 bool AngleSurfaceManager::GetDevice(ID3D11Device** device) {
+  using Microsoft::WRL::ComPtr;
+
   if (!resolved_device_) {
     PFNEGLQUERYDISPLAYATTRIBEXTPROC egl_query_display_attrib_EXT =
         reinterpret_cast<PFNEGLQUERYDISPLAYATTRIBEXTPROC>(

shell/platform/windows/flutter_windows.cc

@@ -302,18 +302,11 @@ int64_t FlutterDesktopTextureRegistrarRegisterExternalTexture(
       ->RegisterTexture(texture_info);
 }
 
-void FlutterDesktopTextureRegistrarUnregisterExternalTexture(
+bool FlutterDesktopTextureRegistrarUnregisterExternalTexture(
     FlutterDesktopTextureRegistrarRef texture_registrar,
-    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);
+    int64_t texture_id) {
+  return TextureRegistrarFromHandle(texture_registrar)
+      ->UnregisterTexture(texture_id);
 }
 
 bool FlutterDesktopTextureRegistrarMarkExternalTextureFrameAvailable(

shell/platform/windows/flutter_windows_engine.cc

@@ -557,26 +557,6 @@ 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,

shell/platform/windows/flutter_windows_engine.h

@@ -190,9 +190,6 @@ 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);
 

shell/platform/windows/flutter_windows_engine_unittests.cc

@@ -446,21 +446,5 @@ 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,28 +77,20 @@ int64_t FlutterWindowsTextureRegistrar::EmplaceTexture(
   return texture_id;
 }
 
-void FlutterWindowsTextureRegistrar::UnregisterTexture(int64_t texture_id,
-                                                       fml::closure callback) {
+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);
+  }
+
   engine_->task_runner()->RunNowOrPostTask([engine = engine_, texture_id]() {
     engine->UnregisterExternalTexture(texture_id);
   });
-
-  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();
-  }
+  return true;
 }
 
 bool FlutterWindowsTextureRegistrar::MarkTextureFrameAvailable(

shell/platform/windows/flutter_windows_texture_registrar.h

@@ -9,7 +9,6 @@
 #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"
 
@@ -29,7 +28,8 @@ class FlutterWindowsTextureRegistrar {
   int64_t RegisterTexture(const FlutterDesktopTextureInfo* texture_info);
 
   // Attempts to unregister the texture identified by |texture_id|.
-  void UnregisterTexture(int64_t texture_id, fml::closure callback = nullptr);
+  // Returns true if the texture was successfully unregistered.
+  bool UnregisterTexture(int64_t texture_id);
 
   // Notifies the engine about a new frame being available.
   // Returns true on success.