diff --git a/shell/platform/common/client_wrapper/core_implementations.cc b/shell/platform/common/client_wrapper/core_implementations.cc index 41be7515ca9e0..e90b26cd4d6e5 100644 --- a/shell/platform/common/client_wrapper/core_implementations.cc +++ b/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 callback) { - if (callback == nullptr) { - FlutterDesktopTextureRegistrarUnregisterExternalTexture( - texture_registrar_ref_, texture_id, nullptr, nullptr); - return; - } - - struct Captures { - std::function callback; - }; - auto captures = new Captures(); - captures->callback = std::move(callback); - FlutterDesktopTextureRegistrarUnregisterExternalTexture( - texture_registrar_ref_, texture_id, - [](void* opaque) { - auto captures = reinterpret_cast(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 47daf7c42fbcd..7a2078f13daad 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 @@ 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 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 b9a18ba660b95..0612e8249b3b2 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 @@ 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( 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 0c1c94b8d27ae..8676cab6c2808 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 @@ 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; } diff --git a/shell/platform/common/client_wrapper/texture_registrar_impl.h b/shell/platform/common/client_wrapper/texture_registrar_impl.h index bd01839d40f42..df51f6ddb5232 100644 --- a/shell/platform/common/client_wrapper/texture_registrar_impl.h +++ b/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 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 index e770f06611c77..02ccc6fa76891 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 #include -#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,17 +110,15 @@ 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(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(1); @@ -133,9 +126,8 @@ TEST(TextureRegistrarTest, UnregisterInvalidTexture) { 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 9979e9a2c15fc..45921613833e3 100644 --- a/shell/platform/common/public/flutter_texture_registrar.h +++ b/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. diff --git a/shell/platform/glfw/flutter_glfw.cc b/shell/platform/glfw/flutter_glfw.cc index a388d218f1823..5079f8c7c2d3f 100644 --- a/shell/platform/glfw/flutter_glfw.cc +++ b/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 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( diff --git a/shell/platform/windows/angle_surface_manager.cc b/shell/platform/windows/angle_surface_manager.cc index cd8973dad6d16..95eb1d1224873 100644 --- a/shell/platform/windows/angle_surface_manager.cc +++ b/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( diff --git a/shell/platform/windows/flutter_windows.cc b/shell/platform/windows/flutter_windows.cc index 0d8ca85fb7225..c5db187e926ba 100644 --- a/shell/platform/windows/flutter_windows.cc +++ b/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( diff --git a/shell/platform/windows/flutter_windows_engine.cc b/shell/platform/windows/flutter_windows_engine.cc index e862a94029b11..8eabd6d1f9e72 100644 --- a/shell/platform/windows/flutter_windows_engine.cc +++ b/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(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 579212183d657..2e261917ce2fa 100644 --- a/shell/platform/windows/flutter_windows_engine.h +++ b/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); diff --git a/shell/platform/windows/flutter_windows_engine_unittests.cc b/shell/platform/windows/flutter_windows_engine_unittests.cc index 30a91f0e9959a..87482e1988e09 100644 --- a/shell/platform/windows/flutter_windows_engine_unittests.cc +++ b/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 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 c3371b8d7b5ca..0abc73b75afe3 100644 --- a/shell/platform/windows/flutter_windows_texture_registrar.cc +++ b/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 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 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 60534328eee4e..f1d2ac0a3b25c 100644 --- a/shell/platform/windows/flutter_windows_texture_registrar.h +++ b/shell/platform/windows/flutter_windows_texture_registrar.h @@ -9,7 +9,6 @@ #include #include -#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. diff --git a/shell/platform/windows/flutter_windows_texture_registrar_unittests.cc b/shell/platform/windows/flutter_windows_texture_registrar_unittests.cc index af12b41fde674..f0bddc1ae9f3f 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 -#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 @@ 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); @@ -129,10 +121,8 @@ TEST(FlutterWindowsTextureRegistrarTest, RegisterUnregisterTexture) { 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 @@ TEST(FlutterWindowsTextureRegistrarTest, PopulateInvalidTexture) { EXPECT_FALSE(result); } -TEST(FlutterWindowsTextureRegistrarTest, - UnregisterTextureWithEngineDownInvokesCallback) { - std::unique_ptr engine = GetTestEngine(); - std::unique_ptr gl = std::make_unique(); - - FlutterWindowsTextureRegistrar registrar(engine.get(), gl->gl_procs()); - - fml::AutoResetWaitableEvent latch; - registrar.UnregisterTexture(1234, [&]() { latch.Signal(); }); - latch.Wait(); -} - } // namespace testing } // namespace flutter