FlutterFragment predictive back #52302

justinmc · 2024-04-22T20:59:45Z

Following up on #39208 and #42789, which added predictive back support for FlutterActivity.

My test app for this is: https://github.com/justinmc/flutter-add-to-app

I tried this both with the Flutter app as the main activity, and with it as a secondary activity after an initial Android route. Predictive back worked in both cases. However, this PR does not include support for in-app route transitions in Flutter.

Related: flutter/flutter#109558, and my comment flutter/flutter#109558 (comment).

b/242216228

math1man · 2024-04-23T14:14:38Z

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

+      // can be changed by calling setFrameworkHandlesBack. For example, the
+      // framework will call this automatically in a typical app when it has
+      // routes to pop.
+      onBackPressedCallback.setEnabled(false);


You probably also need to update the implementation of popSystemNavigator() to restore the previous state of the callback-enablement, instead of unconditionally re-enabling it on line 1673.

@math1man Thanks for jumping back in here, I've been trying to catch back up on your comments from the last PR. Pushing a fix for that now.

No worries. Honestly, I think besides this comment everything looks good. Happy to look into anything specific you might be concerned about though.

…tate, not assume enabled

…rking

justinmc · 2024-04-24T20:48:25Z

Could @math1man or @reidbaker point me in the right direction for how to test this? Sorry I'm pretty inexperienced here, I'm struggling to even get into a real workflow. It seems like I need to recompile every time I touch the engine code (but not the test code). I'm doing:

$ autoninja -C out/android_debug_unopt                                                                     
$ ./flutter/testing/run_tests.py --type java --java-filter io.flutter.embedding.android.FlutterFragmentTest

But for one, I don't know how to view the output of Log statements I put in the test or in the engine code. They never show up in the test output.

I added some code in FlutterFragmentTest to verify popSystemNavigator is enabling/disabling the callback, but it doesn't seem to fail if I revert the PR's changes, and I'm struggling to understand why not without the benefit of logs.

reidbaker · 2024-04-24T20:56:11Z

Honestly @matanlurey is who I go to for engine help but tomorrow I can take a look.

justinmc · 2024-04-24T23:20:53Z

@camsim99 helped me out on Discord, the answer is System.out.println. That should help me make some more progress on the tests tomorrow. Thanks!

math1man · 2024-04-25T12:57:57Z

Sometimes android.util.Log is better, but in most cases either should work.

justinmc · 2024-04-25T16:47:04Z

Ah thanks, I was using io.flutter.Log, which didn't seem to show up.

justinmc · 2024-05-20T18:18:12Z

