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

Conversation

@jiahaog
Copy link
Member

@jiahaog jiahaog commented Sep 16, 2022

Relands #34363 which was reverted in #36091 with fix.

Fix flutter/flutter#110867. The fix is in 6cf5494; the parent commit is a pure revert of the revert.

Explanation of flutter/flutter#110867

Some user might write Dart code like the following to keep the app in full screen mode even after this might be changed in response to certain user actions. There is no built in time out or gesture that will restore the full screen mode the app was in. (Thanks to @Piinks for helping me understand this offline)

SystemChrome.setSystemUIChangeCallback((systemOverlaysAreVisible) async {
  SystemChrome.restoreSystemUIOverlays();
});

This is what happens:

  1. User swipes up or does some gesture which causes the system UI mode to change
  2. OnSystemUiVisibilityChangeListener receives a notification of the change
  3. It propagates a platform message to Dart; the Dart callback defined in setSystemUIChangeCallback (above) is called
  4. SystemChrome.restoreSystemUIOverlays() is called in the callback, sending a platform message to the host
  5. Platform message is scheduled on the host; resulting in a call to setSystemUiVisibility.

1-5 all occur in the same frame. Because of this, OnSystemUiVisibilityChangeListener will silently stop responding to future system UI notifications.

#34363 introduces the behaviour in bold; previously using new Handler instead of Handler.createAsync will always cause platform messages to be scheduled in the next frame, such as in (5).

This PR causes (3) to be scheduled in a subsequent frame, avoiding the issue.

Pre-launch Checklist

  • I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
  • I read the [Tree Hygiene] wiki page, which explains my responsibilities.
  • I read and followed the [Flutter Style Guide] and the [C++, Objective-C, Java style guides].
  • I listed at least one issue that this PR fixes in the description above.
  • I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test-exempt. See [testing the engine] for instructions on
    writing and running engine tests.
  • I updated/added relevant documentation (doc comments with ///).
  • I signed the [CLA].
  • All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on [Discord].

@jiahaog jiahaog marked this pull request as ready for review September 16, 2022 07:38
@jiahaog jiahaog requested a review from dnfield September 16, 2022 07:38
Copy link
Contributor

@dnfield dnfield left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for digging into this!

@dnfield dnfield added the autosubmit Merge PR when tree becomes green via auto submit App label Sep 16, 2022
@auto-submit auto-submit bot merged commit 8f76f17 into flutter:main Sep 16, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 16, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 16, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 16, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 16, 2022
@jiahaog jiahaog deleted the systemchrome-fix branch September 16, 2022 23:56
Oleh-Sv pushed a commit to Oleh-Sv/engine that referenced this pull request Sep 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

autosubmit Merge PR when tree becomes green via auto submit App platform-android

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Android SystemChrome not going away if app is left by a swipe up from the bottom

2 participants