Skip to content

Commit 96479e7

Browse files
ValentinVignalgilnobrega
ValentinVignal
authored and
gilnobrega
committed
Fix memory leak in paginated tables (flutter#146755)
1 parent bba842e commit 96479e7

File tree

4 files changed

+116
-73
lines changed

4 files changed

+116
-73
lines changed

‎packages/flutter/lib/src/material/data_table.dart

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1271,11 +1271,11 @@ class _SortArrow extends StatefulWidget {
12711271
}
12721272

12731273
class _SortArrowState extends State<_SortArrow> with TickerProviderStateMixin {
1274-
late AnimationController _opacityController;
1275-
late Animation<double> _opacityAnimation;
1274+
late final AnimationController _opacityController;
1275+
late final CurvedAnimation _opacityAnimation;
12761276

1277-
late AnimationController _orientationController;
1278-
late Animation<double> _orientationAnimation;
1277+
late final AnimationController _orientationController;
1278+
late final Animation<double> _orientationAnimation;
12791279
double _orientationOffset = 0.0;
12801280

12811281
bool? _up;
@@ -1354,6 +1354,7 @@ class _SortArrowState extends State<_SortArrow> with TickerProviderStateMixin {
13541354
void dispose() {
13551355
_opacityController.dispose();
13561356
_orientationController.dispose();
1357+
_opacityAnimation.dispose();
13571358
super.dispose();
13581359
}
13591360

‎packages/flutter/lib/src/material/dropdown.dart

Lines changed: 52 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,40 @@ class _DropdownMenuItemButton<T> extends StatefulWidget {
118118
}
119119

120120
class _DropdownMenuItemButtonState<T> extends State<_DropdownMenuItemButton<T>> {
121+
CurvedAnimation? _opacityAnimation;
122+
123+
@override
124+
void initState() {
125+
super.initState();
126+
_setOpacityAnimation();
127+
}
128+
129+
@override
130+
void didUpdateWidget(_DropdownMenuItemButton<T> oldWidget) {
131+
super.didUpdateWidget(oldWidget);
132+
if (oldWidget.itemIndex != widget.itemIndex ||
133+
oldWidget.route.animation != widget.route.animation ||
134+
oldWidget.route.selectedIndex != widget.route.selectedIndex ||
135+
widget.route.items.length != oldWidget.route.items.length) {
136+
_setOpacityAnimation();
137+
}
138+
}
139+
140+
void _setOpacityAnimation() {
141+
_opacityAnimation?.dispose();
142+
final double unit = 0.5 / (widget.route.items.length + 1.5);
143+
if (widget.itemIndex == widget.route.selectedIndex) {
144+
_opacityAnimation = CurvedAnimation(
145+
parent: widget.route.animation!, curve: const Threshold(0.0));
146+
} else {
147+
final double start =
148+
clampDouble(0.5 + (widget.itemIndex + 1) * unit, 0.0, 1.0);
149+
final double end = clampDouble(start + 1.5 * unit, 0.0, 1.0);
150+
_opacityAnimation = CurvedAnimation(
151+
parent: widget.route.animation!, curve: Interval(start, end));
152+
}
153+
}
154+
121155
void _handleFocusChange(bool focused) {
122156
final bool inTraditionalMode = switch (FocusManager.instance.highlightMode) {
123157
FocusHighlightMode.touch => false,
@@ -156,18 +190,16 @@ class _DropdownMenuItemButtonState<T> extends State<_DropdownMenuItemButton<T>>
156190
SingleActivator(LogicalKeyboardKey.arrowUp): DirectionalFocusIntent(TraversalDirection.up),
157191
};
158192

193+
@override
194+
void dispose() {
195+
_opacityAnimation?.dispose();
196+
super.dispose();
197+
}
198+
159199
@override
160200
Widget build(BuildContext context) {
161-
final DropdownMenuItem<T> dropdownMenuItem = widget.route.items[widget.itemIndex].item!;
162-
final CurvedAnimation opacity;
163-
final double unit = 0.5 / (widget.route.items.length + 1.5);
164-
if (widget.itemIndex == widget.route.selectedIndex) {
165-
opacity = CurvedAnimation(parent: widget.route.animation!, curve: const Threshold(0.0));
166-
} else {
167-
final double start = clampDouble(0.5 + (widget.itemIndex + 1) * unit, 0.0, 1.0);
168-
final double end = clampDouble(start + 1.5 * unit, 0.0, 1.0);
169-
opacity = CurvedAnimation(parent: widget.route.animation!, curve: Interval(start, end));
170-
}
201+
final DropdownMenuItem<T> dropdownMenuItem =
202+
widget.route.items[widget.itemIndex].item!;
171203
Widget child = Container(
172204
padding: widget.padding,
173205
height: widget.route.itemHeight,
@@ -183,7 +215,7 @@ class _DropdownMenuItemButtonState<T> extends State<_DropdownMenuItemButton<T>>
183215
child: child,
184216
);
185217
}
186-
child = FadeTransition(opacity: opacity, child: child);
218+
child = FadeTransition(opacity: _opacityAnimation!, child: child);
187219
if (kIsWeb && dropdownMenuItem.enabled) {
188220
child = Shortcuts(
189221
shortcuts: _webShortcuts,
@@ -221,8 +253,8 @@ class _DropdownMenu<T> extends StatefulWidget {
221253
}
222254

223255
class _DropdownMenuState<T> extends State<_DropdownMenu<T>> {
224-
late CurvedAnimation _fadeOpacity;
225-
late CurvedAnimation _resize;
256+
late final CurvedAnimation _fadeOpacity;
257+
late final CurvedAnimation _resize;
226258

227259
@override
228260
void initState() {
@@ -243,6 +275,13 @@ class _DropdownMenuState<T> extends State<_DropdownMenu<T>> {
243275
);
244276
}
245277

278+
@override
279+
void dispose() {
280+
_fadeOpacity.dispose();
281+
_resize.dispose();
282+
super.dispose();
283+
}
284+
246285
@override
247286
Widget build(BuildContext context) {
248287
// The menu is shown in three stages (unit timing in brackets):

‎packages/flutter/test/material/paginated_data_table_test.dart

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import 'package:flutter/gestures.dart' show DragStartBehavior;
66
import 'package:flutter/material.dart';
77
import 'package:flutter/rendering.dart';
88
import 'package:flutter_test/flutter_test.dart';
9+
import 'package:leak_tracker_flutter_testing/leak_tracker_flutter_testing.dart';
910

1011
import 'data_table_test_utils.dart';
1112

@@ -70,7 +71,10 @@ void main() {
7071
setUp(() => source = TestDataSource());
7172
tearDown(() => source.dispose());
7273

73-
testWidgets('PaginatedDataTable paging', (WidgetTester tester) async {
74+
testWidgets('PaginatedDataTable paging',
75+
// TODO(polina-c): remove when fixed https://github.com/flutter/flutter/issues/145600 [leak-tracking-opt-in]
76+
experimentalLeakTesting: LeakTesting.settings.withTracked(classes: const <String>['CurvedAnimation']),
77+
(WidgetTester tester) async {
7478
final List<String> log = <String>[];
7579

7680
await tester.pumpWidget(MaterialApp(
@@ -379,7 +383,10 @@ void main() {
379383
expect(find.byWidgetPredicate((Widget widget) => widget is SizedBox && widget.height == (rowsPerPage - (rowCount % rowsPerPage)) * 46.0), findsOneWidget);
380384
});
381385

382-
testWidgets('PaginatedDataTable control test', (WidgetTester tester) async {
386+
testWidgets('PaginatedDataTable control test',
387+
// TODO(polina-c): remove when fixed https://github.com/flutter/flutter/issues/145600 [leak-tracking-opt-in]
388+
experimentalLeakTesting: LeakTesting.settings.withTracked(classes: const <String>['CurvedAnimation']),
389+
(WidgetTester tester) async {
383390
TestDataSource source = TestDataSource()
384391
..generation = 42;
385392
addTearDown(source.dispose);

‎packages/flutter/test/widgets/transitions_test.dart

Lines changed: 50 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -96,60 +96,56 @@ void main() {
9696
expect(actualDecoration.boxShadow, null);
9797
});
9898

99-
testWidgets(
100-
'animations work with curves test',
101-
// TODO(polina-c): remove when fixed https://github.com/flutter/flutter/issues/145600 [leak-tracking-opt-in]
102-
experimentalLeakTesting: LeakTesting.settings.withTracked(classes: const <String>['CurvedAnimation']),
103-
(WidgetTester tester) async {
104-
final CurvedAnimation animation = CurvedAnimation(
105-
parent: controller,
106-
curve: Curves.easeOut,
107-
);
108-
addTearDown(animation.dispose);
109-
final Animation<Decoration> curvedDecorationAnimation =
110-
decorationTween.animate(animation);
111-
112-
final DecoratedBoxTransition transitionUnderTest = DecoratedBoxTransition(
113-
decoration: curvedDecorationAnimation,
114-
position: DecorationPosition.foreground,
115-
child: const Text(
116-
"Doesn't matter",
117-
textDirection: TextDirection.ltr,
118-
),
119-
);
120-
121-
await tester.pumpWidget(transitionUnderTest);
122-
123-
RenderDecoratedBox actualBox = tester.renderObject(find.byType(DecoratedBox));
124-
BoxDecoration actualDecoration = actualBox.decoration as BoxDecoration;
125-
126-
expect(actualDecoration.color, const Color(0xFFFFFFFF));
127-
expect(actualDecoration.boxShadow![0].blurRadius, 10.0);
128-
expect(actualDecoration.boxShadow![0].spreadRadius, 4.0);
129-
expect(actualDecoration.boxShadow![0].color, const Color(0x66000000));
130-
131-
controller.value = 0.5;
132-
133-
await tester.pump();
134-
actualBox = tester.renderObject(find.byType(DecoratedBox));
135-
actualDecoration = actualBox.decoration as BoxDecoration;
136-
137-
// Same as the test above but the values should be much closer to the
138-
// tween's end values given the easeOut curve.
139-
expect(actualDecoration.color, const Color(0xFF505050));
140-
expect(actualDecoration.border, isA<Border>());
141-
final Border border = actualDecoration.border! as Border;
142-
expect(border.left.width, moreOrLessEquals(1.9, epsilon: 0.1));
143-
expect(border.left.style, BorderStyle.solid);
144-
expect(border.left.color, const Color(0xFF151515));
145-
expect(actualDecoration.borderRadius!.resolve(TextDirection.ltr).topLeft.x, moreOrLessEquals(6.8, epsilon: 0.1));
146-
expect(actualDecoration.shape, BoxShape.rectangle);
147-
expect(actualDecoration.boxShadow![0].blurRadius, moreOrLessEquals(3.1, epsilon: 0.1));
148-
expect(actualDecoration.boxShadow![0].spreadRadius, moreOrLessEquals(1.2, epsilon: 0.1));
149-
// Scaling a shadow doesn't change the color.
150-
expect(actualDecoration.boxShadow![0].color, const Color(0x66000000));
151-
},
152-
);
99+
testWidgets('animations work with curves test',
100+
// TODO(polina-c): remove when fixed https://github.com/flutter/flutter/issues/145600 [leak-tracking-opt-in]
101+
experimentalLeakTesting: LeakTesting.settings.withTracked(classes: const <String>['CurvedAnimation']),
102+
(WidgetTester tester) async {
103+
final Animation<Decoration> curvedDecorationAnimation =
104+
decorationTween.animate(CurvedAnimation(
105+
parent: controller,
106+
curve: Curves.easeOut,
107+
));
108+
109+
final DecoratedBoxTransition transitionUnderTest = DecoratedBoxTransition(
110+
decoration: curvedDecorationAnimation,
111+
position: DecorationPosition.foreground,
112+
child: const Text(
113+
"Doesn't matter",
114+
textDirection: TextDirection.ltr,
115+
),
116+
);
117+
118+
await tester.pumpWidget(transitionUnderTest);
119+
120+
RenderDecoratedBox actualBox = tester.renderObject(find.byType(DecoratedBox));
121+
BoxDecoration actualDecoration = actualBox.decoration as BoxDecoration;
122+
123+
expect(actualDecoration.color, const Color(0xFFFFFFFF));
124+
expect(actualDecoration.boxShadow![0].blurRadius, 10.0);
125+
expect(actualDecoration.boxShadow![0].spreadRadius, 4.0);
126+
expect(actualDecoration.boxShadow![0].color, const Color(0x66000000));
127+
128+
controller.value = 0.5;
129+
130+
await tester.pump();
131+
actualBox = tester.renderObject(find.byType(DecoratedBox));
132+
actualDecoration = actualBox.decoration as BoxDecoration;
133+
134+
// Same as the test above but the values should be much closer to the
135+
// tween's end values given the easeOut curve.
136+
expect(actualDecoration.color, const Color(0xFF505050));
137+
expect(actualDecoration.border, isA<Border>());
138+
final Border border = actualDecoration.border! as Border;
139+
expect(border.left.width, moreOrLessEquals(1.9, epsilon: 0.1));
140+
expect(border.left.style, BorderStyle.solid);
141+
expect(border.left.color, const Color(0xFF151515));
142+
expect(actualDecoration.borderRadius!.resolve(TextDirection.ltr).topLeft.x, moreOrLessEquals(6.8, epsilon: 0.1));
143+
expect(actualDecoration.shape, BoxShape.rectangle);
144+
expect(actualDecoration.boxShadow![0].blurRadius, moreOrLessEquals(3.1, epsilon: 0.1));
145+
expect(actualDecoration.boxShadow![0].spreadRadius, moreOrLessEquals(1.2, epsilon: 0.1));
146+
// Scaling a shadow doesn't change the color.
147+
expect(actualDecoration.boxShadow![0].color, const Color(0x66000000));
148+
});
153149
});
154150

155151
testWidgets('AlignTransition animates', (WidgetTester tester) async {

0 commit comments

Comments
 (0)