Revert "Windows: Texture Registrar: Destroy textures on raster thread (#33688)"
This reverts commit 90815e5f75dee1c20ce6d61d5161157513d90b84.
diff --git a/shell/platform/common/client_wrapper/core_implementations.cc b/shell/platform/common/client_wrapper/core_implementations.cc
index 41be751..e90b26c 100644
--- a/shell/platform/common/client_wrapper/core_implementations.cc
+++ b/shell/platform/common/client_wrapper/core_implementations.cc
@@ -195,32 +195,9 @@
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
diff --git a/shell/platform/common/client_wrapper/include/flutter/texture_registrar.h b/shell/platform/common/client_wrapper/include/flutter/texture_registrar.h
index 47daf7c..7a2078f 100644
--- a/shell/platform/common/client_wrapper/include/flutter/texture_registrar.h
+++ b/shell/platform/common/client_wrapper/include/flutter/texture_registrar.h
@@ -95,13 +95,8 @@
// 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;
};
diff --git a/shell/platform/common/client_wrapper/testing/stub_flutter_api.cc b/shell/platform/common/client_wrapper/testing/stub_flutter_api.cc
index b9a18ba..0612e82 100644
--- a/shell/platform/common/client_wrapper/testing/stub_flutter_api.cc
+++ b/shell/platform/common/client_wrapper/testing/stub_flutter_api.cc
@@ -109,17 +109,15 @@
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(
diff --git a/shell/platform/common/client_wrapper/testing/stub_flutter_api.h b/shell/platform/common/client_wrapper/testing/stub_flutter_api.h
index 0c1c94b..8676cab 100644
--- a/shell/platform/common/client_wrapper/testing/stub_flutter_api.h
+++ b/shell/platform/common/client_wrapper/testing/stub_flutter_api.h
@@ -65,19 +65,18 @@
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;
}
diff --git a/shell/platform/common/client_wrapper/texture_registrar_impl.h b/shell/platform/common/client_wrapper/texture_registrar_impl.h
index bd01839..df51f6d 100644
--- a/shell/platform/common/client_wrapper/texture_registrar_impl.h
+++ b/shell/platform/common/client_wrapper/texture_registrar_impl.h
@@ -28,10 +28,6 @@
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;
private:
diff --git a/shell/platform/common/client_wrapper/texture_registrar_unittests.cc b/shell/platform/common/client_wrapper/texture_registrar_unittests.cc
index e770f06..02ccc6f 100644
--- a/shell/platform/common/client_wrapper/texture_registrar_unittests.cc
+++ b/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 @@
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,17 +110,15 @@
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);
@@ -133,9 +126,8 @@
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
diff --git a/shell/platform/common/public/flutter_texture_registrar.h b/shell/platform/common/public/flutter_texture_registrar.h
index 9979e9a..4592161 100644
--- a/shell/platform/common/public/flutter_texture_registrar.h
+++ b/shell/platform/common/public/flutter_texture_registrar.h
@@ -162,15 +162,13 @@
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.
diff --git a/shell/platform/glfw/flutter_glfw.cc b/shell/platform/glfw/flutter_glfw.cc
index a388d21..5079f8c 100644
--- a/shell/platform/glfw/flutter_glfw.cc
+++ b/shell/platform/glfw/flutter_glfw.cc
@@ -728,9 +728,10 @@
// 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 @@
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(
diff --git a/shell/platform/windows/angle_surface_manager.cc b/shell/platform/windows/angle_surface_manager.cc
index cd8973d..95eb1d1 100644
--- a/shell/platform/windows/angle_surface_manager.cc
+++ b/shell/platform/windows/angle_surface_manager.cc
@@ -301,6 +301,8 @@
}
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
index 0d8ca85..c5db187 100644
--- a/shell/platform/windows/flutter_windows.cc
+++ b/shell/platform/windows/flutter_windows.cc
@@ -302,18 +302,11 @@
->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(
diff --git a/shell/platform/windows/flutter_windows_engine.cc b/shell/platform/windows/flutter_windows_engine.cc
index e862a94..8eabd6d 100644
--- a/shell/platform/windows/flutter_windows_engine.cc
+++ b/shell/platform/windows/flutter_windows_engine.cc
@@ -557,26 +557,6 @@
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
index 57921218..2e26191 100644
--- a/shell/platform/windows/flutter_windows_engine.h
+++ b/shell/platform/windows/flutter_windows_engine.h
@@ -190,9 +190,6 @@
// 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
index 30a91f0..87482e1 100644
--- a/shell/platform/windows/flutter_windows_engine_unittests.cc
+++ b/shell/platform/windows/flutter_windows_engine_unittests.cc
@@ -446,21 +446,5 @@
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
index c3371b8..0abc73b 100644
--- a/shell/platform/windows/flutter_windows_texture_registrar.cc
+++ b/shell/platform/windows/flutter_windows_texture_registrar.cc
@@ -77,28 +77,20 @@
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(
diff --git a/shell/platform/windows/flutter_windows_texture_registrar.h b/shell/platform/windows/flutter_windows_texture_registrar.h
index 6053432..f1d2ac0 100644
--- a/shell/platform/windows/flutter_windows_texture_registrar.h
+++ b/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 @@
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.
diff --git a/shell/platform/windows/flutter_windows_texture_registrar_unittests.cc b/shell/platform/windows/flutter_windows_texture_registrar_unittests.cc
index af12b41..f0bddc1 100644
--- a/shell/platform/windows/flutter_windows_texture_registrar_unittests.cc
+++ b/shell/platform/windows/flutter_windows_texture_registrar_unittests.cc
@@ -4,7 +4,6 @@
#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"
@@ -114,13 +113,6 @@
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);
@@ -129,10 +121,8 @@
EXPECT_TRUE(registrar.MarkTextureFrameAvailable(texture_id));
EXPECT_TRUE(mark_frame_available_called);
- fml::AutoResetWaitableEvent latch;
- registrar.UnregisterTexture(texture_id, [&]() { latch.Signal(); });
- latch.Wait();
- ASSERT_TRUE(unregister_called);
+ EXPECT_TRUE(registrar.UnregisterTexture(texture_id));
+ EXPECT_TRUE(unregister_called);
}
TEST(FlutterWindowsTextureRegistrarTest, RegisterUnknownTextureType) {
@@ -305,17 +295,5 @@
EXPECT_FALSE(result);
}
-TEST(FlutterWindowsTextureRegistrarTest,
- UnregisterTextureWithEngineDownInvokesCallback) {
- std::unique_ptr<FlutterWindowsEngine> engine = GetTestEngine();
- std::unique_ptr<MockGlFunctions> gl = std::make_unique<MockGlFunctions>();
-
- FlutterWindowsTextureRegistrar registrar(engine.get(), gl->gl_procs());
-
- fml::AutoResetWaitableEvent latch;
- registrar.UnregisterTexture(1234, [&]() { latch.Signal(); });
- latch.Wait();
-}
-
} // namespace testing
} // namespace flutter
