From dfd4147ceaf0204feeb2e2c5ca25b6ddc4c84483 Mon Sep 17 00:00:00 2001 From: Justin McCandless Date: Mon, 28 Aug 2023 09:23:52 -0700 Subject: [PATCH] Fix stuck predictive back platform channel calls (#133368) Fix a Google test flakiness increase. --- packages/flutter/lib/src/widgets/app.dart | 17 ++-- .../test/cupertino/tab_scaffold_test.dart | 13 +-- packages/flutter/test/widgets/app_test.dart | 84 +++++++++++++++++++ .../flutter/test/widgets/navigator_test.dart | 13 +-- .../flutter/test/widgets/pop_scope_test.dart | 13 +-- 5 files changed, 117 insertions(+), 23 deletions(-) diff --git a/packages/flutter/lib/src/widgets/app.dart b/packages/flutter/lib/src/widgets/app.dart index f8dc851ab0..d8d649c103 100644 --- a/packages/flutter/lib/src/widgets/app.dart +++ b/packages/flutter/lib/src/widgets/app.dart @@ -1346,12 +1346,18 @@ class _WidgetsAppState extends State with WidgetsBindingObserver { /// the platform with [NavigationNotification.canHandlePop] and stops /// bubbling. bool _defaultOnNavigationNotification(NavigationNotification notification) { - // Don't do anything with navigation notifications if there is no engine - // attached. - if (_appLifecycleState != AppLifecycleState.detached) { - SystemNavigator.setFrameworkHandlesBack(notification.canHandlePop); + switch (_appLifecycleState) { + case null: + case AppLifecycleState.detached: + case AppLifecycleState.inactive: + // Avoid updating the engine when the app isn't ready. + return true; + case AppLifecycleState.resumed: + case AppLifecycleState.hidden: + case AppLifecycleState.paused: + SystemNavigator.setFrameworkHandlesBack(notification.canHandlePop); + return true; } - return true; } @override @@ -1366,6 +1372,7 @@ class _WidgetsAppState extends State with WidgetsBindingObserver { _updateRouting(); _locale = _resolveLocales(WidgetsBinding.instance.platformDispatcher.locales, widget.supportedLocales); WidgetsBinding.instance.addObserver(this); + _appLifecycleState = WidgetsBinding.instance.lifecycleState; } @override diff --git a/packages/flutter/test/cupertino/tab_scaffold_test.dart b/packages/flutter/test/cupertino/tab_scaffold_test.dart index b376a92ecf..22270d83f0 100644 --- a/packages/flutter/test/cupertino/tab_scaffold_test.dart +++ b/packages/flutter/test/cupertino/tab_scaffold_test.dart @@ -1219,11 +1219,7 @@ void main() { group('Android Predictive Back', () { bool? lastFrameworkHandlesBack; - setUp(() { - // Initialize to false. Because this uses a static boolean internally, it - // is not reset between tests or calls to pumpWidget. Explicitly setting - // it to false before each test makes them behave deterministically. - SystemNavigator.setFrameworkHandlesBack(false); + setUp(() async { lastFrameworkHandlesBack = null; TestDefaultBinaryMessengerBinding.instance.defaultBinaryMessenger .setMockMethodCallHandler(SystemChannels.platform, (MethodCall methodCall) async { @@ -1233,12 +1229,17 @@ void main() { } return; }); + await TestDefaultBinaryMessengerBinding.instance.defaultBinaryMessenger + .handlePlatformMessage( + 'flutter/lifecycle', + const StringCodec().encodeMessage(AppLifecycleState.resumed.toString()), + (ByteData? data) {}, + ); }); tearDown(() { TestDefaultBinaryMessengerBinding.instance.defaultBinaryMessenger .setMockMethodCallHandler(SystemChannels.platform, null); - SystemNavigator.setFrameworkHandlesBack(true); }); testWidgets('System back navigation inside of tabs', (WidgetTester tester) async { diff --git a/packages/flutter/test/widgets/app_test.dart b/packages/flutter/test/widgets/app_test.dart index c742733106..3540930d10 100644 --- a/packages/flutter/test/widgets/app_test.dart +++ b/packages/flutter/test/widgets/app_test.dart @@ -683,6 +683,90 @@ void main() { expect(copySpy.invoked, isTrue); expect(pasteSpy.invoked, isTrue); }, variant: const TargetPlatformVariant({ TargetPlatform.iOS, TargetPlatform.macOS })); + + group('Android Predictive Back', () { + Future setAppLifeCycleState(AppLifecycleState state) async { + final ByteData? message = const StringCodec().encodeMessage(state.toString()); + await TestDefaultBinaryMessengerBinding.instance.defaultBinaryMessenger + .handlePlatformMessage('flutter/lifecycle', message, (ByteData? data) {}); + } + + final List frameworkHandlesBacks = []; + setUp(() async { + frameworkHandlesBacks.clear(); + TestDefaultBinaryMessengerBinding.instance.defaultBinaryMessenger + .setMockMethodCallHandler(SystemChannels.platform, (MethodCall methodCall) async { + if (methodCall.method == 'SystemNavigator.setFrameworkHandlesBack') { + expect(methodCall.arguments, isA()); + frameworkHandlesBacks.add(methodCall.arguments as bool); + } + return; + }); + }); + + tearDown(() async { + TestDefaultBinaryMessengerBinding.instance.defaultBinaryMessenger + .setMockMethodCallHandler(SystemChannels.platform, null); + await setAppLifeCycleState(AppLifecycleState.resumed); + }); + + testWidgets('WidgetsApp calls setFrameworkHandlesBack only when app is ready', (WidgetTester tester) async { + // Start in the `resumed` state, where setFrameworkHandlesBack should be + // called like normal. + await setAppLifeCycleState(AppLifecycleState.resumed); + + late BuildContext currentContext; + await tester.pumpWidget( + WidgetsApp( + color: const Color(0xFF123456), + builder: (BuildContext context, Widget? child) { + currentContext = context; + return const Placeholder(); + }, + ), + ); + + expect(frameworkHandlesBacks, isEmpty); + + const NavigationNotification(canHandlePop: true).dispatch(currentContext); + await tester.pumpAndSettle(); + expect(frameworkHandlesBacks, isNotEmpty); + expect(frameworkHandlesBacks.last, isTrue); + + const NavigationNotification(canHandlePop: false).dispatch(currentContext); + await tester.pumpAndSettle(); + expect(frameworkHandlesBacks.last, isFalse); + + // Set the app state to inactive, where setFrameworkHandlesBack shouldn't + // be called. + await setAppLifeCycleState(AppLifecycleState.inactive); + + final int finalCallsLength = frameworkHandlesBacks.length; + const NavigationNotification(canHandlePop: true).dispatch(currentContext); + await tester.pumpAndSettle(); + expect(frameworkHandlesBacks, hasLength(finalCallsLength)); + + const NavigationNotification(canHandlePop: false).dispatch(currentContext); + await tester.pumpAndSettle(); + expect(frameworkHandlesBacks, hasLength(finalCallsLength)); + + // Set the app state to detached, which also shouldn't call + // setFrameworkHandlesBack. Must go to paused, then detached. + await setAppLifeCycleState(AppLifecycleState.paused); + await setAppLifeCycleState(AppLifecycleState.detached); + + const NavigationNotification(canHandlePop: true).dispatch(currentContext); + await tester.pumpAndSettle(); + expect(frameworkHandlesBacks, hasLength(finalCallsLength)); + + const NavigationNotification(canHandlePop: false).dispatch(currentContext); + await tester.pumpAndSettle(); + expect(frameworkHandlesBacks, hasLength(finalCallsLength)); + }, + skip: kIsWeb, // [intended] predictive back is only native Android. + variant: const TargetPlatformVariant({ TargetPlatform.android }) + ); + }); } typedef SimpleRouterDelegateBuilder = Widget Function(BuildContext, RouteInformation); diff --git a/packages/flutter/test/widgets/navigator_test.dart b/packages/flutter/test/widgets/navigator_test.dart index d403ceba9c..52cd934893 100644 --- a/packages/flutter/test/widgets/navigator_test.dart +++ b/packages/flutter/test/widgets/navigator_test.dart @@ -4159,11 +4159,7 @@ void main() { group('Android Predictive Back', () { bool? lastFrameworkHandlesBack; - setUp(() { - // Initialize to false. Because this uses a static boolean internally, it - // is not reset between tests or calls to pumpWidget. Explicitly setting - // it to false before each test makes them behave deterministically. - SystemNavigator.setFrameworkHandlesBack(false); + setUp(() async { lastFrameworkHandlesBack = null; TestDefaultBinaryMessengerBinding.instance.defaultBinaryMessenger .setMockMethodCallHandler(SystemChannels.platform, (MethodCall methodCall) async { @@ -4173,12 +4169,17 @@ void main() { } return; }); + await TestDefaultBinaryMessengerBinding.instance.defaultBinaryMessenger + .handlePlatformMessage( + 'flutter/lifecycle', + const StringCodec().encodeMessage(AppLifecycleState.resumed.toString()), + (ByteData? data) {}, + ); }); tearDown(() { TestDefaultBinaryMessengerBinding.instance.defaultBinaryMessenger .setMockMethodCallHandler(SystemChannels.platform, null); - SystemNavigator.setFrameworkHandlesBack(true); }); testWidgets('a single route is already defaulted to false', (WidgetTester tester) async { diff --git a/packages/flutter/test/widgets/pop_scope_test.dart b/packages/flutter/test/widgets/pop_scope_test.dart index c5d0e88545..116951ce78 100644 --- a/packages/flutter/test/widgets/pop_scope_test.dart +++ b/packages/flutter/test/widgets/pop_scope_test.dart @@ -11,11 +11,7 @@ import 'navigator_utils.dart'; void main() { bool? lastFrameworkHandlesBack; - setUp(() { - // Initialize to false. Because this uses a static boolean internally, it - // is not reset between tests or calls to pumpWidget. Explicitly setting - // it to false before each test makes them behave deterministically. - SystemNavigator.setFrameworkHandlesBack(false); + setUp(() async { lastFrameworkHandlesBack = null; TestDefaultBinaryMessengerBinding.instance.defaultBinaryMessenger .setMockMethodCallHandler(SystemChannels.platform, (MethodCall methodCall) async { @@ -25,12 +21,17 @@ void main() { } return; }); + await TestDefaultBinaryMessengerBinding.instance.defaultBinaryMessenger + .handlePlatformMessage( + 'flutter/lifecycle', + const StringCodec().encodeMessage(AppLifecycleState.resumed.toString()), + (ByteData? data) {}, + ); }); tearDown(() { TestDefaultBinaryMessengerBinding.instance.defaultBinaryMessenger .setMockMethodCallHandler(SystemChannels.platform, null); - SystemNavigator.setFrameworkHandlesBack(true); }); testWidgets('toggling canPop on root route allows/prevents backs', (WidgetTester tester) async {