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

[linux] Wait for binding to be ready before requesting exits from framework #41782

Merged
merged 7 commits into from
May 16, 2023
Merged

[linux] Wait for binding to be ready before requesting exits from framework #41782

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
02144b9
Only accept exit requests after the app is initialized
gspencergoog May 5, 2023
a1dc40e
Just don't request an exit if not ready
gspencergoog May 5, 2023
92c8298
Add test
gspencergoog May 5, 2023
a063dd8
Update shell/platform/linux/fl_platform_plugin.cc
gspencergoog May 5, 2023
149bb4a
Send nullptr return value
gspencergoog May 8, 2023
d7ee743
Fix test
gspencergoog May 8, 2023
c13eed2
Add SuccessResponse to ON_CALL
gspencergoog May 9, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 26 additions & 7 deletions shell/platform/linux/fl_platform_plugin.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ struct _FlPlatformPlugin {
FlMethodChannel* channel;
FlMethodCall* exit_application_method_call;
GCancellable* cancellable;
bool app_initialization_complete;
};

G_DEFINE_TYPE(FlPlatformPlugin, fl_platform_plugin, G_TYPE_OBJECT)
Expand Down Expand Up @@ -236,15 +237,32 @@ static void request_app_exit_response_cb(GObject* object,
}
}

// Send a request to Flutter to exit the application.
// Send a request to Flutter to exit the application, but only if it's ready for
// a request.
static void request_app_exit(FlPlatformPlugin* self, const char* type) {
g_autoptr(FlValue) args = fl_value_new_map();
if (!self->app_initialization_complete ||
g_str_equal(type, kExitTypeRequired)) {
quit_application();
return;
}

fl_value_set_string_take(args, kExitTypeKey, fl_value_new_string(type));
fl_method_channel_invoke_method(self->channel, kRequestAppExitMethod, args,
self->cancellable,
request_app_exit_response_cb, self);
}

// Called when the Dart app has finished initialization and is ready to handle
// requests. For the Flutter framework, this means after the ServicesBinding has
// been initialized and it sends a System.initializationComplete message.
static FlMethodResponse* system_intitialization_complete(
FlPlatformPlugin* self,
FlMethodCall* method_call) {
self->app_initialization_complete = TRUE;
return FL_METHOD_RESPONSE(fl_method_success_response_new(nullptr));
}

// Called when Flutter wants to exit the application.
static FlMethodResponse* system_exit_application(FlPlatformPlugin* self,
FlMethodCall* method_call) {
Expand All @@ -270,8 +288,10 @@ static FlMethodResponse* system_exit_application(FlPlatformPlugin* self,
self->exit_application_method_call =
FL_METHOD_CALL(g_object_ref(method_call));

// Requested to immediately quit.
if (g_str_equal(type, kExitTypeRequired)) {
// Requested to immediately quit if the app hasn't yet signaled that it is
// ready to handle requests, or if the type of exit requested is "required".
if (!self->app_initialization_complete ||
g_str_equal(type, kExitTypeRequired)) {
quit_application();
g_autoptr(FlValue) exit_result = fl_value_new_map();
fl_value_set_string_take(exit_result, kExitResponseKey,
Expand Down Expand Up @@ -333,14 +353,12 @@ static void method_call_cb(FlMethodChannel* channel,
response = clipboard_has_strings_async(self, method_call);
} else if (strcmp(method, kExitApplicationMethod) == 0) {
response = system_exit_application(self, method_call);
} else if (strcmp(method, kInitializationCompleteMethod) == 0) {
response = system_intitialization_complete(self, method_call);
} else if (strcmp(method, kPlaySoundMethod) == 0) {
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 Expand Up @@ -381,6 +399,7 @@ FlPlatformPlugin* fl_platform_plugin_new(FlBinaryMessenger* messenger) {
fl_method_channel_new(messenger, kChannelName, FL_METHOD_CODEC(codec));
fl_method_channel_set_method_call_handler(self->channel, method_call_cb, self,
nullptr);
self->app_initialization_complete = FALSE;

return self;
}
Expand Down
5 changes: 5 additions & 0 deletions shell/platform/linux/fl_platform_plugin.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,11 @@ FlPlatformPlugin* fl_platform_plugin_new(FlBinaryMessenger* messenger);
*
* Request the application exits (i.e. due to the window being requested to be
* closed).
*
* Calling this will only send an exit request to the framework if the framework
* has already indicated that it is ready to receive requests by sending a
* "System.initializationComplete" method call on the platform channel. Calls
* before initialization is complete will result in an immediate exit.
*/
void fl_platform_plugin_request_app_exit(FlPlatformPlugin* plugin);

Expand Down
27 changes: 19 additions & 8 deletions shell/platform/linux/fl_platform_plugin_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,22 +112,33 @@ TEST(FlPlatformPluginTest, ExitApplication) {

g_autoptr(FlPlatformPlugin) plugin = fl_platform_plugin_new(messenger);
EXPECT_NE(plugin, nullptr);

g_autoptr(FlValue) args = fl_value_new_map();
fl_value_set_string_take(args, "type", fl_value_new_string("cancelable"));
g_autoptr(FlJsonMethodCodec) codec = fl_json_method_codec_new();
g_autoptr(GBytes) message = fl_method_codec_encode_method_call(
FL_METHOD_CODEC(codec), "System.exitApplication", args, nullptr);

g_autoptr(FlValue) requestArgs = fl_value_new_map();
fl_value_set_string_take(requestArgs, "type",
g_autoptr(FlValue) null = fl_value_new_null();
ON_CALL(messenger, fl_binary_messenger_send_response(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@robert-ancell In order to respond with a null argument, I had to add this ON_CALL to the test so that the mock would return a success response. Otherwise, because the error information isn't set, and it returns false by default, the test SEGVs when trying to access the error information. Is that expected? It doesn't seem to have any issues in production, but I want to make sure I'm not just putting duct tape on it.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't fully understand the gtest mock code but I do notice this has ::testing::_ as the second last argument and not SuccessResponse(null) as cases in other tests? So I think it's going to be returning an empty response, not a dart null if that's what you expect.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The ::testing::_ is a wildcard, so it just means it will return a true result for a call with anything in that position. It doesn't affect what is returned (which is just a bool).

I added it, but it doesn't really change anything.

::testing::Eq<FlBinaryMessenger*>(messenger),
::testing::_, SuccessResponse(null), ::testing::_))
.WillByDefault(testing::Return(TRUE));

// Indicate that the binding is initialized.
g_autoptr(GError) error = nullptr;
g_autoptr(GBytes) init_message = fl_method_codec_encode_method_call(
FL_METHOD_CODEC(codec), "System.initializationComplete", nullptr, &error);
messenger.ReceiveMessage("flutter/platform", init_message);

g_autoptr(FlValue) request_args = fl_value_new_map();
fl_value_set_string_take(request_args, "type",
fl_value_new_string("cancelable"));
EXPECT_CALL(messenger,
fl_binary_messenger_send_on_channel(
::testing::Eq<FlBinaryMessenger*>(messenger),
::testing::StrEq("flutter/platform"),
MethodCall("System.requestAppExit", FlValueEq(requestArgs)),
MethodCall("System.requestAppExit", FlValueEq(request_args)),
::testing::_, ::testing::_, ::testing::_));

g_autoptr(FlValue) args = fl_value_new_map();
fl_value_set_string_take(args, "type", fl_value_new_string("cancelable"));
g_autoptr(GBytes) message = fl_method_codec_encode_method_call(
FL_METHOD_CODEC(codec), "System.exitApplication", args, nullptr);
messenger.ReceiveMessage("flutter/platform", message);
}