From cc9ac7d13cccd24d72faab566f9fb59663c3c3d1 Mon Sep 17 00:00:00 2001 From: Dimil Kalathiya <102401667+Dimilkalathiya@users.noreply.github.com> Date: Sat, 27 Apr 2024 01:49:31 +0530 Subject: [PATCH] fixes `CupertinoFullscreenDialogTransition` leaks (#147168) --- packages/flutter/lib/src/cupertino/route.dart | 98 ++++++++++++++----- .../flutter/test/cupertino/route_test.dart | 48 +++++++++ 2 files changed, 123 insertions(+), 23 deletions(-) diff --git a/packages/flutter/lib/src/cupertino/route.dart b/packages/flutter/lib/src/cupertino/route.dart index f698d1aad7..df7e8626cb 100644 --- a/packages/flutter/lib/src/cupertino/route.dart +++ b/packages/flutter/lib/src/cupertino/route.dart @@ -506,44 +506,96 @@ class _CupertinoPageTransitionState extends State { /// /// For example, used when creating a new calendar event by bringing in the next /// screen from the bottom. -class CupertinoFullscreenDialogTransition extends StatelessWidget { +class CupertinoFullscreenDialogTransition extends StatefulWidget { /// Creates an iOS-style transition used for summoning fullscreen dialogs. /// + const CupertinoFullscreenDialogTransition({ + super.key, + required this.primaryRouteAnimation, + required this.secondaryRouteAnimation, + required this.child, + required this.linearTransition, + }); + /// * `primaryRouteAnimation` is a linear route animation from 0.0 to 1.0 /// when this screen is being pushed. + final Animation primaryRouteAnimation; + /// * `secondaryRouteAnimation` is a linear route animation from 0.0 to 1.0 /// when another screen is being pushed on top of this one. - /// * `linearTransition` is whether to perform the secondary transition linearly. + final Animation secondaryRouteAnimation; + + /// * `linearTransition` is whether to perform the transitions linearly. /// Used to precisely track back gesture drags. - CupertinoFullscreenDialogTransition({ - super.key, - required Animation primaryRouteAnimation, - required Animation secondaryRouteAnimation, - required this.child, - required bool linearTransition, - }) : _positionAnimation = CurvedAnimation( - parent: primaryRouteAnimation, + final bool linearTransition; + + /// The widget below this widget in the tree. + final Widget child; + + @override + State createState() => _CupertinoFullscreenDialogTransitionState(); +} + +class _CupertinoFullscreenDialogTransitionState extends State { + /// When this page is coming in to cover another page. + late Animation _primaryPositionAnimation; + /// When this page is becoming covered by another page. + late Animation _secondaryPositionAnimation; + /// Curve of primary page which is coming in to cover another page. + CurvedAnimation? _primaryPositionCurve; + /// Curve of secondary page which is becoming covered by another page. + CurvedAnimation? _secondaryPositionCurve; + + @override + void initState() { + super.initState(); + _setupAnimation(); + } + + @override + void didUpdateWidget(covariant CupertinoFullscreenDialogTransition oldWidget) { + super.didUpdateWidget(oldWidget); + if (oldWidget.primaryRouteAnimation != widget.primaryRouteAnimation || + oldWidget.secondaryRouteAnimation != widget.secondaryRouteAnimation || + oldWidget.child != widget.child || + oldWidget.linearTransition != widget.linearTransition) { + _disposeCurve(); + _setupAnimation(); + } + } + + @override + void dispose() { + _disposeCurve(); + super.dispose(); + } + + void _disposeCurve() { + _primaryPositionCurve?.dispose(); + _secondaryPositionCurve?.dispose(); + _primaryPositionCurve = null; + _secondaryPositionCurve = null; + } + + void _setupAnimation() { + _primaryPositionAnimation = (_primaryPositionCurve = CurvedAnimation( + parent: widget.primaryRouteAnimation, curve: Curves.linearToEaseOut, // The curve must be flipped so that the reverse animation doesn't play // an ease-in curve, which iOS does not use. reverseCurve: Curves.linearToEaseOut.flipped, - ).drive(_kBottomUpTween), + )).drive(_kBottomUpTween); _secondaryPositionAnimation = - (linearTransition - ? secondaryRouteAnimation - : CurvedAnimation( - parent: secondaryRouteAnimation, + (widget.linearTransition + ? widget.secondaryRouteAnimation + : _secondaryPositionCurve = CurvedAnimation( + parent: widget.secondaryRouteAnimation, curve: Curves.linearToEaseOut, reverseCurve: Curves.easeInToLinear, ) ).drive(_kMiddleLeftTween); + } - final Animation _positionAnimation; - // When this page is becoming covered by another page. - final Animation _secondaryPositionAnimation; - - /// The widget below this widget in the tree. - final Widget child; @override Widget build(BuildContext context) { @@ -554,8 +606,8 @@ class CupertinoFullscreenDialogTransition extends StatelessWidget { textDirection: textDirection, transformHitTests: false, child: SlideTransition( - position: _positionAnimation, - child: child, + position: _primaryPositionAnimation, + child: widget.child, ), ); } diff --git a/packages/flutter/test/cupertino/route_test.dart b/packages/flutter/test/cupertino/route_test.dart index 181e5d0144..f05b299b3a 100644 --- a/packages/flutter/test/cupertino/route_test.dart +++ b/packages/flutter/test/cupertino/route_test.dart @@ -12,6 +12,7 @@ import 'package:flutter/foundation.dart'; import 'package:flutter/material.dart'; import 'package:flutter/rendering.dart'; import 'package:flutter_test/flutter_test.dart'; +import 'package:leak_tracker_flutter_testing/leak_tracker_flutter_testing.dart'; import '../widgets/semantics_tester.dart'; @@ -2431,6 +2432,53 @@ void main() { expect(tester.getBottomRight(find.byType(Placeholder)).dx, 390.0); }); }); + + testWidgets( + // TODO(polina-c): remove when fixed https://github.com/flutter/flutter/issues/145600 [leak-tracking-opt-in] + experimentalLeakTesting: LeakTesting.settings.withTracked(classes: ['CurvedAnimation']), + 'Fullscreen route does not leak CurveAnimation', (WidgetTester tester) async { + await tester.pumpWidget( + MaterialApp( + home: Builder( + builder: (BuildContext context) { + return CupertinoButton( + child: const Text('Button'), + onPressed: () { + Navigator.push(context, CupertinoPageRoute( + fullscreenDialog: true, + builder: (BuildContext context) { + return Column( + children: [ + const Placeholder(), + CupertinoButton( + child: const Text('Close'), + onPressed: () { + Navigator.pop(context); + }, + ), + ], + ); + }, + )); + }, + ); + }, + ), + ), + ); + + // Enter animation. + await tester.tap(find.text('Button')); + await tester.pump(); + + await tester.pump(const Duration(milliseconds: 400)); + + // Exit animation + await tester.tap(find.text('Close')); + await tester.pump(); + + await tester.pump(const Duration(milliseconds: 400)); + }); } class MockNavigatorObserver extends NavigatorObserver {