Unverified Commit 3348a550 authored by chunhtai's avatar chunhtai Committed by GitHub

fixes navigator to be able to handle route with duplicate page key in… (#97394)

parent b8460d4d
......@@ -2801,7 +2801,7 @@ class _RouteEntry extends RouteTransitionRecord {
bool get hasPage => route.settings is Page;
bool canUpdateFrom(Page<dynamic> page) {
if (currentState.index > _RouteLifecycle.idle.index)
if (!willBePresent)
return false;
if (!hasPage)
return false;
......@@ -3635,6 +3635,9 @@ class NavigatorState extends State<Navigator> with TickerProviderStateMixin, Res
// Scans middle of the old entries and records the page key to old entry map.
int oldEntriesBottomToScan = oldEntriesBottom;
final Map<LocalKey, _RouteEntry> pageKeyToOldEntry = <LocalKey, _RouteEntry>{};
// 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<Navigator> with TickerProviderStateMixin, Res
assert(oldEntry.hasPage);
final Page<dynamic> page = oldEntry.route.settings as Page<dynamic>;
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<Navigator> with TickerProviderStateMixin, Res
() => <_RouteEntry>[],
);
pagelessRoutes.add(potentialEntryToRemove);
if (previousOldPageRouteEntry!.isWaitingForExitingDecision && potentialEntryToRemove.isPresent)
if (previousOldPageRouteEntry!.isWaitingForExitingDecision && potentialEntryToRemove.willBePresent)
potentialEntryToRemove.markNeedsExitingDecision();
continue;
}
final Page<dynamic> potentialPageToRemove = potentialEntryToRemove.route.settings as Page<dynamic>;
// 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;
......
......@@ -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<NavigatorState> navigator = GlobalKey<NavigatorState>();
final List<TestPage> myPages1 = <TestPage>[
const TestPage(key: ValueKey<String>('1'), name:'initial'),
];
final List<TestPage> myPages2 = <TestPage>[
const TestPage(key: ValueKey<String>('2'), name:'second'),
];
bool onPopPage(Route<dynamic> 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<TestPage> myPages = <TestPage>[
const TestPage(key: ValueKey<String>('1'), name:'initial'),
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment