Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Commit f53ad9c

Browse files
Reintroduce Windows lifecycle with guard for posthumous OnWindowStateEvent (#44344)
Previously, destruction of `Window` called `DestroyWindow`, which may send `WM_KILLFOCUS` to the to-be-destroyed window. Because `Window`'s destructor is called after `FlutterWindow`'s, the `FlutterWindow` vtable was already destroyed at this point, and the subsequent call to the virtual method `OnWindowStateEvent` would cause a crash. This PR reintroduces the reverted changes for Windows lifecycle with a check before calling the virtual method that the `FlutterWindow` object has not yet been destructed. flutter/flutter#131872 *If you had to change anything in the [flutter/tests] repo, include a link to the migration guide as per the [breaking change policy].* ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide] and the [C++, Objective-C, Java style guides]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test-exempt. See [testing the engine] for instructions on writing and running engine tests. - [x] I updated/added relevant documentation (doc comments with `///`). - [ ] I signed the [CLA]. - [x] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. <!-- Links --> [Contributor Guide]: https://github.com/flutter/flutter/wiki/Tree-hygiene#overview [Tree Hygiene]: https://github.com/flutter/flutter/wiki/Tree-hygiene [Flutter Style Guide]: https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style [testing the engine]: https://github.com/flutter/flutter/wiki/Testing-the-engine [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/wiki/Tree-hygiene#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/wiki/Chat --------- Co-authored-by: Loïc Sharma <737941+loic-sharma@users.noreply.github.com>
1 parent d5db728 commit f53ad9c

25 files changed

+673
-18
lines changed

ci/licenses_golden/excluded_files

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -354,6 +354,7 @@
354354
../../../flutter/shell/platform/windows/text_input_plugin_unittest.cc
355355
../../../flutter/shell/platform/windows/window_proc_delegate_manager_unittests.cc
356356
../../../flutter/shell/platform/windows/window_unittests.cc
357+
../../../flutter/shell/platform/windows/windows_lifecycle_manager_unittests.cc
357358
../../../flutter/shell/profiling/sampling_profiler_unittest.cc
358359
../../../flutter/shell/testing
359360
../../../flutter/shell/vmservice/.dart_tool

shell/platform/windows/BUILD.gn

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -221,6 +221,7 @@ executable("flutter_windows_unittests") {
221221
"text_input_plugin_unittest.cc",
222222
"window_proc_delegate_manager_unittests.cc",
223223
"window_unittests.cc",
224+
"windows_lifecycle_manager_unittests.cc",
224225
]
225226

226227
configs +=

shell/platform/windows/client_wrapper/flutter_engine.cc

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,19 @@ void FlutterEngine::SetNextFrameCallback(std::function<void()> callback) {
100100
this);
101101
}
102102

