Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Platform channel for predictive back #39208

Merged
merged 22 commits into from
Jun 9, 2023

Conversation

justinmc
Copy link
Contributor

@justinmc justinmc commented Jan 27, 2023

This PR enables Android's predictive back animation when leaving a Flutter app.

How

In order to show the predictive back animation when a back gesture happens, Android needs to know ahead of time that it, not Flutter, should handle the back gesture. This is communicated with the following new platform channel method:

setFrameworkHandlesBacks(boolean frameworkHandlesBacks)

In the framework, this will be set to true whenever the framework has a reason to prevent native predictive back, such as when there are multiple routes in the navigation history. Otherwise it will be set to false and predictive back will be enabled, such as when there is only a single route of navigation history.

By default, no back handler is registered for Flutter and predictive back is enabled.

References

Part of flutter/flutter#109513
Framework PR: flutter/flutter#120385
Design doc: https://docs.google.com/document/d/1BGCWy1_LRrXEB6qeqTAKlk-U2CZlKJ5xI97g45U7azk/edit#

@justinmc
Copy link
Contributor Author

justinmc commented May 9, 2023

This is currently waiting on the framework PR (flutter/flutter#120385) to sort out some dependencies on existing APIs. For example, things like WillPopScope need to be ported to work ahead-of-time. See that PR and the design doc for the latest.

@justinmc
Copy link
Contributor Author

I tried this on a very old device (API 24) and the regular back button worked fine (obviously no predictive back).

@justinmc
Copy link
Contributor Author

justinmc commented May 30, 2023

I was thinking I might have to implement this again for FlutterFragment and/or FlutterFragmentActivity. I'm concluding that no, I don't, but any reviewers let me know if I'm wrong. It seems like there is no OnBackInvokedDispatcher in Fragments. The PR that moved to back callbacks didn't touch those files either.

Edit: I really tried to run this in an add-to-app scenario, but ran into a problem (flutter/flutter#128090). If any reviewers know for sure if I should be adding this to FlutterFragment.java too or not, please let me know.

@justinmc justinmc changed the title (WIP) Root predictive back animation Root predictive back animation Jun 2, 2023
@justinmc justinmc changed the title Root predictive back animation Platform channel for predictive back Jun 2, 2023
@@ -643,8 +645,6 @@ protected void onCreate(@Nullable Bundle savedInstanceState) {

lifecycle.handleLifecycleEvent(Lifecycle.Event.ON_CREATE);

registerOnBackInvokedCallback();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Now that this is removed, it means that by default, Flutter will not handle back gestures. So if you start up a Flutter app that does nothing, has no Navigator or WidgetsApp etc., root predictive back will work. The framework will have to first call setFrameworkHandlesBacks in order to prevent predictive back and to handle back gestures itself. This will be handled automatically in Flutter apps that use WidgetsApp.

@@ -698,6 +700,15 @@ public void onBackInvoked() {
}
: null;

@Override
public void setFrameworkHandlesBacks(boolean frameworkHandlesBacks) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

How do people feel about this name?

Copy link
Member

Choose a reason for hiding this comment

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

suggestion: setFrameworkHandlesBack (without the plural s)?

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe something to indicate it registers/unregisters a callback based on frameworkHandlesBacks since it isn't actually setting that, e.g. toggleOnBackRegistry?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm going to go with setFrameworkHandlesBack for now.

I kind of wanted to be generic about what is actually happening under the hood in case that changes, or in case we ever needed to do this on another platform that handles it differently. I think setFrameworkHandlesBack works well enough since it at least sounds declarative, like it's setting some state that decides what should happen to back gestures.

Let me know if anyone has other thoughts though.

@justinmc
Copy link
Contributor Author

justinmc commented Jun 5, 2023

@reidbaker I added you as a reviewer to make sure someone on the Android team is aware of this, but feel free to point me elsewhere if you're not the right person to review this.

@reidbaker
Copy link
Contributor

On vacation for 2 weeks. @camsim99 for Android

Copy link
Contributor

@chunhtai chunhtai left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@goderbauer goderbauer left a comment

Choose a reason for hiding this comment

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

LGTM, but please get input from an Android expert.

@@ -217,6 +217,8 @@ public class FlutterActivity extends Activity
implements FlutterActivityAndFragmentDelegate.Host, LifecycleOwner {
private static final String TAG = "FlutterActivity";

private boolean hasRegisteredCallback = false;
Copy link
Member

Choose a reason for hiding this comment

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

nit: should this get a more specific name as to what callback has been registered for context?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed to: hasRegisteredBackCallback

@@ -698,6 +700,15 @@ public void onBackInvoked() {
}
: null;

@Override
public void setFrameworkHandlesBacks(boolean frameworkHandlesBacks) {
Copy link
Member

Choose a reason for hiding this comment

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

suggestion: setFrameworkHandlesBack (without the plural s)?

Copy link
Contributor

@camsim99 camsim99 left a comment

Choose a reason for hiding this comment

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

LGTM! Just left a suggestion on naming

@@ -698,6 +700,15 @@ public void onBackInvoked() {
}
: null;

@Override
public void setFrameworkHandlesBacks(boolean frameworkHandlesBacks) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe something to indicate it registers/unregisters a callback based on frameworkHandlesBacks since it isn't actually setting that, e.g. toggleOnBackRegistry?

@zanderso
Copy link
Member

zanderso commented Jun 8, 2023

From Engine PR triage: It looks like the mac_unopt presubs failed due to a signing issue in the scenario test. This PR probably needs to be rebased, and to run the presubs again.

@justinmc
Copy link
Contributor Author

justinmc commented Jun 8, 2023

I think I tried rerunning it without seeing your comment and it looks like it worked 🤷

@justinmc justinmc merged commit ff66711 into flutter:main Jun 9, 2023
@justinmc justinmc deleted the predictive-back-root branch June 9, 2023 17:06
loic-sharma pushed a commit to loic-sharma/flutter-engine that referenced this pull request Jun 9, 2023
Adds a platform channel method for enabling/disabling Android's predictive back feature.
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 9, 2023
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Jun 9, 2023
…128612)

flutter/engine@071e1fb...488876e

2023-06-09 skia-flutter-autoroll@skia.org Roll ANGLE from a185cb8c8924 to 3e4f4caebcb0 (2 revisions) (flutter/engine#42701)
2023-06-09 bdero@google.com [Impeller] Add CPU implementations for all color filters (flutter/engine#42692)
2023-06-09 jonahwilliams@google.com [Impeller] add explicit VMA flush to device memory writes. (flutter/engine#42685)
2023-06-09 30870216+gaaclarke@users.noreply.github.com [Impeller] Makes validation layers flag work for android (flutter/engine#42625)
2023-06-09 jmccandless@google.com Platform channel for predictive back (flutter/engine#39208)

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://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 to flutter/flutter that referenced this pull request Aug 4, 2023
This PR aims to support Android's predictive back gesture when popping the entire Flutter app.  Predictive route transitions between routes inside of a Flutter app will come later.

<img width="200" src="https://user-images.githubusercontent.com/389558/217918109-945febaa-9086-41cc-a476-1a189c7831d8.gif" />

### Trying it out

If you want to try this feature yourself, here are the necessary steps:

  1. Run Android 33 or above.
  1. Enable the feature flag for predictive back on the device under "Developer
     options".
  1. Create a Flutter project, or clone [my example project](https://github.com/justinmc/flutter_predictive_back_examples).
  1. Set `android:enableOnBackInvokedCallback="true"` in
     android/app/src/main/AndroidManifest.xml (already done in the example project).
  1. Check out this branch.
  1. Run the app. Perform a back gesture (swipe from the left side of the
     screen).

You should see the predictive back animation like in the animation above and be able to commit or cancel it.

### go_router support

go_router works with predictive back out of the box because it uses a Navigator internally that dispatches NavigationNotifications!

~~go_router can be supported by adding a listener to the router and updating SystemNavigator.setFrameworkHandlesBack.~~

Similar to with nested Navigators, nested go_routers is supported by using a PopScope widget.

<details>

<summary>Full example of nested go_routers</summary>

```dart
// Copyright 2014 The Flutter Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

import 'package:go_router/go_router.dart';

import 'package:flutter/material.dart';
import 'package:flutter/scheduler.dart';

void main() => runApp(_MyApp());

class _MyApp extends StatelessWidget {
  final GoRouter router = GoRouter(
    routes: <RouteBase>[
      GoRoute(
        path: '/',
        builder: (BuildContext context, GoRouterState state) => _HomePage(),
      ),
      GoRoute(
        path: '/nested_navigators',
        builder: (BuildContext context, GoRouterState state) => _NestedGoRoutersPage(),
      ),
    ],
  );

  @OverRide
  Widget build(BuildContext context) {
    return MaterialApp.router(
      routerConfig: router,
    );
  }
}

class _HomePage extends StatelessWidget {
  @OverRide
  Widget build(BuildContext context) {
    return Scaffold(
      appBar: AppBar(
        title: const Text('Nested Navigators Example'),
      ),
      body: Center(
        child: Column(
          mainAxisAlignment: MainAxisAlignment.center,
          children: <Widget>[
            const Text('Home Page'),
            const Text('A system back gesture here will exit the app.'),
            const SizedBox(height: 20.0),
            ListTile(
              title: const Text('Nested go_router route'),
              subtitle: const Text('This route has another go_router in addition to the one used with MaterialApp above.'),
              onTap: () {
                context.push('/nested_navigators');
              },
            ),
          ],
        ),
      ),
    );
  }
}

class _NestedGoRoutersPage extends StatefulWidget {
  @OverRide
  State<_NestedGoRoutersPage> createState() => _NestedGoRoutersPageState();
}

class _NestedGoRoutersPageState extends State<_NestedGoRoutersPage> {
  late final GoRouter _router;
  final GlobalKey<NavigatorState> _nestedNavigatorKey = GlobalKey<NavigatorState>();

  // If the nested navigator has routes that can be popped, then we want to
  // block the root navigator from handling the pop so that the nested navigator
  // can handle it instead.
  bool get _popEnabled {
    // canPop will throw an error if called before build. Is this the best way
    // to avoid that?
    return _nestedNavigatorKey.currentState == null ? true : !_router.canPop();
  }

  void _onRouterChanged() {
    // Here the _router reports the location correctly, but canPop is still out
    // of date.  Hence the post frame callback.
    SchedulerBinding.instance.addPostFrameCallback((Duration duration) {
      setState(() {});
    });
  }

  @OverRide
  void initState() {
    super.initState();

    final BuildContext rootContext = context;
    _router = GoRouter(
      navigatorKey: _nestedNavigatorKey,
      routes: [
        GoRoute(
          path: '/',
          builder: (BuildContext context, GoRouterState state) => _LinksPage(
            title: 'Nested once - home route',
            backgroundColor: Colors.indigo,
            onBack: () {
              rootContext.pop();
            },
            buttons: <Widget>[
              TextButton(
                onPressed: () {
                  context.push('/two');
                },
                child: const Text('Go to another route in this nested Navigator'),
              ),
            ],
          ),
        ),
        GoRoute(
          path: '/two',
          builder: (BuildContext context, GoRouterState state) => _LinksPage(
            backgroundColor: Colors.indigo.withBlue(255),
            title: 'Nested once - page two',
          ),
        ),
      ],
    );

    _router.addListener(_onRouterChanged);
  }

  @OverRide
  void dispose() {
    _router.removeListener(_onRouterChanged);
    super.dispose();
  }

  @OverRide
  Widget build(BuildContext context) {
    return PopScope(
      popEnabled: _popEnabled,
      onPopped: (bool success) {
        if (success) {
          return;
        }
        _router.pop();
      },
      child: Router<Object>.withConfig(
        restorationScopeId: 'router-2',
        config: _router,
      ),
    );
  }
}

class _LinksPage extends StatelessWidget {
  const _LinksPage ({
    required this.backgroundColor,
    this.buttons = const <Widget>[],
    this.onBack,
    required this.title,
  });

  final Color backgroundColor;
  final List<Widget> buttons;
  final VoidCallback? onBack;
  final String title;

  @OverRide
  Widget build(BuildContext context) {
    return Scaffold(
      backgroundColor: backgroundColor,
      body: Center(
        child: Column(
          mainAxisAlignment: MainAxisAlignment.center,
          children: <Widget>[
            Text(title),
            //const Text('A system back here will go back to Nested Navigators Page One'),
            ...buttons,
            TextButton(
              onPressed: onBack ?? () {
                context.pop();
              },
              child: const Text('Go back'),
            ),
          ],
        ),
      ),
    );
  }
}
```

</details>

### Resources

Fixes #109513
Depends on engine PR flutter/engine#39208 ✔️ 
Design doc: https://docs.google.com/document/d/1BGCWy1_LRrXEB6qeqTAKlk-U2CZlKJ5xI97g45U7azk/edit#
Migration guide: flutter/website#8952
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants