Skip to content

Commit

Permalink
Fix memory leak in paginated tables (#146755)
Browse files Browse the repository at this point in the history
  • Loading branch information
@ValentinVignal
ValentinVignal authored Apr 17, 2024
1 parent f8c3e52 commit 51f1725
Show file tree
Hide file tree
Showing 4 changed files with 116 additions and 73 deletions.
9 changes: 5 additions & 4 deletions packages/flutter/lib/src/material/data_table.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1271,11 +1271,11 @@ class _SortArrow extends StatefulWidget {
}

class _SortArrowState extends State<_SortArrow> with TickerProviderStateMixin {
late AnimationController _opacityController;
late Animation<double> _opacityAnimation;
late final AnimationController _opacityController;
late final CurvedAnimation _opacityAnimation;

late AnimationController _orientationController;
late Animation<double> _orientationAnimation;
late final AnimationController _orientationController;
late final Animation<double> _orientationAnimation;
double _orientationOffset = 0.0;

bool? _up;
Expand Down Expand Up @@ -1354,6 +1354,7 @@ class _SortArrowState extends State<_SortArrow> with TickerProviderStateMixin {
void dispose() {
_opacityController.dispose();
_orientationController.dispose();
_opacityAnimation.dispose();
super.dispose();
}

Expand Down
65 changes: 52 additions & 13 deletions packages/flutter/lib/src/material/dropdown.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,40 @@ class _DropdownMenuItemButton<T> extends StatefulWidget {
}

class _DropdownMenuItemButtonState<T> extends State<_DropdownMenuItemButton<T>> {
CurvedAnimation? _opacityAnimation;

@override
void initState() {
super.initState();
_setOpacityAnimation();
}

@override
void didUpdateWidget(_DropdownMenuItemButton<T> oldWidget) {
super.didUpdateWidget(oldWidget);
if (oldWidget.itemIndex != widget.itemIndex ||
oldWidget.route.animation != widget.route.animation ||
oldWidget.route.selectedIndex != widget.route.selectedIndex ||
widget.route.items.length != oldWidget.route.items.length) {
_setOpacityAnimation();
}
}

void _setOpacityAnimation() {
_opacityAnimation?.dispose();
final double unit = 0.5 / (widget.route.items.length + 1.5);
if (widget.itemIndex == widget.route.selectedIndex) {
_opacityAnimation = CurvedAnimation(
parent: widget.route.animation!, curve: const Threshold(0.0));
} else {
final double start =
clampDouble(0.5 + (widget.itemIndex + 1) * unit, 0.0, 1.0);
final double end = clampDouble(start + 1.5 * unit, 0.0, 1.0);
_opacityAnimation = CurvedAnimation(
parent: widget.route.animation!, curve: Interval(start, end));
}
}

void _handleFocusChange(bool focused) {
final bool inTraditionalMode = switch (FocusManager.instance.highlightMode) {
FocusHighlightMode.touch => false,
Expand Down Expand Up @@ -156,18 +190,16 @@ class _DropdownMenuItemButtonState<T> extends State<_DropdownMenuItemButton<T>>
SingleActivator(LogicalKeyboardKey.arrowUp): DirectionalFocusIntent(TraversalDirection.up),
};

@override
void dispose() {
_opacityAnimation?.dispose();
super.dispose();
}

@override
Widget build(BuildContext context) {
final DropdownMenuItem<T> dropdownMenuItem = widget.route.items[widget.itemIndex].item!;
final CurvedAnimation opacity;
final double unit = 0.5 / (widget.route.items.length + 1.5);
if (widget.itemIndex == widget.route.selectedIndex) {
opacity = CurvedAnimation(parent: widget.route.animation!, curve: const Threshold(0.0));
} else {
final double start = clampDouble(0.5 + (widget.itemIndex + 1) * unit, 0.0, 1.0);
final double end = clampDouble(start + 1.5 * unit, 0.0, 1.0);
opacity = CurvedAnimation(parent: widget.route.animation!, curve: Interval(start, end));
}
final DropdownMenuItem<T> dropdownMenuItem =
widget.route.items[widget.itemIndex].item!;
Widget child = Container(
padding: widget.padding,
height: widget.route.itemHeight,
Expand All @@ -183,7 +215,7 @@ class _DropdownMenuItemButtonState<T> extends State<_DropdownMenuItemButton<T>>
child: child,
);
}
child = FadeTransition(opacity: opacity, child: child);
child = FadeTransition(opacity: _opacityAnimation!, child: child);
if (kIsWeb && dropdownMenuItem.enabled) {
child = Shortcuts(
shortcuts: _webShortcuts,
Expand Down Expand Up @@ -221,8 +253,8 @@ class _DropdownMenu<T> extends StatefulWidget {
}

class _DropdownMenuState<T> extends State<_DropdownMenu<T>> {
late CurvedAnimation _fadeOpacity;
late CurvedAnimation _resize;
late final CurvedAnimation _fadeOpacity;
late final CurvedAnimation _resize;

@override
void initState() {
Expand All @@ -243,6 +275,13 @@ class _DropdownMenuState<T> extends State<_DropdownMenu<T>> {
);
}

@override
void dispose() {
_fadeOpacity.dispose();
_resize.dispose();
super.dispose();
}

@override
Widget build(BuildContext context) {
// The menu is shown in three stages (unit timing in brackets):
Expand Down
11 changes: 9 additions & 2 deletions packages/flutter/test/material/paginated_data_table_test.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import 'package:flutter/gestures.dart' show DragStartBehavior;
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 'data_table_test_utils.dart';

Expand Down Expand Up @@ -70,7 +71,10 @@ void main() {
setUp(() => source = TestDataSource());
tearDown(() => source.dispose());

testWidgets('PaginatedDataTable paging', (WidgetTester tester) async {
testWidgets('PaginatedDataTable paging',
// 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 List<String> log = <String>[];

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

testWidgets('PaginatedDataTable control test', (WidgetTester tester) async {
testWidgets('PaginatedDataTable control test',
// 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 {
TestDataSource source = TestDataSource()
..generation = 42;
addTearDown(source.dispose);
Expand Down
104 changes: 50 additions & 54 deletions packages/flutter/test/widgets/transitions_test.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -96,60 +96,56 @@ void main() {
expect(actualDecoration.boxShadow, null);
});

testWidgets(
'animations work with curves test',
// 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 CurvedAnimation animation = CurvedAnimation(
parent: controller,
curve: Curves.easeOut,
);
addTearDown(animation.dispose);
final Animation<Decoration> curvedDecorationAnimation =
decorationTween.animate(animation);

final DecoratedBoxTransition transitionUnderTest = DecoratedBoxTransition(
decoration: curvedDecorationAnimation,
position: DecorationPosition.foreground,
child: const Text(
"Doesn't matter",
textDirection: TextDirection.ltr,
),
);

await tester.pumpWidget(transitionUnderTest);

RenderDecoratedBox actualBox = tester.renderObject(find.byType(DecoratedBox));
BoxDecoration actualDecoration = actualBox.decoration as BoxDecoration;

expect(actualDecoration.color, const Color(0xFFFFFFFF));
expect(actualDecoration.boxShadow![0].blurRadius, 10.0);
expect(actualDecoration.boxShadow![0].spreadRadius, 4.0);
expect(actualDecoration.boxShadow![0].color, const Color(0x66000000));

controller.value = 0.5;

await tester.pump();
actualBox = tester.renderObject(find.byType(DecoratedBox));
actualDecoration = actualBox.decoration as BoxDecoration;

// Same as the test above but the values should be much closer to the
// tween's end values given the easeOut curve.
expect(actualDecoration.color, const Color(0xFF505050));
expect(actualDecoration.border, isA<Border>());
final Border border = actualDecoration.border! as Border;
expect(border.left.width, moreOrLessEquals(1.9, epsilon: 0.1));
expect(border.left.style, BorderStyle.solid);
expect(border.left.color, const Color(0xFF151515));
expect(actualDecoration.borderRadius!.resolve(TextDirection.ltr).topLeft.x, moreOrLessEquals(6.8, epsilon: 0.1));
expect(actualDecoration.shape, BoxShape.rectangle);
expect(actualDecoration.boxShadow![0].blurRadius, moreOrLessEquals(3.1, epsilon: 0.1));
expect(actualDecoration.boxShadow![0].spreadRadius, moreOrLessEquals(1.2, epsilon: 0.1));
// Scaling a shadow doesn't change the color.
expect(actualDecoration.boxShadow![0].color, const Color(0x66000000));
},
);
testWidgets('animations work with curves test',
// 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 Animation<Decoration> curvedDecorationAnimation =
decorationTween.animate(CurvedAnimation(
parent: controller,
curve: Curves.easeOut,
));

final DecoratedBoxTransition transitionUnderTest = DecoratedBoxTransition(
decoration: curvedDecorationAnimation,
position: DecorationPosition.foreground,
child: const Text(
"Doesn't matter",
textDirection: TextDirection.ltr,
),
);

await tester.pumpWidget(transitionUnderTest);

RenderDecoratedBox actualBox = tester.renderObject(find.byType(DecoratedBox));
BoxDecoration actualDecoration = actualBox.decoration as BoxDecoration;

expect(actualDecoration.color, const Color(0xFFFFFFFF));
expect(actualDecoration.boxShadow![0].blurRadius, 10.0);
expect(actualDecoration.boxShadow![0].spreadRadius, 4.0);
expect(actualDecoration.boxShadow![0].color, const Color(0x66000000));

controller.value = 0.5;

await tester.pump();
actualBox = tester.renderObject(find.byType(DecoratedBox));
actualDecoration = actualBox.decoration as BoxDecoration;

// Same as the test above but the values should be much closer to the
// tween's end values given the easeOut curve.
expect(actualDecoration.color, const Color(0xFF505050));
expect(actualDecoration.border, isA<Border>());
final Border border = actualDecoration.border! as Border;
expect(border.left.width, moreOrLessEquals(1.9, epsilon: 0.1));
expect(border.left.style, BorderStyle.solid);
expect(border.left.color, const Color(0xFF151515));
expect(actualDecoration.borderRadius!.resolve(TextDirection.ltr).topLeft.x, moreOrLessEquals(6.8, epsilon: 0.1));
expect(actualDecoration.shape, BoxShape.rectangle);
expect(actualDecoration.boxShadow![0].blurRadius, moreOrLessEquals(3.1, epsilon: 0.1));
expect(actualDecoration.boxShadow![0].spreadRadius, moreOrLessEquals(1.2, epsilon: 0.1));
// Scaling a shadow doesn't change the color.
expect(actualDecoration.boxShadow![0].color, const Color(0x66000000));
});
});

testWidgets('AlignTransition animates', (WidgetTester tester) async {
Expand Down

0 comments on commit 51f1725

Please sign in to comment.