103+
std::optional<LRESULT> FlutterEngine::ProcessExternalWindowMessage(
104+
HWND hwnd,
105+
UINT message,
106+
WPARAM wparam,
107+
LPARAM lparam) {
108+
LRESULT result;
109+
if (FlutterDesktopEngineProcessExternalWindowMessage(
110+
engine_, hwnd, message, wparam, lparam, &result)) {
111+
return result;
112+
}
113+
return std::nullopt;
114+
}
115+
103116
FlutterDesktopEngineRef FlutterEngine::RelinquishEngine() {
104117
owns_engine_ = false;
105118
return engine_;

shell/platform/windows/client_wrapper/flutter_engine_unittests.cc

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,17 @@ class TestFlutterWindowsApi : public testing::StubFlutterWindowsApi {
5656
// |flutter::testing::StubFlutterWindowsApi|
5757
void EngineReloadSystemFonts() override { reload_fonts_called_ = true; }
5858

59+
// |flutter::testing::StubFlutterWindowsApi|
60+
bool EngineProcessExternalWindowMessage(FlutterDesktopEngineRef engine,
61+
HWND hwnd,
62+
UINT message,
63+
WPARAM wparam,
64+
LPARAM lparam,
65+
LRESULT* result) override {
66+
last_external_message_ = message;
67+
return false;
68+
}
69+
5970
bool create_called() { return create_called_; }
6071

6172
bool run_called() { return run_called_; }
@@ -74,6 +85,8 @@ class TestFlutterWindowsApi : public testing::StubFlutterWindowsApi {
7485
next_frame_callback_ = nullptr;
7586
}
7687

88+
UINT last_external_message() { return last_external_message_; }
89+
7790
private:
7891
bool create_called_ = false;
7992
bool run_called_ = false;
@@ -82,6 +95,7 @@ class TestFlutterWindowsApi : public testing::StubFlutterWindowsApi {
8295
std::vector<std::string> dart_entrypoint_arguments_;
8396
VoidCallback next_frame_callback_ = nullptr;
8497
void* next_frame_user_data_ = nullptr;
98+
UINT last_external_message_ = 0;
8599
};
86100

87101
} // namespace
@@ -201,4 +215,16 @@ TEST(FlutterEngineTest, SetNextFrameCallback) {
201215
EXPECT_TRUE(success);
202216
}
203217

218+
TEST(FlutterEngineTest, ProcessExternalWindowMessage) {
219+
testing::ScopedStubFlutterWindowsApi scoped_api_stub(
220+
std::make_unique<TestFlutterWindowsApi>());
221+
auto test_api = static_cast<TestFlutterWindowsApi*>(scoped_api_stub.stub());
222+
223+
FlutterEngine engine(DartProject(L"fake/project/path"));
224+
225+
engine.ProcessExternalWindowMessage(reinterpret_cast<HWND>(1), 1234, 0, 0);
226+
227+
EXPECT_EQ(test_api->last_external_message(), 1234);
228+
}
229+
204230
} // namespace flutter

shell/platform/windows/client_wrapper/include/flutter/flutter_engine.h

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99

1010
#include <chrono>
1111
#include <memory>
12+
#include <optional>
1213
#include <string>
1314

