Skip to content

Commit

Permalink
Fix janks and memory leaks in CupertinoPageTransition and `Cupertin…
Browse files Browse the repository at this point in the history 
…oFullscreenDialogTransition` (#146999)
  • Loading branch information
@ValentinVignal
ValentinVignal authored May 6, 2024
1 parent 9dac4ed commit f3978c7
Show file tree
Hide file tree
Showing 2 changed files with 198 additions and 9 deletions.
8 changes: 5 additions & 3 deletions packages/flutter/lib/src/cupertino/route.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -435,22 +435,25 @@ class _CupertinoPageTransitionState extends State<CupertinoPageTransition> {
super.didUpdateWidget(oldWidget);
if (oldWidget.primaryRouteAnimation != widget.primaryRouteAnimation
|| oldWidget.secondaryRouteAnimation != widget.secondaryRouteAnimation
|| oldWidget.child != widget.child || oldWidget.linearTransition != widget.linearTransition) {
|| oldWidget.linearTransition != widget.linearTransition) {
_disposeCurve();
_setupAnimation();
}
}

@override
void dispose() {
super.dispose();
_disposeCurve();
super.dispose();
}

void _disposeCurve() {
_primaryPositionCurve?.dispose();
_secondaryPositionCurve?.dispose();
_primaryShadowCurve?.dispose();
_primaryPositionCurve = null;
_secondaryPositionCurve = null;
_primaryShadowCurve = null;
}

void _setupAnimation() {
Expand Down Expand Up @@ -557,7 +560,6 @@ class _CupertinoFullscreenDialogTransitionState extends State<CupertinoFullscree
super.didUpdateWidget(oldWidget);
if (oldWidget.primaryRouteAnimation != widget.primaryRouteAnimation ||
oldWidget.secondaryRouteAnimation != widget.secondaryRouteAnimation ||
oldWidget.child != widget.child ||
oldWidget.linearTransition != widget.linearTransition) {
_disposeCurve();
_setupAnimation();
Expand Down
199 changes: 193 additions & 6 deletions packages/flutter/test/cupertino/route_test.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -644,7 +644,10 @@ void main() {
);
});

testWidgets('Fullscreen route animates correct transform values over time', (WidgetTester tester) async {
testWidgets('Fullscreen route animates correct transform values over time',
// TODO(polina-c): remove when fixed https:/flutter/flutter/issues/145600 [leak-tracking-opt-in]
experimentalLeakTesting: LeakTesting.settings.withTracked(classes: const <String>['CurvedAnimation']),
(WidgetTester tester) async {
await tester.pumpWidget(
CupertinoApp(
home: Builder(
Expand Down Expand Up @@ -712,11 +715,27 @@ void main() {
await tester.pump(const Duration(milliseconds: 40));
expect(tester.getTopLeft(find.byType(Placeholder)).dy, moreOrLessEquals(7.4, epsilon: 0.1));

await tester.pump(const Duration(milliseconds: 50));
expect(tester.getTopLeft(find.byType(Placeholder)).dy, moreOrLessEquals(3, epsilon: 0.1));

await tester.pump(const Duration(milliseconds: 50));
expect(tester.getTopLeft(find.byType(Placeholder)).dy, moreOrLessEquals(0, epsilon: 0.1));

// Give time to the animation to finish and update its status to
// AnimationState.completed, so the reverse curved can be used in the next
// step.
await tester.pumpAndSettle(const Duration(milliseconds: 1));

// Exit animation
await tester.tap(find.text('Close'));
await tester.pump();

await tester.pump(const Duration(milliseconds: 50));
expect(tester.getTopLeft(find.byType(Placeholder)).dy, moreOrLessEquals(156.3, epsilon: 0.1));

await tester.pump(const Duration(milliseconds: 50));
expect(tester.getTopLeft(find.byType(Placeholder)).dy, moreOrLessEquals(308.1, epsilon: 0.1));

await tester.pump(const Duration(milliseconds: 40));
expect(tester.getTopLeft(find.byType(Placeholder)).dy, moreOrLessEquals(411.03, epsilon: 0.1));

Expand Down Expand Up @@ -818,25 +837,173 @@ void main() {
await tester.pump(const Duration(milliseconds: 40));
expect(tester.getTopLeft(find.byType(Placeholder)).dx, moreOrLessEquals(-263.0, epsilon: 1.0));

await tester.pump(const Duration(milliseconds: 50));
expect(tester.getTopLeft(find.byType(Placeholder)).dx, moreOrLessEquals(-265.0, epsilon: 1.0));

await tester.pump(const Duration(milliseconds: 50));
expect(tester.getTopLeft(find.byType(Placeholder)).dx, moreOrLessEquals(-266.0, epsilon: 1.0));

// Give time to the animation to finish and update its status to
// AnimationState.completed, so the reverse curved can be used in the next
// step.
await tester.pumpAndSettle(const Duration(milliseconds: 1));

// Exit animation
await tester.tap(find.text('Close'));
await tester.pump();

await tester.pump(const Duration(milliseconds: 50));
expect(tester.getTopLeft(find.byType(Placeholder)).dx, moreOrLessEquals(-197.0, epsilon: 1.0));

await tester.pump(const Duration(milliseconds: 50));
expect(tester.getTopLeft(find.byType(Placeholder)).dx, moreOrLessEquals(-129.0, epsilon: 1.0));

await tester.pump(const Duration(milliseconds: 40));
expect(tester.getTopLeft(find.byType(Placeholder)).dx, moreOrLessEquals(-83.0, epsilon: 1.0));

await tester.pump(const Duration(milliseconds: 360));
expect(tester.getTopLeft(find.byType(Placeholder)).dx, moreOrLessEquals(-0.0, epsilon: 1.0));
}

testWidgets('CupertinoPageRoute has parallax when non fullscreenDialog route is pushed on top', (WidgetTester tester) async {
testWidgets('CupertinoPageRoute has parallax when non fullscreenDialog route is pushed on top',
// TODO(polina-c): remove when fixed https:/flutter/flutter/issues/145600 [leak-tracking-opt-in]
experimentalLeakTesting: LeakTesting.settings.withTracked(classes: const <String>['CurvedAnimation']),
(WidgetTester tester) async {
await testParallax(tester, fromFullscreenDialog: false);
});

testWidgets('FullscreenDialog CupertinoPageRoute has parallax when non fullscreenDialog route is pushed on top', (WidgetTester tester) async {
testWidgets('FullscreenDialog CupertinoPageRoute has parallax when non fullscreenDialog route is pushed on top',
// TODO(polina-c): remove when fixed https:/flutter/flutter/issues/145600 [leak-tracking-opt-in]
experimentalLeakTesting: LeakTesting.settings.withTracked(classes: const <String>['CurvedAnimation']),
(WidgetTester tester) async {
await testParallax(tester, fromFullscreenDialog: true);
});

group('Interrupted push', () {
Future<void> testParallax(WidgetTester tester, {required bool fromFullscreenDialog}) async {
await tester.pumpWidget(
CupertinoApp(
onGenerateRoute: (RouteSettings settings) => CupertinoPageRoute<void>(
fullscreenDialog: fromFullscreenDialog,
settings: settings,
builder: (BuildContext context) {
return Column(
children: <Widget>[
const Placeholder(),
CupertinoButton(
child: const Text('Button'),
onPressed: () {
Navigator.push<void>(context, CupertinoPageRoute<void>(
builder: (BuildContext context) {
return CupertinoButton(
child: const Text('Close'),
onPressed: () {
Navigator.pop<void>(context);
},
);
},
));
},
),
],
);
},
),
),
);

// Enter animation.
await tester.tap(find.text('Button'));
expect(tester.getTopLeft(find.byType(Placeholder)).dx, moreOrLessEquals(0.0, epsilon: 0.1));
await tester.pump();

// The push animation duration is 500ms. We let it run for 400ms before
// interrupting and popping it.

await tester.pump(const Duration(milliseconds: 40));
expect(tester.getTopLeft(find.byType(Placeholder)).dx, moreOrLessEquals(-55.0, epsilon: 1.0));

await tester.pump(const Duration(milliseconds: 40));
expect(tester.getTopLeft(find.byType(Placeholder)).dx, moreOrLessEquals(-111.0, epsilon: 1.0));

await tester.pump(const Duration(milliseconds: 40));
expect(tester.getTopLeft(find.byType(Placeholder)).dx, moreOrLessEquals(-161.0, epsilon: 1.0));

await tester.pump(const Duration(milliseconds: 40));
expect(tester.getTopLeft(find.byType(Placeholder)).dx, moreOrLessEquals(-200.0, epsilon: 1.0));

await tester.pump(const Duration(milliseconds: 40));
expect(tester.getTopLeft(find.byType(Placeholder)).dx, moreOrLessEquals(-226.0, epsilon: 1.0));

await tester.pump(const Duration(milliseconds: 40));
expect(tester.getTopLeft(find.byType(Placeholder)).dx, moreOrLessEquals(-242.0, epsilon: 1.0));

await tester.pump(const Duration(milliseconds: 40));
expect(tester.getTopLeft(find.byType(Placeholder)).dx, moreOrLessEquals(-251.0, epsilon: 1.0));

await tester.pump(const Duration(milliseconds: 40));
expect(tester.getTopLeft(find.byType(Placeholder)).dx, moreOrLessEquals(-257.0, epsilon: 1.0));

await tester.pump(const Duration(milliseconds: 40));
expect(tester.getTopLeft(find.byType(Placeholder)).dx, moreOrLessEquals(-261.0, epsilon: 1.0));

await tester.pump(const Duration(milliseconds: 40));
expect(tester.getTopLeft(find.byType(Placeholder)).dx, moreOrLessEquals(-263.0, epsilon: 1.0));

// Exit animation
await tester.tap(find.text('Close'));
await tester.pump();

// When the push animation is interrupted, the forward curved is used for
// the reversed animation to avoid discontinuities.


await tester.pump(const Duration(milliseconds: 40));
expect(tester.getTopLeft(find.byType(Placeholder)).dx, moreOrLessEquals(-261.0, epsilon: 1.0));

await tester.pump(const Duration(milliseconds: 40));
expect(tester.getTopLeft(find.byType(Placeholder)).dx, moreOrLessEquals(-257.0, epsilon: 1.0));

await tester.pump(const Duration(milliseconds: 40));
expect(tester.getTopLeft(find.byType(Placeholder)).dx, moreOrLessEquals(-251.0, epsilon: 1.0));

await tester.pump(const Duration(milliseconds: 40));
expect(tester.getTopLeft(find.byType(Placeholder)).dx, moreOrLessEquals(-242.0, epsilon: 1.0));

await tester.pump(const Duration(milliseconds: 40));
expect(tester.getTopLeft(find.byType(Placeholder)).dx, moreOrLessEquals(-226.0, epsilon: 1.0));

await tester.pump(const Duration(milliseconds: 40));
expect(tester.getTopLeft(find.byType(Placeholder)).dx, moreOrLessEquals(-200.0, epsilon: 1.0));

await tester.pump(const Duration(milliseconds: 40));
expect(tester.getTopLeft(find.byType(Placeholder)).dx, moreOrLessEquals(-161.0, epsilon: 1.0));

await tester.pump(const Duration(milliseconds: 40));
expect(tester.getTopLeft(find.byType(Placeholder)).dx, moreOrLessEquals(-111.0, epsilon: 1.0));

await tester.pump(const Duration(milliseconds: 40));
expect(tester.getTopLeft(find.byType(Placeholder)).dx, moreOrLessEquals(-55.0, epsilon: 1.0));

await tester.pump(const Duration(milliseconds: 40));
expect(tester.getTopLeft(find.byType(Placeholder)).dx, moreOrLessEquals(0.0, epsilon: 1.0));
}

testWidgets('CupertinoPageRoute has parallax when non fullscreenDialog route is pushed on top and gets popped before the end of the animation',
// TODO(polina-c): remove when fixed https:/flutter/flutter/issues/145600 [leak-tracking-opt-in]
experimentalLeakTesting: LeakTesting.settings.withTracked(classes: const <String>['CurvedAnimation']),
(WidgetTester tester) async {
await testParallax(tester, fromFullscreenDialog: false);
});

testWidgets('FullscreenDialog CupertinoPageRoute has parallax when non fullscreenDialog route is pushed on top and gets popped before the end of the animation',
// TODO(polina-c): remove when fixed https:/flutter/flutter/issues/145600 [leak-tracking-opt-in]
experimentalLeakTesting: LeakTesting.settings.withTracked(classes: const <String>['CurvedAnimation']),
(WidgetTester tester) async {
await testParallax(tester, fromFullscreenDialog: true);
});
});

Future<void> testNoParallax(WidgetTester tester, {required bool fromFullscreenDialog}) async{
await tester.pumpWidget(
CupertinoApp(
Expand Down Expand Up @@ -918,15 +1085,24 @@ void main() {
expect(tester.getTopLeft(find.byType(Placeholder)).dx, 0.0);
}

testWidgets('CupertinoPageRoute has no parallax when fullscreenDialog route is pushed on top', (WidgetTester tester) async {
testWidgets('CupertinoPageRoute has no parallax when fullscreenDialog route is pushed on top',
// TODO(polina-c): remove when fixed https:/flutter/flutter/issues/145600 [leak-tracking-opt-in]
experimentalLeakTesting: LeakTesting.settings.withTracked(classes: const <String>['CurvedAnimation']),
(WidgetTester tester) async {
await testNoParallax(tester, fromFullscreenDialog: false);
});

testWidgets('FullscreenDialog CupertinoPageRoute has no parallax when fullscreenDialog route is pushed on top', (WidgetTester tester) async {
testWidgets('FullscreenDialog CupertinoPageRoute has no parallax when fullscreenDialog route is pushed on top',
// TODO(polina-c): remove when fixed https:/flutter/flutter/issues/145600 [leak-tracking-opt-in]
experimentalLeakTesting: LeakTesting.settings.withTracked(classes: const <String>['CurvedAnimation']),
(WidgetTester tester) async {
await testNoParallax(tester, fromFullscreenDialog: true);
});

testWidgets('Animated push/pop is not linear', (WidgetTester tester) async {
testWidgets('Animated push/pop is not linear',
// TODO(polina-c): remove when fixed https:/flutter/flutter/issues/145600 [leak-tracking-opt-in]
experimentalLeakTesting: LeakTesting.settings.withTracked(classes: const <String>['CurvedAnimation']),
(WidgetTester tester) async {
await tester.pumpWidget(
const CupertinoApp(
home: Text('1'),
Expand All @@ -944,6 +1120,11 @@ void main() {
tester.state<NavigatorState>(find.byType(Navigator)).push(route2);
// The whole transition is 500ms based on CupertinoPageRoute.transitionDuration.
// Break it up into small chunks.
//
// The screen width is 800.
// The top left corner of the text 1 will go from 0 to -800 / 3 = - 266.67.
// The top left corner of the text 2 will go from 800 to 0.


await tester.pump();
await tester.pump(const Duration(milliseconds: 50));
Expand All @@ -961,8 +1142,14 @@ void main() {

// Finish the rest of the animation
await tester.pump(const Duration(milliseconds: 350));
// Give time to the animation to finish and update its status to
// AnimationState.completed, so the reverse curved can be used in the next
// step.
await tester.pumpAndSettle(const Duration(milliseconds: 1));

tester.state<NavigatorState>(find.byType(Navigator)).pop();
// The top left corner of the text 1 will go from -800 / 3 = - 266.67 to 0.
// The top left corner of the text 2 will go from 0 to 800.
await tester.pump();
await tester.pump(const Duration(milliseconds: 50));
expect(tester.getTopLeft(find.text('1')).dx, moreOrLessEquals(-197, epsilon: 1));
Expand Down

0 comments on commit f3978c7

Please sign in to comment.