From 87b8a883d461a767427a41e8eaa9aab5a49374fe Mon Sep 17 00:00:00 2001 From: yaakovschectman <109111084+yaakovschectman@users.noreply.github.com> Date: Wed, 2 Aug 2023 14:53:33 -0400 Subject: [PATCH 1/8] Reintroduce win lifecycle changes --- ci/licenses_golden/excluded_files | 1 + shell/platform/windows/BUILD.gn | 1 + .../windows/client_wrapper/flutter_engine.cc | 13 ++ .../flutter_engine_unittests.cc | 26 ++++ .../include/flutter/flutter_engine.h | 11 ++ .../testing/stub_flutter_windows_api.cc | 14 ++ .../testing/stub_flutter_windows_api.h | 11 ++ shell/platform/windows/flutter_window.cc | 32 ++++- shell/platform/windows/flutter_window.h | 9 ++ .../windows/flutter_window_unittests.cc | 62 ++++++++- shell/platform/windows/flutter_windows.cc | 16 +++ .../windows/flutter_windows_engine.cc | 24 +++- .../platform/windows/flutter_windows_engine.h | 16 +++ .../flutter_windows_engine_unittests.cc | 107 +++++++++++++++ .../platform/windows/flutter_windows_view.cc | 6 + shell/platform/windows/flutter_windows_view.h | 3 + .../platform/windows/public/flutter_windows.h | 13 ++ shell/platform/windows/testing/mock_window.h | 2 + .../mock_window_binding_handler_delegate.h | 2 + shell/platform/windows/window.cc | 5 + shell/platform/windows/window.h | 4 + .../windows/window_binding_handler_delegate.h | 5 + .../windows/windows_lifecycle_manager.cc | 124 ++++++++++++++++++ .../windows/windows_lifecycle_manager.h | 63 ++++++++- .../windows_lifecycle_manager_unittests.cc | 61 +++++++++ 25 files changed, 620 insertions(+), 11 deletions(-) create mode 100644 shell/platform/windows/windows_lifecycle_manager_unittests.cc diff --git a/ci/licenses_golden/excluded_files b/ci/licenses_golden/excluded_files index 46fec1b3ca9a0..95fd0061eec3a 100644 --- a/ci/licenses_golden/excluded_files +++ b/ci/licenses_golden/excluded_files @@ -352,6 +352,7 @@ ../../../flutter/shell/platform/windows/text_input_plugin_unittest.cc ../../../flutter/shell/platform/windows/window_proc_delegate_manager_unittests.cc ../../../flutter/shell/platform/windows/window_unittests.cc +../../../flutter/shell/platform/windows/windows_lifecycle_manager_unittests.cc ../../../flutter/shell/profiling/sampling_profiler_unittest.cc ../../../flutter/shell/testing ../../../flutter/shell/vmservice/.dart_tool diff --git a/shell/platform/windows/BUILD.gn b/shell/platform/windows/BUILD.gn index 5ee5b431991c8..10369da9c097e 100644 --- a/shell/platform/windows/BUILD.gn +++ b/shell/platform/windows/BUILD.gn @@ -221,6 +221,7 @@ executable("flutter_windows_unittests") { "text_input_plugin_unittest.cc", "window_proc_delegate_manager_unittests.cc", "window_unittests.cc", + "windows_lifecycle_manager_unittests.cc", ] configs += diff --git a/shell/platform/windows/client_wrapper/flutter_engine.cc b/shell/platform/windows/client_wrapper/flutter_engine.cc index 00ccbbcff7cab..53d32f443f88f 100644 --- a/shell/platform/windows/client_wrapper/flutter_engine.cc +++ b/shell/platform/windows/client_wrapper/flutter_engine.cc @@ -100,6 +100,19 @@ void FlutterEngine::SetNextFrameCallback(std::function callback) { this); } +std::optional FlutterEngine::ProcessExternalWindowMessage( + HWND hwnd, + UINT message, + WPARAM wparam, + LPARAM lparam) { + LRESULT result; + if (FlutterDesktopEngineProcessExternalWindowMessage( + engine_, hwnd, message, wparam, lparam, &result)) { + return result; + } + return std::nullopt; +} + FlutterDesktopEngineRef FlutterEngine::RelinquishEngine() { owns_engine_ = false; return engine_; diff --git a/shell/platform/windows/client_wrapper/flutter_engine_unittests.cc b/shell/platform/windows/client_wrapper/flutter_engine_unittests.cc index a1274998eadce..78ad17b1efdf1 100644 --- a/shell/platform/windows/client_wrapper/flutter_engine_unittests.cc +++ b/shell/platform/windows/client_wrapper/flutter_engine_unittests.cc @@ -56,6 +56,17 @@ class TestFlutterWindowsApi : public testing::StubFlutterWindowsApi { // |flutter::testing::StubFlutterWindowsApi| void EngineReloadSystemFonts() override { reload_fonts_called_ = true; } + // |flutter::testing::StubFlutterWindowsApi| + bool EngineProcessExternalWindowMessage(FlutterDesktopEngineRef engine, + HWND hwnd, + UINT message, + WPARAM wparam, + LPARAM lparam, + LRESULT* result) override { + last_external_message_ = message; + return false; + } + bool create_called() { return create_called_; } bool run_called() { return run_called_; } @@ -74,6 +85,8 @@ class TestFlutterWindowsApi : public testing::StubFlutterWindowsApi { next_frame_callback_ = nullptr; } + UINT last_external_message() { return last_external_message_; } + private: bool create_called_ = false; bool run_called_ = false; @@ -82,6 +95,7 @@ class TestFlutterWindowsApi : public testing::StubFlutterWindowsApi { std::vector dart_entrypoint_arguments_; VoidCallback next_frame_callback_ = nullptr; void* next_frame_user_data_ = nullptr; + UINT last_external_message_ = 0; }; } // namespace @@ -201,4 +215,16 @@ TEST(FlutterEngineTest, SetNextFrameCallback) { EXPECT_TRUE(success); } +TEST(FlutterEngineTest, ProcessExternalWindowMessage) { + testing::ScopedStubFlutterWindowsApi scoped_api_stub( + std::make_unique()); + auto test_api = static_cast(scoped_api_stub.stub()); + + FlutterEngine engine(DartProject(L"fake/project/path")); + + engine.ProcessExternalWindowMessage(reinterpret_cast(1), 1234, 0, 0); + + EXPECT_EQ(test_api->last_external_message(), 1234); +} + } // namespace flutter diff --git a/shell/platform/windows/client_wrapper/include/flutter/flutter_engine.h b/shell/platform/windows/client_wrapper/include/flutter/flutter_engine.h index 63a820ce2d6bd..b71c5cd479611 100644 --- a/shell/platform/windows/client_wrapper/include/flutter/flutter_engine.h +++ b/shell/platform/windows/client_wrapper/include/flutter/flutter_engine.h @@ -9,6 +9,7 @@ #include #include +#include #include #include "binary_messenger.h" @@ -84,6 +85,16 @@ class FlutterEngine : public PluginRegistry { // once on the platform thread. void SetNextFrameCallback(std::function callback); + // Called to pass an external window message to the engine for lifecycle + // state updates. This does not consume the window message. Non-Flutter + // windows must call this method in their WndProc in order to be included in + // the logic for application lifecycle state updates. Returns a result when + // the message has been consumed. + std::optional ProcessExternalWindowMessage(HWND hwnd, + UINT message, + WPARAM wparam, + LPARAM lparam); + private: // For access to RelinquishEngine. friend class FlutterViewController; diff --git a/shell/platform/windows/client_wrapper/testing/stub_flutter_windows_api.cc b/shell/platform/windows/client_wrapper/testing/stub_flutter_windows_api.cc index ed9f9d3bcb2ff..da0a8c57712d3 100644 --- a/shell/platform/windows/client_wrapper/testing/stub_flutter_windows_api.cc +++ b/shell/platform/windows/client_wrapper/testing/stub_flutter_windows_api.cc @@ -162,6 +162,20 @@ IDXGIAdapter* FlutterDesktopViewGetGraphicsAdapter(FlutterDesktopViewRef view) { return nullptr; } +bool FlutterDesktopEngineProcessExternalWindowMessage( + FlutterDesktopEngineRef engine, + HWND hwnd, + UINT message, + WPARAM wparam, + LPARAM lparam, + LRESULT* result) { + if (s_stub_implementation) { + return s_stub_implementation->EngineProcessExternalWindowMessage( + engine, hwnd, message, wparam, lparam, result); + } + return false; +} + FlutterDesktopViewRef FlutterDesktopPluginRegistrarGetView( FlutterDesktopPluginRegistrarRef controller) { // The stub ignores this, so just return an arbitrary non-zero value. diff --git a/shell/platform/windows/client_wrapper/testing/stub_flutter_windows_api.h b/shell/platform/windows/client_wrapper/testing/stub_flutter_windows_api.h index a1864d4f07720..73e920d949e32 100644 --- a/shell/platform/windows/client_wrapper/testing/stub_flutter_windows_api.h +++ b/shell/platform/windows/client_wrapper/testing/stub_flutter_windows_api.h @@ -89,6 +89,17 @@ class StubFlutterWindowsApi { // FlutterDesktopPluginRegistrarUnregisterTopLevelWindowProcDelegate. virtual void PluginRegistrarUnregisterTopLevelWindowProcDelegate( FlutterDesktopWindowProcCallback delegate) {} + + // Claled for FlutterDesktopEngineProcessExternalWindowMessage. + virtual bool EngineProcessExternalWindowMessage( + FlutterDesktopEngineRef engine, + HWND hwnd, + UINT message, + WPARAM wparam, + LPARAM lparam, + LRESULT* result) { + return false; + } }; // A test helper that owns a stub implementation, making it the test stub for diff --git a/shell/platform/windows/flutter_window.cc b/shell/platform/windows/flutter_window.cc index 6dba8cadc368b..61e51afa67841 100644 --- a/shell/platform/windows/flutter_window.cc +++ b/shell/platform/windows/flutter_window.cc @@ -74,11 +74,19 @@ FlutterWindow::FlutterWindow(int width, int height) current_cursor_ = ::LoadCursor(nullptr, IDC_ARROW); } -FlutterWindow::~FlutterWindow() {} +FlutterWindow::~FlutterWindow() { + OnWindowStateEvent(WindowStateEvent::kHide); +} void FlutterWindow::SetView(WindowBindingHandlerDelegate* window) { binding_handler_delegate_ = window; direct_manipulation_owner_->SetBindingHandlerDelegate(window); + if (restored_) { + OnWindowStateEvent(WindowStateEvent::kShow); + } + if (focused_) { + OnWindowStateEvent(WindowStateEvent::kFocus); + } } WindowsRenderTarget FlutterWindow::GetRenderTarget() { @@ -328,4 +336,26 @@ bool FlutterWindow::NeedsVSync() { return true; } +void FlutterWindow::OnWindowStateEvent(WindowStateEvent event) { + switch (event) { + case WindowStateEvent::kShow: + restored_ = true; + break; + case WindowStateEvent::kHide: + restored_ = false; + focused_ = false; + break; + case WindowStateEvent::kFocus: + focused_ = true; + break; + case WindowStateEvent::kUnfocus: + focused_ = false; + break; + } + HWND hwnd = GetPlatformWindow(); + if (hwnd && binding_handler_delegate_) { + binding_handler_delegate_->OnWindowStateEvent(hwnd, event); + } +} + } // namespace flutter diff --git a/shell/platform/windows/flutter_window.h b/shell/platform/windows/flutter_window.h index 5725af5de1a6a..55d9db885a4d0 100644 --- a/shell/platform/windows/flutter_window.h +++ b/shell/platform/windows/flutter_window.h @@ -162,6 +162,9 @@ class FlutterWindow : public Window, public WindowBindingHandler { // |Window| ui::AXFragmentRootDelegateWin* GetAxFragmentRootDelegate() override; + // |Window| + void OnWindowStateEvent(WindowStateEvent event) override; + private: // A pointer to a FlutterWindowsView that can be used to update engine // windowing and input state. @@ -173,6 +176,12 @@ class FlutterWindow : public Window, public WindowBindingHandler { // The cursor rect set by Flutter. RECT cursor_rect_; + // The window receives resize and focus messages before its view is set, so + // these values cache the state of the window in the meantime so that the + // proper application lifecycle state can be updated once the view is set. + bool restored_ = false; + bool focused_ = false; + FML_DISALLOW_COPY_AND_ASSIGN(FlutterWindow); }; diff --git a/shell/platform/windows/flutter_window_unittests.cc b/shell/platform/windows/flutter_window_unittests.cc index e8ec2820666b1..8ba12aecd7cda 100644 --- a/shell/platform/windows/flutter_window_unittests.cc +++ b/shell/platform/windows/flutter_window_unittests.cc @@ -27,7 +27,7 @@ class MockFlutterWindow : public FlutterWindow { ON_CALL(*this, GetDpiScale()) .WillByDefault(Return(this->FlutterWindow::GetDpiScale())); } - virtual ~MockFlutterWindow() {} + virtual ~MockFlutterWindow() { SetView(nullptr); } // Wrapper for GetCurrentDPI() which is a protected method. UINT GetDpi() { return GetCurrentDPI(); } @@ -229,6 +229,10 @@ TEST(FlutterWindowTest, OnPointerStarSendsDeviceType) { kDefaultPointerDeviceId, WM_LBUTTONDOWN); win32window.OnPointerLeave(10.0, 10.0, kFlutterPointerDeviceKindStylus, kDefaultPointerDeviceId); + + // Destruction of win32window sends a HIDE update. In situ, the window is + // owned by the delegate, and so is destructed first. Not so here. + win32window.SetView(nullptr); } // Tests that calls to OnScroll in turn calls GetScrollOffsetMultiplier @@ -324,5 +328,61 @@ TEST(FlutterWindowTest, AlertNode) { EXPECT_EQ(role.lVal, ROLE_SYSTEM_ALERT); } +TEST(FlutterWindowTest, LifecycleFocusMessages) { + MockFlutterWindow win32window; + ON_CALL(win32window, GetPlatformWindow).WillByDefault([]() { + return reinterpret_cast(1); + }); + MockWindowBindingHandlerDelegate delegate; + win32window.SetView(&delegate); + + WindowStateEvent last_event; + ON_CALL(delegate, OnWindowStateEvent) + .WillByDefault([&last_event](HWND hwnd, WindowStateEvent event) { + last_event = event; + }); + + win32window.InjectWindowMessage(WM_SIZE, 0, 0); + EXPECT_EQ(last_event, WindowStateEvent::kHide); + + win32window.InjectWindowMessage(WM_SIZE, 0, MAKEWORD(1, 1)); + EXPECT_EQ(last_event, WindowStateEvent::kShow); + + win32window.InjectWindowMessage(WM_SETFOCUS, 0, 0); + EXPECT_EQ(last_event, WindowStateEvent::kFocus); + + win32window.InjectWindowMessage(WM_KILLFOCUS, 0, 0); + EXPECT_EQ(last_event, WindowStateEvent::kUnfocus); +} + +TEST(FlutterWindowTest, CachedLifecycleMessage) { + MockFlutterWindow win32window; + ON_CALL(win32window, GetPlatformWindow).WillByDefault([]() { + return reinterpret_cast(1); + }); + + // Restore + win32window.InjectWindowMessage(WM_SIZE, 0, MAKEWORD(1, 1)); + + // Focus + win32window.InjectWindowMessage(WM_SETFOCUS, 0, 0); + + MockWindowBindingHandlerDelegate delegate; + bool focused = false; + bool restored = false; + ON_CALL(delegate, OnWindowStateEvent) + .WillByDefault([&](HWND hwnd, WindowStateEvent event) { + if (event == WindowStateEvent::kFocus) { + focused = true; + } else if (event == WindowStateEvent::kShow) { + restored = true; + } + }); + + win32window.SetView(&delegate); + EXPECT_TRUE(focused); + EXPECT_TRUE(restored); +} + } // namespace testing } // namespace flutter diff --git a/shell/platform/windows/flutter_windows.cc b/shell/platform/windows/flutter_windows.cc index 50bc538f1f8e0..fd3071d898505 100644 --- a/shell/platform/windows/flutter_windows.cc +++ b/shell/platform/windows/flutter_windows.cc @@ -207,6 +207,22 @@ IDXGIAdapter* FlutterDesktopViewGetGraphicsAdapter(FlutterDesktopViewRef view) { return nullptr; } +bool FlutterDesktopEngineProcessExternalWindowMessage( + FlutterDesktopEngineRef engine, + HWND hwnd, + UINT message, + WPARAM wparam, + LPARAM lparam, + LRESULT* result) { + std::optional lresult = + EngineFromHandle(engine)->ProcessExternalWindowMessage(hwnd, message, + wparam, lparam); + if (result && lresult.has_value()) { + *result = lresult.value(); + } + return lresult.has_value(); +} + FlutterDesktopViewRef FlutterDesktopPluginRegistrarGetView( FlutterDesktopPluginRegistrarRef registrar) { return HandleForView(registrar->engine->view()); diff --git a/shell/platform/windows/flutter_windows_engine.cc b/shell/platform/windows/flutter_windows_engine.cc index 68cccd27ab19f..8ab6b7d5fae74 100644 --- a/shell/platform/windows/flutter_windows_engine.cc +++ b/shell/platform/windows/flutter_windows_engine.cc @@ -584,10 +584,9 @@ void FlutterWindowsEngine::SetNextFrameCallback(fml::closure callback) { } void FlutterWindowsEngine::SetLifecycleState(flutter::AppLifecycleState state) { - const char* state_name = flutter::AppLifecycleStateToString(state); - SendPlatformMessage("flutter/lifecycle", - reinterpret_cast(state_name), - strlen(state_name), nullptr, nullptr); + if (lifecycle_manager_) { + lifecycle_manager_->SetLifecycleState(state); + } } void FlutterWindowsEngine::SendSystemLocales() { @@ -796,4 +795,21 @@ void FlutterWindowsEngine::OnApplicationLifecycleEnabled() { lifecycle_manager_->BeginProcessingClose(); } +void FlutterWindowsEngine::OnWindowStateEvent(HWND hwnd, + WindowStateEvent event) { + lifecycle_manager_->OnWindowStateEvent(hwnd, event); +} + +std::optional FlutterWindowsEngine::ProcessExternalWindowMessage( + HWND hwnd, + UINT message, + WPARAM wparam, + LPARAM lparam) { + if (lifecycle_manager_) { + return lifecycle_manager_->ExternalWindowMessage(hwnd, message, wparam, + lparam); + } + return std::nullopt; +} + } // namespace flutter diff --git a/shell/platform/windows/flutter_windows_engine.h b/shell/platform/windows/flutter_windows_engine.h index 3e5a7343287b6..36147c00c821c 100644 --- a/shell/platform/windows/flutter_windows_engine.h +++ b/shell/platform/windows/flutter_windows_engine.h @@ -262,6 +262,22 @@ class FlutterWindowsEngine { // Registers the top level handler for the WM_CLOSE window message. void OnApplicationLifecycleEnabled(); + // Called when a Window receives an event that may alter the application + // lifecycle state. + void OnWindowStateEvent(HWND hwnd, WindowStateEvent event); + + // Handle a message from a non-Flutter window in the same application. + // Returns a result when the message is consumed and should not be processed + // further. + std::optional ProcessExternalWindowMessage(HWND hwnd, + UINT message, + WPARAM wparam, + LPARAM lparam); + + WindowsLifecycleManager* lifecycle_manager() { + return lifecycle_manager_.get(); + } + protected: // Creates the keyboard key handler. // diff --git a/shell/platform/windows/flutter_windows_engine_unittests.cc b/shell/platform/windows/flutter_windows_engine_unittests.cc index 80fb26f6cd3b3..8dcb50846f23c 100644 --- a/shell/platform/windows/flutter_windows_engine_unittests.cc +++ b/shell/platform/windows/flutter_windows_engine_unittests.cc @@ -608,6 +608,7 @@ class MockFlutterWindowsView : public FlutterWindowsView { MOCK_METHOD2(NotifyWinEventWrapper, void(ui::AXPlatformNodeWin*, ax::mojom::Event)); + MOCK_METHOD0(GetPlatformWindow, HWND()); private: FML_DISALLOW_COPY_AND_ASSIGN(MockFlutterWindowsView); @@ -668,6 +669,7 @@ class MockWindowsLifecycleManager : public WindowsLifecycleManager { UINT)); MOCK_METHOD4(DispatchMessage, void(HWND, UINT, WPARAM, LPARAM)); MOCK_METHOD0(IsLastWindowOfProcess, bool(void)); + MOCK_METHOD1(SetLifecycleState, void(AppLifecycleState)); }; TEST_F(FlutterWindowsEngineTest, TestExit) { @@ -895,5 +897,110 @@ TEST_F(FlutterWindowsEngineTest, EnableApplicationLifecycle) { 0); } +TEST_F(FlutterWindowsEngineTest, AppStartsInResumedState) { + FlutterWindowsEngineBuilder builder{GetContext()}; + + auto window_binding_handler = + std::make_unique<::testing::NiceMock>(); + MockFlutterWindowsView view(std::move(window_binding_handler)); + view.SetEngine(builder.Build()); + FlutterWindowsEngine* engine = view.GetEngine(); + + EngineModifier modifier(engine); + modifier.embedder_api().RunsAOTCompiledDartCode = []() { return false; }; + auto handler = std::make_unique(engine); + EXPECT_CALL(*handler, SetLifecycleState(AppLifecycleState::kResumed)) + .Times(1); + modifier.SetLifecycleManager(std::move(handler)); + engine->Run(); +} + +TEST_F(FlutterWindowsEngineTest, LifecycleStateTransition) { + FlutterWindowsEngineBuilder builder{GetContext()}; + + auto window_binding_handler = + std::make_unique<::testing::NiceMock>(); + MockFlutterWindowsView view(std::move(window_binding_handler)); + view.SetEngine(builder.Build()); + FlutterWindowsEngine* engine = view.GetEngine(); + + EngineModifier modifier(engine); + modifier.embedder_api().RunsAOTCompiledDartCode = []() { return false; }; + engine->Run(); + + engine->window_proc_delegate_manager()->OnTopLevelWindowProc( + (HWND)1, WM_SIZE, SIZE_RESTORED, 0); + EXPECT_EQ(engine->lifecycle_manager()->GetLifecycleState(), + AppLifecycleState::kResumed); + + engine->window_proc_delegate_manager()->OnTopLevelWindowProc( + (HWND)1, WM_SIZE, SIZE_MINIMIZED, 0); + EXPECT_EQ(engine->lifecycle_manager()->GetLifecycleState(), + AppLifecycleState::kHidden); + + engine->window_proc_delegate_manager()->OnTopLevelWindowProc( + (HWND)1, WM_SIZE, SIZE_RESTORED, 0); + EXPECT_EQ(engine->lifecycle_manager()->GetLifecycleState(), + AppLifecycleState::kInactive); +} + +TEST_F(FlutterWindowsEngineTest, ExternalWindowMessage) { + FlutterWindowsEngineBuilder builder{GetContext()}; + + auto window_binding_handler = + std::make_unique<::testing::NiceMock>(); + MockFlutterWindowsView view(std::move(window_binding_handler)); + view.SetEngine(builder.Build()); + FlutterWindowsEngine* engine = view.GetEngine(); + + EngineModifier modifier(engine); + modifier.embedder_api().RunsAOTCompiledDartCode = []() { return false; }; + // Sets lifecycle state to resumed. + engine->Run(); + + // Ensure HWND(1) is in the set of visible windows before hiding it. + engine->ProcessExternalWindowMessage(reinterpret_cast(1), WM_SHOWWINDOW, + TRUE, NULL); + engine->ProcessExternalWindowMessage(reinterpret_cast(1), WM_SHOWWINDOW, + FALSE, NULL); + + EXPECT_EQ(engine->lifecycle_manager()->GetLifecycleState(), + AppLifecycleState::kHidden); +} + +TEST_F(FlutterWindowsEngineTest, InnerWindowHidden) { + FlutterWindowsEngineBuilder builder{GetContext()}; + HWND outer = reinterpret_cast(1); + HWND inner = reinterpret_cast(2); + + auto window_binding_handler = + std::make_unique<::testing::NiceMock>(); + MockFlutterWindowsView view(std::move(window_binding_handler)); + ON_CALL(view, GetPlatformWindow).WillByDefault([=]() { return inner; }); + view.SetEngine(builder.Build()); + FlutterWindowsEngine* engine = view.GetEngine(); + + EngineModifier modifier(engine); + modifier.embedder_api().RunsAOTCompiledDartCode = []() { return false; }; + // Sets lifecycle state to resumed. + engine->Run(); + + // Show both top-level and Flutter window. + engine->window_proc_delegate_manager()->OnTopLevelWindowProc( + outer, WM_SHOWWINDOW, TRUE, NULL); + view.OnWindowStateEvent(inner, WindowStateEvent::kShow); + view.OnWindowStateEvent(inner, WindowStateEvent::kFocus); + + EXPECT_EQ(engine->lifecycle_manager()->GetLifecycleState(), + AppLifecycleState::kResumed); + + // Hide Flutter window, but not top level window. + view.OnWindowStateEvent(inner, WindowStateEvent::kHide); + + // The top-level window is still visible, so we ought not enter hidden state. + EXPECT_EQ(engine->lifecycle_manager()->GetLifecycleState(), + AppLifecycleState::kInactive); +} + } // namespace testing } // namespace flutter diff --git a/shell/platform/windows/flutter_windows_view.cc b/shell/platform/windows/flutter_windows_view.cc index 131cd2a52a82c..d668d2f447b6c 100644 --- a/shell/platform/windows/flutter_windows_view.cc +++ b/shell/platform/windows/flutter_windows_view.cc @@ -673,4 +673,10 @@ void FlutterWindowsView::OnDwmCompositionChanged() { } } +void FlutterWindowsView::OnWindowStateEvent(HWND hwnd, WindowStateEvent event) { + if (engine_) { + engine_->OnWindowStateEvent(hwnd, event); + } +} + } // namespace flutter diff --git a/shell/platform/windows/flutter_windows_view.h b/shell/platform/windows/flutter_windows_view.h index 8dc31fafee39c..792db3f5b9f95 100644 --- a/shell/platform/windows/flutter_windows_view.h +++ b/shell/platform/windows/flutter_windows_view.h @@ -211,6 +211,9 @@ class FlutterWindowsView : public WindowBindingHandlerDelegate, return accessibility_bridge_; } + // |WindowBindingHandlerDelegate| + void OnWindowStateEvent(HWND hwnd, WindowStateEvent event) override; + protected: virtual void NotifyWinEventWrapper(ui::AXPlatformNodeWin* node, ax::mojom::Event event); diff --git a/shell/platform/windows/public/flutter_windows.h b/shell/platform/windows/public/flutter_windows.h index 767c3468d45d3..beee0f029f4ff 100644 --- a/shell/platform/windows/public/flutter_windows.h +++ b/shell/platform/windows/public/flutter_windows.h @@ -212,6 +212,19 @@ FLUTTER_EXPORT HWND FlutterDesktopViewGetHWND(FlutterDesktopViewRef view); FLUTTER_EXPORT IDXGIAdapter* FlutterDesktopViewGetGraphicsAdapter( FlutterDesktopViewRef view); +// Called to pass an external window message to the engine for lifecycle +// state updates. This does not consume the window message. Non-Flutter windows +// must call this method in their WndProc in order to be included in the logic +// for application lifecycle state updates. Returns true when the message is +// consumed. +FLUTTER_EXPORT bool FlutterDesktopEngineProcessExternalWindowMessage( + FlutterDesktopEngineRef engine, + HWND hwnd, + UINT message, + WPARAM wparam, + LPARAM lparam, + LRESULT* result); + // ========== Plugin Registrar (extensions) ========== // These are Windows-specific extensions to flutter_plugin_registrar.h diff --git a/shell/platform/windows/testing/mock_window.h b/shell/platform/windows/testing/mock_window.h index 53567b723dafc..4294476d91750 100644 --- a/shell/platform/windows/testing/mock_window.h +++ b/shell/platform/windows/testing/mock_window.h @@ -66,6 +66,8 @@ class MockWindow : public Window { MOCK_METHOD3(OnGetObject, LRESULT(UINT, WPARAM, LPARAM)); + MOCK_METHOD1(OnWindowStateEvent, void(WindowStateEvent)); + void CallOnImeComposition(UINT const message, WPARAM const wparam, LPARAM const lparam); diff --git a/shell/platform/windows/testing/mock_window_binding_handler_delegate.h b/shell/platform/windows/testing/mock_window_binding_handler_delegate.h index 35147d4e09200..fb58b354cb72d 100644 --- a/shell/platform/windows/testing/mock_window_binding_handler_delegate.h +++ b/shell/platform/windows/testing/mock_window_binding_handler_delegate.h @@ -61,6 +61,8 @@ class MockWindowBindingHandlerDelegate : public WindowBindingHandlerDelegate { MOCK_METHOD0(GetAxFragmentRootDelegate, ui::AXFragmentRootDelegateWin*()); + MOCK_METHOD2(OnWindowStateEvent, void(HWND, WindowStateEvent)); + private: FML_DISALLOW_COPY_AND_ASSIGN(MockWindowBindingHandlerDelegate); }; diff --git a/shell/platform/windows/window.cc b/shell/platform/windows/window.cc index ceb9cb8379ff9..a4cc4efec6754 100644 --- a/shell/platform/windows/window.cc +++ b/shell/platform/windows/window.cc @@ -352,6 +352,9 @@ Window::HandleMessage(UINT const message, current_width_ = width; current_height_ = height; HandleResize(width, height); + + OnWindowStateEvent(width == 0 && height == 0 ? WindowStateEvent::kHide + : WindowStateEvent::kShow); break; case WM_PAINT: OnPaint(); @@ -430,9 +433,11 @@ Window::HandleMessage(UINT const message, break; } case WM_SETFOCUS: + OnWindowStateEvent(WindowStateEvent::kFocus); ::CreateCaret(window_handle_, nullptr, 1, 1); break; case WM_KILLFOCUS: + OnWindowStateEvent(WindowStateEvent::kUnfocus); ::DestroyCaret(); break; case WM_LBUTTONDOWN: diff --git a/shell/platform/windows/window.h b/shell/platform/windows/window.h index 632e140979575..b3605cd7b19d5 100644 --- a/shell/platform/windows/window.h +++ b/shell/platform/windows/window.h @@ -18,6 +18,7 @@ #include "flutter/shell/platform/windows/keyboard_manager.h" #include "flutter/shell/platform/windows/sequential_id_generator.h" #include "flutter/shell/platform/windows/text_input_manager.h" +#include "flutter/shell/platform/windows/windows_lifecycle_manager.h" #include "flutter/shell/platform/windows/windows_proc_table.h" #include "flutter/shell/platform/windows/windowsx_shim.h" #include "flutter/third_party/accessibility/ax/platform/ax_fragment_root_delegate_win.h" @@ -223,6 +224,9 @@ class Window : public KeyboardManager::WindowDelegate { // Called to obtain a pointer to the fragment root delegate. virtual ui::AXFragmentRootDelegateWin* GetAxFragmentRootDelegate() = 0; + // Called on a resize or focus event. + virtual void OnWindowStateEvent(WindowStateEvent event) = 0; + protected: // Win32's DefWindowProc. // diff --git a/shell/platform/windows/window_binding_handler_delegate.h b/shell/platform/windows/window_binding_handler_delegate.h index 1812107f2dfa9..27c5687adf168 100644 --- a/shell/platform/windows/window_binding_handler_delegate.h +++ b/shell/platform/windows/window_binding_handler_delegate.h @@ -9,6 +9,7 @@ #include "flutter/shell/platform/common/geometry.h" #include "flutter/shell/platform/embedder/embedder.h" +#include "flutter/shell/platform/windows/windows_lifecycle_manager.h" #include "flutter/third_party/accessibility/ax/platform/ax_fragment_root_delegate_win.h" #include "flutter/third_party/accessibility/gfx/native_widget_types.h" @@ -144,6 +145,10 @@ class WindowBindingHandlerDelegate { // MSAA, UIA elements do not explicitly store or enumerate their // children and parents, so a method such as this is required. virtual ui::AXFragmentRootDelegateWin* GetAxFragmentRootDelegate() = 0; + + // Called when a window receives an event that may alter application lifecycle + // state. + virtual void OnWindowStateEvent(HWND hwnd, WindowStateEvent event) = 0; }; } // namespace flutter diff --git a/shell/platform/windows/windows_lifecycle_manager.cc b/shell/platform/windows/windows_lifecycle_manager.cc index f8bf5de37f7bf..dbd5e71658a2e 100644 --- a/shell/platform/windows/windows_lifecycle_manager.cc +++ b/shell/platform/windows/windows_lifecycle_manager.cc @@ -75,6 +75,26 @@ bool WindowsLifecycleManager::WindowProc(HWND hwnd, case WM_DWMCOMPOSITIONCHANGED: engine_->OnDwmCompositionChanged(); break; + + case WM_SIZE: + if (wpar == SIZE_MAXIMIZED || wpar == SIZE_RESTORED) { + OnWindowStateEvent(hwnd, WindowStateEvent::kShow); + } else if (wpar == SIZE_MINIMIZED) { + OnWindowStateEvent(hwnd, WindowStateEvent::kHide); + } + break; + + case WM_SHOWWINDOW: + if (!wpar) { + OnWindowStateEvent(hwnd, WindowStateEvent::kHide); + } else { + OnWindowStateEvent(hwnd, WindowStateEvent::kShow); + } + break; + + case WM_DESTROY: + OnWindowStateEvent(hwnd, WindowStateEvent::kHide); + break; } return false; } @@ -163,4 +183,108 @@ void WindowsLifecycleManager::BeginProcessingClose() { process_close_ = true; } +void WindowsLifecycleManager::SetLifecycleState(AppLifecycleState state) { + if (state_ == state) { + return; + } + state_ = state; + if (engine_) { + const char* state_name = AppLifecycleStateToString(state); + engine_->SendPlatformMessage("flutter/lifecycle", + reinterpret_cast(state_name), + strlen(state_name), nullptr, nullptr); + } +} + +void WindowsLifecycleManager::OnWindowStateEvent(HWND hwnd, + WindowStateEvent event) { + // Synthesize an unfocus event when a focused window is hidden. + if (event == WindowStateEvent::kHide && + focused_windows_.find(hwnd) != focused_windows_.end()) { + OnWindowStateEvent(hwnd, WindowStateEvent::kUnfocus); + } + + std::lock_guard guard(state_update_lock_); + switch (event) { + case WindowStateEvent::kShow: { + bool first_shown_window = visible_windows_.empty(); + auto pair = visible_windows_.insert(hwnd); + if (first_shown_window && pair.second && + state_ == AppLifecycleState::kHidden) { + SetLifecycleState(AppLifecycleState::kInactive); + } + break; + } + case WindowStateEvent::kHide: { + bool present = visible_windows_.erase(hwnd); + bool empty = visible_windows_.empty(); + if (present && empty && + (state_ == AppLifecycleState::kResumed || + state_ == AppLifecycleState::kInactive)) { + SetLifecycleState(AppLifecycleState::kHidden); + } + break; + } + case WindowStateEvent::kFocus: { + bool first_focused_window = focused_windows_.empty(); + auto pair = focused_windows_.insert(hwnd); + if (first_focused_window && pair.second && + state_ == AppLifecycleState::kInactive) { + SetLifecycleState(AppLifecycleState::kResumed); + } + break; + } + case WindowStateEvent::kUnfocus: { + if (focused_windows_.erase(hwnd) && focused_windows_.empty() && + state_ == AppLifecycleState::kResumed) { + SetLifecycleState(AppLifecycleState::kInactive); + } + break; + } + } +} + +std::optional WindowsLifecycleManager::ExternalWindowMessage( + HWND hwnd, + UINT message, + WPARAM wparam, + LPARAM lparam) { + std::optional event = std::nullopt; + + // TODO (schectman): Handle WM_CLOSE messages. + // https://github.com/flutter/flutter/issues/131497 + switch (message) { + case WM_SHOWWINDOW: + event = wparam ? flutter::WindowStateEvent::kShow + : flutter::WindowStateEvent::kHide; + break; + case WM_SIZE: + switch (wparam) { + case SIZE_MINIMIZED: + event = flutter::WindowStateEvent::kHide; + break; + case SIZE_RESTORED: + case SIZE_MAXIMIZED: + event = flutter::WindowStateEvent::kShow; + break; + } + break; + case WM_SETFOCUS: + event = flutter::WindowStateEvent::kFocus; + break; + case WM_KILLFOCUS: + event = flutter::WindowStateEvent::kUnfocus; + break; + case WM_DESTROY: + event = flutter::WindowStateEvent::kHide; + break; + } + + if (event.has_value()) { + OnWindowStateEvent(hwnd, *event); + } + + return std::nullopt; +} + } // namespace flutter diff --git a/shell/platform/windows/windows_lifecycle_manager.h b/shell/platform/windows/windows_lifecycle_manager.h index b08ec57a4dbd8..2a2ca7a3160c1 100644 --- a/shell/platform/windows/windows_lifecycle_manager.h +++ b/shell/platform/windows/windows_lifecycle_manager.h @@ -9,17 +9,31 @@ #include #include +#include #include +#include + +#include "flutter/shell/platform/common/app_lifecycle_state.h" namespace flutter { class FlutterWindowsEngine; -/// A manager for lifecycle events of the top-level window. +/// An event representing a change in window state that may update the +// application lifecycle state. +enum class WindowStateEvent { + kShow, + kHide, + kFocus, + kUnfocus, +}; + +/// A manager for lifecycle events of the top-level windows. /// -/// Currently handles the following events: -/// 1. WM_CLOSE -/// 2. WM_DWMCOMPOSITIONCHANGED +/// WndProc is called for window messages of the top-level Flutter window. +/// ExternalWindowMessage is called for non-flutter top-level window messages. +/// OnWindowStateEvent is called when the visibility or focus state of a window +/// is changed, including the FlutterView window. class WindowsLifecycleManager { public: WindowsLifecycleManager(FlutterWindowsEngine* engine); @@ -34,12 +48,43 @@ class WindowsLifecycleManager { std::optional lparam, UINT exit_code); - // Intercept top level window messages, only paying attention to WM_CLOSE. + // Intercept top level window WM_CLOSE message and listen to events that may + // update the application lifecycle. bool WindowProc(HWND hwnd, UINT msg, WPARAM w, LPARAM l, LRESULT* result); // Signal to start consuming WM_CLOSE messages. void BeginProcessingClose(); + // Update the app lifecycle state in response to a change in window state. + // When the app lifecycle state actually changes, this sends a platform + // message to the framework notifying it of the state change. + virtual void SetLifecycleState(AppLifecycleState state); + + // Respond to a change in window state. Transitions as follows: + // When the only visible window is hidden, transition from resumed or + // inactive to hidden. + // When the only focused window is unfocused, transition from resumed to + // inactive. + // When a window is focused, transition from inactive to resumed. + // When a window is shown, transition from hidden to inactive. + virtual void OnWindowStateEvent(HWND hwnd, WindowStateEvent event); + + AppLifecycleState GetLifecycleState() { return state_; } + + // Called by the engine when a non-Flutter window receives an event that may + // alter the lifecycle state. The logic for external windows must differ from + // that used for FlutterWindow instances, because: + // - FlutterWindow does not receive WM_SHOW messages, + // - When FlutterWindow receives WM_SIZE messages, wparam stores no meaningful + // information, whereas it usually indicates the action which changed the + // window size. + // When this returns a result, the message has been consumed and should not be + // processed further. Currently, it will always return nullopt. + std::optional ExternalWindowMessage(HWND hwnd, + UINT message, + WPARAM wparam, + LPARAM lparam); + protected: // Check the number of top-level windows associated with this process, and // return true only if there are 1 or fewer. @@ -56,6 +101,14 @@ class WindowsLifecycleManager { std::map, int> sent_close_messages_; bool process_close_; + + std::set visible_windows_; + + std::set focused_windows_; + + std::mutex state_update_lock_; + + flutter::AppLifecycleState state_; }; } // namespace flutter diff --git a/shell/platform/windows/windows_lifecycle_manager_unittests.cc b/shell/platform/windows/windows_lifecycle_manager_unittests.cc new file mode 100644 index 0000000000000..6414d0ade78a0 --- /dev/null +++ b/shell/platform/windows/windows_lifecycle_manager_unittests.cc @@ -0,0 +1,61 @@ +// 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. + +#include "flutter/shell/platform/windows/windows_lifecycle_manager.h" + +#include "flutter/shell/platform/windows/testing/windows_test.h" +#include "gtest/gtest.h" + +namespace flutter { +namespace testing { + +class WindowsLifecycleManagerTest : public WindowsTest {}; + +TEST_F(WindowsLifecycleManagerTest, StateTransitions) { + WindowsLifecycleManager manager(nullptr); + HWND win1 = reinterpret_cast(1); + HWND win2 = reinterpret_cast(2); + + // Hidden to inactive upon window shown. + manager.SetLifecycleState(AppLifecycleState::kHidden); + manager.OnWindowStateEvent(win1, WindowStateEvent::kShow); + EXPECT_EQ(manager.GetLifecycleState(), AppLifecycleState::kInactive); + + // Showing a second window does not change state. + manager.OnWindowStateEvent(win2, WindowStateEvent::kShow); + EXPECT_EQ(manager.GetLifecycleState(), AppLifecycleState::kInactive); + + // Inactive to resumed upon window focus. + manager.OnWindowStateEvent(win2, WindowStateEvent::kFocus); + EXPECT_EQ(manager.GetLifecycleState(), AppLifecycleState::kResumed); + + // Showing a second window does not change state. + manager.OnWindowStateEvent(win1, WindowStateEvent::kFocus); + EXPECT_EQ(manager.GetLifecycleState(), AppLifecycleState::kResumed); + + // Unfocusing one window does not change state while another is focused. + manager.OnWindowStateEvent(win1, WindowStateEvent::kUnfocus); + EXPECT_EQ(manager.GetLifecycleState(), AppLifecycleState::kResumed); + + // Unfocusing final remaining focused window transitions to inactive. + manager.OnWindowStateEvent(win2, WindowStateEvent::kUnfocus); + EXPECT_EQ(manager.GetLifecycleState(), AppLifecycleState::kInactive); + + // Hiding one of two visible windows does not change state. + manager.OnWindowStateEvent(win2, WindowStateEvent::kHide); + EXPECT_EQ(manager.GetLifecycleState(), AppLifecycleState::kInactive); + + // Hiding only visible window transitions to hidden. + manager.OnWindowStateEvent(win1, WindowStateEvent::kHide); + EXPECT_EQ(manager.GetLifecycleState(), AppLifecycleState::kHidden); + + // Transition directly from resumed to hidden when the window is hidden. + manager.OnWindowStateEvent(win1, WindowStateEvent::kShow); + manager.OnWindowStateEvent(win1, WindowStateEvent::kFocus); + manager.OnWindowStateEvent(win1, WindowStateEvent::kHide); + EXPECT_EQ(manager.GetLifecycleState(), AppLifecycleState::kHidden); +} + +} // namespace testing +} // namespace flutter From 7584237ab9c16b7aef8c54f56d40ebfe3df8d1f2 Mon Sep 17 00:00:00 2001 From: schectman Date: Thu, 3 Aug 2023 10:45:04 -0400 Subject: [PATCH 2/8] Check for posthumous --- shell/platform/windows/flutter_window.cc | 1 + .../windows/flutter_window_unittests.cc | 21 +++++++++++++++++++ shell/platform/windows/window.cc | 14 +++++++++---- shell/platform/windows/window.h | 3 +++ 4 files changed, 35 insertions(+), 4 deletions(-) diff --git a/shell/platform/windows/flutter_window.cc b/shell/platform/windows/flutter_window.cc index 61e51afa67841..d71d36230f56d 100644 --- a/shell/platform/windows/flutter_window.cc +++ b/shell/platform/windows/flutter_window.cc @@ -76,6 +76,7 @@ FlutterWindow::FlutterWindow(int width, int height) FlutterWindow::~FlutterWindow() { OnWindowStateEvent(WindowStateEvent::kHide); + vtable_is_alive = false; } void FlutterWindow::SetView(WindowBindingHandlerDelegate* window) { diff --git a/shell/platform/windows/flutter_window_unittests.cc b/shell/platform/windows/flutter_window_unittests.cc index 8ba12aecd7cda..44b27ca5cd45b 100644 --- a/shell/platform/windows/flutter_window_unittests.cc +++ b/shell/platform/windows/flutter_window_unittests.cc @@ -384,5 +384,26 @@ TEST(FlutterWindowTest, CachedLifecycleMessage) { EXPECT_TRUE(restored); } +TEST(FlutterWindowTest, PosthumousWindowMessage) { + MockWindowBindingHandlerDelegate delegate; + bool expect_messages = true; + ON_CALL(delegate, OnWindowStateEvent).WillByDefault([&](HWND hwnd, WindowStateEvent event) { + FML_LOG(ERROR) << "Got event " << static_cast(event); + EXPECT_TRUE(expect_messages); + }); + + { + MockFlutterWindow win32window; + ON_CALL(win32window, GetPlatformWindow).WillByDefault([&]() { + return win32window.FlutterWindow::GetPlatformWindow(); + }); + win32window.SetView(&delegate); + win32window.InitializeChild("Title", 1, 1); + HWND hwnd = win32window.GetPlatformWindow(); + SendMessage(hwnd, WM_SIZE, 0, MAKEWORD(1, 1)); + SendMessage(hwnd, WM_SETFOCUS, 0, 0); + } +} + } // namespace testing } // namespace flutter diff --git a/shell/platform/windows/window.cc b/shell/platform/windows/window.cc index a4cc4efec6754..8acf27bdf12c7 100644 --- a/shell/platform/windows/window.cc +++ b/shell/platform/windows/window.cc @@ -353,8 +353,10 @@ Window::HandleMessage(UINT const message, current_height_ = height; HandleResize(width, height); - OnWindowStateEvent(width == 0 && height == 0 ? WindowStateEvent::kHide - : WindowStateEvent::kShow); + if (vtable_is_alive) { + OnWindowStateEvent(width == 0 && height == 0 ? WindowStateEvent::kHide + : WindowStateEvent::kShow); + } break; case WM_PAINT: OnPaint(); @@ -433,11 +435,15 @@ Window::HandleMessage(UINT const message, break; } case WM_SETFOCUS: - OnWindowStateEvent(WindowStateEvent::kFocus); + if (vtable_is_alive) { + OnWindowStateEvent(WindowStateEvent::kFocus); + } ::CreateCaret(window_handle_, nullptr, 1, 1); break; case WM_KILLFOCUS: - OnWindowStateEvent(WindowStateEvent::kUnfocus); + if (vtable_is_alive) { + OnWindowStateEvent(WindowStateEvent::kUnfocus); + } ::DestroyCaret(); break; case WM_LBUTTONDOWN: diff --git a/shell/platform/windows/window.h b/shell/platform/windows/window.h index b3605cd7b19d5..2d9801cb28657 100644 --- a/shell/platform/windows/window.h +++ b/shell/platform/windows/window.h @@ -253,6 +253,9 @@ class Window : public KeyboardManager::WindowDelegate { // Accessibility node that represents an alert. std::unique_ptr alert_node_; + // Guard against posthumous vtable access; + bool vtable_is_alive = true; + private: // Release OS resources associated with window. void Destroy(); From 0631b8a3a545a7fa176ef17ed4427aec56e9878f Mon Sep 17 00:00:00 2001 From: schectman Date: Thu, 3 Aug 2023 10:45:54 -0400 Subject: [PATCH 3/8] Format --- shell/platform/windows/flutter_window_unittests.cc | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/shell/platform/windows/flutter_window_unittests.cc b/shell/platform/windows/flutter_window_unittests.cc index 44b27ca5cd45b..e75a1cbf52780 100644 --- a/shell/platform/windows/flutter_window_unittests.cc +++ b/shell/platform/windows/flutter_window_unittests.cc @@ -387,10 +387,11 @@ TEST(FlutterWindowTest, CachedLifecycleMessage) { TEST(FlutterWindowTest, PosthumousWindowMessage) { MockWindowBindingHandlerDelegate delegate; bool expect_messages = true; - ON_CALL(delegate, OnWindowStateEvent).WillByDefault([&](HWND hwnd, WindowStateEvent event) { - FML_LOG(ERROR) << "Got event " << static_cast(event); - EXPECT_TRUE(expect_messages); - }); + ON_CALL(delegate, OnWindowStateEvent) + .WillByDefault([&](HWND hwnd, WindowStateEvent event) { + FML_LOG(ERROR) << "Got event " << static_cast(event); + EXPECT_TRUE(expect_messages); + }); { MockFlutterWindow win32window; From d334d04a11b600791dfb9e446d72e46b30f117a0 Mon Sep 17 00:00:00 2001 From: schectman Date: Tue, 8 Aug 2023 14:31:24 -0400 Subject: [PATCH 4/8] Move Destroy --- shell/platform/windows/flutter_window.cc | 6 +-- shell/platform/windows/flutter_window.h | 2 +- .../windows/flutter_window_unittests.cc | 41 ++++++++++++++----- shell/platform/windows/window.cc | 18 +++----- shell/platform/windows/window.h | 9 ++-- 5 files changed, 43 insertions(+), 33 deletions(-) diff --git a/shell/platform/windows/flutter_window.cc b/shell/platform/windows/flutter_window.cc index d71d36230f56d..b8060a7a92dbb 100644 --- a/shell/platform/windows/flutter_window.cc +++ b/shell/platform/windows/flutter_window.cc @@ -76,16 +76,16 @@ FlutterWindow::FlutterWindow(int width, int height) FlutterWindow::~FlutterWindow() { OnWindowStateEvent(WindowStateEvent::kHide); - vtable_is_alive = false; + Destroy(); } void FlutterWindow::SetView(WindowBindingHandlerDelegate* window) { binding_handler_delegate_ = window; direct_manipulation_owner_->SetBindingHandlerDelegate(window); - if (restored_) { + if (restored_ && window) { OnWindowStateEvent(WindowStateEvent::kShow); } - if (focused_) { + if (focused_ && window) { OnWindowStateEvent(WindowStateEvent::kFocus); } } diff --git a/shell/platform/windows/flutter_window.h b/shell/platform/windows/flutter_window.h index 55d9db885a4d0..2b242315acc89 100644 --- a/shell/platform/windows/flutter_window.h +++ b/shell/platform/windows/flutter_window.h @@ -163,7 +163,7 @@ class FlutterWindow : public Window, public WindowBindingHandler { ui::AXFragmentRootDelegateWin* GetAxFragmentRootDelegate() override; // |Window| - void OnWindowStateEvent(WindowStateEvent event) override; + virtual void OnWindowStateEvent(WindowStateEvent event) override; private: // A pointer to a FlutterWindowsView that can be used to update engine diff --git a/shell/platform/windows/flutter_window_unittests.cc b/shell/platform/windows/flutter_window_unittests.cc index e75a1cbf52780..9daa9dc67cd5f 100644 --- a/shell/platform/windows/flutter_window_unittests.cc +++ b/shell/platform/windows/flutter_window_unittests.cc @@ -23,11 +23,15 @@ static constexpr int32_t kDefaultPointerDeviceId = 0; class MockFlutterWindow : public FlutterWindow { public: - MockFlutterWindow() : FlutterWindow(800, 600) { + MockFlutterWindow(bool reset_view_on_exit = true) : FlutterWindow(800, 600), reset_view_on_exit_(reset_view_on_exit) { ON_CALL(*this, GetDpiScale()) .WillByDefault(Return(this->FlutterWindow::GetDpiScale())); } - virtual ~MockFlutterWindow() { SetView(nullptr); } + virtual ~MockFlutterWindow() { + if (reset_view_on_exit_) { + SetView(nullptr); + } + } // Wrapper for GetCurrentDPI() which is a protected method. UINT GetDpi() { return GetCurrentDPI(); } @@ -61,6 +65,7 @@ class MockFlutterWindow : public FlutterWindow { MOCK_METHOD1(Win32MapVkToChar, uint32_t(uint32_t)); MOCK_METHOD0(GetPlatformWindow, HWND()); MOCK_METHOD0(GetAxFragmentRootDelegate, ui::AXFragmentRootDelegateWin*()); + MOCK_METHOD1(OnWindowStateEvent, void(WindowStateEvent)); protected: // |KeyboardManager::WindowDelegate| @@ -72,6 +77,7 @@ class MockFlutterWindow : public FlutterWindow { } private: + bool reset_view_on_exit_; FML_DISALLOW_COPY_AND_ASSIGN(MockFlutterWindow); }; @@ -341,6 +347,9 @@ TEST(FlutterWindowTest, LifecycleFocusMessages) { .WillByDefault([&last_event](HWND hwnd, WindowStateEvent event) { last_event = event; }); + ON_CALL(win32window, OnWindowStateEvent).WillByDefault([&](WindowStateEvent event) { + win32window.FlutterWindow::OnWindowStateEvent(event); + }); win32window.InjectWindowMessage(WM_SIZE, 0, 0); EXPECT_EQ(last_event, WindowStateEvent::kHide); @@ -360,6 +369,9 @@ TEST(FlutterWindowTest, CachedLifecycleMessage) { ON_CALL(win32window, GetPlatformWindow).WillByDefault([]() { return reinterpret_cast(1); }); + ON_CALL(win32window, OnWindowStateEvent).WillByDefault([&](WindowStateEvent event) { + win32window.FlutterWindow::OnWindowStateEvent(event); + }); // Restore win32window.InjectWindowMessage(WM_SIZE, 0, MAKEWORD(1, 1)); @@ -386,24 +398,33 @@ TEST(FlutterWindowTest, CachedLifecycleMessage) { TEST(FlutterWindowTest, PosthumousWindowMessage) { MockWindowBindingHandlerDelegate delegate; - bool expect_messages = true; - ON_CALL(delegate, OnWindowStateEvent) - .WillByDefault([&](HWND hwnd, WindowStateEvent event) { - FML_LOG(ERROR) << "Got event " << static_cast(event); - EXPECT_TRUE(expect_messages); - }); + int msg_count = 0; + HWND hwnd; + ON_CALL(delegate, OnWindowStateEvent).WillByDefault([&](HWND hwnd, WindowStateEvent event) { + msg_count++; + }); { - MockFlutterWindow win32window; + MockFlutterWindow win32window(false); ON_CALL(win32window, GetPlatformWindow).WillByDefault([&]() { return win32window.FlutterWindow::GetPlatformWindow(); }); + ON_CALL(win32window, OnWindowStateEvent).WillByDefault([&](WindowStateEvent event) { + win32window.FlutterWindow::OnWindowStateEvent(event); + }); win32window.SetView(&delegate); win32window.InitializeChild("Title", 1, 1); - HWND hwnd = win32window.GetPlatformWindow(); + hwnd = win32window.GetPlatformWindow(); SendMessage(hwnd, WM_SIZE, 0, MAKEWORD(1, 1)); SendMessage(hwnd, WM_SETFOCUS, 0, 0); + + // By setting this to zero before exiting the scope that contains + // win32window, and then checking its value afterwards, enforce that the + // window receive and process messages from its destructor without + // accessing out-of-bounds memory. + msg_count = 0; } + EXPECT_GE(msg_count, 1); } } // namespace testing diff --git a/shell/platform/windows/window.cc b/shell/platform/windows/window.cc index 8acf27bdf12c7..646a2ec9c84a6 100644 --- a/shell/platform/windows/window.cc +++ b/shell/platform/windows/window.cc @@ -81,9 +81,7 @@ Window::Window(std::unique_ptr windows_proc_table, keyboard_manager_ = std::make_unique(this); } -Window::~Window() { - Destroy(); -} +Window::~Window() {} void Window::InitializeChild(const char* title, unsigned int width, @@ -353,10 +351,8 @@ Window::HandleMessage(UINT const message, current_height_ = height; HandleResize(width, height); - if (vtable_is_alive) { - OnWindowStateEvent(width == 0 && height == 0 ? WindowStateEvent::kHide - : WindowStateEvent::kShow); - } + OnWindowStateEvent(width == 0 && height == 0 ? WindowStateEvent::kHide + : WindowStateEvent::kShow); break; case WM_PAINT: OnPaint(); @@ -435,15 +431,11 @@ Window::HandleMessage(UINT const message, break; } case WM_SETFOCUS: - if (vtable_is_alive) { - OnWindowStateEvent(WindowStateEvent::kFocus); - } + OnWindowStateEvent(WindowStateEvent::kFocus); ::CreateCaret(window_handle_, nullptr, 1, 1); break; case WM_KILLFOCUS: - if (vtable_is_alive) { - OnWindowStateEvent(WindowStateEvent::kUnfocus); - } + OnWindowStateEvent(WindowStateEvent::kUnfocus); ::DestroyCaret(); break; case WM_LBUTTONDOWN: diff --git a/shell/platform/windows/window.h b/shell/platform/windows/window.h index 2d9801cb28657..047370ee259f5 100644 --- a/shell/platform/windows/window.h +++ b/shell/platform/windows/window.h @@ -240,6 +240,9 @@ class Window : public KeyboardManager::WindowDelegate { // Returns the root view accessibility node, or nullptr if none. virtual gfx::NativeViewAccessible GetNativeViewAccessible() = 0; + // Release OS resources associated with window. + void Destroy(); + // Handles running DirectManipulation on the window to receive trackpad // gestures. std::unique_ptr direct_manipulation_owner_; @@ -253,13 +256,7 @@ class Window : public KeyboardManager::WindowDelegate { // Accessibility node that represents an alert. std::unique_ptr alert_node_; - // Guard against posthumous vtable access; - bool vtable_is_alive = true; - private: - // Release OS resources associated with window. - void Destroy(); - // Activates tracking for a "mouse leave" event. void TrackMouseLeaveEvent(HWND hwnd); From 4d445e2bce1b998aece5f6db67e40f6ec83e26d4 Mon Sep 17 00:00:00 2001 From: schectman Date: Tue, 8 Aug 2023 14:39:38 -0400 Subject: [PATCH 5/8] Format --- .../windows/flutter_window_unittests.cc | 29 ++++++++++--------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/shell/platform/windows/flutter_window_unittests.cc b/shell/platform/windows/flutter_window_unittests.cc index 9daa9dc67cd5f..2c988d26e4763 100644 --- a/shell/platform/windows/flutter_window_unittests.cc +++ b/shell/platform/windows/flutter_window_unittests.cc @@ -23,7 +23,8 @@ static constexpr int32_t kDefaultPointerDeviceId = 0; class MockFlutterWindow : public FlutterWindow { public: - MockFlutterWindow(bool reset_view_on_exit = true) : FlutterWindow(800, 600), reset_view_on_exit_(reset_view_on_exit) { + MockFlutterWindow(bool reset_view_on_exit = true) + : FlutterWindow(800, 600), reset_view_on_exit_(reset_view_on_exit) { ON_CALL(*this, GetDpiScale()) .WillByDefault(Return(this->FlutterWindow::GetDpiScale())); } @@ -347,9 +348,10 @@ TEST(FlutterWindowTest, LifecycleFocusMessages) { .WillByDefault([&last_event](HWND hwnd, WindowStateEvent event) { last_event = event; }); - ON_CALL(win32window, OnWindowStateEvent).WillByDefault([&](WindowStateEvent event) { - win32window.FlutterWindow::OnWindowStateEvent(event); - }); + ON_CALL(win32window, OnWindowStateEvent) + .WillByDefault([&](WindowStateEvent event) { + win32window.FlutterWindow::OnWindowStateEvent(event); + }); win32window.InjectWindowMessage(WM_SIZE, 0, 0); EXPECT_EQ(last_event, WindowStateEvent::kHide); @@ -369,9 +371,10 @@ TEST(FlutterWindowTest, CachedLifecycleMessage) { ON_CALL(win32window, GetPlatformWindow).WillByDefault([]() { return reinterpret_cast(1); }); - ON_CALL(win32window, OnWindowStateEvent).WillByDefault([&](WindowStateEvent event) { - win32window.FlutterWindow::OnWindowStateEvent(event); - }); + ON_CALL(win32window, OnWindowStateEvent) + .WillByDefault([&](WindowStateEvent event) { + win32window.FlutterWindow::OnWindowStateEvent(event); + }); // Restore win32window.InjectWindowMessage(WM_SIZE, 0, MAKEWORD(1, 1)); @@ -400,18 +403,18 @@ TEST(FlutterWindowTest, PosthumousWindowMessage) { MockWindowBindingHandlerDelegate delegate; int msg_count = 0; HWND hwnd; - ON_CALL(delegate, OnWindowStateEvent).WillByDefault([&](HWND hwnd, WindowStateEvent event) { - msg_count++; - }); + ON_CALL(delegate, OnWindowStateEvent) + .WillByDefault([&](HWND hwnd, WindowStateEvent event) { msg_count++; }); { MockFlutterWindow win32window(false); ON_CALL(win32window, GetPlatformWindow).WillByDefault([&]() { return win32window.FlutterWindow::GetPlatformWindow(); }); - ON_CALL(win32window, OnWindowStateEvent).WillByDefault([&](WindowStateEvent event) { - win32window.FlutterWindow::OnWindowStateEvent(event); - }); + ON_CALL(win32window, OnWindowStateEvent) + .WillByDefault([&](WindowStateEvent event) { + win32window.FlutterWindow::OnWindowStateEvent(event); + }); win32window.SetView(&delegate); win32window.InitializeChild("Title", 1, 1); hwnd = win32window.GetPlatformWindow(); From 54b7629d09532ccc64f58af6a4743426d2393850 Mon Sep 17 00:00:00 2001 From: schectman Date: Wed, 9 Aug 2023 12:21:46 -0400 Subject: [PATCH 6/8] PR comments --- .../windows/client_wrapper/include/flutter/flutter_engine.h | 6 +++--- .../client_wrapper/testing/stub_flutter_windows_api.h | 2 +- shell/platform/windows/public/flutter_windows.h | 6 +++--- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/shell/platform/windows/client_wrapper/include/flutter/flutter_engine.h b/shell/platform/windows/client_wrapper/include/flutter/flutter_engine.h index b71c5cd479611..32c011774d9e4 100644 --- a/shell/platform/windows/client_wrapper/include/flutter/flutter_engine.h +++ b/shell/platform/windows/client_wrapper/include/flutter/flutter_engine.h @@ -86,10 +86,10 @@ class FlutterEngine : public PluginRegistry { void SetNextFrameCallback(std::function callback); // Called to pass an external window message to the engine for lifecycle - // state updates. This does not consume the window message. Non-Flutter + // state updates. Non-Flutter // windows must call this method in their WndProc in order to be included in - // the logic for application lifecycle state updates. Returns a result when - // the message has been consumed. + // the logic for application lifecycle state updates. Returns a result if + // the message should be consumed. std::optional ProcessExternalWindowMessage(HWND hwnd, UINT message, WPARAM wparam, diff --git a/shell/platform/windows/client_wrapper/testing/stub_flutter_windows_api.h b/shell/platform/windows/client_wrapper/testing/stub_flutter_windows_api.h index 73e920d949e32..475fdb0c32088 100644 --- a/shell/platform/windows/client_wrapper/testing/stub_flutter_windows_api.h +++ b/shell/platform/windows/client_wrapper/testing/stub_flutter_windows_api.h @@ -90,7 +90,7 @@ class StubFlutterWindowsApi { virtual void PluginRegistrarUnregisterTopLevelWindowProcDelegate( FlutterDesktopWindowProcCallback delegate) {} - // Claled for FlutterDesktopEngineProcessExternalWindowMessage. + // Called for FlutterDesktopEngineProcessExternalWindowMessage. virtual bool EngineProcessExternalWindowMessage( FlutterDesktopEngineRef engine, HWND hwnd, diff --git a/shell/platform/windows/public/flutter_windows.h b/shell/platform/windows/public/flutter_windows.h index beee0f029f4ff..935908bef5223 100644 --- a/shell/platform/windows/public/flutter_windows.h +++ b/shell/platform/windows/public/flutter_windows.h @@ -213,10 +213,10 @@ FLUTTER_EXPORT IDXGIAdapter* FlutterDesktopViewGetGraphicsAdapter( FlutterDesktopViewRef view); // Called to pass an external window message to the engine for lifecycle -// state updates. This does not consume the window message. Non-Flutter windows +// state updates. Non-Flutter windows // must call this method in their WndProc in order to be included in the logic -// for application lifecycle state updates. Returns true when the message is -// consumed. +// for application lifecycle state updates. Returns true if the message should +// be consumed. FLUTTER_EXPORT bool FlutterDesktopEngineProcessExternalWindowMessage( FlutterDesktopEngineRef engine, HWND hwnd, From b91b1fafc42fb06c47586101049bdbc14de778e1 Mon Sep 17 00:00:00 2001 From: schectman Date: Wed, 9 Aug 2023 13:32:57 -0400 Subject: [PATCH 7/8] More comments --- .../client_wrapper/include/flutter/flutter_engine.h | 7 +++---- shell/platform/windows/public/flutter_windows.h | 7 +++---- shell/platform/windows/window.h | 2 +- shell/platform/windows/windows_lifecycle_manager.cc | 3 +++ 4 files changed, 10 insertions(+), 9 deletions(-) diff --git a/shell/platform/windows/client_wrapper/include/flutter/flutter_engine.h b/shell/platform/windows/client_wrapper/include/flutter/flutter_engine.h index 32c011774d9e4..61bda1597ca82 100644 --- a/shell/platform/windows/client_wrapper/include/flutter/flutter_engine.h +++ b/shell/platform/windows/client_wrapper/include/flutter/flutter_engine.h @@ -86,10 +86,9 @@ class FlutterEngine : public PluginRegistry { void SetNextFrameCallback(std::function callback); // Called to pass an external window message to the engine for lifecycle - // state updates. Non-Flutter - // windows must call this method in their WndProc in order to be included in - // the logic for application lifecycle state updates. Returns a result if - // the message should be consumed. + // state updates. Non-Flutter windows must call this method in their WndProc + // in order to be included in the logic for application lifecycle state + // updates. Returns a result if the message should be consumed. std::optional ProcessExternalWindowMessage(HWND hwnd, UINT message, WPARAM wparam, diff --git a/shell/platform/windows/public/flutter_windows.h b/shell/platform/windows/public/flutter_windows.h index 935908bef5223..c93e33476d0fb 100644 --- a/shell/platform/windows/public/flutter_windows.h +++ b/shell/platform/windows/public/flutter_windows.h @@ -213,10 +213,9 @@ FLUTTER_EXPORT IDXGIAdapter* FlutterDesktopViewGetGraphicsAdapter( FlutterDesktopViewRef view); // Called to pass an external window message to the engine for lifecycle -// state updates. Non-Flutter windows -// must call this method in their WndProc in order to be included in the logic -// for application lifecycle state updates. Returns true if the message should -// be consumed. +// state updates. Non-Flutter windows must call this method in their WndProc +// in order to be included in the logic for application lifecycle state +// updates. Returns a result if the message should be consumed. FLUTTER_EXPORT bool FlutterDesktopEngineProcessExternalWindowMessage( FlutterDesktopEngineRef engine, HWND hwnd, diff --git a/shell/platform/windows/window.h b/shell/platform/windows/window.h index 047370ee259f5..015619c3d95cb 100644 --- a/shell/platform/windows/window.h +++ b/shell/platform/windows/window.h @@ -240,7 +240,7 @@ class Window : public KeyboardManager::WindowDelegate { // Returns the root view accessibility node, or nullptr if none. virtual gfx::NativeViewAccessible GetNativeViewAccessible() = 0; - // Release OS resources associated with window. + // Release OS resources associated with the window. void Destroy(); // Handles running DirectManipulation on the window to receive trackpad diff --git a/shell/platform/windows/windows_lifecycle_manager.cc b/shell/platform/windows/windows_lifecycle_manager.cc index dbd5e71658a2e..1676374bd935a 100644 --- a/shell/platform/windows/windows_lifecycle_manager.cc +++ b/shell/platform/windows/windows_lifecycle_manager.cc @@ -183,6 +183,9 @@ void WindowsLifecycleManager::BeginProcessingClose() { process_close_ = true; } +// TODO(schectman): Wait until the platform channel is registered to send +// the platform message. +// https://github.com/flutter/engine/pull/44238 void WindowsLifecycleManager::SetLifecycleState(AppLifecycleState state) { if (state_ == state) { return; From a29e31bc89527e1de4b481e3130077a693fef8f8 Mon Sep 17 00:00:00 2001 From: yaakovschectman <109111084+yaakovschectman@users.noreply.github.com> Date: Thu, 10 Aug 2023 13:31:02 -0400 Subject: [PATCH 8/8] Update shell/platform/windows/windows_lifecycle_manager.cc MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Loïc Sharma <737941+loic-sharma@users.noreply.github.com> --- shell/platform/windows/windows_lifecycle_manager.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shell/platform/windows/windows_lifecycle_manager.cc b/shell/platform/windows/windows_lifecycle_manager.cc index 1676374bd935a..5d572e165924e 100644 --- a/shell/platform/windows/windows_lifecycle_manager.cc +++ b/shell/platform/windows/windows_lifecycle_manager.cc @@ -185,7 +185,7 @@ void WindowsLifecycleManager::BeginProcessingClose() { // TODO(schectman): Wait until the platform channel is registered to send // the platform message. -// https://github.com/flutter/engine/pull/44238 +// https://github.com/flutter/flutter/issues/131616 void WindowsLifecycleManager::SetLifecycleState(AppLifecycleState state) { if (state_ == state) { return;