diff --git a/packages/flutter/lib/src/widgets/heroes.dart b/packages/flutter/lib/src/widgets/heroes.dart index 572beb195e..6a1e497a87 100644 --- a/packages/flutter/lib/src/widgets/heroes.dart +++ b/packages/flutter/lib/src/widgets/heroes.dart @@ -534,15 +534,7 @@ class _HeroFlight { ); } - void _handleAnimationUpdate(AnimationStatus status) { - // The animation will not finish until the user lifts their finger, so we - // should ignore the status update if the gesture is in progress. - // - // This also relies on the animation to update its status at the end of the - // gesture. See the _CupertinoBackGestureController.dragEnd for how - // cupertino page route achieves that. - if (manifest!.fromRoute.navigator?.userGestureInProgress == true) - return; + void _performAnimationUpdate(AnimationStatus status) { if (status == AnimationStatus.completed || status == AnimationStatus.dismissed) { _proxyAnimation.parent = null; @@ -560,6 +552,35 @@ class _HeroFlight { } } + bool _scheduledPerformAnimtationUpdate = false; + void _handleAnimationUpdate(AnimationStatus status) { + // The animation will not finish until the user lifts their finger, so we + // should suppress the status update if the gesture is in progress, and + // delay it until the finger is lifted. + if (manifest!.fromRoute.navigator?.userGestureInProgress != true) { + _performAnimationUpdate(status); + return; + } + + if (_scheduledPerformAnimtationUpdate) + return; + + // The `navigator` must be non-null here, or the first if clause above would + // have returned from this method. + final NavigatorState navigator = manifest!.fromRoute.navigator!; + + void delayedPerformAnimtationUpdate() { + assert(!navigator.userGestureInProgress); + assert(_scheduledPerformAnimtationUpdate); + _scheduledPerformAnimtationUpdate = false; + navigator.userGestureInProgressNotifier.removeListener(delayedPerformAnimtationUpdate); + _performAnimationUpdate(_proxyAnimation.status); + } + assert(navigator.userGestureInProgress); + _scheduledPerformAnimtationUpdate = true; + navigator.userGestureInProgressNotifier.addListener(delayedPerformAnimtationUpdate); + } + // The simple case: we're either starting a push or a pop animation. void start(_HeroFlightManifest initialManifest) { assert(!_aborted); diff --git a/packages/flutter/lib/src/widgets/navigator.dart b/packages/flutter/lib/src/widgets/navigator.dart index 6884e8908b..5481641d12 100644 --- a/packages/flutter/lib/src/widgets/navigator.dart +++ b/packages/flutter/lib/src/widgets/navigator.dart @@ -3041,8 +3041,36 @@ class _RouteEntry extends RouteTransitionRecord { void dispose() { assert(currentState.index < _RouteLifecycle.disposed.index); - route.dispose(); currentState = _RouteLifecycle.disposed; + + // If the overlay entries are still mounted, widgets in the route's subtree + // may still reference resources from the route and we delay disposal of + // the route until the overlay entries are no longer mounted. + // Since the overlay entry is the root of the route's subtree it will only + // get unmounted after every other widget in the subtree has been unmounted. + + final Iterable mountedEntries = route.overlayEntries.where((OverlayEntry e) => e.mounted); + + if (mountedEntries.isEmpty) { + route.dispose(); + } else { + int mounted = mountedEntries.length; + assert(mounted > 0); + for (final OverlayEntry entry in mountedEntries) { + late VoidCallback listener; + listener = () { + assert(mounted > 0); + assert(!entry.mounted); + mounted--; + entry.removeListener(listener); + if (mounted == 0) { + assert(route.overlayEntries.every((OverlayEntry e) => !e.mounted)); + route.dispose(); + } + }; + entry.addListener(listener); + } + } } bool get willBePresent { diff --git a/packages/flutter/lib/src/widgets/overlay.dart b/packages/flutter/lib/src/widgets/overlay.dart index 444f76cfc0..ac95784fb2 100644 --- a/packages/flutter/lib/src/widgets/overlay.dart +++ b/packages/flutter/lib/src/widgets/overlay.dart @@ -45,13 +45,17 @@ import 'ticker_provider.dart'; /// if widgets in an overlay entry with [maintainState] set to true repeatedly /// call [State.setState], the user's battery will be drained unnecessarily. /// +/// [OverlayEntry] is a [ChangeNotifier] that notifies when the widget built by +/// [builder] is mounted or unmounted, whose exact state can be queried by +/// [mounted]. +/// /// See also: /// /// * [Overlay] /// * [OverlayState] /// * [WidgetsApp] /// * [MaterialApp] -class OverlayEntry { +class OverlayEntry extends ChangeNotifier { /// Creates an overlay entry. /// /// To insert the entry into an [Overlay], first find the overlay using @@ -113,6 +117,19 @@ class OverlayEntry { _overlay!._didChangeEntryOpacity(); } + /// Whether the [OverlayEntry] is currently mounted in the widget tree. + /// + /// The [OverlayEntry] notifies its listeners when this value changes. + bool get mounted => _mounted; + bool _mounted = false; + void _updateMounted(bool value) { + if (value == _mounted) { + return; + } + _mounted = value; + notifyListeners(); + } + OverlayState? _overlay; final GlobalKey<_OverlayEntryWidgetState> _key = GlobalKey<_OverlayEntryWidgetState>(); @@ -172,6 +189,18 @@ class _OverlayEntryWidget extends StatefulWidget { } class _OverlayEntryWidgetState extends State<_OverlayEntryWidget> { + @override + void initState() { + super.initState(); + widget.entry._updateMounted(true); + } + + @override + void dispose() { + widget.entry._updateMounted(false); + super.dispose(); + } + @override Widget build(BuildContext context) { return TickerMode( diff --git a/packages/flutter/lib/src/widgets/routes.dart b/packages/flutter/lib/src/widgets/routes.dart index cf136f80c9..d3874a4f17 100644 --- a/packages/flutter/lib/src/widgets/routes.dart +++ b/packages/flutter/lib/src/widgets/routes.dart @@ -189,7 +189,6 @@ abstract class TransitionRoute extends OverlayRoute { // removing the route and disposing it. if (!isActive) { navigator!.finalizeRoute(this); - assert(overlayEntries.isEmpty); } break; } diff --git a/packages/flutter/test/cupertino/route_test.dart b/packages/flutter/test/cupertino/route_test.dart index 0a9a8bd1cc..682f078b06 100644 --- a/packages/flutter/test/cupertino/route_test.dart +++ b/packages/flutter/test/cupertino/route_test.dart @@ -1632,6 +1632,57 @@ void main() { expect(find.byType(CupertinoButton), findsOneWidget); expect(find.text('PointerCancelEvents: 1'), findsOneWidget); }); + + testWidgets('Popping routes during back swipe should not crash', (WidgetTester tester) async { + // Regression test for https://github.com/flutter/flutter/issues/63984#issuecomment-675679939 + + final CupertinoPageRoute r = CupertinoPageRoute(builder: (BuildContext context) { + return const Scaffold( + body: Center( + child: Text('child'), + ), + ); + }); + + late NavigatorState navigator; + + await tester.pumpWidget(CupertinoApp( + home: Center( + child: Builder(builder: (BuildContext context) { + return RaisedButton( + child: const Text('Home'), + onPressed: () { + navigator = Navigator.of(context)!; + assert(navigator != null); + navigator.push(r); + }, + ); + }), + ), + )); + + final TestGesture gesture = await tester.createGesture(); + await gesture.down(tester.getCenter(find.byType(RaisedButton))); + await gesture.up(); + + await tester.pumpAndSettle(); + + await gesture.down(const Offset(3, 300), timeStamp: Duration.zero); + + // Need 2 events to form a valid drag + await tester.pump(const Duration(milliseconds: 100)); + await gesture.moveTo(const Offset(30, 300), timeStamp: const Duration(milliseconds: 100)); + await tester.pump(const Duration(milliseconds: 200)); + await gesture.moveTo(const Offset(50, 300), timeStamp: const Duration(milliseconds: 200)); + + // Pause a while so that the route is popped when the drag is canceled + await tester.pump(const Duration(milliseconds: 1000)); + await gesture.moveTo(const Offset(51, 300), timeStamp: const Duration(milliseconds: 1200)); + + // Remove the drag + navigator.removeRoute(r); + await tester.pump(); + }); } class MockNavigatorObserver extends NavigatorObserver { diff --git a/packages/flutter/test/widgets/routes_test.dart b/packages/flutter/test/widgets/routes_test.dart index b5fdfb8df3..58a26492f3 100644 --- a/packages/flutter/test/widgets/routes_test.dart +++ b/packages/flutter/test/widgets/routes_test.dart @@ -101,13 +101,16 @@ Future runNavigatorTest( WidgetTester tester, NavigatorState host, VoidCallback test, - List expectations, -) async { + List expectations, [ + List expectationsAfterAnotherPump = const [], +]) async { expect(host, isNotNull); test(); expect(results, equals(expectations)); results.clear(); await tester.pump(); + expect(results, equals(expectationsAfterAnotherPump)); + results.clear(); } void main() { @@ -199,6 +202,8 @@ void main() { [ // stack is: initial, two 'third: didPop hello', 'two: didPopNext third', + ], + [ 'third: dispose', ], ); @@ -209,6 +214,8 @@ void main() { [ // stack is: initial 'two: didPop good bye', 'initial: didPopNext two', + ], + [ 'two: dispose', ], ); @@ -278,6 +285,8 @@ void main() { [ 'third: didPop good bye', 'second: didPopNext third', + ], + [ 'third: dispose', ], ); @@ -320,6 +329,8 @@ void main() { [ 'four: didPop the end', 'second: didPopNext four', + ], + [ 'four: dispose', ], ); @@ -395,6 +406,8 @@ void main() { [ 'C: didPop null', 'b: didPopNext C', + ], + [ 'C: dispose', ], );