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 all 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
4 changes: 4 additions & 0 deletions shell/platform/darwin/macos/framework/Source/FlutterEngine.mm
Expand Up @@ -1031,6 +1031,10 @@ - (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.initializationComplete"]) {
// TODO(gspencergoog): Handle this message to enable exit message listening.
// https://github.com/flutter/flutter/issues/126033
result(nil);
loic-sharma marked this conversation as resolved.
Show resolved Hide resolved
} else {
result(FlutterMethodNotImplemented);
}
Expand Down
6 changes: 6 additions & 0 deletions shell/platform/linux/fl_platform_plugin.cc
Expand Up @@ -20,6 +20,8 @@ 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 kInitializationCompleteMethod[] =
"System.initializationComplete";
static constexpr char kPlaySoundMethod[] = "SystemSound.play";
static constexpr char kSystemNavigatorPopMethod[] = "SystemNavigator.pop";
static constexpr char kTextKey[] = "text";
Expand Down Expand Up @@ -335,6 +337,10 @@ 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, kInitializationCompleteMethod) == 0) {
// TODO(gspencergoog): Handle this message to enable exit message listening.
// https://github.com/flutter/flutter/issues/126033
response = FL_METHOD_RESPONSE(fl_method_success_response_new(nullptr));
} 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
Expand Up @@ -778,4 +778,8 @@ void FlutterWindowsEngine::OnDwmCompositionChanged() {
view_->OnDwmCompositionChanged();
}

void FlutterWindowsEngine::OnApplicationLifecycleEnabled() {
lifecycle_manager_->BeginProcessingClose();
}

} // namespace flutter
4 changes: 4 additions & 0 deletions shell/platform/windows/flutter_windows_engine.h
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 OnApplicationLifecycleEnabled();

protected:
// Creates the keyboard key handler.
//
Expand Down
46 changes: 46 additions & 0 deletions shell/platform/windows/flutter_windows_engine_unittests.cc
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->OnApplicationLifecycleEnabled();

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->OnApplicationLifecycleEnabled();

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->OnApplicationLifecycleEnabled();

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->OnApplicationLifecycleEnabled();

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->OnApplicationLifecycleEnabled();

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

} // namespace testing
} // namespace flutter
5 changes: 5 additions & 0 deletions shell/platform/windows/platform_handler.cc
Expand Up @@ -23,6 +23,8 @@ 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 kInitializationCompleteMethod[] =
"System.initializationComplete";
static constexpr char kPlaySoundMethod[] = "SystemSound.play";

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

SystemSoundPlay(sound_type.GetString(), std::move(result));
} else if (method.compare(kInitializationCompleteMethod) == 0) {
engine_->OnApplicationLifecycleEnabled();
result->Success();
} else {
result->NotImplemented();
}
Expand Down
9 changes: 8 additions & 1 deletion shell/platform/windows/windows_lifecycle_manager.cc
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
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