Unverified Commit 27e1efc1 authored by Michael Goderbauer's avatar Michael Goderbauer Committed by GitHub

Delay Route disposal until OverlayEntries are unmounted (#68913)

parent e7b66ac7
...@@ -534,15 +534,7 @@ class _HeroFlight { ...@@ -534,15 +534,7 @@ class _HeroFlight {
); );
} }
void _handleAnimationUpdate(AnimationStatus status) { void _performAnimationUpdate(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;
if (status == AnimationStatus.completed || status == AnimationStatus.dismissed) { if (status == AnimationStatus.completed || status == AnimationStatus.dismissed) {
_proxyAnimation.parent = null; _proxyAnimation.parent = null;
...@@ -560,6 +552,35 @@ class _HeroFlight { ...@@ -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. // The simple case: we're either starting a push or a pop animation.
void start(_HeroFlightManifest initialManifest) { void start(_HeroFlightManifest initialManifest) {
assert(!_aborted); assert(!_aborted);
......
...@@ -3041,8 +3041,36 @@ class _RouteEntry extends RouteTransitionRecord { ...@@ -3041,8 +3041,36 @@ class _RouteEntry extends RouteTransitionRecord {
void dispose() { void dispose() {
assert(currentState.index < _RouteLifecycle.disposed.index); assert(currentState.index < _RouteLifecycle.disposed.index);
route.dispose();
currentState = _RouteLifecycle.disposed; 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<OverlayEntry> 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 { bool get willBePresent {
......
...@@ -45,13 +45,17 @@ import 'ticker_provider.dart'; ...@@ -45,13 +45,17 @@ import 'ticker_provider.dart';
/// if widgets in an overlay entry with [maintainState] set to true repeatedly /// if widgets in an overlay entry with [maintainState] set to true repeatedly
/// call [State.setState], the user's battery will be drained unnecessarily. /// 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: /// See also:
/// ///
/// * [Overlay] /// * [Overlay]
/// * [OverlayState] /// * [OverlayState]
/// * [WidgetsApp] /// * [WidgetsApp]
/// * [MaterialApp] /// * [MaterialApp]
class OverlayEntry { class OverlayEntry extends ChangeNotifier {
/// Creates an overlay entry. /// Creates an overlay entry.
/// ///
/// To insert the entry into an [Overlay], first find the overlay using /// To insert the entry into an [Overlay], first find the overlay using
...@@ -113,6 +117,19 @@ class OverlayEntry { ...@@ -113,6 +117,19 @@ class OverlayEntry {
_overlay!._didChangeEntryOpacity(); _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; OverlayState? _overlay;
final GlobalKey<_OverlayEntryWidgetState> _key = GlobalKey<_OverlayEntryWidgetState>(); final GlobalKey<_OverlayEntryWidgetState> _key = GlobalKey<_OverlayEntryWidgetState>();
...@@ -172,6 +189,18 @@ class _OverlayEntryWidget extends StatefulWidget { ...@@ -172,6 +189,18 @@ class _OverlayEntryWidget extends StatefulWidget {
} }
class _OverlayEntryWidgetState extends State<_OverlayEntryWidget> { class _OverlayEntryWidgetState extends State<_OverlayEntryWidget> {
@override
void initState() {
super.initState();
widget.entry._updateMounted(true);
}
@override
void dispose() {
widget.entry._updateMounted(false);
super.dispose();
}
@override @override
Widget build(BuildContext context) { Widget build(BuildContext context) {
return TickerMode( return TickerMode(
......
...@@ -189,7 +189,6 @@ abstract class TransitionRoute<T> extends OverlayRoute<T> { ...@@ -189,7 +189,6 @@ abstract class TransitionRoute<T> extends OverlayRoute<T> {
// removing the route and disposing it. // removing the route and disposing it.
if (!isActive) { if (!isActive) {
navigator!.finalizeRoute(this); navigator!.finalizeRoute(this);
assert(overlayEntries.isEmpty);
} }
break; break;
} }
......
...@@ -1632,6 +1632,57 @@ void main() { ...@@ -1632,6 +1632,57 @@ void main() {
expect(find.byType(CupertinoButton), findsOneWidget); expect(find.byType(CupertinoButton), findsOneWidget);
expect(find.text('PointerCancelEvents: 1'), 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<void> r = CupertinoPageRoute<void>(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<void>(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 { class MockNavigatorObserver extends NavigatorObserver {
......
...@@ -101,13 +101,16 @@ Future<void> runNavigatorTest( ...@@ -101,13 +101,16 @@ Future<void> runNavigatorTest(
WidgetTester tester, WidgetTester tester,
NavigatorState host, NavigatorState host,
VoidCallback test, VoidCallback test,
List<String> expectations, List<String> expectations, [
) async { List<String> expectationsAfterAnotherPump = const <String>[],
]) async {
expect(host, isNotNull); expect(host, isNotNull);
test(); test();
expect(results, equals(expectations)); expect(results, equals(expectations));
results.clear(); results.clear();
await tester.pump(); await tester.pump();
expect(results, equals(expectationsAfterAnotherPump));
results.clear();
} }
void main() { void main() {
...@@ -199,6 +202,8 @@ void main() { ...@@ -199,6 +202,8 @@ void main() {
<String>[ // stack is: initial, two <String>[ // stack is: initial, two
'third: didPop hello', 'third: didPop hello',
'two: didPopNext third', 'two: didPopNext third',
],
<String>[
'third: dispose', 'third: dispose',
], ],
); );
...@@ -209,6 +214,8 @@ void main() { ...@@ -209,6 +214,8 @@ void main() {
<String>[ // stack is: initial <String>[ // stack is: initial
'two: didPop good bye', 'two: didPop good bye',
'initial: didPopNext two', 'initial: didPopNext two',
],
<String>[
'two: dispose', 'two: dispose',
], ],
); );
...@@ -278,6 +285,8 @@ void main() { ...@@ -278,6 +285,8 @@ void main() {
<String>[ <String>[
'third: didPop good bye', 'third: didPop good bye',
'second: didPopNext third', 'second: didPopNext third',
],
<String>[
'third: dispose', 'third: dispose',
], ],
); );
...@@ -320,6 +329,8 @@ void main() { ...@@ -320,6 +329,8 @@ void main() {
<String>[ <String>[
'four: didPop the end', 'four: didPop the end',
'second: didPopNext four', 'second: didPopNext four',
],
<String>[
'four: dispose', 'four: dispose',
], ],
); );
...@@ -395,6 +406,8 @@ void main() { ...@@ -395,6 +406,8 @@ void main() {
<String>[ <String>[
'C: didPop null', 'C: didPop null',
'b: didPopNext C', 'b: didPopNext C',
],
<String>[
'C: dispose', 'C: dispose',
], ],
); );
......
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