Revert "Fixes zero route transition duration crash (#90461)" (#94564)

This reverts commit 0eeb026e.

packages/flutter/lib/src/widgets/navigator.dart

@@ -2697,11 +2697,8 @@ class Navigator extends StatefulWidget {
 //                           \      /     /
 //                           idle--+-----+
 //                           /  \
-//                          /    +------+
-//                         /     |      |
-//                        /      |  complete*
-//                        |      |    /
-//                       pop*  remove*
+//                          /    \
+//                        pop*  remove*
 //                        /        \
 //                       /       removing#
 //                     popping#       |
@@ -2738,7 +2735,6 @@ enum _RouteLifecycle {
   //
   // routes that should be included in route announcement and should still listen to transition changes.
   pop, // we'll want to call didPop
-  complete, // we'll want to call didComplete,
   remove, // we'll want to run didReplace/didRemove etc
   // routes should not be included in route announcement but should still listen to transition changes.
   popping, // we're waiting for the route to call finalizeRoute to switch to dispose
@@ -2874,38 +2870,14 @@ class _RouteEntry extends RouteTransitionRecord {
     lastAnnouncedPoppedNextRoute = poppedRoute;
   }
 
-  /// Process the to-be-popped route.
-  ///
-  /// A route can be marked for pop by transition delegate or Navigator.pop,
-  /// this method actually pops the route by calling Route.didPop.
-  ///
-  /// Returns true if the route is popped; otherwise, returns false if the route
-  /// refuses to be popped.
-  bool handlePop({ required NavigatorState navigator, required Route<dynamic>? previousPresent }) {
+  void handlePop({ required NavigatorState navigator, required Route<dynamic>? previousPresent }) {
     assert(navigator != null);
     assert(navigator._debugLocked);
     assert(route._navigator == navigator);
     currentState = _RouteLifecycle.popping;
-    if (route._popCompleter.isCompleted) {
-      // This is a page-based route popped through the Navigator.pop. The
-      // didPop should have been called. No further action is needed.
-      assert(hasPage);
-      assert(pendingResult == null);
-      return true;
-    }
-    if (!route.didPop(pendingResult)) {
-      currentState = _RouteLifecycle.idle;
-      return false;
-    }
-    pendingResult = null;
-    return true;
-  }
-
-  void handleComplete() {
-    route.didComplete(pendingResult);
-    pendingResult = null;
-    assert(route._popCompleter.isCompleted); // implies didComplete was called
-    currentState = _RouteLifecycle.remove;
+    navigator._observedRouteDeletions.add(
+      _NavigatorPopObservation(route, previousPresent),
+    );
   }
 
   void handleRemoval({ required NavigatorState navigator, required Route<dynamic>? previousPresent }) {
@@ -2920,6 +2892,8 @@ class _RouteEntry extends RouteTransitionRecord {
     }
   }
 
+  bool doingPop = false;
+
   void didAdd({ required NavigatorState navigator, required bool isNewFirst}) {
     route.didAdd();
     currentState = _RouteLifecycle.idle;
@@ -2928,12 +2902,13 @@ class _RouteEntry extends RouteTransitionRecord {
     }
   }
 
-  Object? pendingResult;
-
   void pop<T>(T? result) {
     assert(isPresent);
-    pendingResult = result;
-    currentState = _RouteLifecycle.pop;
+    doingPop = true;
+    if (route.didPop(result) && doingPop) {
+      currentState = _RouteLifecycle.pop;
+    }
+    doingPop = false;
   }
 
   bool _reportRemovalToObserver = true;
@@ -2963,8 +2938,9 @@ class _RouteEntry extends RouteTransitionRecord {
       return;
     assert(isPresent);
     _reportRemovalToObserver = !isReplaced;
-    pendingResult = result;
-    currentState = _RouteLifecycle.complete;
+    route.didComplete(result);
+    assert(route._popCompleter.isCompleted); // implies didComplete was called
+    currentState = _RouteLifecycle.remove;
   }
 
   void finalize() {
@@ -3787,11 +3763,8 @@ class NavigatorState extends State<Navigator> with TickerProviderStateMixin, Res
     assert(() { _debugLocked = false; return true; }());
   }
 
-  bool _flushingHistory = false;
-
   void _flushHistoryUpdates({bool rearrangeOverlay = true}) {
     assert(_debugLocked && !_debugUpdatingPage);
-    _flushingHistory = true;
     // Clean up the list, sending updates to the routes that changed. Notably,
     // we don't send the didChangePrevious/didChangeNext updates to those that
     // did not change at this point, because we're not yet sure exactly what the
@@ -3855,35 +3828,21 @@ class NavigatorState extends State<Navigator> with TickerProviderStateMixin, Res
           canRemoveOrAdd = true;
           break;
         case _RouteLifecycle.pop:
-          if (!entry.handlePop(
-                navigator: this,
-                previousPresent: _getRouteBefore(index, _RouteEntry.willBePresentPredicate)?.route)){
-            assert(entry.currentState == _RouteLifecycle.idle);
-            continue;
-          }
           if (!seenTopActiveRoute) {
             if (poppedRoute != null)
               entry.handleDidPopNext(poppedRoute);
             poppedRoute = entry.route;
           }
-          _observedRouteDeletions.add(
-            _NavigatorPopObservation(entry.route, _getRouteBefore(index, _RouteEntry.willBePresentPredicate)?.route),
+          entry.handlePop(
+            navigator: this,
+            previousPresent: _getRouteBefore(index, _RouteEntry.willBePresentPredicate)?.route,
           );
-          if (entry.currentState == _RouteLifecycle.dispose) {
-            // The pop finished synchronously. This can happen if transition
-            // duration is zero.
-            continue;
-          }
           assert(entry.currentState == _RouteLifecycle.popping);
           canRemoveOrAdd = true;
           break;
         case _RouteLifecycle.popping:
           // Will exit this state when animation completes.
           break;
-        case _RouteLifecycle.complete:
-          entry.handleComplete();
-          assert(entry.currentState == _RouteLifecycle.remove);
-          continue;
         case _RouteLifecycle.remove:
           if (!seenTopActiveRoute) {
             if (poppedRoute != null)
@@ -3918,6 +3877,7 @@ class NavigatorState extends State<Navigator> with TickerProviderStateMixin, Res
       entry = previous;
       previous = index > 0 ? _history[index - 1] : null;
     }
+
     // Informs navigator observers about route changes.
     _flushObserverNotifications();
 
@@ -3950,7 +3910,6 @@ class NavigatorState extends State<Navigator> with TickerProviderStateMixin, Res
     if (bucket != null) {
       _serializableHistory.update(_history);
     }
-    _flushingHistory = false;
   }
 
   void _flushObserverNotifications() {
@@ -4887,18 +4846,17 @@ class NavigatorState extends State<Navigator> with TickerProviderStateMixin, Res
     }());
     final _RouteEntry entry = _history.lastWhere(_RouteEntry.isPresentPredicate);
     if (entry.hasPage) {
-      if (widget.onPopPage!(entry.route, result) && entry.currentState == _RouteLifecycle.idle) {
-        // The entry may have been disposed if the pop finishes synchronously.
-        assert(entry.route._popCompleter.isCompleted);
+      if (widget.onPopPage!(entry.route, result))
         entry.currentState = _RouteLifecycle.pop;
-      }
     } else {
       entry.pop<T>(result);
-      assert (entry.currentState == _RouteLifecycle.pop);
     }
-    if (entry.currentState == _RouteLifecycle.pop)
+    if (entry.currentState == _RouteLifecycle.pop) {
+      // Flush the history if the route actually wants to be popped (the pop
+      // wasn't handled internally).
       _flushHistoryUpdates(rearrangeOverlay: false);
-    assert(entry.currentState == _RouteLifecycle.idle || entry.route._popCompleter.isCompleted);
+      assert(entry.route._popCompleter.isCompleted);
+    }
     assert(() {
       _debugLocked = false;
       return true;
@@ -5014,13 +4972,15 @@ class NavigatorState extends State<Navigator> with TickerProviderStateMixin, Res
     assert(() { wasDebugLocked = _debugLocked; _debugLocked = true; return true; }());
     assert(_history.where(_RouteEntry.isRoutePredicate(route)).length == 1);
     final _RouteEntry entry =  _history.firstWhere(_RouteEntry.isRoutePredicate(route));
-    assert(entry.currentState == _RouteLifecycle.popping);
-    entry.finalize();
-    // finalizeRoute can be called during _flushHistoryUpdates if a pop
-    // finishes synchronously.
-    if (!_flushingHistory)
+    if (entry.doingPop) {
+      // We were called synchronously from Route.didPop(), but didn't process
+      // the pop yet. Let's do that now before finalizing.
+      entry.currentState = _RouteLifecycle.pop;
       _flushHistoryUpdates(rearrangeOverlay: false);
-
+    }
+    assert(entry.currentState != _RouteLifecycle.pop);
+    entry.finalize();
+    _flushHistoryUpdates(rearrangeOverlay: false);
     assert(() { _debugLocked = wasDebugLocked!; return true; }());
   }
 

packages/flutter/lib/src/widgets/routes.dart

@@ -129,9 +129,7 @@ abstract class TransitionRoute<T> extends OverlayRoute<T> {
   //
   // This situation arises when dealing with the Cupertino dismiss gesture.
   @override
-  bool get finishedWhenPopped => _controller!.status == AnimationStatus.dismissed && !_popFinalized;
-
-  bool _popFinalized = false;
+  bool get finishedWhenPopped => _controller!.status == AnimationStatus.dismissed;
 
   /// The animation that drives the route's transition and the previous route's
   /// forward transition.
@@ -208,7 +206,6 @@ abstract class TransitionRoute<T> extends OverlayRoute<T> {
         // removing the route and disposing it.
         if (!isActive) {
           navigator!.finalizeRoute(this);
-          _popFinalized = true;
         }
         break;
     }

packages/flutter/test/widgets/navigator_test.dart

@@ -2969,32 +2969,6 @@ void main() {
       expect(primaryAnimationOfRouteOne.status, AnimationStatus.dismissed);
     });
 
-    testWidgets('Pop no animation page does not crash', (WidgetTester tester) async {
-      // Regression Test for https://github.com/flutter/flutter/issues/86604.
-      Widget buildNavigator(bool secondPage) {
-        return Directionality(
-          textDirection: TextDirection.ltr,
-          child: Navigator(
-            pages: <Page<void>>[
-              const ZeroDurationPage(
-                child: Text('page1'),
-              ),
-              if (secondPage)
-                const ZeroDurationPage(
-                  child: Text('page2'),
-                ),
-            ],
-            onPopPage: (Route<dynamic> route, dynamic result) => false,
-          ),
-        );
-      }
-      await tester.pumpWidget(buildNavigator(true));
-      expect(find.text('page2'), findsOneWidget);
-
-      await tester.pumpWidget(buildNavigator(false));
-      expect(find.text('page1'), findsOneWidget);
-    });
-
     testWidgets('can work with pageless route', (WidgetTester tester) async {
       final GlobalKey<NavigatorState> navigator = GlobalKey<NavigatorState>();
       List<TestPage> myPages = <TestPage>[
@@ -3925,9 +3899,6 @@ class NavigatorObservation {
   final String? previous;
   final String? current;
   final String operation;
-
-  @override
-  String toString() => 'NavigatorObservation($operation, $current, $previous)';
 }
 
 class BuilderPage extends Page<void> {
@@ -3943,43 +3914,3 @@ class BuilderPage extends Page<void> {
     );
   }
 }
-
-class ZeroDurationPage extends Page<void> {
-  const ZeroDurationPage({required this.child});
-
-  final Widget child;
-
-  @override
-  Route<void> createRoute(BuildContext context) {
-    return ZeroDurationPageRoute(page: this);
-  }
-}
-
-class ZeroDurationPageRoute extends PageRoute<void> {
-  ZeroDurationPageRoute({required ZeroDurationPage page})
-      : super(settings: page);
-
-  @override
-  Duration get transitionDuration => Duration.zero;
-
-  ZeroDurationPage get _page => settings as ZeroDurationPage;
-
-  @override
-  Widget buildPage(BuildContext context, Animation<double> animation, Animation<double> secondaryAnimation) {
-    return _page.child;
-  }
-
-  @override
-  Widget buildTransitions(BuildContext context, Animation<double> animation, Animation<double> secondaryAnimation, Widget child) {
-    return child;
-  }
-
-  @override
-  bool get maintainState => false;
-
-  @override
-  Color? get barrierColor => null;
-
-  @override
-  String? get barrierLabel => null;
-}