Fix memory leaks in context menu (#147822)

engine-flutter-autoroll · May 8, 2024 · 1a8aeb1 · 1a8aeb1
1 parent 8e834b5
commit 1a8aeb1
Show file tree

Hide file tree

Showing 2 changed files with 60 additions and 28 deletions.
diff --git a/packages/flutter/lib/src/cupertino/context_menu.dart b/packages/flutter/lib/src/cupertino/context_menu.dart
@@ -626,6 +626,7 @@ class _DecoyChild extends StatefulWidget {
 class _DecoyChildState extends State<_DecoyChild> with TickerProviderStateMixin {
   late Animation<Rect?> _rect;
   late Animation<Decoration> _boxDecoration;
+  late final CurvedAnimation _boxDecorationCurvedAnimation;
 
   @override
   void initState() {
@@ -670,18 +671,18 @@ class _DecoyChildState extends State<_DecoyChild> with TickerProviderStateMixin
       ),
     ]).animate(widget.controller);
 
+    _boxDecorationCurvedAnimation = CurvedAnimation(
+        parent: widget.controller,
+        curve: Interval(0.0, CupertinoContextMenu.animationOpensAt),
+    );
     _boxDecoration = DecorationTween(
       begin: const BoxDecoration(
         boxShadow: <BoxShadow>[],
       ),
       end: const BoxDecoration(
         boxShadow: _endBoxShadow,
       ),
-    ).animate(CurvedAnimation(
-        parent: widget.controller,
-        curve: Interval(0.0, CupertinoContextMenu.animationOpensAt),
-      ),
-    );
+    ).animate(_boxDecorationCurvedAnimation);
   }
 
   Widget _buildAnimation(BuildContext context, Widget? child) {
@@ -701,6 +702,12 @@ class _DecoyChildState extends State<_DecoyChild> with TickerProviderStateMixin
     );
   }
 
+  @override
+  void dispose() {
+    _boxDecorationCurvedAnimation.dispose();
+    super.dispose();
+  }
+
   @override
   Widget build(BuildContext context) {
     return Stack(
@@ -793,6 +800,10 @@ class _ContextMenuRoute<T> extends PopupRoute<T> {
   @override
   Duration get transitionDuration => _kModalPopupTransitionDuration;
 
+  CurvedAnimation? _curvedAnimation;
+
+  CurvedAnimation? _sheetOpacityCurvedAnimation;
+
   // Getting the RenderBox doesn't include the scale from the Transform.scale,
   // so it's manually accounted for here.
   static Rect _getScaledRect(GlobalKey globalKey, double scale) {
@@ -840,10 +851,11 @@ class _ContextMenuRoute<T> extends PopupRoute<T> {
   void _onDismiss(BuildContext context, double scale, double opacity) {
     _scale = scale;
     _opacityTween.end = opacity;
-    _sheetOpacity = _opacityTween.animate(CurvedAnimation(
+    _sheetOpacityCurvedAnimation = CurvedAnimation(
       parent: animation!,
       curve: const Interval(0.9, 1.0),
-    ));
+    );
+    _sheetOpacity = _opacityTween.animate(_sheetOpacityCurvedAnimation!);
     Navigator.of(context).pop();
   }
 
@@ -918,10 +930,14 @@ class _ContextMenuRoute<T> extends PopupRoute<T> {
   @override
   Animation<double> createAnimation() {
     final Animation<double> animation = super.createAnimation();
-    _sheetOpacity = _opacityTween.animate(CurvedAnimation(
-      parent: animation,
-      curve: Curves.linear,
-    ));
+    if (_curvedAnimation?.parent != animation) {
+      _curvedAnimation?.dispose();
+      _curvedAnimation = CurvedAnimation(
+        parent: animation,
+        curve: Curves.linear,
+      );
+    }
+    _sheetOpacity = _opacityTween.animate(_curvedAnimation!);
     return animation;
   }
 
@@ -995,6 +1011,13 @@ class _ContextMenuRoute<T> extends PopupRoute<T> {
       },
     );
   }
+
+  @override
+  void dispose() {
+    _curvedAnimation?.dispose();
+    _sheetOpacityCurvedAnimation?.dispose();
+    super.dispose();
+  }
 }
 
 // The final state of the _ContextMenuRoute after animating in and before
@@ -1034,8 +1057,10 @@ class _ContextMenuRouteStaticState extends State<_ContextMenuRouteStatic> with T
 
   late Offset _dragOffset;
   double _lastScale = 1.0;
-  late AnimationController _moveController;
-  late AnimationController _sheetController;
+  late final AnimationController _moveController;
+  late final CurvedAnimation _moveCurvedAnimation;
+  late final AnimationController _sheetController;
+  late final CurvedAnimation _sheetCurvedAnimation;
   late Animation<Offset> _moveAnimation;
   late Animation<double> _sheetScaleAnimation;
   late Animation<double> _sheetOpacityAnimation;
@@ -1148,12 +1173,7 @@ class _ContextMenuRouteStaticState extends State<_ContextMenuRouteStatic> with T
           clampDouble(endX, -_kPadding, _kPadding),
           endY,
         ),
-      ).animate(
-        CurvedAnimation(
-          parent: _moveController,
-          curve: Curves.elasticIn,
-        ),
-      );
+      ).animate(_moveCurvedAnimation);
 
       // Fade the _ContextMenuSheet out or in, if needed.
       if (_lastScale <= _kSheetScaleThreshold
@@ -1252,21 +1272,24 @@ class _ContextMenuRouteStaticState extends State<_ContextMenuRouteStatic> with T
       value: 1.0,
       vsync: this,
     );
+    _moveCurvedAnimation = CurvedAnimation(
+      parent: _moveController,
+     curve: Curves.elasticIn,
+    );
     _sheetController = AnimationController(
       duration: const Duration(milliseconds: 100),
       reverseDuration: const Duration(milliseconds: 300),
       vsync: this,
     );
-    _sheetScaleAnimation = Tween<double>(
-      begin: 1.0,
-      end: 0.0,
-    ).animate(
-      CurvedAnimation(
-        parent: _sheetController,
+    _sheetCurvedAnimation = CurvedAnimation(
+      parent: _sheetController,
         curve: Curves.linear,
         reverseCurve: Curves.easeInBack,
-      ),
     );
+    _sheetScaleAnimation = Tween<double>(
+      begin: 1.0,
+      end: 0.0,
+    ).animate(_sheetCurvedAnimation);
     _sheetOpacityAnimation = Tween<double>(
       begin: 1.0,
       end: 0.0,
@@ -1277,7 +1300,9 @@ class _ContextMenuRouteStaticState extends State<_ContextMenuRouteStatic> with T
   @override
   void dispose() {
     _moveController.dispose();
+    _moveCurvedAnimation.dispose();
     _sheetController.dispose();
+    _sheetCurvedAnimation.dispose();
     super.dispose();
   }
 

diff --git a/packages/flutter/test/cupertino/context_menu_test.dart b/packages/flutter/test/cupertino/context_menu_test.dart
@@ -8,6 +8,7 @@ import 'package:flutter/foundation.dart';
 import 'package:flutter/gestures.dart';
 import 'package:flutter/rendering.dart';
 import 'package:flutter_test/flutter_test.dart';
+import 'package:leak_tracker_flutter_testing/leak_tracker_flutter_testing.dart';
 
 void main() {
   final TestWidgetsFlutterBinding binding = TestWidgetsFlutterBinding.ensureInitialized();
@@ -140,7 +141,10 @@ void main() {
       expect(tester.getRect(find.byWidget(child)), childRect);
     });
 
-    testWidgets('Can open CupertinoContextMenu by tap and hold', (WidgetTester tester) async {
+    testWidgets('Can open CupertinoContextMenu by tap and hold',
+      // TODO(polina-c): remove when fixed https://github.com/flutter/flutter/issues/145600 [leak-tracking-opt-in]
+      experimentalLeakTesting: LeakTesting.settings.withTracked(classes: const <String>['CurvedAnimation']),
+      (WidgetTester tester) async {
       final Widget child = getChild();
       await tester.pumpWidget(getContextMenu(child: child));
       expect(find.byWidget(child), findsOneWidget);
@@ -587,7 +591,10 @@ void main() {
       expect(findStatic(), findsNothing);
     });
 
-    testWidgets('Can close CupertinoContextMenu by flinging down', (WidgetTester tester) async {
+    testWidgets('Can close CupertinoContextMenu by flinging down',
+      // TODO(polina-c): remove when fixed https://github.com/flutter/flutter/issues/145600 [leak-tracking-opt-in]
+      experimentalLeakTesting: LeakTesting.settings.withTracked(classes: const <String>['CurvedAnimation']),
+      (WidgetTester tester) async {
       final Widget child = getChild();
       await tester.pumpWidget(getContextMenu(child: child));