From a2558d437de725a8fb1a10b47515a73f3cfe7ac8 Mon Sep 17 00:00:00 2001 From: Loic Sharma Date: Tue, 20 Feb 2024 14:17:41 -0800 Subject: [PATCH] [Windows] Order top-level message procedures --- .../flutter/software_surface_producer.cc | 2 +- .../flutter_windows_engine_unittests.cc | 22 ++++++------ .../windows/window_proc_delegate_manager.cc | 27 ++++++++++---- .../windows/window_proc_delegate_manager.h | 25 +++++++------ .../window_proc_delegate_manager_unittests.cc | 35 +++++++++++++++++++ 5 files changed, 81 insertions(+), 30 deletions(-) diff --git a/shell/platform/fuchsia/flutter/software_surface_producer.cc b/shell/platform/fuchsia/flutter/software_surface_producer.cc index a7be9e3bb2b1a..d279a92fa777d 100644 --- a/shell/platform/fuchsia/flutter/software_surface_producer.cc +++ b/shell/platform/fuchsia/flutter/software_surface_producer.cc @@ -7,7 +7,7 @@ #include #include -#include // Foor std::remove_if +#include // For std::remove_if #include #include #include diff --git a/shell/platform/windows/flutter_windows_engine_unittests.cc b/shell/platform/windows/flutter_windows_engine_unittests.cc index 5b48ef53f381f..d72a2e4e3dc46 100644 --- a/shell/platform/windows/flutter_windows_engine_unittests.cc +++ b/shell/platform/windows/flutter_windows_engine_unittests.cc @@ -784,9 +784,9 @@ TEST_F(FlutterWindowsEngineTest, TestExitCancel) { } } -// TODO(loicsharma): This test is passing incorrectly on the first -// WM_CLOSE message when instead it should pass on the second WM_CLOSE message. -// https://github.com/flutter/flutter/issues/137963 +// Flutter consumes the first WM_CLOSE message to allow the app to cancel the +// exit. If the app does not cancel the exit, Flutter synthesizes a second +// WM_CLOSE message. TEST_F(FlutterWindowsEngineTest, TestExitSecondCloseMessage) { FlutterWindowsEngineBuilder builder{GetContext()}; builder.SetDartEntrypoint("exitTestExit"); @@ -802,18 +802,16 @@ TEST_F(FlutterWindowsEngineTest, TestExitSecondCloseMessage) { modifier.embedder_api().RunsAOTCompiledDartCode = []() { return false; }; auto handler = std::make_unique(engine.get()); EXPECT_CALL(*handler, SetLifecycleState(AppLifecycleState::kResumed)); - // TODO(loicsharma): These should be `EXPECT_CALL`s - // https://github.com/flutter/flutter/issues/137963 - ON_CALL(*handler, IsLastWindowOfProcess).WillByDefault(Return(true)); - ON_CALL(*handler, Quit) - .WillByDefault([handler_ptr = handler.get()]( - std::optional hwnd, std::optional wparam, - std::optional lparam, UINT exit_code) { + EXPECT_CALL(*handler, IsLastWindowOfProcess).WillOnce(Return(true)); + EXPECT_CALL(*handler, Quit) + .WillOnce([handler_ptr = handler.get()]( + std::optional hwnd, std::optional wparam, + std::optional lparam, UINT exit_code) { handler_ptr->WindowsLifecycleManager::Quit(hwnd, wparam, lparam, exit_code); }); - ON_CALL(*handler, DispatchMessage) - .WillByDefault( + EXPECT_CALL(*handler, DispatchMessage) + .WillRepeatedly( [&engine](HWND hwnd, UINT msg, WPARAM wparam, LPARAM lparam) { engine->window_proc_delegate_manager()->OnTopLevelWindowProc( hwnd, msg, wparam, lparam); diff --git a/shell/platform/windows/window_proc_delegate_manager.cc b/shell/platform/windows/window_proc_delegate_manager.cc index b2aae25958a16..8fab1f6360c8c 100644 --- a/shell/platform/windows/window_proc_delegate_manager.cc +++ b/shell/platform/windows/window_proc_delegate_manager.cc @@ -4,6 +4,8 @@ #include "flutter/shell/platform/windows/window_proc_delegate_manager.h" +#include + #include "flutter/shell/platform/embedder/embedder.h" namespace flutter { @@ -12,26 +14,37 @@ WindowProcDelegateManager::WindowProcDelegateManager() = default; WindowProcDelegateManager::~WindowProcDelegateManager() = default; void WindowProcDelegateManager::RegisterTopLevelWindowProcDelegate( - FlutterDesktopWindowProcCallback delegate, + FlutterDesktopWindowProcCallback callback, void* user_data) { - top_level_window_proc_handlers_[delegate] = user_data; + UnregisterTopLevelWindowProcDelegate(callback); + + delegates_.push_back(WindowProcDelegate{ + .callback = callback, + .user_data = user_data, + }); } void WindowProcDelegateManager::UnregisterTopLevelWindowProcDelegate( - FlutterDesktopWindowProcCallback delegate) { - top_level_window_proc_handlers_.erase(delegate); + FlutterDesktopWindowProcCallback callback) { + delegates_.erase( + std::remove_if(delegates_.begin(), delegates_.end(), + [&callback](const WindowProcDelegate& delegate) { + return delegate.callback == callback; + }), + delegates_.end()); } std::optional WindowProcDelegateManager::OnTopLevelWindowProc( HWND hwnd, UINT message, WPARAM wparam, - LPARAM lparam) { + LPARAM lparam) const { std::optional result; - for (const auto& [handler, user_data] : top_level_window_proc_handlers_) { + for (const auto& delegate : delegates_) { LPARAM handler_result; // Stop as soon as any delegate indicates that it has handled the message. - if (handler(hwnd, message, wparam, lparam, user_data, &handler_result)) { + if (delegate.callback(hwnd, message, wparam, lparam, delegate.user_data, + &handler_result)) { result = handler_result; break; } diff --git a/shell/platform/windows/window_proc_delegate_manager.h b/shell/platform/windows/window_proc_delegate_manager.h index bd6116f0058f1..e66f343f1a4e8 100644 --- a/shell/platform/windows/window_proc_delegate_manager.h +++ b/shell/platform/windows/window_proc_delegate_manager.h @@ -7,8 +7,8 @@ #include -#include #include +#include #include "flutter/fml/macros.h" #include "flutter/shell/platform/windows/public/flutter_windows.h" @@ -22,30 +22,35 @@ class WindowProcDelegateManager { explicit WindowProcDelegateManager(); ~WindowProcDelegateManager(); - // Adds |delegate| as a delegate to be called for |OnTopLevelWindowProc|. + // Adds |callback| as a delegate to be called for |OnTopLevelWindowProc|. // - // Multiple calls with the same |delegate| will replace the previous + // Multiple calls with the same |callback| will replace the previous // registration, even if |user_data| is different. void RegisterTopLevelWindowProcDelegate( - FlutterDesktopWindowProcCallback delegate, + FlutterDesktopWindowProcCallback callback, void* user_data); - // Unregisters |delegate| as a delate for |OnTopLevelWindowProc|. + // Unregisters |callback| as a delegate for |OnTopLevelWindowProc|. void UnregisterTopLevelWindowProcDelegate( - FlutterDesktopWindowProcCallback delegate); + FlutterDesktopWindowProcCallback callback); - // Calls any registered WindowProc delegates. + // Calls any registered WindowProc delegates in the order they were + // registered. // // If a result is returned, then the message was handled in such a way that // further handling should not be done. std::optional OnTopLevelWindowProc(HWND hwnd, UINT message, WPARAM wparam, - LPARAM lparam); + LPARAM lparam) const; private: - std::map - top_level_window_proc_handlers_; + struct WindowProcDelegate { + FlutterDesktopWindowProcCallback callback = nullptr; + void* user_data = nullptr; + }; + + std::vector delegates_; FML_DISALLOW_COPY_AND_ASSIGN(WindowProcDelegateManager); }; diff --git a/shell/platform/windows/window_proc_delegate_manager_unittests.cc b/shell/platform/windows/window_proc_delegate_manager_unittests.cc index 195fb5b88cd0e..625310e6a6bba 100644 --- a/shell/platform/windows/window_proc_delegate_manager_unittests.cc +++ b/shell/platform/windows/window_proc_delegate_manager_unittests.cc @@ -125,6 +125,41 @@ TEST(WindowProcDelegateManagerTest, RegisterMultiple) { EXPECT_TRUE(called_b); } +TEST(WindowProcDelegateManagerTest, Ordered) { + TestWindowProcDelegate delegate_1 = [](HWND hwnd, UINT message, WPARAM wparam, + LPARAM lparam) { return 1; }; + TestWindowProcDelegate delegate_2 = [](HWND hwnd, UINT message, WPARAM wparam, + LPARAM lparam) { return 2; }; + + // Result should be 1 if delegate '1' is registered before delegate '2'. + { + WindowProcDelegateManager manager; + manager.RegisterTopLevelWindowProcDelegate(TestWindowProcCallback, + &delegate_1); + manager.RegisterTopLevelWindowProcDelegate(TestWindowProcCallback2, + &delegate_2); + + std::optional result = + manager.OnTopLevelWindowProc(nullptr, 0, 0, 0); + + EXPECT_EQ(result, 1); + } + + // Result should be 2 if delegate '2' is registered before delegate '1'. + { + WindowProcDelegateManager manager; + manager.RegisterTopLevelWindowProcDelegate(TestWindowProcCallback2, + &delegate_2); + manager.RegisterTopLevelWindowProcDelegate(TestWindowProcCallback, + &delegate_1); + + std::optional result = + manager.OnTopLevelWindowProc(nullptr, 0, 0, 0); + + EXPECT_EQ(result, 2); + } +} + TEST(WindowProcDelegateManagerTest, ConflictingDelegates) { WindowProcDelegateManager manager;