Fix flaky Windows exit unit test, remove error messages (#40945)
Shutdown is async, so we can not guarantee the order of execution and,
by extension, ignorance of execution used as an auxiliary part of a unit
test. Also, mock the windows view/binding handler for the engine in
order to avoid error messages.
Addresses https://github.com/flutter/flutter/issues/124162
## 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
diff --git a/shell/platform/windows/fixtures/main.dart b/shell/platform/windows/fixtures/main.dart
index c9794f6..dd88b7e 100644
--- a/shell/platform/windows/fixtures/main.dart
+++ b/shell/platform/windows/fixtures/main.dart
@@ -97,24 +97,6 @@
closed.complete(data);
});
await closed.future;
-
- // From here down, nothing should be called, because the application will have already been closed.
- final Completer<ByteData?> exited = Completer<ByteData?>();
- final String jsonString = json.encode(<String, dynamic>{
- 'method': 'System.exitApplication',
- 'args': <String, dynamic>{
- 'type': 'required', 'exitCode': 0
- }
- });
- ui.PlatformDispatcher.instance.sendPlatformMessage(
- 'flutter/platform',
- ByteData.sublistView(
- Uint8List.fromList(utf8.encode(jsonString))
- ),
- (ByteData? reply) {
- exited.complete(reply);
- });
- await exited.future;
}
@pragma('vm:entry-point')
diff --git a/shell/platform/windows/flutter_windows_engine_unittests.cc b/shell/platform/windows/flutter_windows_engine_unittests.cc
index f21e3aa..161181c 100644
--- a/shell/platform/windows/flutter_windows_engine_unittests.cc
+++ b/shell/platform/windows/flutter_windows_engine_unittests.cc
@@ -639,13 +639,16 @@
FlutterWindowsEngineBuilder builder{GetContext()};
builder.SetDartEntrypoint("exitTestExit");
bool finished = false;
- bool did_call = false;
- std::unique_ptr<FlutterWindowsEngine> engine = builder.Build();
+ auto window_binding_handler =
+ std::make_unique<::testing::NiceMock<MockWindowBindingHandler>>();
+ MockFlutterWindowsView view(std::move(window_binding_handler));
+ view.SetEngine(builder.Build());
+ FlutterWindowsEngine* engine = view.GetEngine();
- EngineModifier modifier(engine.get());
+ EngineModifier modifier(engine);
modifier.embedder_api().RunsAOTCompiledDartCode = []() { return false; };
- auto handler = std::make_unique<MockWindowsLifecycleManager>(engine.get());
+ auto handler = std::make_unique<MockWindowsLifecycleManager>(engine);
ON_CALL(*handler, Quit)
.WillByDefault(
[&finished](std::optional<HWND> hwnd, std::optional<WPARAM> wparam,
@@ -655,28 +658,16 @@
EXPECT_CALL(*handler, Quit).Times(1);
modifier.SetLifecycleManager(std::move(handler));
- auto binary_messenger =
- std::make_unique<BinaryMessengerImpl>(engine->messenger());
- // If this callback is triggered, the test fails. The exit procedure should be
- // called without checking with the framework.
- binary_messenger->SetMessageHandler(
- "flutter/platform", [&did_call](const uint8_t* message,
- size_t message_size, BinaryReply reply) {
- did_call = true;
- char response[] = "";
- reply(reinterpret_cast<uint8_t*>(response), 0);
- });
-
engine->Run();
engine->window_proc_delegate_manager()->OnTopLevelWindowProc(0, WM_CLOSE, 0,
0);
+ // The test will only succeed when this while loop exits. Otherwise it will
+ // timeout.
while (!finished) {
engine->task_runner()->ProcessTasks();
}
-
- EXPECT_FALSE(did_call);
}
TEST_F(FlutterWindowsEngineTest, TestExitCancel) {
@@ -685,11 +676,15 @@
bool finished = false;
bool did_call = false;
- std::unique_ptr<FlutterWindowsEngine> engine = builder.Build();
+ auto window_binding_handler =
+ std::make_unique<::testing::NiceMock<MockWindowBindingHandler>>();
+ MockFlutterWindowsView view(std::move(window_binding_handler));
+ view.SetEngine(builder.Build());
+ FlutterWindowsEngine* engine = view.GetEngine();
- EngineModifier modifier(engine.get());
+ EngineModifier modifier(engine);
modifier.embedder_api().RunsAOTCompiledDartCode = []() { return false; };
- auto handler = std::make_unique<MockWindowsLifecycleManager>(engine.get());
+ auto handler = std::make_unique<MockWindowsLifecycleManager>(engine);
ON_CALL(*handler, Quit)
.WillByDefault([&finished](std::optional<HWND> hwnd,
std::optional<WPARAM> wparam,
@@ -729,22 +724,29 @@
TEST_F(FlutterWindowsEngineTest, TestExitSecondCloseMessage) {
FlutterWindowsEngineBuilder builder{GetContext()};
builder.SetDartEntrypoint("exitTestExit");
- bool did_call = false;
bool second_close = false;
- std::unique_ptr<FlutterWindowsEngine> engine = builder.Build();
+ auto window_binding_handler =
+ std::make_unique<::testing::NiceMock<MockWindowBindingHandler>>();
+ MockFlutterWindowsView view(std::move(window_binding_handler));
+ view.SetEngine(builder.Build());
+ FlutterWindowsEngine* engine = view.GetEngine();
- EngineModifier modifier(engine.get());
+ EngineModifier modifier(engine);
modifier.embedder_api().RunsAOTCompiledDartCode = []() { return false; };
- auto handler = std::make_unique<MockWindowsLifecycleManager>(engine.get());
- ON_CALL(*handler, IsLastWindowOfProcess).WillByDefault([]() { return true; });
- ON_CALL(*handler, Quit)
- .WillByDefault([&handler](std::optional<HWND> hwnd,
- std::optional<WPARAM> wparam,
- std::optional<LPARAM> lparam, UINT exit_code) {
- handler->WindowsLifecycleManager::Quit(hwnd, wparam, lparam, exit_code);
- });
- ON_CALL(*handler, DispatchMessage)
+ auto handler = std::make_unique<MockWindowsLifecycleManager>(engine);
+ auto& handler_obj = *handler;
+ ON_CALL(handler_obj, IsLastWindowOfProcess).WillByDefault([]() {
+ return true;
+ });
+ ON_CALL(handler_obj, Quit)
+ .WillByDefault(
+ [&handler_obj](std::optional<HWND> hwnd, std::optional<WPARAM> wparam,
+ std::optional<LPARAM> lparam, UINT exit_code) {
+ handler_obj.WindowsLifecycleManager::Quit(hwnd, wparam, lparam,
+ exit_code);
+ });
+ ON_CALL(handler_obj, DispatchMessage)
.WillByDefault(
[&engine](HWND hwnd, UINT msg, WPARAM wparam, LPARAM lparam) {
engine->window_proc_delegate_manager()->OnTopLevelWindowProc(
@@ -752,16 +754,6 @@
});
modifier.SetLifecycleManager(std::move(handler));
- auto binary_messenger =
- std::make_unique<BinaryMessengerImpl>(engine->messenger());
- binary_messenger->SetMessageHandler(
- "flutter/platform", [&did_call](const uint8_t* message,
- size_t message_size, BinaryReply reply) {
- did_call = true;
- char response[] = "";
- reply(reinterpret_cast<uint8_t*>(response), 0);
- });
-
engine->Run();
// This delegate will be registered after the lifecycle manager, so it will be
@@ -788,21 +780,22 @@
while (!second_close) {
engine->task_runner()->ProcessTasks();
}
-
- EXPECT_FALSE(did_call);
}
TEST_F(FlutterWindowsEngineTest, TestExitCloseMultiWindow) {
FlutterWindowsEngineBuilder builder{GetContext()};
builder.SetDartEntrypoint("exitTestExit");
bool finished = false;
- bool did_call = false;
- std::unique_ptr<FlutterWindowsEngine> engine = builder.Build();
+ auto window_binding_handler =
+ std::make_unique<::testing::NiceMock<MockWindowBindingHandler>>();
+ MockFlutterWindowsView view(std::move(window_binding_handler));
+ view.SetEngine(builder.Build());
+ FlutterWindowsEngine* engine = view.GetEngine();
- EngineModifier modifier(engine.get());
+ EngineModifier modifier(engine);
modifier.embedder_api().RunsAOTCompiledDartCode = []() { return false; };
- auto handler = std::make_unique<MockWindowsLifecycleManager>(engine.get());
+ auto handler = std::make_unique<MockWindowsLifecycleManager>(engine);
ON_CALL(*handler, IsLastWindowOfProcess).WillByDefault([&finished]() {
finished = true;
return false;
@@ -811,18 +804,6 @@
EXPECT_CALL(*handler, Quit).Times(0);
modifier.SetLifecycleManager(std::move(handler));
- auto binary_messenger =
- std::make_unique<BinaryMessengerImpl>(engine->messenger());
- // If this callback is triggered, the test fails. The exit procedure should be
- // called without checking with the framework.
- binary_messenger->SetMessageHandler(
- "flutter/platform", [&did_call](const uint8_t* message,
- size_t message_size, BinaryReply reply) {
- did_call = true;
- char response[] = "";
- reply(reinterpret_cast<uint8_t*>(response), 0);
- });
-
engine->Run();
engine->window_proc_delegate_manager()->OnTopLevelWindowProc(0, WM_CLOSE, 0,
@@ -831,8 +812,6 @@
while (!finished) {
engine->task_runner()->ProcessTasks();
}
-
- EXPECT_FALSE(did_call);
}
} // namespace testing
