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

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

Merged
merged 3 commits into from
May 16, 2023

Conversation

gspencergoog
Copy link
Contributor

@gspencergoog gspencergoog commented May 4, 2023

Description

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.

Related PRs

Related Issues

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.

Copy link
Member

@loic-sharma loic-sharma left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@cbracken cbracken left a comment

Choose a reason for hiding this comment

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

lgtm!

@gspencergoog gspencergoog changed the title Wait for binding to be ready before requesting exits from framework [macOS] Wait for binding to be ready before requesting exits from framework May 5, 2023
@chinmaygarde
Copy link
Member

Can we rebase and land this before more conflicts pile up?

@gspencergoog
Copy link
Contributor Author

@chinmaygarde I'll fix the conflicts, but this can't land until the associated flutter/flutter change (flutter/flutter#126075) lands.

@gspencergoog gspencergoog added the autosubmit Merge PR when tree becomes green via auto submit App label May 16, 2023
@flutter-dashboard
Copy link

Golden file changes have been found for this pull request. Click here to view and triage (e.g. because this is an intentional change).

If you are still iterating on this change and are not ready to resolve the images on the Flutter Gold dashboard, consider marking this PR as a draft pull request above. You will still be able to view image results on the dashboard, commenting will be silenced, and the check will not try to resolve itself until marked ready for review.

Changes reported for pull request #41753 at sha 917809f

auto-submit bot pushed a commit that referenced this pull request May 16, 2023
…mework (#41782)

## 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
 - #41733
 - #41753

## Related Issues
 - flutter/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.
@gspencergoog
Copy link
Contributor Author

I think the Gold changes are hallucinations. There are no diffs in the check link.

@gspencergoog gspencergoog merged commit 49b4317 into flutter:main May 16, 2023
@gspencergoog gspencergoog deleted the fix_macos_exit branch May 16, 2023 21:29
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 16, 2023
zanderso pushed a commit to flutter/flutter that referenced this pull request May 16, 2023
…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
CaseyHillers pushed a commit to CaseyHillers/flutter that referenced this pull request May 24, 2023
…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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
autosubmit Merge PR when tree becomes green via auto submit App platform-macos
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants