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

gspencergoog · 2023-05-05T19:17:25Z

Description

Similar to #41733 and #41753 this causes the linux embedding to wait until it hears that the scheduler binding has registered itself before proceeding to send termination requests to the framework.

This allows applications that don't use the framework (just use dart:ui directly) to exit automatically when the last window is closed. Without this change, the app does not exit when the window is closed.

Depends on framework PR flutter/flutter#126075 landing first.

Related PRs

Only register top level window message listener upon registering service binding #41733
[macOS] Wait for binding to be ready before requesting exits from framework #41753

Related Issues

[Desktop] Cannot close app if using dart:ui directly flutter#126033.

Tests

Added a test to make sure that it doesn't send a termination request if the binding hasn't notified that it is ready yet.

cbracken

lgtm!

shell/platform/linux/fl_platform_plugin.cc

loic-sharma

Nice!

shell/platform/linux/fl_platform_plugin.cc

gspencergoog · 2023-05-08T22:51:21Z

shell/platform/linux/fl_platform_plugin_test.cc


-  g_autoptr(FlValue) requestArgs = fl_value_new_map();
-  fl_value_set_string_take(requestArgs, "type",
+  ON_CALL(messenger, fl_binary_messenger_send_response(


@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.

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.

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.

robert-ancell

lgtm

Co-authored-by: Loïc Sharma <737941+loic-sharma@users.noreply.github.com>

…mework (#41753) Similar to #41733 and #41782, this causes the macos embedding to wait until it hears that the scheduler binding has registered itself before proceeding to send termination requests to the framework. This allows applications that don't use the framework (just use `dart:ui` directly) to exit automatically when the last window is closed. Without this change, the last window closes, but the app does not exit. Depends on framework PR flutter/flutter#126075 landing first.

…s from framework (flutter/engine#41782)

…sions) (#126961) Manual roll requested by zra@google.com flutter/engine@fe24767...1c775e3 2023-05-16 zanderso@users.noreply.github.com Revert "[ios_platform_view] only recycle maskView when the view is applying mutators" (flutter/engine#42080) 2023-05-16 gspencergoog@users.noreply.github.com [macOS] Wait for binding to be ready before requesting exits from framework (flutter/engine#41753) 2023-05-16 gspencergoog@users.noreply.github.com [linux] Wait for binding to be ready before requesting exits from framework (flutter/engine#41782) 2023-05-16 jacksongardner@google.com Initial support for images in Skwasm (flutter/engine#42019) 2023-05-16 jacksongardner@google.com Use new `unresolvedCodePoints` API from skia. (flutter/engine#41991) 2023-05-16 jason-simmons@users.noreply.github.com Convert public API NativeFieldWrapper classes to abstract interfaces (flutter/engine#41945) 2023-05-16 737941+loic-sharma@users.noreply.github.com [Windows] Add force redraw to the C++ client wrapper (flutter/engine#42061) 2023-05-16 godofredoc@google.com Fix drone_dimension host_engine_builder. (flutter/engine#42068) 2023-05-16 godofredoc@google.com Add linux_clang_tidy builder. (flutter/engine#41990) 2023-05-16 ychris@google.com [ios_platform_view] only recycle maskView when the view is applying mutators (flutter/engine#41573) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Please CC rmistry@google.com,zra@google.com on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

…sions) (flutter#126961) Manual roll requested by zra@google.com flutter/engine@fe24767...1c775e3 2023-05-16 zanderso@users.noreply.github.com Revert "[ios_platform_view] only recycle maskView when the view is applying mutators" (flutter/engine#42080) 2023-05-16 gspencergoog@users.noreply.github.com [macOS] Wait for binding to be ready before requesting exits from framework (flutter/engine#41753) 2023-05-16 gspencergoog@users.noreply.github.com [linux] Wait for binding to be ready before requesting exits from framework (flutter/engine#41782) 2023-05-16 jacksongardner@google.com Initial support for images in Skwasm (flutter/engine#42019) 2023-05-16 jacksongardner@google.com Use new `unresolvedCodePoints` API from skia. (flutter/engine#41991) 2023-05-16 jason-simmons@users.noreply.github.com Convert public API NativeFieldWrapper classes to abstract interfaces (flutter/engine#41945) 2023-05-16 737941+loic-sharma@users.noreply.github.com [Windows] Add force redraw to the C++ client wrapper (flutter/engine#42061) 2023-05-16 godofredoc@google.com Fix drone_dimension host_engine_builder. (flutter/engine#42068) 2023-05-16 godofredoc@google.com Add linux_clang_tidy builder. (flutter/engine#41990) 2023-05-16 ychris@google.com [ios_platform_view] only recycle maskView when the view is applying mutators (flutter/engine#41573) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Please CC rmistry@google.com,zra@google.com on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

flutter-dashboard bot added the platform-linux label May 5, 2023

gspencergoog mentioned this pull request May 5, 2023

[macOS] Wait for binding to be ready before requesting exits from framework #41753

Merged

gspencergoog requested review from cbracken and robert-ancell May 5, 2023 19:17

cbracken approved these changes May 5, 2023

View reviewed changes

loic-sharma reviewed May 5, 2023

View reviewed changes

shell/platform/linux/fl_platform_plugin.cc Outdated Show resolved Hide resolved

loic-sharma approved these changes May 5, 2023

View reviewed changes

robert-ancell reviewed May 8, 2023

View reviewed changes

shell/platform/linux/fl_platform_plugin.cc Outdated Show resolved Hide resolved

gspencergoog force-pushed the fix_linux_exit branch from d652f34 to 427fe21 Compare May 8, 2023 22:38

gspencergoog commented May 8, 2023

View reviewed changes

robert-ancell approved these changes May 10, 2023

View reviewed changes

gspencergoog and others added 6 commits May 16, 2023 12:39

Only accept exit requests after the app is initialized

02144b9

Just don't request an exit if not ready

a1dc40e

Add test

92c8298

Update shell/platform/linux/fl_platform_plugin.cc

a063dd8

Co-authored-by: Loïc Sharma <737941+loic-sharma@users.noreply.github.com>

Send nullptr return value

149bb4a

Fix test

d7ee743

gspencergoog force-pushed the fix_linux_exit branch 2 times, most recently from 8100ef1 to 6ccbdc5 Compare May 16, 2023 19:42

Add SuccessResponse to ON_CALL

c13eed2

gspencergoog force-pushed the fix_linux_exit branch from 6ccbdc5 to c13eed2 Compare May 16, 2023 19:47

gspencergoog added the autosubmit Merge PR when tree becomes green via auto submit App label May 16, 2023

auto-submit bot merged commit a49138e into flutter:main May 16, 2023

engine-flutter-autoroll mentioned this pull request May 16, 2023

Manual roll Flutter Engine from fe2476743b59 to 1c775e34e2d5 (10 revisions) flutter/flutter#126961

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 16, 2023

a49138ea3 [linux] Wait for binding to be ready before requesting exit…

3e24d61

…s from framework (flutter/engine#41782)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

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

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

Uh oh!

gspencergoog commented May 5, 2023 •

edited

Loading

Uh oh!

cbracken left a comment

Uh oh!

Uh oh!

loic-sharma left a comment

Uh oh!

Uh oh!

gspencergoog May 8, 2023

Uh oh!

robert-ancell May 9, 2023

Uh oh!

gspencergoog May 9, 2023

Uh oh!

robert-ancell left a comment

Uh oh!

Uh oh!

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

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

Uh oh!

Conversation

gspencergoog commented May 5, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related PRs

Related Issues

Tests

Uh oh!

cbracken left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

loic-sharma left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

gspencergoog May 8, 2023

Choose a reason for hiding this comment

Uh oh!

robert-ancell May 9, 2023

Choose a reason for hiding this comment

Uh oh!

gspencergoog May 9, 2023

Choose a reason for hiding this comment

Uh oh!

robert-ancell left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

gspencergoog commented May 5, 2023 •

edited

Loading