From 7d23780c7f3b5eca9f07c0b974e6c19bb37c38c1 Mon Sep 17 00:00:00 2001 From: Gray Mackall <34871572+gmackall@users.noreply.github.com> Date: Fri, 21 Feb 2025 14:22:24 -0800 Subject: [PATCH] [remake] Restore old back handling for FlutterFragmentActivity (#161545) Remake of https://github.com/flutter/engine/pull/56565. Quoting from an old comment, slightly edited: ------------ https://github.com/flutter/engine/pull/52302 seems to have unintentionally had the effect of not allowing people to "opt out" of predictive back. This is actually aligned with what the android docs say should happen: https://developer.android.com/guide/navigation/custom-back/predictive-back-gesture > Note: OnBackPressedCallback is always called regardless of the value of android:enableOnBackInvokedCallback. In other words, disabling the system animation doesn't affect your app's back handling logic if it uses OnBackPressedCallback. But this wasn't actually true for flutter apps before https://github.com/flutter/engine/pull/52302, because we were not calling `super`, and `FlutterFragmentActivity` extends a `FragmentActivity` which in turn extends a `ComponentActivity`, which uses the old `onBackPressed` to [invoke the new back handling](https://cs.android.com/androidx/platform/frameworks/support/+/androidx-main:activity/activity/src/main/java/androidx/activity/ComponentActivity.kt;l=587?q=ComponentActivity): ```kotlin override fun onBackPressed() { onBackPressedDispatcher.onBackPressed() } ``` So while the docs imply that removing the `onBackPressed` in `FlutterFragmentActivity` shouldn't have had an effect, that wasn't true because in our case we were consuming the back event and ignoring the warning ```java @Override @SuppressWarnings("MissingSuperCall") public void onBackPressed() { flutterFragment.onBackPressed(); } ``` What all this means is that apps that _aren't_ opting in to predictive back had their back handling migrated to the new code path automatically. FlutterFragmentActivity was uniquely is forced into the new back handling codepath by https://github.com/flutter/engine/pull/52302, which this PR fixes. ------------ ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [ ] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md --------- Co-authored-by: Gray Mackall --- .../android/FlutterFragmentActivity.java | 9 +++++++++ .../android/FlutterFragmentActivityTest.java | 18 ++++++++++++++++++ 2 files changed, 27 insertions(+) diff --git a/engine/src/flutter/shell/platform/android/io/flutter/embedding/android/FlutterFragmentActivity.java b/engine/src/flutter/shell/platform/android/io/flutter/embedding/android/FlutterFragmentActivity.java index d0562be102..033ea576b1 100644 --- a/engine/src/flutter/shell/platform/android/io/flutter/embedding/android/FlutterFragmentActivity.java +++ b/engine/src/flutter/shell/platform/android/io/flutter/embedding/android/FlutterFragmentActivity.java @@ -620,6 +620,15 @@ public class FlutterFragmentActivity extends FragmentActivity super.onNewIntent(intent); } + // Intentionally missing super call to align FlutterFragmentActivity predictive back behavior + // with FlutterActivity, with respect to how it responds to the + // android:enableOnBackInvokedCallback manifest property. + @Override + @SuppressWarnings("MissingSuperCall") + public void onBackPressed() { + flutterFragment.onBackPressed(); + } + @Override public void onRequestPermissionsResult( int requestCode, @NonNull String[] permissions, @NonNull int[] grantResults) { diff --git a/engine/src/flutter/shell/platform/android/test/io/flutter/embedding/android/FlutterFragmentActivityTest.java b/engine/src/flutter/shell/platform/android/test/io/flutter/embedding/android/FlutterFragmentActivityTest.java index 73274cbce9..f25b134137 100644 --- a/engine/src/flutter/shell/platform/android/test/io/flutter/embedding/android/FlutterFragmentActivityTest.java +++ b/engine/src/flutter/shell/platform/android/test/io/flutter/embedding/android/FlutterFragmentActivityTest.java @@ -13,6 +13,7 @@ import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import android.annotation.TargetApi; @@ -299,6 +300,23 @@ public class FlutterFragmentActivityTest { flutterFragmentActivityActivityScenario.close(); } + @Test + @Config(minSdk = Build.API_LEVELS.API_34) + @TargetApi(Build.API_LEVELS.API_34) + public void onBackPressedCallsGetForwardedToFragment() { + FlutterFragmentActivityWithProvidedEngine activity = + spy(Robolectric.buildActivity(FlutterFragmentActivityWithProvidedEngine.class).get()); + + FlutterFragment fragment = mock(FlutterFragment.class); + when(activity.retrieveExistingFlutterFragmentIfPossible()).thenReturn(null, fragment); + FlutterEngine engine = mock(FlutterEngine.class); + when(fragment.getFlutterEngine()).thenReturn(engine); + activity.onCreate(null); + + activity.onBackPressed(); + verify(fragment).onBackPressed(); + } + @Test @Config(minSdk = Build.API_LEVELS.API_34) @TargetApi(Build.API_LEVELS.API_34)