Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Only register top level window message listener upon registering service binding #41733

Merged
merged 22 commits into from
May 10, 2023
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions shell/platform/darwin/macos/framework/Source/FlutterEngine.mm
Original file line number Diff line number Diff line change
Expand Up @@ -1031,6 +1031,8 @@ - (void)handleMethodCall:(FlutterMethodCall*)call result:(FlutterResult)result {
result(@{@"value" : @([self clipboardHasStrings])});
} else if ([call.method isEqualToString:@"System.exitApplication"]) {
[[self terminationHandler] handleRequestAppExitMethodCall:call.arguments result:result];
} else if ([call.method isEqualToString:@"System.enableApplicationLifecycle"]) {
result(nil);
loic-sharma marked this conversation as resolved.
Show resolved Hide resolved
} else {
result(FlutterMethodNotImplemented);
}
Expand Down
4 changes: 4 additions & 0 deletions shell/platform/linux/fl_platform_plugin.cc
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ static constexpr char kSetClipboardDataMethod[] = "Clipboard.setData";
static constexpr char kClipboardHasStringsMethod[] = "Clipboard.hasStrings";
static constexpr char kExitApplicationMethod[] = "System.exitApplication";
static constexpr char kRequestAppExitMethod[] = "System.requestAppExit";
static constexpr char kRegisterBindingMethod[] = "System.enableApplicationLifecycle";
static constexpr char kPlaySoundMethod[] = "SystemSound.play";
static constexpr char kSystemNavigatorPopMethod[] = "SystemNavigator.pop";
static constexpr char kTextKey[] = "text";
Expand Down Expand Up @@ -335,6 +336,9 @@ static void method_call_cb(FlMethodChannel* channel,
response = system_sound_play(self, args);
} else if (strcmp(method, kSystemNavigatorPopMethod) == 0) {
response = system_navigator_pop(self);
} else if (strcmp(method, kRegisterBindingMethod) == 0) {
response =
FL_METHOD_RESPONSE(fl_method_success_response_new(fl_value_new_null()));
robert-ancell marked this conversation as resolved.
Show resolved Hide resolved
} else {
response = FL_METHOD_RESPONSE(fl_method_not_implemented_response_new());
}
Expand Down
4 changes: 4 additions & 0 deletions shell/platform/windows/flutter_windows_engine.cc
Original file line number Diff line number Diff line change
Expand Up @@ -778,4 +778,8 @@ void FlutterWindowsEngine::OnDwmCompositionChanged() {
view_->OnDwmCompositionChanged();
}

void FlutterWindowsEngine::OnServiceBindingsRegistered() {
yaakovschectman marked this conversation as resolved.
Show resolved Hide resolved
lifecycle_manager_->BeginProcessingClose();
}

} // namespace flutter
4 changes: 4 additions & 0 deletions shell/platform/windows/flutter_windows_engine.h
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,10 @@ class FlutterWindowsEngine {
// Called when a WM_DWMCOMPOSITIONCHANGED message is received.
void OnDwmCompositionChanged();

// Called in response to the framework registering a ServiceBindings.
// Registers the top level handler for the WM_CLOSE window message.
void OnServiceBindingsRegistered();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't forget to update these names to reflect the latest framework message name :)


protected:
// Creates the keyboard key handler.
//
Expand Down
46 changes: 46 additions & 0 deletions shell/platform/windows/flutter_windows_engine_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -657,6 +657,7 @@ TEST_F(FlutterWindowsEngineTest, TestExit) {
ON_CALL(*handler, IsLastWindowOfProcess).WillByDefault([]() { return true; });
EXPECT_CALL(*handler, Quit).Times(1);
modifier.SetLifecycleManager(std::move(handler));
engine->OnServiceBindingsRegistered();

engine->Run();

Expand Down Expand Up @@ -693,6 +694,7 @@ TEST_F(FlutterWindowsEngineTest, TestExitCancel) {
ON_CALL(*handler, IsLastWindowOfProcess).WillByDefault([]() { return true; });
EXPECT_CALL(*handler, Quit).Times(0);
modifier.SetLifecycleManager(std::move(handler));
engine->OnServiceBindingsRegistered();

auto binary_messenger =
std::make_unique<BinaryMessengerImpl>(engine->messenger());
Expand Down Expand Up @@ -753,6 +755,7 @@ TEST_F(FlutterWindowsEngineTest, TestExitSecondCloseMessage) {
hwnd, msg, wparam, lparam);
});
modifier.SetLifecycleManager(std::move(handler));
engine->OnServiceBindingsRegistered();

engine->Run();

Expand Down Expand Up @@ -803,6 +806,7 @@ TEST_F(FlutterWindowsEngineTest, TestExitCloseMultiWindow) {
// Quit should not be called when there is more than one window.
EXPECT_CALL(*handler, Quit).Times(0);
modifier.SetLifecycleManager(std::move(handler));
engine->OnServiceBindingsRegistered();

engine->Run();

Expand All @@ -814,5 +818,47 @@ TEST_F(FlutterWindowsEngineTest, TestExitCloseMultiWindow) {
}
}

TEST_F(FlutterWindowsEngineTest, LifecycleManagerDisabledByDefault) {
FlutterWindowsEngineBuilder builder{GetContext()};

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);
modifier.embedder_api().RunsAOTCompiledDartCode = []() { return false; };
auto handler = std::make_unique<MockWindowsLifecycleManager>(engine);
EXPECT_CALL(*handler, IsLastWindowOfProcess).Times(0);
modifier.SetLifecycleManager(std::move(handler));

engine->window_proc_delegate_manager()->OnTopLevelWindowProc(0, WM_CLOSE, 0,
0);
}

