From 3348a55035ef079cb46e3ea99ecd23dd55f44eee Mon Sep 17 00:00:00 2001 From: chunhtai <47866232+chunhtai@users.noreply.github.com> Date: Fri, 28 Jan 2022 09:40:11 -0800 Subject: [PATCH] =?UTF-8?q?fixes=20navigator=20to=20be=20able=20to=20handl?= =?UTF-8?q?e=20route=20with=20duplicate=20page=20key=20in=E2=80=A6=20(#973?= =?UTF-8?q?94)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../flutter/lib/src/widgets/navigator.dart | 24 ++++++++----- .../flutter/test/widgets/navigator_test.dart | 36 +++++++++++++++++++ 2 files changed, 52 insertions(+), 8 deletions(-) diff --git a/packages/flutter/lib/src/widgets/navigator.dart b/packages/flutter/lib/src/widgets/navigator.dart index bcc3b94c1d3..75e25670c16 100644 --- a/packages/flutter/lib/src/widgets/navigator.dart +++ b/packages/flutter/lib/src/widgets/navigator.dart @@ -2801,7 +2801,7 @@ class _RouteEntry extends RouteTransitionRecord { bool get hasPage => route.settings is Page; bool canUpdateFrom(Page page) { - if (currentState.index > _RouteLifecycle.idle.index) + if (!willBePresent) return false; if (!hasPage) return false; @@ -3635,6 +3635,9 @@ class NavigatorState extends State with TickerProviderStateMixin, Res // Scans middle of the old entries and records the page key to old entry map. int oldEntriesBottomToScan = oldEntriesBottom; final Map pageKeyToOldEntry = {}; + // This set contains entries that are transitioning out but are still in + // the route stack. + final Set<_RouteEntry> phantomEntries = <_RouteEntry>{}; while (oldEntriesBottomToScan <= oldEntriesTop) { final _RouteEntry oldEntry = _history[oldEntriesBottomToScan]; oldEntriesBottomToScan += 1; @@ -3650,9 +3653,14 @@ class NavigatorState extends State with TickerProviderStateMixin, Res assert(oldEntry.hasPage); final Page page = oldEntry.route.settings as Page; + if (page.key == null) continue; + if (!oldEntry.willBePresent) { + phantomEntries.add(oldEntry); + continue; + } assert(!pageKeyToOldEntry.containsKey(page.key)); pageKeyToOldEntry[page.key!] = oldEntry; } @@ -3703,21 +3711,21 @@ class NavigatorState extends State with TickerProviderStateMixin, Res () => <_RouteEntry>[], ); pagelessRoutes.add(potentialEntryToRemove); - if (previousOldPageRouteEntry!.isWaitingForExitingDecision && potentialEntryToRemove.isPresent) + if (previousOldPageRouteEntry!.isWaitingForExitingDecision && potentialEntryToRemove.willBePresent) potentialEntryToRemove.markNeedsExitingDecision(); continue; } final Page potentialPageToRemove = potentialEntryToRemove.route.settings as Page; // Marks for transition delegate to remove if this old page does not have - // a key or was not taken during updating the middle of new page. - if ( - potentialPageToRemove.key == null || - pageKeyToOldEntry.containsKey(potentialPageToRemove.key) - ) { + // a key, was not taken during updating the middle of new page, or is + // already transitioning out. + if (potentialPageToRemove.key == null || + pageKeyToOldEntry.containsKey(potentialPageToRemove.key) || + phantomEntries.contains(potentialEntryToRemove)) { locationToExitingPageRoute[previousOldPageRouteEntry] = potentialEntryToRemove; // We only need a decision if it has not already been popped. - if (potentialEntryToRemove.isPresent) + if (potentialEntryToRemove.willBePresent) potentialEntryToRemove.markNeedsExitingDecision(); } previousOldPageRouteEntry = potentialEntryToRemove; diff --git a/packages/flutter/test/widgets/navigator_test.dart b/packages/flutter/test/widgets/navigator_test.dart index 2198015e795..512d5b6c6d5 100644 --- a/packages/flutter/test/widgets/navigator_test.dart +++ b/packages/flutter/test/widgets/navigator_test.dart @@ -2644,6 +2644,42 @@ void main() { expect(find.text('initial'), findsOneWidget); }); + testWidgets('can handle duplicate page key if update before transition finishes', (WidgetTester tester) async { + // Regression test for https://github.com/flutter/flutter/issues/97363. + final GlobalKey navigator = GlobalKey(); + final List myPages1 = [ + const TestPage(key: ValueKey('1'), name:'initial'), + ]; + final List myPages2 = [ + const TestPage(key: ValueKey('2'), name:'second'), + ]; + + bool onPopPage(Route route, dynamic result) => false; + + await tester.pumpWidget( + buildNavigator(pages: myPages1, onPopPage: onPopPage, key: navigator), + ); + await tester.pump(); + expect(find.text('initial'), findsOneWidget); + // Update multiple times without waiting for pop to finish to leave + // multiple popping route entries in route stack with the same page key. + await tester.pumpWidget( + buildNavigator(pages: myPages2, onPopPage: onPopPage, key: navigator), + ); + await tester.pump(); + await tester.pumpWidget( + buildNavigator(pages: myPages1, onPopPage: onPopPage, key: navigator), + ); + await tester.pump(); + await tester.pumpWidget( + buildNavigator(pages: myPages2, onPopPage: onPopPage, key: navigator), + ); + + await tester.pumpAndSettle(); + expect(find.text('second'), findsOneWidget); + expect(tester.takeException(), isNull); + }); + testWidgets('throw if onPopPage callback is not provided', (WidgetTester tester) async { final List myPages = [ const TestPage(key: ValueKey('1'), name:'initial'),