@math1man This works in an add-to-app setup when I try it and the tests pass, but it does not include the changes for predictive back route transitions within Flutter apps (#49093). I guess it will be best to do that in a separate PR?

math1man

LGTM

justinmc · 2024-05-23T16:29:20Z

@reidbaker This is ready for a final check when you have a chance.

reidbaker · 2024-05-23T16:30:12Z

Thank you I will get this on my list

justinmc · 2024-05-23T16:30:50Z

Thanks! No rush from me.

reidbaker

Approved but there are some comments on trying to understand this code.

reidbaker · 2024-05-28T14:41:18Z

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

+      // By default, Android handles backs, and predictive back is enabled. This
+      // can be changed by calling setFrameworkHandlesBack. For example, the
+      // framework will call this automatically in a typical app when it has
+      // routes to pop.


I am still confused by this code. It looks like we are on a true branch of ARG_SHOULD_AUTOMATICALLY_HANDLE_ON_BACK_PRESSED.
Is this just saying that onBackPressedCallback.setEnabled should be the opposite of the argument passed to ARG_SHOULD_AUTOMATICALLY_HANDLE_ON_BACK_PRESSED?

I'm going to push a commit to reword this comment to try to make it more clear.

ARG_SHOULD_AUTOMATICALLY_HANDLE_ON_BACK_PRESSED means that predictive back is enabled. onBackPressedCallback enabled means that Flutter is handling back gestures (typically because Flutter has Flutter routes to on its navigation stack). Disabled means that Android is handling back gestures, so it will either pop the Activity or go back to the phone's home screen.

So when the app starts, by default we let Android handle backs. If/when the Flutter app wants to change this, it calls setFrameworkHandlesBack and enables this callback.

reidbaker · 2024-05-28T14:48:12Z

shell/platform/android/test/io/flutter/embedding/android/FlutterFragmentTest.java

@@ -318,8 +320,15 @@ public void itDelegatesOnBackPressedAutomaticallyWhenEnabled() {
    TestDelegateFactory delegateFactory = new TestDelegateFactory(mockDelegate);
    fragment.setDelegateFactory(delegateFactory);

+    // Calling onBackPressed now will still be handled by Android (the default),


reading this test shouldn't the back pressed be handled by the framework because of the code on 306?

My comment above should help explain this a bit. shouldAutomaticallyHandleOnBackPressed enables predictive back, while setFrameworkHandlesBack determines whether Android or the framework handle the back gesture.

This reverts commit c364b29.

…sions) (#149220) Manual roll requested by jacksongardner@google.com flutter/engine@d032390...b26e1b0 2024-05-28 34871572+gmackall@users.noreply.github.com Manual revert of #53001 (flutter/engine#53075) 2024-05-28 chinmaygarde@google.com Remove --ios-cpu flag. Only the arm64 variant is supported. (flutter/engine#53044) 2024-05-28 skia-flutter-autoroll@skia.org Roll Skia from 229d94a8807e to ac454b80130c (1 revision) (flutter/engine#53074) 2024-05-28 jonahwilliams@google.com [Impeller] make strokes slightly lighter. (flutter/engine#53067) 2024-05-28 skia-flutter-autoroll@skia.org Roll Skia from 23ddbb590e44 to 229d94a8807e (2 revisions) (flutter/engine#53071) 2024-05-28 jmccandless@google.com FlutterFragment predictive back (flutter/engine#52302) 2024-05-28 skia-flutter-autoroll@skia.org Roll Skia from 02c359cf8233 to 23ddbb590e44 (2 revisions) (flutter/engine#53070) 2024-05-28 skia-flutter-autoroll@skia.org Roll Skia from 4f91b3865441 to 02c359cf8233 (1 revision) (flutter/engine#53069) 2024-05-28 30870216+gaaclarke@users.noreply.github.com [Impeller] shrunk the buffer for the rrect_blur (flutter/engine#53068) 2024-05-28 skia-flutter-autoroll@skia.org Roll Skia from 91cd2b48377a to 4f91b3865441 (4 revisions) (flutter/engine#53066) 2024-05-28 34871572+gmackall@users.noreply.github.com Upgrade all[most] androidx dependencies to latest (flutter/engine#53001) 2024-05-28 skia-flutter-autoroll@skia.org Roll Skia from 0c2c490021b7 to 91cd2b48377a (3 revisions) (flutter/engine#53065) 2024-05-28 skia-flutter-autoroll@skia.org Roll Skia from 545203f95d4e to 0c2c490021b7 (2 revisions) (flutter/engine#53063) 2024-05-28 skia-flutter-autoroll@skia.org Roll Skia from 74b4d97be6ab to 545203f95d4e (1 revision) (flutter/engine#53062) 2024-05-28 skia-flutter-autoroll@skia.org Roll Skia from 848d9498fd68 to 74b4d97be6ab (1 revision) (flutter/engine#53061) 2024-05-28 johnniwinther@google.com Remove use of --nnbd-agnostic (flutter/engine#53055) 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

justinmc added 3 commits August 18, 2023 15:15

Seems to work whether Flutter is the first acitivty or not

89629fb

Work with original onBackPressed mechanics

bf4e1ec

Merge branch 'main' into flutter-fragment-predictive-back

c396216

github-actions bot added the platform-android label Apr 22, 2024

justinmc mentioned this pull request Apr 22, 2024

FlutterFragment predictive back #44865

Closed

Fix from math1man review flutter#44865

1ae637d

math1man reviewed Apr 23, 2024

View reviewed changes

justinmc added 3 commits April 23, 2024 16:32

popSystemNavigator should restore onBackPressedCallback to original s…

7cd3383

…tate, not assume enabled

Test that popSystemNavigator preserves enabled state

3733332

An attempt at testing the callback being enabled/disabled, but not wo…

0d95e27

…rking

justinmc requested a review from reidbaker April 24, 2024 20:41

justinmc added 2 commits May 16, 2024 10:54

Merge branch 'main' into flutter-fragment-predictive-back

6d69cfc

Fix flipped logic, and add comments explaining what's going on

c06300a

justinmc marked this pull request as ready for review May 20, 2024 18:15

justinmc requested a review from math1man May 20, 2024 18:15

math1man reviewed May 23, 2024

View reviewed changes

justinmc mentioned this pull request May 23, 2024

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

Open

reidbaker approved these changes May 28, 2024

View reviewed changes

justinmc added 2 commits May 28, 2024 10:44

Merge branch 'main' into flutter-fragment-predictive-back

48765ee

Clarify comment

c785e98

reidbaker approved these changes May 28, 2024

View reviewed changes

Whoops, should have gotten this in the merge commit

1ff7c32

justinmc merged commit c364b29 into flutter:main May 28, 2024
26 checks passed

justinmc deleted the flutter-fragment-predictive-back branch May 28, 2024 19:52

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

Roll Flutter Engine from d0323905fc2f to c364b294c1f3 (11 revisions) flutter/flutter#149213

Closed

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

c364b294c FlutterFragment predictive back (flutter/engine#52302)

fe2e9ec

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

Roll Flutter Engine from d0323905fc2f to dd019f6d111d (12 revisions) flutter/flutter#149215

Closed

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

c364b294c FlutterFragment predictive back (flutter/engine#52302)

9f1afc2

eyebrowsoffire added a commit to eyebrowsoffire/engine that referenced this pull request May 28, 2024

Revert "FlutterFragment predictive back (flutter#52302)"

ac5133b

This reverts commit c364b29.

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

Roll Flutter Engine from d0323905fc2f to 06944ac16662 (13 revisions) flutter/flutter#149217

Closed

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

c364b294c FlutterFragment predictive back (flutter/engine#52302)

5cf9467

engine-flutter-autoroll mentioned this pull request May 29, 2024

Manual roll Flutter Engine from d0323905fc2f to b26e1b023cdb (16 revisions) flutter/flutter#149220

Merged

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

c364b294c FlutterFragment predictive back (flutter/engine#52302)

2b01db6

This was referenced May 29, 2024

Back gesture on Android 12+ kills app rather than backgrounding it flutter/flutter#117061

Open

Switch from onBackPressed to OnBackPressedDispatcher flutter/flutter#109558

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

FlutterFragment predictive back #52302

FlutterFragment predictive back #52302

justinmc commented Apr 22, 2024 •

edited

math1man Apr 23, 2024

justinmc Apr 23, 2024

math1man Apr 24, 2024

justinmc commented Apr 24, 2024

reidbaker commented Apr 24, 2024

justinmc commented Apr 24, 2024

math1man commented Apr 25, 2024

justinmc commented Apr 25, 2024

justinmc commented May 20, 2024

math1man left a comment

justinmc commented May 23, 2024

reidbaker commented May 23, 2024

justinmc commented May 23, 2024

reidbaker left a comment

reidbaker May 28, 2024

justinmc May 28, 2024

reidbaker May 28, 2024

justinmc May 28, 2024

FlutterFragment predictive back #52302

FlutterFragment predictive back #52302

Conversation

justinmc commented Apr 22, 2024 • edited

math1man Apr 23, 2024

Choose a reason for hiding this comment

justinmc Apr 23, 2024

Choose a reason for hiding this comment

math1man Apr 24, 2024

Choose a reason for hiding this comment

justinmc commented Apr 24, 2024

reidbaker commented Apr 24, 2024

justinmc commented Apr 24, 2024

math1man commented Apr 25, 2024

justinmc commented Apr 25, 2024

justinmc commented May 20, 2024

math1man left a comment

Choose a reason for hiding this comment

justinmc commented May 23, 2024

reidbaker commented May 23, 2024

justinmc commented May 23, 2024

reidbaker left a comment

Choose a reason for hiding this comment

reidbaker May 28, 2024

Choose a reason for hiding this comment

justinmc May 28, 2024

Choose a reason for hiding this comment

reidbaker May 28, 2024

Choose a reason for hiding this comment

justinmc May 28, 2024

Choose a reason for hiding this comment

justinmc commented Apr 22, 2024 •

edited