Platform channel for predictive back in route transitions on android #49093

maRci002 · 2023-12-15T17:22:53Z

This pull request introduces platform channel support for predictive back route transitions on Android, as part of the ongoing efforts in flutter/flutter#131961.

For Android SDK 33: Utilizes OnBackInvokedCallback to handle back navigation events (Platform channel for predictive back #39208).
For Android SDK 34 and above: Employs OnBackAnimationCallback, taking advantage of the enhanced back navigation capabilities introduced in this version.

In both scenarios, the callbacks are appropriately forwarded to the Flutter framework.
This PR focuses on the engine part, setting the stage for integrating these enhancements into the broader Flutter ecosystem.

References

Part of flutter/flutter#131961
Framework PR: flutter/flutter#141373
Fixes flutter/flutter#111295

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

camsim99

This LGTM overall just left an idea for a test and some nits!

shell/platform/android/io/flutter/embedding/engine/systemchannels/NavigationChannel.java

shell/platform/android/io/flutter/embedding/android/FlutterActivity.java

justinmc

@maRci002 You are a hero for implementing this, thank you for the PR.

Have you tried tying into these methods on the framework side and seeing what happens? I'd like to verify that it is going to meet our needs on the framework before merging this PR. I added some requirements to the description in flutter/flutter#131961 that I'd like to verify are possible.

You mentioned in flutter/flutter#131961 (comment) that you might want to work on the framework side. Feel free to open a framework PR (even a draft) if so. We don't have to hit all of those requirements in the first PR, I just want to make sure we've got the API right so that they're possible.

maRci002 · 2024-01-11T14:56:02Z

Thanks, @justinmc. I've just created the PR for the framework part flutter/flutter#141373

jonahwilliams · 2024-01-25T21:18:22Z

What is the status of this PR, is it waiting on review comments or framework changes? Friendly ping @justinmc / @maRci002

maRci002 · 2024-01-25T22:01:42Z

In my opinion, this PR is ready for further review. However, I've encountered some issues with testing, particularly related to the @Config annotation. I'm currently awaiting some inputs from @camsim99 to resolve these concerns. Once these are addressed, I believe the PR will be in good shape for final review.

justinmc · 2024-01-25T23:24:25Z

I owe a major review on this, sorry for the delay. I've been struggling to get my engine development set up on a new machine...

camsim99 · 2024-02-05T16:59:53Z

@maRci002 Looks like tests are passing! Is this ready for re-review?

justinmc

Sorry for such a long delay in reviewing this. I was able to get it running locally and it worked great for me! I think overall this approach looks good.

My main concern is any possible breakages due to no longer calling onBackPressed. See my comment on that below.

I'll also leave a review on the framework PR in a bit.

justinmc · 2024-02-05T21:33:48Z

shell/platform/android/io/flutter/embedding/android/FlutterActivity.java

-          ? new OnBackInvokedCallback() {
-            // TODO(garyq): Remove SuppressWarnings annotation. This was added to workaround
-            // a google3 bug where the linter is not properly running against API 33, causing
-            // a failure here. See b/243609613 and https://github.com/flutter/flutter/issues/111295


I've edited the PR description to say "Fixes flutter/flutter#111295". It looks like that Google bug is closed now, so I think it should be fine to remove the annotation like you did.

shell/platform/android/io/flutter/embedding/android/FlutterActivityAndFragmentDelegate.java

justinmc · 2024-02-05T22:49:27Z

shell/platform/android/io/flutter/embedding/android/FlutterActivity.java

+  @TargetApi(33)
+  @RequiresApi(33)
+  private OnBackInvokedCallback createOnBackInvokedCallback() {
+    if (Build.VERSION.SDK_INT >= 34) {


So for API >= 34, popping will be done by the framework via commitBackGesture. For API <= 33, the framework will handle it by listening to popRoute called by onBackPressed as usual.

Could there be any situations where users that are expecting onBackPressed won't get it and won't be listening for commitBackGesture? Like what if someone has a custom Android route transition and they don't know about these new changes?

Ok at the route transition level it won't matter because either way it will just result in a pop. But I wonder if this could mess with users at any other level...

But I wonder if this could mess with users at any other level...

If someone directly sets their own method for SystemChannels.navigation.setMethodCallHandler, it might cause a problem. However, using the recommended WidgetsBindingObserver to register their own handlers shouldn't be a problem, since the handleCommitBackGesture method might delegate work to the handlePopRoute method if it's not handled by registered observers.

shell/platform/android/io/flutter/embedding/android/FlutterActivityAndFragmentDelegate.java

maRci002 · 2024-02-06T14:34:22Z

@maRci002 Looks like tests are passing! Is this ready for re-review?

@camsim99, thanks, but the API might change over time. I will definitely let you know when it's ready for review from an Android perspective.

camsim99

Looks great! Just left an optional nit on documentation

camsim99 · 2024-03-18T16:08:37Z

shell/platform/android/io/flutter/embedding/android/FlutterActivityAndFragmentDelegate.java

+   * Invoke this from {@link OnBackAnimationCallback#onBackStarted(BackEvent)}.
+   *
+   * <p>This method should be called when the back gesture is initiated, as part of the
+   * implementation of {@link OnBackAnimationCallback}. It's responsible for handling the initial


Nit: can this doc is some way clarify that what it's responsible for is defined in the framework?

Same nit for the other methods you added here

flutter-dashboard · 2024-03-19T23:56:59Z

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 #49093 at sha 7bcd50f

shell/platform/android/io/flutter/embedding/engine/systemchannels/BackGestureChannel.java

justinmc · 2024-03-20T22:05:04Z

Thanks for the quick turnaround on that!

…n android (flutter/engine#49093)

…145877) flutter/engine@c602abd...922c7b1 2024-03-28 skia-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from Lk8KBU-c97ROj-YHm... to uzI3wnbEGlZ_dtO0Z... (flutter/engine#51737) 2024-03-28 skia-flutter-autoroll@skia.org Roll Skia from b6160ffc0b96 to 66241be7c81c (16 revisions) (flutter/engine#51733) 2024-03-28 bdero@google.com [Impeller] Reland: Use the scissor to limit all draws by clip coverage. (flutter/engine#51731) 2024-03-27 30870216+gaaclarke@users.noreply.github.com [Impeller] adds a `plus` advanced blend for f16 pixel formats (flutter/engine#51589) 2024-03-27 maRci002@users.noreply.github.com Platform channel for predictive back in route transitions on android (flutter/engine#49093) 2024-03-27 jonahwilliams@google.com [Impeller] render empty filled paths without crashing. (flutter/engine#51713) 2024-03-27 jonahwilliams@google.com [Impeller] use optimal depth attachment, remove useless barrier. (flutter/engine#51723) Also rolling transitive DEPS: fuchsia/sdk/core/linux-amd64 from Lk8KBU-c97RO to uzI3wnbEGlZ_ 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 jacksongardner@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://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

…isions) (#145877)" (#145901) Reverts: #145877 Initiated by: zanderso Reason for reverting: #145899 Original PR Author: engine-flutter-autoroll Reviewed By: {fluttergithubbot} This change reverts the following previous change: flutter/engine@c602abd...922c7b1 2024-03-28 skia-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from Lk8KBU-c97ROj-YHm... to uzI3wnbEGlZ_dtO0Z... (flutter/engine#51737) 2024-03-28 skia-flutter-autoroll@skia.org Roll Skia from b6160ffc0b96 to 66241be7c81c (16 revisions) (flutter/engine#51733) 2024-03-28 bdero@google.com [Impeller] Reland: Use the scissor to limit all draws by clip coverage. (flutter/engine#51731) 2024-03-27 30870216+gaaclarke@users.noreply.github.com [Impeller] adds a `plus` advanced blend for f16 pixel formats (flutter/engine#51589) 2024-03-27 maRci002@users.noreply.github.com Platform channel for predictive back in route transitions on android (flutter/engine#49093) 2024-03-27 jonahwilliams@google.com [Impeller] render empty filled paths without crashing. (flutter/engine#51713) 2024-03-27 jonahwilliams@google.com [Impeller] use optimal depth attachment, remove useless barrier. (flutter/engine#51723) Also rolling transitive DEPS: fuchsia/sdk/core/linux-amd64 from Lk8KBU-c97RO to uzI3wnbEGlZ_ 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 jacksongardner@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://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

…n android (flutter/engine#49093)

…sions) (#145903) Manual roll requested by zra@google.com flutter/engine@c602abd...9df2d3a 2024-03-28 98614782+auto-submit[bot]@users.noreply.github.com Reverts "[Impeller] adds a `plus` advanced blend for f16 pixel formats (#51589)" (flutter/engine#51741) 2024-03-28 jonahwilliams@google.com [Impeller] correct multisample resolve/store configuration for Vulkan. (flutter/engine#51740) 2024-03-28 matanlurey@users.noreply.github.com Remove Impeller/OpenGLES from CI branch for Android e2e tests. (flutter/engine#51734) 2024-03-28 skia-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from Lk8KBU-c97ROj-YHm... to uzI3wnbEGlZ_dtO0Z... (flutter/engine#51737) 2024-03-28 skia-flutter-autoroll@skia.org Roll Skia from b6160ffc0b96 to 66241be7c81c (16 revisions) (flutter/engine#51733) 2024-03-28 bdero@google.com [Impeller] Reland: Use the scissor to limit all draws by clip coverage. (flutter/engine#51731) 2024-03-27 30870216+gaaclarke@users.noreply.github.com [Impeller] adds a `plus` advanced blend for f16 pixel formats (flutter/engine#51589) 2024-03-27 maRci002@users.noreply.github.com Platform channel for predictive back in route transitions on android (flutter/engine#49093) 2024-03-27 jonahwilliams@google.com [Impeller] render empty filled paths without crashing. (flutter/engine#51713) 2024-03-27 jonahwilliams@google.com [Impeller] use optimal depth attachment, remove useless barrier. (flutter/engine#51723) Also rolling transitive DEPS: fuchsia/sdk/core/linux-amd64 from Lk8KBU-c97RO to uzI3wnbEGlZ_ 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 jacksongardner@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://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

timbotimbo · 2024-07-14T17:26:02Z

Any idea how these changes can somehow causeOnBackInvokedCallback to be invoked on a different platformview, causing an exception on Android < 13?
flutter/flutter#151733

Apologies if this is unwanted here. I simply traced my exception back to this change and I'm seriously out of my depth here.

github-actions bot added the platform-android label Dec 15, 2023

maRci002 marked this pull request as ready for review December 15, 2023 22:39

maRci002 force-pushed the predictive-back-route branch from 1353dcd to e79dace Compare December 16, 2023 17:23

maRci002 mentioned this pull request Dec 20, 2023

Android predictive back route transitions flutter/flutter#131961

Closed

2 tasks

mossmana requested review from HansMuller and a team December 21, 2023 19:33

camsim99 reviewed Jan 8, 2024

View reviewed changes

camsim99 requested a review from justinmc January 8, 2024 17:42

justinmc reviewed Jan 8, 2024

View reviewed changes

justinmc mentioned this pull request Jan 8, 2024

Predictive back gesture Android flutter/flutter#132504

Closed

2 tasks

maRci002 mentioned this pull request Jan 11, 2024

Predictive back support for routes flutter/flutter#141373

Merged

8 tasks

maRci002 requested review from justinmc and camsim99 January 11, 2024 14:56

maRci002 force-pushed the predictive-back-route branch 3 times, most recently from acc36fd to d9033d8 Compare January 23, 2024 15:23

maRci002 force-pushed the predictive-back-route branch 3 times, most recently from de5ba9b to 92730fe Compare January 30, 2024 23:18

HansMuller removed their request for review January 30, 2024 23:19

maRci002 force-pushed the predictive-back-route branch from 92730fe to 99e31b4 Compare January 31, 2024 00:16

justinmc mentioned this pull request Feb 5, 2024

Remove @SuppressWarning workaround in FlutterActivity flutter/flutter#111295

Closed

justinmc reviewed Feb 5, 2024

View reviewed changes

chinmaygarde added the Work in progress (WIP) Not ready (yet) for review! label Feb 15, 2024

justinmc mentioned this pull request Mar 14, 2024

(WIP) Predictive back OpenContainer transition flutter/packages#6321

Draft

camsim99 approved these changes Mar 18, 2024

View reviewed changes

flutter-dashboard bot added the will affect goldens label Mar 19, 2024

justinmc reviewed Mar 20, 2024

View reviewed changes

shell/platform/android/io/flutter/embedding/engine/systemchannels/BackGestureChannel.java Outdated Show resolved Hide resolved

justinmc mentioned this pull request Mar 26, 2024

FlutterFragment predictive back #44865

Closed

maRci002 added 6 commits March 27, 2024 08:44

API 34+ use OnBackAnimationCallback

7edfa84

rename BackEvent touchX -> x and touchY -> y

5ddbce4

update doc

4941bae

rename method channel to lowercase

6e42194

use StandardMethodCodec / Offset

9392932

format

0caacd8

maRci002 force-pushed the predictive-back-route branch from 5934345 to 0caacd8 Compare March 27, 2024 07:51

justinmc merged commit 00b0905 into flutter:main Mar 27, 2024
25 checks passed

engine-flutter-autoroll mentioned this pull request Mar 28, 2024

Roll Flutter Engine from c602abdbae16 to 922c7b133bc2 (7 revisions) flutter/flutter#145877

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Mar 28, 2024

00b0905af Platform channel for predictive back in route transitions o…

8c6ae4f

…n android (flutter/engine#49093)

auto-submit bot mentioned this pull request Mar 28, 2024

Reverts "Roll Flutter Engine from c602abdbae16 to 922c7b133bc2 (7 revisions) (#145877)" flutter/flutter#145901

Merged

engine-flutter-autoroll mentioned this pull request Mar 28, 2024

Manual roll Flutter Engine from c602abdbae16 to 9df2d3a0778e (10 revisions) flutter/flutter#145903

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Mar 28, 2024

00b0905af Platform channel for predictive back in route transitions o…

36e245f

…n android (flutter/engine#49093)

This was referenced May 20, 2024

FlutterFragment predictive back #52302

Merged

Add-to-app should support predictive back route transitions flutter/flutter#148997

Open

justinmc mentioned this pull request Jun 5, 2024

FlutterFragmentActivity support for predictive back flutter/flutter#149753

Open

timbotimbo mentioned this pull request Jul 14, 2024

[Android] Flutter 3.22 - Predictive back implementation causes exception in platform view on Android < 13 (api level 33) flutter/flutter#151733

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Platform channel for predictive back in route transitions on android #49093

Platform channel for predictive back in route transitions on android #49093

maRci002 commented Dec 15, 2023 •

edited by justinmc

Loading

camsim99 left a comment

justinmc left a comment

maRci002 commented Jan 11, 2024

jonahwilliams commented Jan 25, 2024

maRci002 commented Jan 25, 2024

justinmc commented Jan 25, 2024

camsim99 commented Feb 5, 2024

justinmc left a comment

justinmc Feb 5, 2024

justinmc Feb 5, 2024

justinmc Feb 5, 2024

maRci002 Feb 6, 2024

maRci002 commented Feb 6, 2024

camsim99 left a comment

camsim99 Mar 18, 2024

camsim99 Mar 18, 2024

flutter-dashboard bot commented Mar 19, 2024

justinmc commented Mar 20, 2024

timbotimbo commented Jul 14, 2024

Platform channel for predictive back in route transitions on android #49093

Platform channel for predictive back in route transitions on android #49093

Conversation

maRci002 commented Dec 15, 2023 • edited by justinmc Loading

References

Pre-launch Checklist

camsim99 left a comment

Choose a reason for hiding this comment

justinmc left a comment

Choose a reason for hiding this comment

maRci002 commented Jan 11, 2024

jonahwilliams commented Jan 25, 2024

maRci002 commented Jan 25, 2024

justinmc commented Jan 25, 2024

camsim99 commented Feb 5, 2024

justinmc left a comment

Choose a reason for hiding this comment

justinmc Feb 5, 2024

Choose a reason for hiding this comment

justinmc Feb 5, 2024

Choose a reason for hiding this comment

justinmc Feb 5, 2024

Choose a reason for hiding this comment

maRci002 Feb 6, 2024

Choose a reason for hiding this comment

maRci002 commented Feb 6, 2024

camsim99 left a comment

Choose a reason for hiding this comment

camsim99 Mar 18, 2024

Choose a reason for hiding this comment

camsim99 Mar 18, 2024

Choose a reason for hiding this comment

flutter-dashboard bot commented Mar 19, 2024

justinmc commented Mar 20, 2024

timbotimbo commented Jul 14, 2024

maRci002 commented Dec 15, 2023 •

edited by justinmc

Loading