Capture additional final inset states in ImeSyncDeferringInsetsCallback #42700
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
gmackall
changed the title
|
@jason-simmons I requested your review as you had made the most changes to this callback, but let me know if you're not the right person to review! I've also reached out on #hackers-engine to see if anyone there knows who best to review |
jason-simmons
approved these changes
Jun 12, 2023
gmackall
added
the
autosubmit
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Jun 13, 2023
auto-submit bot
pushed a commit
to flutter/flutter
that referenced
this pull request
Jun 14, 2023
…128813) flutter/engine@b6bf3a6...66a5761 2023-06-13 34871572+gmackall@users.noreply.github.com Capture additional final inset states in ImeSyncDeferringInsetsCallback (flutter/engine#42700) 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 chinmaygarde@google.com,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
auto-submit bot
pushed a commit
that referenced
this pull request
Jun 23, 2023
…hind capturing the latest final inset state (#43109) Solely documentation change to explain the reasoning behind the change in #42700. [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes flutter/flutter#118761.
For some context: ImeSyncDeferringInsetsCallback extends WindowInsetsAnimation.Callback specifically to handle insets related to keyboard animations.
When a keyboard animation happens, we get a call to onPrepare with the animation, and then to onApplyWindowInsets with WindowInsets that represent the post animation state. For example, for hiding the keyboard, we would get WindowInsets with height of 0 for insets of type WindowInsets.Type.ime() (input method editor). We save these post-animation insets.
We then get calls to onProgress for each frame of the animation, and use the WindowInsets passed to onProgress to build new WindowInsets based on the saved post-animation insets, but with the keyboard insets overridden by the WindowInsets passed to onProgress.
Finally, we get a call to onEnd, where we apply the saved insets.
However, there can be multiple animation running at the same time. For example, dismissing the keyboard can result in an animation associated solely with the keyboard happening, as well as an animation associated only with the navigation bar happening. And they can start at slightly different times. This can result in the situation described in this comment: flutter/flutter#118761 (comment)
Re-setting needsSave to false ensures we don't ignore any updates to the final state, ensuring correct insets in onProgress and onEnd.
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.