TEST_F(FlutterWindowsEngineTest, EnableApplicationLifecycle) {
FlutterWindowsEngineBuilder builder{GetContext()};

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);
modifier.embedder_api().RunsAOTCompiledDartCode = []() { return false; };
auto handler = std::make_unique<MockWindowsLifecycleManager>(engine);
ON_CALL(*handler, IsLastWindowOfProcess).WillByDefault([]() {
return false;
});
EXPECT_CALL(*handler, IsLastWindowOfProcess).Times(1);
modifier.SetLifecycleManager(std::move(handler));
engine->OnServiceBindingsRegistered();

engine->window_proc_delegate_manager()->OnTopLevelWindowProc(0, WM_CLOSE, 0,
0);
}

} // namespace testing
} // namespace flutter
4 changes: 4 additions & 0 deletions shell/platform/windows/platform_handler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ static constexpr char kHasStringsClipboardMethod[] = "Clipboard.hasStrings";
static constexpr char kSetClipboardDataMethod[] = "Clipboard.setData";
static constexpr char kExitApplicationMethod[] = "System.exitApplication";
static constexpr char kRequestAppExitMethod[] = "System.requestAppExit";
static constexpr char kRegisterBindingMethod[] = "System.enableApplicationLifecycle";
static constexpr char kPlaySoundMethod[] = "SystemSound.play";

static constexpr char kExitCodeKey[] = "exitCode";
Expand Down Expand Up @@ -495,6 +496,9 @@ void PlatformHandler::HandleMethodCall(
const rapidjson::Value& sound_type = method_call.arguments()[0];

SystemSoundPlay(sound_type.GetString(), std::move(result));
} else if (method.compare(kRegisterBindingMethod) == 0) {
engine_->OnServiceBindingsRegistered();
result->Success();
} else {
result->NotImplemented();
}
Expand Down
9 changes: 8 additions & 1 deletion shell/platform/windows/windows_lifecycle_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
namespace flutter {

WindowsLifecycleManager::WindowsLifecycleManager(FlutterWindowsEngine* engine)
: engine_(engine) {}
: engine_(engine), process_close_(false) {}

WindowsLifecycleManager::~WindowsLifecycleManager() {}

Expand Down Expand Up @@ -49,6 +49,9 @@ bool WindowsLifecycleManager::WindowProc(HWND hwnd,
// is, we re-dispatch a new WM_CLOSE message. In order to allow the new
// message to reach other delegates, we ignore it here.
case WM_CLOSE: {
if (!process_close_) {
return false;
}
auto key = std::make_tuple(hwnd, wpar, lpar);
auto itr = sent_close_messages_.find(key);
if (itr != sent_close_messages_.end()) {
Expand Down Expand Up @@ -156,4 +159,8 @@ bool WindowsLifecycleManager::IsLastWindowOfProcess() {
return num_windows <= 1;
}

void WindowsLifecycleManager::BeginProcessingClose() {
process_close_ = true;
}

} // namespace flutter
5 changes: 5 additions & 0 deletions shell/platform/windows/windows_lifecycle_manager.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,9 @@ class WindowsLifecycleManager {
// Intercept top level window messages, only paying attention to WM_CLOSE.
bool WindowProc(HWND hwnd, UINT msg, WPARAM w, LPARAM l, LRESULT* result);

// Signal to start consuming WM_CLOSE messages.
void BeginProcessingClose();

protected:
// Check the number of top-level windows associated with this process, and
// return true only if there are 1 or fewer.
Expand All @@ -51,6 +54,8 @@ class WindowsLifecycleManager {
FlutterWindowsEngine* engine_;

std::map<std::tuple<HWND, WPARAM, LPARAM>, int> sent_close_messages_;

bool process_close_;
};

} // namespace flutter
Expand Down