1415
#include "binary_messenger.h"
@@ -84,6 +85,15 @@ class FlutterEngine : public PluginRegistry {
8485
// once on the platform thread.
8586
void SetNextFrameCallback(std::function<void()> callback);
8687

88+
// Called to pass an external window message to the engine for lifecycle
89+
// state updates. Non-Flutter windows must call this method in their WndProc
90+
// in order to be included in the logic for application lifecycle state
91+
// updates. Returns a result if the message should be consumed.
92+
std::optional<LRESULT> ProcessExternalWindowMessage(HWND hwnd,
93+
UINT message,
94+
WPARAM wparam,
95+
LPARAM lparam);
96+
8797
private:
8898
// For access to RelinquishEngine.
8999
friend class FlutterViewController;

shell/platform/windows/client_wrapper/testing/stub_flutter_windows_api.cc

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -162,6 +162,20 @@ IDXGIAdapter* FlutterDesktopViewGetGraphicsAdapter(FlutterDesktopViewRef view) {
162162
return nullptr;
163163
}
164164

165+
bool FlutterDesktopEngineProcessExternalWindowMessage(
166+
FlutterDesktopEngineRef engine,
167+
HWND hwnd,
168+
UINT message,
169+
WPARAM wparam,
170+
LPARAM lparam,
171+
LRESULT* result) {
172+
if (s_stub_implementation) {
173+
return s_stub_implementation->EngineProcessExternalWindowMessage(
174+
engine, hwnd, message, wparam, lparam, result);
175+
}
176+
return false;
177+
}
178+
165179
FlutterDesktopViewRef FlutterDesktopPluginRegistrarGetView(
166180
FlutterDesktopPluginRegistrarRef controller) {
167181
// The stub ignores this, so just return an arbitrary non-zero value.

shell/platform/windows/client_wrapper/testing/stub_flutter_windows_api.h

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,17 @@ class StubFlutterWindowsApi {
8989
// FlutterDesktopPluginRegistrarUnregisterTopLevelWindowProcDelegate.
9090
virtual void PluginRegistrarUnregisterTopLevelWindowProcDelegate(
9191
FlutterDesktopWindowProcCallback delegate) {}
92+
93+
// Called for FlutterDesktopEngineProcessExternalWindowMessage.
94+
virtual bool EngineProcessExternalWindowMessage(
95+
FlutterDesktopEngineRef engine,
96+
HWND hwnd,
97+
UINT message,
98+
WPARAM wparam,
99+
LPARAM lparam,
100+
LRESULT* result) {
101+
return false;
102+
}
92103
};
93104

94105
// A test helper that owns a stub implementation, making it the test stub for

shell/platform/windows/flutter_window.cc

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,11 +74,20 @@ FlutterWindow::FlutterWindow(int width, int height)
7474
current_cursor_ = ::LoadCursor(nullptr, IDC_ARROW);
7575
}
7676

77-
FlutterWindow::~FlutterWindow() {}
77+
FlutterWindow::~FlutterWindow() {
78+
OnWindowStateEvent(WindowStateEvent::kHide);
79+
Destroy();
80+
}
7881

7982
void FlutterWindow::SetView(WindowBindingHandlerDelegate* window) {
8083
binding_handler_delegate_ = window;
8184
direct_manipulation_owner_->SetBindingHandlerDelegate(window);
85+
if (restored_ && window) {
86+
OnWindowStateEvent(WindowStateEvent::kShow);
87+
}
88+
if (focused_ && window) {
89+
OnWindowStateEvent(WindowStateEvent::kFocus);
90+
}
8291
}
8392

8493
WindowsRenderTarget FlutterWindow::GetRenderTarget() {
@@ -328,4 +337,26 @@ bool FlutterWindow::NeedsVSync() {
328337
return true;
329338
}
330339

340+
void FlutterWindow::OnWindowStateEvent(WindowStateEvent event) {
341+
switch (event) {
342+
case WindowStateEvent::kShow:
343+
restored_ = true;
344+
break;
345+
case WindowStateEvent::kHide:
346+
restored_ = false;
347+
focused_ = false;
348+
break;
349+
case WindowStateEvent::kFocus:
350+
focused_ = true;
351+
break;
352+
case WindowStateEvent::kUnfocus:
353+
focused_ = false;
354+
break;
355+
}
356+
HWND hwnd = GetPlatformWindow();
357+
if (hwnd && binding_handler_delegate_) {
358+
binding_handler_delegate_->OnWindowStateEvent(hwnd, event);
359+
}
360+
}
361+
331362
} // namespace flutter

shell/platform/windows/flutter_window.h

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -162,6 +162,9 @@ class FlutterWindow : public Window, public WindowBindingHandler {
162162
// |Window|
163163
ui::AXFragmentRootDelegateWin* GetAxFragmentRootDelegate() override;
164164

165+
// |Window|
166+
virtual void OnWindowStateEvent(WindowStateEvent event) override;
167+
165168
private:
166169
// A pointer to a FlutterWindowsView that can be used to update engine
167170
// windowing and input state.
@@ -173,6 +176,12 @@ class FlutterWindow : public Window, public WindowBindingHandler {
173176
// The cursor rect set by Flutter.
174177
RECT cursor_rect_;
175178

179+
// The window receives resize and focus messages before its view is set, so
180+
// these values cache the state of the window in the meantime so that the
181+
// proper application lifecycle state can be updated once the view is set.
182+
bool restored_ = false;
183+
bool focused_ = false;
184+
176185
FML_DISALLOW_COPY_AND_ASSIGN(FlutterWindow);
177186
};
178187

shell/platform/windows/flutter_window_unittests.cc

Lines changed: 108 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,11 +23,16 @@ static constexpr int32_t kDefaultPointerDeviceId = 0;
2323

2424
class MockFlutterWindow : public FlutterWindow {
2525
public:
26-
MockFlutterWindow() : FlutterWindow(800, 600) {
26+
MockFlutterWindow(bool reset_view_on_exit = true)
27+
: FlutterWindow(800, 600), reset_view_on_exit_(reset_view_on_exit) {
2728
ON_CALL(*this, GetDpiScale())
2829
.WillByDefault(Return(this->FlutterWindow::GetDpiScale()));
2930
}
30-
virtual ~MockFlutterWindow() {}
31+
virtual ~MockFlutterWindow() {
32+
if (reset_view_on_exit_) {
33+
SetView(nullptr);
34+
}
35+
}
3136

3237
// Wrapper for GetCurrentDPI() which is a protected method.
3338
UINT GetDpi() { return GetCurrentDPI(); }
@@ -61,6 +66,7 @@ class MockFlutterWindow : public FlutterWindow {
6166
MOCK_METHOD1(Win32MapVkToChar, uint32_t(uint32_t));
6267
MOCK_METHOD0(GetPlatformWindow, HWND());
6368
MOCK_METHOD0(GetAxFragmentRootDelegate, ui::AXFragmentRootDelegateWin*());
69+
MOCK_METHOD1(OnWindowStateEvent, void(WindowStateEvent));
6470

6571
protected:
6672
// |KeyboardManager::WindowDelegate|
@@ -72,6 +78,7 @@ class MockFlutterWindow : public FlutterWindow {
7278
}
7379

7480
private:
81+
bool reset_view_on_exit_;
7582
FML_DISALLOW_COPY_AND_ASSIGN(MockFlutterWindow);
7683
};
7784

@@ -229,6 +236,10 @@ TEST(FlutterWindowTest, OnPointerStarSendsDeviceType) {
229236
kDefaultPointerDeviceId, WM_LBUTTONDOWN);
230237
win32window.OnPointerLeave(10.0, 10.0, kFlutterPointerDeviceKindStylus,
231238
kDefaultPointerDeviceId);
239+
240+
// Destruction of win32window sends a HIDE update. In situ, the window is
241+
// owned by the delegate, and so is destructed first. Not so here.
242+
win32window.SetView(nullptr);
232243
}
233244

234245
// Tests that calls to OnScroll in turn calls GetScrollOffsetMultiplier
@@ -324,5 +335,100 @@ TEST(FlutterWindowTest, AlertNode) {
324335
EXPECT_EQ(role.lVal, ROLE_SYSTEM_ALERT);
325336
}
326337

338+
TEST(FlutterWindowTest, LifecycleFocusMessages) {
339+
MockFlutterWindow win32window;
340+
ON_CALL(win32window, GetPlatformWindow).WillByDefault([]() {
341+
return reinterpret_cast<HWND>(1);
342+
});
343+
MockWindowBindingHandlerDelegate delegate;
344+
win32window.SetView(&delegate);
345+
346+
WindowStateEvent last_event;
347+
ON_CALL(delegate, OnWindowStateEvent)
348+
.WillByDefault([&last_event](HWND hwnd, WindowStateEvent event) {
349+
last_event = event;
350+
});
351+
ON_CALL(win32window, OnWindowStateEvent)
352+
.WillByDefault([&](WindowStateEvent event) {
353+
win32window.FlutterWindow::OnWindowStateEvent(event);
354+
});
355+
356+
win32window.InjectWindowMessage(WM_SIZE, 0, 0);
357+
EXPECT_EQ(last_event, WindowStateEvent::kHide);
358+
359+
win32window.InjectWindowMessage(WM_SIZE, 0, MAKEWORD(1, 1));
360+
EXPECT_EQ(last_event, WindowStateEvent::kShow);
361+
362+
win32window.InjectWindowMessage(WM_SETFOCUS, 0, 0);
363+
EXPECT_EQ(last_event, WindowStateEvent::kFocus);
364+
365+
win32window.InjectWindowMessage(WM_KILLFOCUS, 0, 0);
366+
EXPECT_EQ(last_event, WindowStateEvent::kUnfocus);
367+
}
368+
369+
TEST(FlutterWindowTest, CachedLifecycleMessage) {
370+
MockFlutterWindow win32window;
371+
ON_CALL(win32window, GetPlatformWindow).WillByDefault([]() {
372+
return reinterpret_cast<HWND>(1);
373+
});
374+
ON_CALL(win32window, OnWindowStateEvent)
375+
.WillByDefault([&](WindowStateEvent event) {
376+
win32window.FlutterWindow::OnWindowStateEvent(event);
377+
});
378+
379+
// Restore
380+
win32window.InjectWindowMessage(WM_SIZE, 0, MAKEWORD(1, 1));
381+
382+
// Focus
383+
win32window.InjectWindowMessage(WM_SETFOCUS, 0, 0);
384+
385+
MockWindowBindingHandlerDelegate delegate;
386+
bool focused = false;
387+
bool restored = false;
388+
ON_CALL(delegate, OnWindowStateEvent)
389+
.WillByDefault([&](HWND hwnd, WindowStateEvent event) {
390+
if (event == WindowStateEvent::kFocus) {
391+
focused = true;
392+
} else if (event == WindowStateEvent::kShow) {
393+
restored = true;
394+
}
395+
});
396+
397+
win32window.SetView(&delegate);
398+
EXPECT_TRUE(focused);
399+
EXPECT_TRUE(restored);
400+
}
401+
402+
TEST(FlutterWindowTest, PosthumousWindowMessage) {
403+
MockWindowBindingHandlerDelegate delegate;
404+
int msg_count = 0;
405+
HWND hwnd;
406+
ON_CALL(delegate, OnWindowStateEvent)
407+
.WillByDefault([&](HWND hwnd, WindowStateEvent event) { msg_count++; });
408+
409+
{
410+
MockFlutterWindow win32window(false);
411+
ON_CALL(win32window, GetPlatformWindow).WillByDefault([&]() {
412+
return win32window.FlutterWindow::GetPlatformWindow();
413+
});
414+
ON_CALL(win32window, OnWindowStateEvent)
415+
.WillByDefault([&](WindowStateEvent event) {
416+
win32window.FlutterWindow::OnWindowStateEvent(event);
417+
});
418+
win32window.SetView(&delegate);
419+
win32window.InitializeChild("Title", 1, 1);
420+
hwnd = win32window.GetPlatformWindow();
421+
SendMessage(hwnd, WM_SIZE, 0, MAKEWORD(1, 1));
422+
SendMessage(hwnd, WM_SETFOCUS, 0, 0);
423+
424+
// By setting this to zero before exiting the scope that contains
425+
// win32window, and then checking its value afterwards, enforce that the
426+
// window receive and process messages from its destructor without
427+
// accessing out-of-bounds memory.
428+
msg_count = 0;
429+
}
430+
EXPECT_GE(msg_count, 1);
431+
}
432+
327433
} // namespace testing
328434
} // namespace flutter

shell/platform/windows/flutter_windows.cc

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -207,6 +207,22 @@ IDXGIAdapter* FlutterDesktopViewGetGraphicsAdapter(FlutterDesktopViewRef view) {
207207
return nullptr;
208208
}
209209

210+
bool FlutterDesktopEngineProcessExternalWindowMessage(
211+
FlutterDesktopEngineRef engine,
212+
HWND hwnd,
213+
UINT message,
214+
WPARAM wparam,
215+
LPARAM lparam,
216+
LRESULT* result) {
217+
std::optional<LRESULT> lresult =
218+
EngineFromHandle(engine)->ProcessExternalWindowMessage(hwnd, message,
219+
wparam, lparam);
220+
if (result && lresult.has_value()) {
221+
*result = lresult.value();
222+
}
223+
return lresult.has_value();
224+
}
225+
210226
FlutterDesktopViewRef FlutterDesktopPluginRegistrarGetView(
211227
FlutterDesktopPluginRegistrarRef registrar) {
212228
return HandleForView(registrar->engine->view());

0 commit comments

Comments
 (0)