From e64a9ae2bd28b494e9a6b4c3ce6620ee0cf7a12c Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Thu, 20 Oct 2022 16:39:21 -0700 Subject: [PATCH 01/13] Implemented threadsafe platform channel replies on windows --- .../client_wrapper/core_implementations.cc | 19 +++++++++-- .../testing/stub_flutter_api.cc | 27 +++++++++++++++ .../common/public/flutter_messenger.h | 16 +++++++++ shell/platform/glfw/flutter_glfw.cc | 25 ++++++++++++++ shell/platform/windows/flutter_windows.cc | 34 ++++++++++++++++--- .../windows/flutter_windows_engine.cc | 6 ++-- .../platform/windows/flutter_windows_engine.h | 2 +- shell/platform/windows/window_state.h | 25 ++++++++++++++ 8 files changed, 143 insertions(+), 11 deletions(-) diff --git a/shell/platform/common/client_wrapper/core_implementations.cc b/shell/platform/common/client_wrapper/core_implementations.cc index 41be7515ca9e0..4a43ba01d7534 100644 --- a/shell/platform/common/client_wrapper/core_implementations.cc +++ b/shell/platform/common/client_wrapper/core_implementations.cc @@ -36,17 +36,30 @@ void ForwardToHandler(FlutterDesktopMessengerRef messenger, const FlutterDesktopMessage* message, void* user_data) { auto* response_handle = message->response_handle; - BinaryReply reply_handler = [messenger, response_handle]( + auto messenger_ptr = std::shared_ptr( + FlutterDesktopMessengerAddRef(messenger), + &FlutterDesktopMessengerRelease); + BinaryReply reply_handler = [messenger_ptr, response_handle]( const uint8_t* reply, size_t reply_size) mutable { + auto lock = std::unique_ptr( + FlutterDesktopMessengerLock(messenger_ptr.get()), + &FlutterDesktopMessengerUnlock); + if (!FlutterDesktopMessengerIsAvailable(messenger_ptr.get())) { + std::cerr << "Error: Responding to platform channel after the engine has" + "been deleted." + << std::endl; + return; + } if (!response_handle) { std::cerr << "Error: Response can be set only once. Ignoring " "duplicate response." << std::endl; return; } - FlutterDesktopMessengerSendResponse(messenger, response_handle, reply, - reply_size); + FlutterDesktopMessengerSendResponse(messenger_ptr.get(), response_handle, + reply, reply_size); // The engine frees the response handle once // FlutterDesktopSendMessageResponse is called. response_handle = nullptr; 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..28b01d70ea209 100644 --- a/shell/platform/common/client_wrapper/testing/stub_flutter_api.cc +++ b/shell/platform/common/client_wrapper/testing/stub_flutter_api.cc @@ -4,6 +4,8 @@ #include "flutter/shell/platform/common/client_wrapper/testing/stub_flutter_api.h" +#include + static flutter::testing::StubFlutterApi* s_stub_implementation; namespace flutter { @@ -93,6 +95,31 @@ void FlutterDesktopMessengerSetCallback(FlutterDesktopMessengerRef messenger, } } +FlutterDesktopMessengerRef FlutterDesktopMessengerAddRef( + FlutterDesktopMessengerRef messenger) { + assert(false); // not implemented + return nullptr; +} + +void FlutterDesktopMessengerRelease(FlutterDesktopMessengerRef messenger) { + assert(false); // not implemented +} + +bool FlutterDesktopMessengerIsAvailable(FlutterDesktopMessengerRef messenger) { + assert(false); // not implemented + return false; +} + +FlutterDesktopMessengerRef FlutterDesktopMessengerLock( + FlutterDesktopMessengerRef messenger) { + assert(false); // not implemented + return nullptr; +} + +void FlutterDesktopMessengerUnlock(FlutterDesktopMessengerRef messenger) { + assert(false); // not implemented +} + FlutterDesktopTextureRegistrarRef FlutterDesktopRegistrarGetTextureRegistrar( FlutterDesktopPluginRegistrarRef registrar) { return reinterpret_cast(1); diff --git a/shell/platform/common/public/flutter_messenger.h b/shell/platform/common/public/flutter_messenger.h index 854ff985cec73..6f873c7b753ee 100644 --- a/shell/platform/common/public/flutter_messenger.h +++ b/shell/platform/common/public/flutter_messenger.h @@ -5,6 +5,7 @@ #ifndef FLUTTER_SHELL_PLATFORM_COMMON_PUBLIC_FLUTTER_MESSENGER_H_ #define FLUTTER_SHELL_PLATFORM_COMMON_PUBLIC_FLUTTER_MESSENGER_H_ +#include #include #include @@ -87,6 +88,21 @@ FLUTTER_EXPORT void FlutterDesktopMessengerSetCallback( FlutterDesktopMessageCallback callback, void* user_data); +FLUTTER_EXPORT FlutterDesktopMessengerRef +FlutterDesktopMessengerAddRef(FlutterDesktopMessengerRef messenger); + +FLUTTER_EXPORT void FlutterDesktopMessengerRelease( + FlutterDesktopMessengerRef messenger); + +FLUTTER_EXPORT bool FlutterDesktopMessengerIsAvailable( + FlutterDesktopMessengerRef messenger); + +FLUTTER_EXPORT FlutterDesktopMessengerRef +FlutterDesktopMessengerLock(FlutterDesktopMessengerRef messenger); + +FLUTTER_EXPORT void FlutterDesktopMessengerUnlock( + FlutterDesktopMessengerRef messenger); + #if defined(__cplusplus) } // extern "C" #endif diff --git a/shell/platform/glfw/flutter_glfw.cc b/shell/platform/glfw/flutter_glfw.cc index 55015cfec0d0e..bf2a15c955f22 100644 --- a/shell/platform/glfw/flutter_glfw.cc +++ b/shell/platform/glfw/flutter_glfw.cc @@ -1096,6 +1096,31 @@ void FlutterDesktopMessengerSetCallback(FlutterDesktopMessengerRef messenger, user_data); } +FlutterDesktopMessengerRef FlutterDesktopMessengerAddRef( + FlutterDesktopMessengerRef messenger) { + assert(false); // not implemented + return nullptr; +} + +void FlutterDesktopMessengerRelease(FlutterDesktopMessengerRef messenger) { + assert(false); // not implemented +} + +bool FlutterDesktopMessengerIsAvailable(FlutterDesktopMessengerRef messenger) { + assert(false); // not implemented + return false; +} + +FlutterDesktopMessengerRef FlutterDesktopMessengerLock( + FlutterDesktopMessengerRef messenger) { + assert(false); // not implemented + return nullptr; +} + +void FlutterDesktopMessengerUnlock(FlutterDesktopMessengerRef messenger) { + assert(false); // not implemented +} + FlutterDesktopTextureRegistrarRef FlutterDesktopRegistrarGetTextureRegistrar( FlutterDesktopPluginRegistrarRef registrar) { std::cerr << "GLFW Texture support is not implemented yet." << std::endl; diff --git a/shell/platform/windows/flutter_windows.cc b/shell/platform/windows/flutter_windows.cc index 0d8ca85fb7225..999937d0121ad 100644 --- a/shell/platform/windows/flutter_windows.cc +++ b/shell/platform/windows/flutter_windows.cc @@ -262,8 +262,8 @@ bool FlutterDesktopMessengerSendWithReply(FlutterDesktopMessengerRef messenger, const size_t message_size, const FlutterDesktopBinaryReply reply, void* user_data) { - return messenger->engine->SendPlatformMessage(channel, message, message_size, - reply, user_data); + return messenger->GetEngine()->SendPlatformMessage( + channel, message, message_size, reply, user_data); } bool FlutterDesktopMessengerSend(FlutterDesktopMessengerRef messenger, @@ -279,15 +279,39 @@ void FlutterDesktopMessengerSendResponse( const FlutterDesktopMessageResponseHandle* handle, const uint8_t* data, size_t data_length) { - messenger->engine->SendPlatformMessageResponse(handle, data, data_length); + messenger->GetEngine()->SendPlatformMessageResponse(handle, data, + data_length); } void FlutterDesktopMessengerSetCallback(FlutterDesktopMessengerRef messenger, const char* channel, FlutterDesktopMessageCallback callback, void* user_data) { - messenger->engine->message_dispatcher()->SetMessageCallback(channel, callback, - user_data); + messenger->GetEngine()->message_dispatcher()->SetMessageCallback( + channel, callback, user_data); +} + +FlutterDesktopMessengerRef FlutterDesktopMessengerAddRef( + FlutterDesktopMessengerRef messenger) { + return messenger->AddRef(); +} + +void FlutterDesktopMessengerRelease(FlutterDesktopMessengerRef messenger) { + messenger->Release(); +} + +bool FlutterDesktopMessengerIsAvailable(FlutterDesktopMessengerRef messenger) { + return messenger->GetEngine() != nullptr; +} + +FlutterDesktopMessengerRef FlutterDesktopMessengerLock( + FlutterDesktopMessengerRef messenger) { + messenger->GetMutex().lock(); + return messenger; +} + +void FlutterDesktopMessengerUnlock(FlutterDesktopMessengerRef messenger) { + messenger->GetMutex().unlock(); } FlutterDesktopTextureRegistrarRef FlutterDesktopRegistrarGetTextureRegistrar( diff --git a/shell/platform/windows/flutter_windows_engine.cc b/shell/platform/windows/flutter_windows_engine.cc index 24b17a28ac6af..719b16c2fc8de 100644 --- a/shell/platform/windows/flutter_windows_engine.cc +++ b/shell/platform/windows/flutter_windows_engine.cc @@ -179,8 +179,9 @@ FlutterWindowsEngine::FlutterWindowsEngine( }); // Set up the legacy structs backing the API handles. - messenger_ = std::make_unique(); - messenger_->engine = this; + messenger_ = + fml::RefPtr(new FlutterDesktopMessenger()); + messenger_->SetEngine(this); plugin_registrar_ = std::make_unique(); plugin_registrar_->engine = this; @@ -210,6 +211,7 @@ FlutterWindowsEngine::FlutterWindowsEngine( } FlutterWindowsEngine::~FlutterWindowsEngine() { + messenger_->SetEngine(nullptr); Stop(); } diff --git a/shell/platform/windows/flutter_windows_engine.h b/shell/platform/windows/flutter_windows_engine.h index 64a4f0fa5bfd7..a94ec6bc48bcf 100644 --- a/shell/platform/windows/flutter_windows_engine.h +++ b/shell/platform/windows/flutter_windows_engine.h @@ -283,7 +283,7 @@ class FlutterWindowsEngine { std::unique_ptr task_runner_; // The plugin messenger handle given to API clients. - std::unique_ptr messenger_; + fml::RefPtr messenger_; // A wrapper around messenger_ for interacting with client_wrapper-level APIs. std::unique_ptr messenger_wrapper_; diff --git a/shell/platform/windows/window_state.h b/shell/platform/windows/window_state.h index 56897f7e1a269..d1ea14839c956 100644 --- a/shell/platform/windows/window_state.h +++ b/shell/platform/windows/window_state.h @@ -5,6 +5,8 @@ #ifndef FLUTTER_SHELL_PLATFORM_WINDOWS_FLUTTER_WINDOW_STATE_H_ #define FLUTTER_SHELL_PLATFORM_WINDOWS_FLUTTER_WINDOW_STATE_H_ +#include + #include "flutter/shell/platform/common/client_wrapper/include/flutter/plugin_registrar.h" #include "flutter/shell/platform/common/incoming_message_dispatcher.h" #include "flutter/shell/platform/embedder/embedder.h" @@ -36,8 +38,31 @@ struct FlutterDesktopPluginRegistrar { // Wrapper to distinguish the messenger ref from the engine ref given out // in the C API. struct FlutterDesktopMessenger { + public: + FlutterDesktopMessenger() = default; + flutter::FlutterWindowsEngine* GetEngine() const { return engine; } + void SetEngine(flutter::FlutterWindowsEngine* arg_engine) { + std::scoped_lock lock(mutex_); + engine = arg_engine; + } + FlutterDesktopMessenger* AddRef() { + ref_count_.fetch_add(1); + return this; + } + void Release() { + int32_t old_count = ref_count_.fetch_sub(1); + if (old_count <= 1) { + delete this; + } + } + std::mutex& GetMutex() { return mutex_; } + + private: + FML_DISALLOW_COPY_AND_ASSIGN(FlutterDesktopMessenger); // The engine that owns this state object. flutter::FlutterWindowsEngine* engine = nullptr; + std::mutex mutex_; + std::atomic ref_count_ = 0; }; #endif // FLUTTER_SHELL_PLATFORM_WINDOWS_FLUTTER_WINDOW_STATE_H_ From cf133e76954dd2d163794135ebf44ba1d48be05e Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Fri, 21 Oct 2022 16:02:15 -0700 Subject: [PATCH 02/13] added unit test --- .../flutter_windows_engine_unittests.cc | 54 +++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/shell/platform/windows/flutter_windows_engine_unittests.cc b/shell/platform/windows/flutter_windows_engine_unittests.cc index 5649d583d2b37..9714bd8767144 100644 --- a/shell/platform/windows/flutter_windows_engine_unittests.cc +++ b/shell/platform/windows/flutter_windows_engine_unittests.cc @@ -311,6 +311,60 @@ TEST_F(FlutterWindowsEngineTest, PlatformMessageRoundTrip) { } } +TEST_F(FlutterWindowsEngineTest, PlatformMessageRespondOnDifferentThread) { + FlutterDesktopEngineProperties properties = {}; + properties.assets_path = GetContext().GetAssetsPath().c_str(); + properties.icu_data_path = GetContext().GetIcuDataPath().c_str(); + properties.dart_entrypoint = "hiPlatformChannels"; + + FlutterProjectBundle project(properties); + auto engine = std::make_unique(project); + + EngineModifier modifier(engine.get()); + modifier.embedder_api().RunsAOTCompiledDartCode = []() { return false; }; + + auto binary_messenger = + std::make_unique(engine->messenger()); + + engine->Run(); + bool did_call_callback = false; + bool did_call_reply = false; + bool did_call_dart_reply = false; + std::string channel = "hi"; + std::unique_ptr reply_thread; + binary_messenger->SetMessageHandler( + channel, + [&did_call_callback, &did_call_dart_reply, &reply_thread]( + const uint8_t* message, size_t message_size, BinaryReply reply) { + if (message_size == 5) { + EXPECT_EQ(message[0], static_cast('h')); + reply_thread.reset(new std::thread([reply = std::move(reply)]() { + char response[] = {'b', 'y', 'e'}; + reply(reinterpret_cast(response), 3); + })); + did_call_callback = true; + } else { + EXPECT_EQ(message_size, 3); + EXPECT_EQ(message[0], static_cast('b')); + did_call_dart_reply = true; + } + }); + char payload[] = {'h', 'e', 'l', 'l', 'o'}; + binary_messenger->Send( + channel, reinterpret_cast(payload), 5, + [&did_call_reply](const uint8_t* reply, size_t reply_size) { + EXPECT_EQ(reply_size, 5); + EXPECT_EQ(reply[0], static_cast('h')); + did_call_reply = true; + }); + // Rely on timeout mechanism in CI. + while (!did_call_callback || !did_call_reply || !did_call_dart_reply) { + engine->task_runner()->ProcessTasks(); + } + ASSERT_TRUE(reply_thread); + reply_thread->join(); +} + TEST_F(FlutterWindowsEngineTest, SendPlatformMessageWithResponse) { std::unique_ptr engine = GetTestEngine(); EngineModifier modifier(engine.get()); From e689f0ad5161cdb895ccdbfac6e8c7c625042111 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Mon, 24 Oct 2022 10:08:52 -0700 Subject: [PATCH 03/13] added docstrings --- .../common/public/flutter_messenger.h | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/shell/platform/common/public/flutter_messenger.h b/shell/platform/common/public/flutter_messenger.h index 6f873c7b753ee..4c080fcc417cc 100644 --- a/shell/platform/common/public/flutter_messenger.h +++ b/shell/platform/common/public/flutter_messenger.h @@ -88,18 +88,44 @@ FLUTTER_EXPORT void FlutterDesktopMessengerSetCallback( FlutterDesktopMessageCallback callback, void* user_data); +// Increments the reference count for the |messenger|. +// +// See also: |FlutterDesktopMessengerRelease| FLUTTER_EXPORT FlutterDesktopMessengerRef FlutterDesktopMessengerAddRef(FlutterDesktopMessengerRef messenger); +// Decrements the reference count for the |messenger|. +// +// The reference count starts at zero so it is valid to call +// FlutterDesktopMessengerRelease without having called +// FlutterDesktopMessengerAddRef. +// +// See also: |FlutterDesktopMessengerAddRef| FLUTTER_EXPORT void FlutterDesktopMessengerRelease( FlutterDesktopMessengerRef messenger); +// Returns `true` if the |FlutterDesktopMessengerRef| still references a running +// engine. +// +// This check should be made inside of a |FlutterDesktopMessengerLock| and +// before any other calls are made to the FlutterDesktopMessengerRef when using +// it from a thread other than the platform thread. FLUTTER_EXPORT bool FlutterDesktopMessengerIsAvailable( FlutterDesktopMessengerRef messenger); +// Locks the `FlutterDesktopMessengerRef` ensuring that +// |FlutterDesktopMessengerIsAvailable| does not change while locked. +// +// All calls to the FlutterDesktopMessengerRef from threads other than the +// platform thread should happen inside of a lock. +// +// See also: |FlutterDesktopMessengerUnlock| FLUTTER_EXPORT FlutterDesktopMessengerRef FlutterDesktopMessengerLock(FlutterDesktopMessengerRef messenger); +// Unlocks the `FlutterDesktopMessengerRef`. +// +// See also: |FlutterDesktopMessengerLock| FLUTTER_EXPORT void FlutterDesktopMessengerUnlock( FlutterDesktopMessengerRef messenger); From c7a53763c97b335c8920dd2baf20c4a25f97edac Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Mon, 24 Oct 2022 10:50:04 -0700 Subject: [PATCH 04/13] implemented glfw --- shell/platform/glfw/flutter_glfw.cc | 95 ++++++++++++++++++----------- 1 file changed, 59 insertions(+), 36 deletions(-) diff --git a/shell/platform/glfw/flutter_glfw.cc b/shell/platform/glfw/flutter_glfw.cc index bf2a15c955f22..ddee807f125b5 100644 --- a/shell/platform/glfw/flutter_glfw.cc +++ b/shell/platform/glfw/flutter_glfw.cc @@ -116,7 +116,7 @@ struct FlutterDesktopEngineState { std::unique_ptr event_loop; // The plugin messenger handle given to API clients. - std::unique_ptr messenger; + std::shared_ptr messenger; // Message dispatch manager for messages from the Flutter engine. std::unique_ptr message_dispatcher; @@ -149,10 +149,54 @@ struct FlutterDesktopPluginRegistrar { // State associated with the messenger used to communicate with the engine. struct FlutterDesktopMessenger { + void AddRef() { ref_count_.fetch_add(1); } + + void Release() { + int32_t old_count = ref_count_.fetch_sub(1); + if (old_count <= 1) { + delete this; + } + } + + FlutterDesktopEngineState* GetEngine() const { return engine_; } + void SetEngine(FlutterDesktopEngineState* engine) { + std::scoped_lock lock(mutex_); + engine_ = engine; + } + + std::mutex& GetMutex() { return mutex_; } + + private: // The engine that backs this messenger. - FlutterDesktopEngineState* engine; + FlutterDesktopEngineState* engine_; + std::atomic ref_count_ = 0; + std::mutex mutex_; }; +FlutterDesktopMessengerRef FlutterDesktopMessengerAddRef( + FlutterDesktopMessengerRef messenger) { + messenger->AddRef(); + return messenger; +} + +void FlutterDesktopMessengerRelease(FlutterDesktopMessengerRef messenger) { + messenger->Release(); +} + +bool FlutterDesktopMessengerIsAvailable(FlutterDesktopMessengerRef messenger) { + return messenger->GetEngine() != nullptr; +} + +FlutterDesktopMessengerRef FlutterDesktopMessengerLock( + FlutterDesktopMessengerRef messenger) { + messenger->GetMutex().lock(); + return messenger; +} + +void FlutterDesktopMessengerUnlock(FlutterDesktopMessengerRef messenger) { + messenger->GetMutex().unlock(); +} + // Retrieves state bag for the window in question from the GLFWWindow. static FlutterDesktopWindowControllerState* GetWindowController( GLFWwindow* window) { @@ -743,8 +787,10 @@ static void SetUpLocales(FlutterDesktopEngineState* state) { static void SetUpCommonEngineState(FlutterDesktopEngineState* state, GLFWwindow* window) { // Messaging. - state->messenger = std::make_unique(); - state->messenger->engine = state; + state->messenger = std::shared_ptr( + FlutterDesktopMessengerAddRef(new FlutterDesktopMessenger()), + &FlutterDesktopMessengerRelease); + state->messenger->SetEngine(state); state->message_dispatcher = std::make_unique( state->messenger.get()); @@ -846,6 +892,7 @@ FlutterDesktopWindowControllerRef FlutterDesktopCreateWindow( } void FlutterDesktopDestroyWindow(FlutterDesktopWindowControllerRef controller) { + controller->engine->messenger->SetEngine(nullptr); FlutterDesktopPluginRegistrarRef registrar = controller->engine->plugin_registrar.get(); if (registrar->destruction_handler) { @@ -1045,7 +1092,8 @@ bool FlutterDesktopMessengerSendWithReply(FlutterDesktopMessengerRef messenger, FlutterPlatformMessageResponseHandle* response_handle = nullptr; if (reply != nullptr && user_data != nullptr) { FlutterEngineResult result = FlutterPlatformMessageCreateResponseHandle( - messenger->engine->flutter_engine, reply, user_data, &response_handle); + messenger->GetEngine()->flutter_engine, reply, user_data, + &response_handle); if (result != kSuccess) { std::cout << "Failed to create response handle\n"; return false; @@ -1061,11 +1109,11 @@ bool FlutterDesktopMessengerSendWithReply(FlutterDesktopMessengerRef messenger, }; FlutterEngineResult message_result = FlutterEngineSendPlatformMessage( - messenger->engine->flutter_engine, &platform_message); + messenger->GetEngine()->flutter_engine, &platform_message); if (response_handle != nullptr) { FlutterPlatformMessageReleaseResponseHandle( - messenger->engine->flutter_engine, response_handle); + messenger->GetEngine()->flutter_engine, response_handle); } return message_result == kSuccess; @@ -1084,41 +1132,16 @@ void FlutterDesktopMessengerSendResponse( const FlutterDesktopMessageResponseHandle* handle, const uint8_t* data, size_t data_length) { - FlutterEngineSendPlatformMessageResponse(messenger->engine->flutter_engine, - handle, data, data_length); + FlutterEngineSendPlatformMessageResponse( + messenger->GetEngine()->flutter_engine, handle, data, data_length); } void FlutterDesktopMessengerSetCallback(FlutterDesktopMessengerRef messenger, const char* channel, FlutterDesktopMessageCallback callback, void* user_data) { - messenger->engine->message_dispatcher->SetMessageCallback(channel, callback, - user_data); -} - -FlutterDesktopMessengerRef FlutterDesktopMessengerAddRef( - FlutterDesktopMessengerRef messenger) { - assert(false); // not implemented - return nullptr; -} - -void FlutterDesktopMessengerRelease(FlutterDesktopMessengerRef messenger) { - assert(false); // not implemented -} - -bool FlutterDesktopMessengerIsAvailable(FlutterDesktopMessengerRef messenger) { - assert(false); // not implemented - return false; -} - -FlutterDesktopMessengerRef FlutterDesktopMessengerLock( - FlutterDesktopMessengerRef messenger) { - assert(false); // not implemented - return nullptr; -} - -void FlutterDesktopMessengerUnlock(FlutterDesktopMessengerRef messenger) { - assert(false); // not implemented + messenger->GetEngine()->message_dispatcher->SetMessageCallback( + channel, callback, user_data); } FlutterDesktopTextureRegistrarRef FlutterDesktopRegistrarGetTextureRegistrar( From 77a85e9b9376317f8f37e4e0d1219476a2a0f41a Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Mon, 24 Oct 2022 14:03:23 -0700 Subject: [PATCH 05/13] added comments --- shell/platform/common/client_wrapper/core_implementations.cc | 1 + shell/platform/embedder/embedder.cc | 1 + 2 files changed, 2 insertions(+) diff --git a/shell/platform/common/client_wrapper/core_implementations.cc b/shell/platform/common/client_wrapper/core_implementations.cc index 4a43ba01d7534..bbc6c9c9f55bf 100644 --- a/shell/platform/common/client_wrapper/core_implementations.cc +++ b/shell/platform/common/client_wrapper/core_implementations.cc @@ -42,6 +42,7 @@ void ForwardToHandler(FlutterDesktopMessengerRef messenger, BinaryReply reply_handler = [messenger_ptr, response_handle]( const uint8_t* reply, size_t reply_size) mutable { + // Note: This can be called on any thread. auto lock = std::unique_ptr( FlutterDesktopMessengerLock(messenger_ptr.get()), diff --git a/shell/platform/embedder/embedder.cc b/shell/platform/embedder/embedder.cc index 4106afc533dd0..f8bca14bc1b62 100644 --- a/shell/platform/embedder/embedder.cc +++ b/shell/platform/embedder/embedder.cc @@ -2330,6 +2330,7 @@ FlutterEngineResult FlutterEngineSendPlatformMessageResponse( const FlutterPlatformMessageResponseHandle* handle, const uint8_t* data, size_t data_length) { + // Note: This can execute on any thread. if (data_length != 0 && data == nullptr) { return LOG_EMBEDDER_ERROR( kInvalidArguments, From 06d1b05c7e2a04a04ce267eb2d7745ef9efb082a Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Mon, 24 Oct 2022 16:01:59 -0700 Subject: [PATCH 06/13] made glfw messenger unable to be copied --- shell/platform/glfw/flutter_glfw.cc | 3 +++ 1 file changed, 3 insertions(+) diff --git a/shell/platform/glfw/flutter_glfw.cc b/shell/platform/glfw/flutter_glfw.cc index ddee807f125b5..ecf48b53f92ba 100644 --- a/shell/platform/glfw/flutter_glfw.cc +++ b/shell/platform/glfw/flutter_glfw.cc @@ -149,6 +149,8 @@ struct FlutterDesktopPluginRegistrar { // State associated with the messenger used to communicate with the engine. struct FlutterDesktopMessenger { + FlutterDesktopMessenger() = default; + void AddRef() { ref_count_.fetch_add(1); } void Release() { @@ -167,6 +169,7 @@ struct FlutterDesktopMessenger { std::mutex& GetMutex() { return mutex_; } private: + FML_DISALLOW_COPY_AND_ASSIGN(FlutterDesktopMessenger); // The engine that backs this messenger. FlutterDesktopEngineState* engine_; std::atomic ref_count_ = 0; From da8322250a532e79bf7f0b025bbfe119fda63a2e Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Tue, 1 Nov 2022 13:45:21 -0700 Subject: [PATCH 07/13] stuart feedback 1 --- shell/platform/common/public/flutter_messenger.h | 12 +++++++++--- shell/platform/glfw/flutter_glfw.cc | 4 +++- shell/platform/windows/public/flutter_windows.h | 2 ++ 3 files changed, 14 insertions(+), 4 deletions(-) diff --git a/shell/platform/common/public/flutter_messenger.h b/shell/platform/common/public/flutter_messenger.h index 4c080fcc417cc..019267cbe7dfa 100644 --- a/shell/platform/common/public/flutter_messenger.h +++ b/shell/platform/common/public/flutter_messenger.h @@ -90,15 +90,15 @@ FLUTTER_EXPORT void FlutterDesktopMessengerSetCallback( // Increments the reference count for the |messenger|. // +// Operation is thread-safe. +// // See also: |FlutterDesktopMessengerRelease| FLUTTER_EXPORT FlutterDesktopMessengerRef FlutterDesktopMessengerAddRef(FlutterDesktopMessengerRef messenger); // Decrements the reference count for the |messenger|. // -// The reference count starts at zero so it is valid to call -// FlutterDesktopMessengerRelease without having called -// FlutterDesktopMessengerAddRef. +// Operation is thread-safe. // // See also: |FlutterDesktopMessengerAddRef| FLUTTER_EXPORT void FlutterDesktopMessengerRelease( @@ -119,12 +119,18 @@ FLUTTER_EXPORT bool FlutterDesktopMessengerIsAvailable( // All calls to the FlutterDesktopMessengerRef from threads other than the // platform thread should happen inside of a lock. // +// Operation is thread-safe. +// +// Returns the |messenger| value. +// // See also: |FlutterDesktopMessengerUnlock| FLUTTER_EXPORT FlutterDesktopMessengerRef FlutterDesktopMessengerLock(FlutterDesktopMessengerRef messenger); // Unlocks the `FlutterDesktopMessengerRef`. // +// Operation is thread-safe. +// // See also: |FlutterDesktopMessengerLock| FLUTTER_EXPORT void FlutterDesktopMessengerUnlock( FlutterDesktopMessengerRef messenger); diff --git a/shell/platform/glfw/flutter_glfw.cc b/shell/platform/glfw/flutter_glfw.cc index ecf48b53f92ba..d9417850f823d 100644 --- a/shell/platform/glfw/flutter_glfw.cc +++ b/shell/platform/glfw/flutter_glfw.cc @@ -169,7 +169,9 @@ struct FlutterDesktopMessenger { std::mutex& GetMutex() { return mutex_; } private: - FML_DISALLOW_COPY_AND_ASSIGN(FlutterDesktopMessenger); + FlutterDesktopMessenger(const FlutterDesktopMessenger& value) = delete; + FlutterDesktopMessenger& operator=(const FlutterDesktopMessenger& value) = + delete; // The engine that backs this messenger. FlutterDesktopEngineState* engine_; std::atomic ref_count_ = 0; diff --git a/shell/platform/windows/public/flutter_windows.h b/shell/platform/windows/public/flutter_windows.h index 89195454117c6..33a4dfd7bfc91 100644 --- a/shell/platform/windows/public/flutter_windows.h +++ b/shell/platform/windows/public/flutter_windows.h @@ -181,6 +181,8 @@ FlutterDesktopEngineGetPluginRegistrar(FlutterDesktopEngineRef engine, const char* plugin_name); // Returns the messenger associated with the engine. +// +// This does not effect the reference count of the FlutterDesktopMessenger. FLUTTER_EXPORT FlutterDesktopMessengerRef FlutterDesktopEngineGetMessenger(FlutterDesktopEngineRef engine); From cd82e4d5ace7623f891f041d9b50b25ce50bc09e Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Wed, 2 Nov 2022 13:10:52 -0700 Subject: [PATCH 08/13] stuart feedback 2: replaced the shared_ptr --- shell/platform/glfw/flutter_glfw.cc | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/shell/platform/glfw/flutter_glfw.cc b/shell/platform/glfw/flutter_glfw.cc index d9417850f823d..7428a0776f91a 100644 --- a/shell/platform/glfw/flutter_glfw.cc +++ b/shell/platform/glfw/flutter_glfw.cc @@ -106,6 +106,10 @@ struct AOTDataDeleter { }; using UniqueAotDataPtr = std::unique_ptr<_FlutterEngineAOTData, AOTDataDeleter>; +/// Maintains one ref on the FlutterDesktopMessenger's internal reference count. +using FlutterDesktopMessengerReferenceOwner = + std::unique_ptr; // Struct for storing state of a Flutter engine instance. struct FlutterDesktopEngineState { @@ -116,7 +120,8 @@ struct FlutterDesktopEngineState { std::unique_ptr event_loop; // The plugin messenger handle given to API clients. - std::shared_ptr messenger; + FlutterDesktopMessengerReferenceOwner messenger = { + nullptr, [](FlutterDesktopMessengerRef ref) {}}; // Message dispatch manager for messages from the Flutter engine. std::unique_ptr message_dispatcher; @@ -792,7 +797,7 @@ static void SetUpLocales(FlutterDesktopEngineState* state) { static void SetUpCommonEngineState(FlutterDesktopEngineState* state, GLFWwindow* window) { // Messaging. - state->messenger = std::shared_ptr( + state->messenger = FlutterDesktopMessengerReferenceOwner( FlutterDesktopMessengerAddRef(new FlutterDesktopMessenger()), &FlutterDesktopMessengerRelease); state->messenger->SetEngine(state); From 5781f32924bd4c8c708a27f53f643ee9d3bb725d Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Wed, 2 Nov 2022 13:39:05 -0700 Subject: [PATCH 09/13] stuart feedback 3 --- .../client_wrapper/core_implementations.cc | 10 +++++--- shell/platform/embedder/embedder.cc | 2 +- shell/platform/glfw/flutter_glfw.cc | 18 +++++++++++++- .../platform/windows/public/flutter_windows.h | 6 ++++- shell/platform/windows/window_state.h | 24 ++++++++++++++++++- 5 files changed, 53 insertions(+), 7 deletions(-) diff --git a/shell/platform/common/client_wrapper/core_implementations.cc b/shell/platform/common/client_wrapper/core_implementations.cc index bbc6c9c9f55bf..a0e06578b2c19 100644 --- a/shell/platform/common/client_wrapper/core_implementations.cc +++ b/shell/platform/common/client_wrapper/core_implementations.cc @@ -26,6 +26,11 @@ namespace flutter { // ========== binary_messenger_impl.h ========== namespace { + +using FlutterDesktopMessengerScopedLock = + std::unique_ptr; + // Passes |message| to |user_data|, which must be a BinaryMessageHandler, along // with a BinaryReply that will send a response on |message|'s response handle. // @@ -42,9 +47,8 @@ void ForwardToHandler(FlutterDesktopMessengerRef messenger, BinaryReply reply_handler = [messenger_ptr, response_handle]( const uint8_t* reply, size_t reply_size) mutable { - // Note: This can be called on any thread. - auto lock = std::unique_ptr( + // Note: This lambda can be called on any thread. + auto lock = FlutterDesktopMessengerScopedLock( FlutterDesktopMessengerLock(messenger_ptr.get()), &FlutterDesktopMessengerUnlock); if (!FlutterDesktopMessengerIsAvailable(messenger_ptr.get())) { diff --git a/shell/platform/embedder/embedder.cc b/shell/platform/embedder/embedder.cc index f8bca14bc1b62..3511162c89d40 100644 --- a/shell/platform/embedder/embedder.cc +++ b/shell/platform/embedder/embedder.cc @@ -2325,12 +2325,12 @@ FlutterEngineResult FlutterPlatformMessageReleaseResponseHandle( return kSuccess; } +// Note: This can execute on any thread. FlutterEngineResult FlutterEngineSendPlatformMessageResponse( FLUTTER_API_SYMBOL(FlutterEngine) engine, const FlutterPlatformMessageResponseHandle* handle, const uint8_t* data, size_t data_length) { - // Note: This can execute on any thread. if (data_length != 0 && data == nullptr) { return LOG_EMBEDDER_ERROR( kInvalidArguments, diff --git a/shell/platform/glfw/flutter_glfw.cc b/shell/platform/glfw/flutter_glfw.cc index 7428a0776f91a..2509067a7ccc9 100644 --- a/shell/platform/glfw/flutter_glfw.cc +++ b/shell/platform/glfw/flutter_glfw.cc @@ -156,8 +156,15 @@ struct FlutterDesktopPluginRegistrar { struct FlutterDesktopMessenger { FlutterDesktopMessenger() = default; + /// Increments the reference count. + /// + /// Thread-safe. void AddRef() { ref_count_.fetch_add(1); } + /// Decrements the reference count and deletes the object if the count has + /// gone to zero. + /// + /// Thread-safe. void Release() { int32_t old_count = ref_count_.fetch_sub(1); if (old_count <= 1) { @@ -165,18 +172,27 @@ struct FlutterDesktopMessenger { } } + /// Getter for the engine field. FlutterDesktopEngineState* GetEngine() const { return engine_; } + + /// Setter for the engine field. + /// Thread-safe. void SetEngine(FlutterDesktopEngineState* engine) { std::scoped_lock lock(mutex_); engine_ = engine; } + /// Returns the mutex associated with the |FlutterDesktopMessenger|. + /// + /// This mutex is used to synchronize reading or writing state inside the + /// |FlutterDesktopMessenger| (ie |engine_|). std::mutex& GetMutex() { return mutex_; } - private: FlutterDesktopMessenger(const FlutterDesktopMessenger& value) = delete; FlutterDesktopMessenger& operator=(const FlutterDesktopMessenger& value) = delete; + + private: // The engine that backs this messenger. FlutterDesktopEngineState* engine_; std::atomic ref_count_ = 0; diff --git a/shell/platform/windows/public/flutter_windows.h b/shell/platform/windows/public/flutter_windows.h index 33a4dfd7bfc91..8c1e365ec1571 100644 --- a/shell/platform/windows/public/flutter_windows.h +++ b/shell/platform/windows/public/flutter_windows.h @@ -182,7 +182,11 @@ FlutterDesktopEngineGetPluginRegistrar(FlutterDesktopEngineRef engine, // Returns the messenger associated with the engine. // -// This does not effect the reference count of the FlutterDesktopMessenger. +// This does not provide an owning reference, so should *not* be balanced with a +// call to |FlutterDesktopMessengerRelease|. +// +// Callers should use |FlutterDesktopMessengerAddRef| if the returned pointer +// will potentially outlive 'engine', such as when passing it to another thread. FLUTTER_EXPORT FlutterDesktopMessengerRef FlutterDesktopEngineGetMessenger(FlutterDesktopEngineRef engine); diff --git a/shell/platform/windows/window_state.h b/shell/platform/windows/window_state.h index d1ea14839c956..2b7318229f755 100644 --- a/shell/platform/windows/window_state.h +++ b/shell/platform/windows/window_state.h @@ -40,25 +40,47 @@ struct FlutterDesktopPluginRegistrar { struct FlutterDesktopMessenger { public: FlutterDesktopMessenger() = default; + + /// Getter for the engine field. flutter::FlutterWindowsEngine* GetEngine() const { return engine; } + + /// Setter for the engine field. + /// Thread-safe. void SetEngine(flutter::FlutterWindowsEngine* arg_engine) { std::scoped_lock lock(mutex_); engine = arg_engine; } + + /// Increments the reference count. + /// + /// Thread-safe. FlutterDesktopMessenger* AddRef() { ref_count_.fetch_add(1); return this; } + + /// Decrements the reference count and deletes the object if the count has + /// gone to zero. + /// + /// Thread-safe. void Release() { int32_t old_count = ref_count_.fetch_sub(1); if (old_count <= 1) { delete this; } } + + /// Returns the mutex associated with the |FlutterDesktopMessenger|. + /// + /// This mutex is used to synchronize reading or writing state inside the + /// |FlutterDesktopMessenger| (ie |engine|). std::mutex& GetMutex() { return mutex_; } + FlutterDesktopMessenger(const FlutterDesktopMessenger& value) = delete; + FlutterDesktopMessenger& operator=(const FlutterDesktopMessenger& value) = + delete; + private: - FML_DISALLOW_COPY_AND_ASSIGN(FlutterDesktopMessenger); // The engine that owns this state object. flutter::FlutterWindowsEngine* engine = nullptr; std::mutex mutex_; From e18fafcc6c518d5ad73c747a4f9f322169bdb648 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Thu, 3 Nov 2022 10:40:24 -0700 Subject: [PATCH 10/13] stuart feedback: remove error log --- shell/platform/common/client_wrapper/core_implementations.cc | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/shell/platform/common/client_wrapper/core_implementations.cc b/shell/platform/common/client_wrapper/core_implementations.cc index a0e06578b2c19..1eb5a5898117d 100644 --- a/shell/platform/common/client_wrapper/core_implementations.cc +++ b/shell/platform/common/client_wrapper/core_implementations.cc @@ -52,9 +52,7 @@ void ForwardToHandler(FlutterDesktopMessengerRef messenger, FlutterDesktopMessengerLock(messenger_ptr.get()), &FlutterDesktopMessengerUnlock); if (!FlutterDesktopMessengerIsAvailable(messenger_ptr.get())) { - std::cerr << "Error: Responding to platform channel after the engine has" - "been deleted." - << std::endl; + // Drop reply if it comes in after the engine is destroyed. return; } if (!response_handle) { From 3f519f448718e79af018a516d21c86641e3ee057 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Fri, 4 Nov 2022 14:35:12 -0700 Subject: [PATCH 11/13] Moved FlutterDesktopMessenger to its own file. --- .../windows/flutter_desktop_messenger.h | 78 +++++++++++++++++++ shell/platform/windows/flutter_windows.cc | 29 ++++--- .../windows/flutter_windows_engine.cc | 5 +- .../platform/windows/flutter_windows_engine.h | 5 +- shell/platform/windows/window_state.h | 52 +------------ 5 files changed, 103 insertions(+), 66 deletions(-) create mode 100644 shell/platform/windows/flutter_desktop_messenger.h diff --git a/shell/platform/windows/flutter_desktop_messenger.h b/shell/platform/windows/flutter_desktop_messenger.h new file mode 100644 index 0000000000000..682ca9d364ca0 --- /dev/null +++ b/shell/platform/windows/flutter_desktop_messenger.h @@ -0,0 +1,78 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef FLUTTER_SHELL_PLATFORM_WINDOWS_FLUTTER_DESKTOP_MESSENGER_H_ +#define FLUTTER_SHELL_PLATFORM_WINDOWS_FLUTTER_DESKTOP_MESSENGER_H_ + +#include +#include + +#include "flutter/shell/platform/common/public/flutter_messenger.h" + +namespace flutter { + +class FlutterWindowsEngine; + +class FlutterDesktopMessenger { + public: + FlutterDesktopMessenger() = default; + + /// Convert to FlutterDesktopMessengerRef. + FlutterDesktopMessengerRef ToRef() { + return reinterpret_cast(this); + } + + /// Convert from FlutterDesktopMessengerRef. + static FlutterDesktopMessenger* FromRef(FlutterDesktopMessengerRef ref) { + return reinterpret_cast(ref); + } + + /// Getter for the engine field. + flutter::FlutterWindowsEngine* GetEngine() const { return engine; } + + /// Setter for the engine field. + /// Thread-safe. + void SetEngine(flutter::FlutterWindowsEngine* arg_engine) { + std::scoped_lock lock(mutex_); + engine = arg_engine; + } + + /// Increments the reference count. + /// + /// Thread-safe. + FlutterDesktopMessenger* AddRef() { + ref_count_.fetch_add(1); + return this; + } + + /// Decrements the reference count and deletes the object if the count has + /// gone to zero. + /// + /// Thread-safe. + void Release() { + int32_t old_count = ref_count_.fetch_sub(1); + if (old_count <= 1) { + delete this; + } + } + + /// Returns the mutex associated with the |FlutterDesktopMessenger|. + /// + /// This mutex is used to synchronize reading or writing state inside the + /// |FlutterDesktopMessenger| (ie |engine|). + std::mutex& GetMutex() { return mutex_; } + + FlutterDesktopMessenger(const FlutterDesktopMessenger& value) = delete; + FlutterDesktopMessenger& operator=(const FlutterDesktopMessenger& value) = + delete; + + private: + // The engine that owns this state object. + flutter::FlutterWindowsEngine* engine = nullptr; + std::mutex mutex_; + std::atomic ref_count_ = 0; +}; +} // namespace flutter + +#endif // FLUTTER_SHELL_PLATFORM_WINDOWS_FLUTTER_WINDOW_STATE_H_ diff --git a/shell/platform/windows/flutter_windows.cc b/shell/platform/windows/flutter_windows.cc index 999937d0121ad..0756efc9dc4f0 100644 --- a/shell/platform/windows/flutter_windows.cc +++ b/shell/platform/windows/flutter_windows.cc @@ -262,8 +262,9 @@ bool FlutterDesktopMessengerSendWithReply(FlutterDesktopMessengerRef messenger, const size_t message_size, const FlutterDesktopBinaryReply reply, void* user_data) { - return messenger->GetEngine()->SendPlatformMessage( - channel, message, message_size, reply, user_data); + return flutter::FlutterDesktopMessenger::FromRef(messenger) + ->GetEngine() + ->SendPlatformMessage(channel, message, message_size, reply, user_data); } bool FlutterDesktopMessengerSend(FlutterDesktopMessengerRef messenger, @@ -279,39 +280,45 @@ void FlutterDesktopMessengerSendResponse( const FlutterDesktopMessageResponseHandle* handle, const uint8_t* data, size_t data_length) { - messenger->GetEngine()->SendPlatformMessageResponse(handle, data, - data_length); + flutter::FlutterDesktopMessenger::FromRef(messenger) + ->GetEngine() + ->SendPlatformMessageResponse(handle, data, data_length); } void FlutterDesktopMessengerSetCallback(FlutterDesktopMessengerRef messenger, const char* channel, FlutterDesktopMessageCallback callback, void* user_data) { - messenger->GetEngine()->message_dispatcher()->SetMessageCallback( - channel, callback, user_data); + flutter::FlutterDesktopMessenger::FromRef(messenger) + ->GetEngine() + ->message_dispatcher() + ->SetMessageCallback(channel, callback, user_data); } FlutterDesktopMessengerRef FlutterDesktopMessengerAddRef( FlutterDesktopMessengerRef messenger) { - return messenger->AddRef(); + return flutter::FlutterDesktopMessenger::FromRef(messenger) + ->AddRef() + ->ToRef(); } void FlutterDesktopMessengerRelease(FlutterDesktopMessengerRef messenger) { - messenger->Release(); + flutter::FlutterDesktopMessenger::FromRef(messenger)->Release(); } bool FlutterDesktopMessengerIsAvailable(FlutterDesktopMessengerRef messenger) { - return messenger->GetEngine() != nullptr; + return flutter::FlutterDesktopMessenger::FromRef(messenger)->GetEngine() != + nullptr; } FlutterDesktopMessengerRef FlutterDesktopMessengerLock( FlutterDesktopMessengerRef messenger) { - messenger->GetMutex().lock(); + flutter::FlutterDesktopMessenger::FromRef(messenger)->GetMutex().lock(); return messenger; } void FlutterDesktopMessengerUnlock(FlutterDesktopMessengerRef messenger) { - messenger->GetMutex().unlock(); + flutter::FlutterDesktopMessenger::FromRef(messenger)->GetMutex().unlock(); } FlutterDesktopTextureRegistrarRef FlutterDesktopRegistrarGetTextureRegistrar( diff --git a/shell/platform/windows/flutter_windows_engine.cc b/shell/platform/windows/flutter_windows_engine.cc index 719b16c2fc8de..1cd2fc408c330 100644 --- a/shell/platform/windows/flutter_windows_engine.cc +++ b/shell/platform/windows/flutter_windows_engine.cc @@ -185,9 +185,10 @@ FlutterWindowsEngine::FlutterWindowsEngine( plugin_registrar_ = std::make_unique(); plugin_registrar_->engine = this; - messenger_wrapper_ = std::make_unique(messenger_.get()); + messenger_wrapper_ = + std::make_unique(messenger_->ToRef()); message_dispatcher_ = - std::make_unique(messenger_.get()); + std::make_unique(messenger_->ToRef()); message_dispatcher_->SetMessageCallback( kAccessibilityChannelName, [](FlutterDesktopMessengerRef messenger, diff --git a/shell/platform/windows/flutter_windows_engine.h b/shell/platform/windows/flutter_windows_engine.h index a94ec6bc48bcf..3b6fdf15e9185 100644 --- a/shell/platform/windows/flutter_windows_engine.h +++ b/shell/platform/windows/flutter_windows_engine.h @@ -20,6 +20,7 @@ #include "flutter/shell/platform/common/incoming_message_dispatcher.h" #include "flutter/shell/platform/embedder/embedder.h" #include "flutter/shell/platform/windows/angle_surface_manager.h" +#include "flutter/shell/platform/windows/flutter_desktop_messenger.h" #include "flutter/shell/platform/windows/flutter_project_bundle.h" #include "flutter/shell/platform/windows/flutter_windows_texture_registrar.h" #include "flutter/shell/platform/windows/public/flutter_windows.h" @@ -124,7 +125,7 @@ class FlutterWindowsEngine { // Sets switches member to the given switches. void SetSwitches(const std::vector& switches); - FlutterDesktopMessengerRef messenger() { return messenger_.get(); } + FlutterDesktopMessengerRef messenger() { return messenger_->ToRef(); } IncomingMessageDispatcher* message_dispatcher() { return message_dispatcher_.get(); @@ -283,7 +284,7 @@ class FlutterWindowsEngine { std::unique_ptr task_runner_; // The plugin messenger handle given to API clients. - fml::RefPtr messenger_; + fml::RefPtr messenger_; // A wrapper around messenger_ for interacting with client_wrapper-level APIs. std::unique_ptr messenger_wrapper_; diff --git a/shell/platform/windows/window_state.h b/shell/platform/windows/window_state.h index 2b7318229f755..debb501894ecb 100644 --- a/shell/platform/windows/window_state.h +++ b/shell/platform/windows/window_state.h @@ -5,8 +5,6 @@ #ifndef FLUTTER_SHELL_PLATFORM_WINDOWS_FLUTTER_WINDOW_STATE_H_ #define FLUTTER_SHELL_PLATFORM_WINDOWS_FLUTTER_WINDOW_STATE_H_ -#include - #include "flutter/shell/platform/common/client_wrapper/include/flutter/plugin_registrar.h" #include "flutter/shell/platform/common/incoming_message_dispatcher.h" #include "flutter/shell/platform/embedder/embedder.h" @@ -37,54 +35,6 @@ struct FlutterDesktopPluginRegistrar { // Wrapper to distinguish the messenger ref from the engine ref given out // in the C API. -struct FlutterDesktopMessenger { - public: - FlutterDesktopMessenger() = default; - - /// Getter for the engine field. - flutter::FlutterWindowsEngine* GetEngine() const { return engine; } - - /// Setter for the engine field. - /// Thread-safe. - void SetEngine(flutter::FlutterWindowsEngine* arg_engine) { - std::scoped_lock lock(mutex_); - engine = arg_engine; - } - - /// Increments the reference count. - /// - /// Thread-safe. - FlutterDesktopMessenger* AddRef() { - ref_count_.fetch_add(1); - return this; - } - - /// Decrements the reference count and deletes the object if the count has - /// gone to zero. - /// - /// Thread-safe. - void Release() { - int32_t old_count = ref_count_.fetch_sub(1); - if (old_count <= 1) { - delete this; - } - } - - /// Returns the mutex associated with the |FlutterDesktopMessenger|. - /// - /// This mutex is used to synchronize reading or writing state inside the - /// |FlutterDesktopMessenger| (ie |engine|). - std::mutex& GetMutex() { return mutex_; } - - FlutterDesktopMessenger(const FlutterDesktopMessenger& value) = delete; - FlutterDesktopMessenger& operator=(const FlutterDesktopMessenger& value) = - delete; - - private: - // The engine that owns this state object. - flutter::FlutterWindowsEngine* engine = nullptr; - std::mutex mutex_; - std::atomic ref_count_ = 0; -}; +struct FlutterDesktopMessenger {}; #endif // FLUTTER_SHELL_PLATFORM_WINDOWS_FLUTTER_WINDOW_STATE_H_ From 81da72e09639f9d654b9433772758180d233bc96 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Mon, 7 Nov 2022 09:13:22 -0800 Subject: [PATCH 12/13] updated licenses --- ci/licenses_golden/licenses_flutter | 1 + 1 file changed, 1 insertion(+) diff --git a/ci/licenses_golden/licenses_flutter b/ci/licenses_golden/licenses_flutter index 974f458968985..d48759ee457c0 100644 --- a/ci/licenses_golden/licenses_flutter +++ b/ci/licenses_golden/licenses_flutter @@ -3099,6 +3099,7 @@ FILE: ../../../flutter/shell/platform/windows/external_texture_d3d.cc FILE: ../../../flutter/shell/platform/windows/external_texture_d3d.h FILE: ../../../flutter/shell/platform/windows/external_texture_pixelbuffer.cc FILE: ../../../flutter/shell/platform/windows/external_texture_pixelbuffer.h +FILE: ../../../flutter/shell/platform/windows/flutter_desktop_messenger.h FILE: ../../../flutter/shell/platform/windows/flutter_key_map.g.cc FILE: ../../../flutter/shell/platform/windows/flutter_platform_node_delegate_windows.cc FILE: ../../../flutter/shell/platform/windows/flutter_platform_node_delegate_windows.h From 1794f65d1abab5a1ab965475d8d905d27fc6b684 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Wed, 9 Nov 2022 13:13:00 -0800 Subject: [PATCH 13/13] stuart feedback --- shell/platform/windows/flutter_desktop_messenger.h | 5 +++++ shell/platform/windows/window_state.h | 4 ---- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/shell/platform/windows/flutter_desktop_messenger.h b/shell/platform/windows/flutter_desktop_messenger.h index 682ca9d364ca0..8920068e511fe 100644 --- a/shell/platform/windows/flutter_desktop_messenger.h +++ b/shell/platform/windows/flutter_desktop_messenger.h @@ -14,6 +14,11 @@ namespace flutter { class FlutterWindowsEngine; +/// A messenger object used to invoke platform messages. +/// +/// On Windows, the message handler is essentially the |FlutterWindowsEngine|, +/// this allows a handle to the |FlutterWindowsEngine| that will become +/// invalidated if the |FlutterWindowsEngine| is destroyed. class FlutterDesktopMessenger { public: FlutterDesktopMessenger() = default; diff --git a/shell/platform/windows/window_state.h b/shell/platform/windows/window_state.h index debb501894ecb..13b282a535862 100644 --- a/shell/platform/windows/window_state.h +++ b/shell/platform/windows/window_state.h @@ -33,8 +33,4 @@ struct FlutterDesktopPluginRegistrar { flutter::FlutterWindowsEngine* engine = nullptr; }; -// Wrapper to distinguish the messenger ref from the engine ref given out -// in the C API. -struct FlutterDesktopMessenger {}; - #endif // FLUTTER_SHELL_PLATFORM_WINDOWS_FLUTTER_WINDOW_STATE_H_