diff --git a/packages/flutter/lib/src/widgets/navigator.dart b/packages/flutter/lib/src/widgets/navigator.dart index 65c199c13e4..e8f5700ab96 100644 --- a/packages/flutter/lib/src/widgets/navigator.dart +++ b/packages/flutter/lib/src/widgets/navigator.dart @@ -184,6 +184,9 @@ abstract class Route extends _RoutePlaceholder { NavigatorState? get navigator => _navigator; NavigatorState? _navigator; + bool get _installed => _navigator != null; + bool _isInstalledIn(NavigatorState state) => _navigator == state; + /// The settings for this route. /// /// See [RouteSettings] for details. @@ -220,7 +223,7 @@ abstract class Route extends _RoutePlaceholder { void _updateSettings(RouteSettings newSettings) { if (_settings != newSettings) { _settings = newSettings; - if (_navigator != null) { + if (_installed) { changedInternalState(); } } @@ -579,7 +582,7 @@ abstract class Route extends _RoutePlaceholder { /// /// If this is true, then [isActive] is also true. bool get isCurrent { - if (_navigator == null) { + if (!_installed) { return false; } final _RouteEntry? currentRouteEntry = _navigator!._lastRouteEntryWhereOrNull( @@ -596,7 +599,7 @@ abstract class Route extends _RoutePlaceholder { /// If [isFirst] and [isCurrent] are both true then this is the only route on /// the navigator (and [isActive] will also be true). bool get isFirst { - if (_navigator == null) { + if (!_installed) { return false; } final _RouteEntry? currentRouteEntry = _navigator!._firstRouteEntryWhereOrNull( @@ -611,7 +614,7 @@ abstract class Route extends _RoutePlaceholder { /// Whether there is at least one active route underneath this route. @protected bool get hasActiveRouteBelow { - if (_navigator == null) { + if (!_installed) { return false; } for (final _RouteEntry entry in _navigator!._history) { @@ -3213,7 +3216,7 @@ class _RouteEntry extends RouteTransitionRecord { ); assert(navigator._debugLocked); assert( - route._navigator == null, + !route._installed, 'The pushed route has already been used. When pushing a route, a new ' 'Route object must be provided.', ); @@ -3292,7 +3295,7 @@ class _RouteEntry extends RouteTransitionRecord { /// refuses to be popped. bool handlePop({required NavigatorState navigator, required Route? previousPresent}) { assert(navigator._debugLocked); - assert(route._navigator == navigator); + assert(route._isInstalledIn(navigator)); currentState = _RouteLifecycle.popping; if (route._popCompleter.isCompleted) { // This is a page-based route popped through the Navigator.pop. The @@ -3322,7 +3325,7 @@ class _RouteEntry extends RouteTransitionRecord { required Route? previousPresent, }) { assert(navigator._debugLocked); - if (route._navigator == navigator) { + if (route._isInstalledIn(navigator)) { currentState = _RouteLifecycle.removing; } else { // This route is still waiting to be added while a top-most push or pop @@ -3336,7 +3339,7 @@ class _RouteEntry extends RouteTransitionRecord { } void didAdd({required NavigatorState navigator, required bool isNewFirst}) { - assert(route._navigator == null); + assert(!route._installed); route._navigator = navigator; route.install(); assert(route.overlayEntries.isNotEmpty); @@ -3436,7 +3439,7 @@ class _RouteEntry extends RouteTransitionRecord { if (!navigator._entryWaitingForSubTreeDisposal.remove(this)) { // This route must have been destroyed as a result of navigator // force dispose. - assert(route._navigator == null && !navigator.mounted); + assert(!route._installed && !navigator.mounted); return; } assert(currentState == _RouteLifecycle.disposing); @@ -4484,9 +4487,12 @@ class NavigatorState extends State with TickerProviderStateMixin, Res assert(entry.currentState == _RouteLifecycle.remove); continue; case _RouteLifecycle.remove: - if (!seenTopActiveRoute) { + // Routes that are not yet added (_installed == false) will exit as if + // they have never been on the navigator. They shouldn't announce + // their presence. + if (!seenTopActiveRoute && entry.route._installed) { if (poppedRoute != null) { - entry.route.didPopNext(poppedRoute); + entry.handleDidPopNext(poppedRoute); } poppedRoute = null; } @@ -5049,7 +5055,7 @@ class NavigatorState extends State with TickerProviderStateMixin, Res _debugLocked = true; return true; }()); - assert(entry.route._navigator == null); + assert(!entry.route._installed); assert(entry.currentState == _RouteLifecycle.push); _history.add(entry); _flushHistoryUpdates(); @@ -5127,7 +5133,7 @@ class NavigatorState extends State with TickerProviderStateMixin, Res Route newRoute, { TO? result, }) { - assert(newRoute._navigator == null); + assert(!newRoute._installed); _pushReplacementEntry( _RouteEntry(newRoute, pageBased: false, initialState: _RouteLifecycle.pushReplace), result, @@ -5181,7 +5187,7 @@ class NavigatorState extends State with TickerProviderStateMixin, Res _debugLocked = true; return true; }()); - assert(entry.route._navigator == null); + assert(!entry.route._installed); assert(_history.isNotEmpty); assert( _history.any(_RouteEntry.isPresentPredicate), @@ -5228,7 +5234,7 @@ class NavigatorState extends State with TickerProviderStateMixin, Res /// restored during state restoration. @optionalTypeArgs Future pushAndRemoveUntil(Route newRoute, RoutePredicate predicate) { - assert(newRoute._navigator == null); + assert(!newRoute._installed); assert(newRoute.overlayEntries.isEmpty); _pushEntryAndRemoveUntil( _RouteEntry(newRoute, pageBased: false, initialState: _RouteLifecycle.push), @@ -5282,7 +5288,7 @@ class NavigatorState extends State with TickerProviderStateMixin, Res _debugLocked = true; return true; }()); - assert(entry.route._navigator == null); + assert(!entry.route._installed); assert(entry.route.overlayEntries.isEmpty); assert(entry.currentState == _RouteLifecycle.push); int index = _history.length - 1; @@ -5315,7 +5321,7 @@ class NavigatorState extends State with TickerProviderStateMixin, Res @optionalTypeArgs void replace({required Route oldRoute, required Route newRoute}) { assert(!_debugLocked); - assert(oldRoute._navigator == this); + assert(oldRoute._isInstalledIn(this)); _replaceEntry( _RouteEntry(newRoute, pageBased: false, initialState: _RouteLifecycle.replace), oldRoute, @@ -5337,7 +5343,7 @@ class NavigatorState extends State with TickerProviderStateMixin, Res required RestorableRouteBuilder newRouteBuilder, Object? arguments, }) { - assert(oldRoute._navigator == this); + assert(oldRoute._isInstalledIn(this)); assert( _debugIsStaticCallback(newRouteBuilder), 'The provided routeBuilder must be a static function.', @@ -5365,7 +5371,7 @@ class NavigatorState extends State with TickerProviderStateMixin, Res return true; }()); assert(entry.currentState == _RouteLifecycle.replace); - assert(entry.route._navigator == null); + assert(!entry.route._installed); final int index = _history.indexWhere(_RouteEntry.isRoutePredicate(oldRoute)); assert(index >= 0, 'This Navigator does not contain the specified oldRoute.'); assert( @@ -5401,8 +5407,8 @@ class NavigatorState extends State with TickerProviderStateMixin, Res required Route anchorRoute, required Route newRoute, }) { - assert(newRoute._navigator == null); - assert(anchorRoute._navigator == this); + assert(!newRoute._installed); + assert(anchorRoute._isInstalledIn(this)); _replaceEntryBelow( _RouteEntry(newRoute, pageBased: false, initialState: _RouteLifecycle.replace), anchorRoute, @@ -5425,7 +5431,7 @@ class NavigatorState extends State with TickerProviderStateMixin, Res required RestorableRouteBuilder newRouteBuilder, Object? arguments, }) { - assert(anchorRoute._navigator == this); + assert(anchorRoute._isInstalledIn(this)); assert( _debugIsStaticCallback(newRouteBuilder), 'The provided routeBuilder must be a static function.', @@ -5515,7 +5521,7 @@ class NavigatorState extends State with TickerProviderStateMixin, Res if (lastEntry == null) { return false; } - assert(lastEntry.route._navigator == this); + assert(lastEntry.route._isInstalledIn(this)); // TODO(justinmc): When the deprecated willPop method is removed, delete // this code and use only popDisposition, below. @@ -5636,7 +5642,7 @@ class NavigatorState extends State with TickerProviderStateMixin, Res _debugLocked = true; return true; }()); - assert(route._navigator == this); + assert(route._isInstalledIn(this)); final bool wasCurrent = route.isCurrent; final _RouteEntry entry = _history.firstWhere(_RouteEntry.isRoutePredicate(route)); entry.complete(result, isReplaced: false, imperativeRemoval: true); @@ -5661,7 +5667,7 @@ class NavigatorState extends State with TickerProviderStateMixin, Res _debugLocked = true; return true; }()); - assert(anchorRoute._navigator == this); + assert(anchorRoute._isInstalledIn(this)); final int anchorIndex = _history.indexWhere(_RouteEntry.isRoutePredicate(anchorRoute)); assert(anchorIndex >= 0, 'This Navigator does not contain the specified anchorRoute.'); assert( diff --git a/packages/flutter/test/widgets/navigator_test.dart b/packages/flutter/test/widgets/navigator_test.dart index 067a81335a1..c4846c8204e 100644 --- a/packages/flutter/test/widgets/navigator_test.dart +++ b/packages/flutter/test/widgets/navigator_test.dart @@ -280,6 +280,7 @@ void main() { expect(tester.widget(find.byType(Overlay)).clipBehavior, Clip.none); }); + // Regression test for https://github.com/flutter/flutter/issues/170250. testWidgets('Can remove page before they are added', (WidgetTester tester) async { final GlobalKey key = GlobalKey(); Future buildPages(List> pages) { @@ -337,6 +338,52 @@ void main() { expect(tester.takeException(), isNull); }); + // Regression test for https://github.com/flutter/flutter/issues/177322. + testWidgets('Can remove page before they are added - case 2', (WidgetTester tester) async { + final GlobalKey key = GlobalKey(); + Future buildPages(List> pages) { + return tester.pumpWidget( + MediaQuery( + data: MediaQueryData.fromView(tester.view), + child: Directionality( + textDirection: TextDirection.ltr, + child: Navigator(key: key, pages: pages, onDidRemovePage: (_) {}), + ), + ), + ); + } + + const MaterialPage page = MaterialPage( + key: ValueKey('page'), + child: Text('page'), + ); + const MaterialPage page1 = MaterialPage( + key: ValueKey('page1'), + child: Text('page1'), + ); + const MaterialPage page2 = MaterialPage( + key: ValueKey('page2'), + child: Text('page2'), + ); + const MaterialPage page3 = MaterialPage( + key: ValueKey('page3'), + child: Text('page3'), + ); + await buildPages(>[page]); + + expect(find.text('page'), findsOneWidget); + + await buildPages(>[page, page1, page2, page3]); + + // In mid transition; + await tester.pump(const Duration(microseconds: 150)); + // Cause page1 and page2 to be removed before they are added. + await buildPages(>[page]); + await tester.pumpAndSettle(); + expect(find.text('page'), findsOneWidget); + expect(tester.takeException(), isNull); + }); + testWidgets('Zero transition page-based route correctly notifies observers when it is popped', ( WidgetTester tester, ) async {