Unverified Commit 5d5da388 authored by Kostia Sokolovskyi's avatar Kostia Sokolovskyi Committed by GitHub

Fix memory leak in _DraggableScrollableSheetState (#134212)

parent 834f5dc2
......@@ -718,7 +718,11 @@ class _DraggableScrollableSheetState extends State<DraggableScrollableSheet> {
@override
void dispose() {
widget.controller?._detach(disposeExtent: true);
if (widget.controller == null) {
_extent.dispose();
} else {
widget.controller!._detach(disposeExtent: true);
}
_scrollController.dispose();
super.dispose();
}
......
......@@ -1590,12 +1590,7 @@ void main() {
expect(titleOffset, const Offset(292.0, 136.0));
expect(titleOffset.dx, lessThan(wideSize.width - 320)); // Default master view width is 320.0.
expect(tester.getCenter(find.byType(ListView)), const Offset(160, 356));
},
leakTrackingTestConfig: const LeakTrackingTestConfig(
// TODO(polina-c): remove after fixing
// https://github.com/flutter/flutter/issues/133705
notDisposedAllowList: <String, int?> {'ValueNotifier<double>':5},
));
});
testWidgets('Material2 - LicensePage master view layout position - rtl', (WidgetTester tester) async {
const TextDirection textDirection = TextDirection.rtl;
......
......@@ -5,6 +5,7 @@
import 'package:flutter/foundation.dart';
import 'package:flutter/material.dart';
import 'package:flutter_test/flutter_test.dart';
import 'package:leak_tracker_flutter_testing/leak_tracker_flutter_testing.dart';
void main() {
Widget boilerplateWidget(VoidCallback? onButtonPressed, {
......@@ -71,7 +72,7 @@ void main() {
);
}
testWidgets('Do not crash when replacing scroll position during the drag', (WidgetTester tester) async {
testWidgetsWithLeakTracking('Do not crash when replacing scroll position during the drag', (WidgetTester tester) async {
// Regression test for https://github.com/flutter/flutter/issues/89681
bool showScrollbars = false;
await tester.pumpWidget(
......@@ -119,7 +120,7 @@ void main() {
// Go without throw.
});
testWidgets('Scrolls correct amount when maxChildSize < 1.0', (WidgetTester tester) async {
testWidgetsWithLeakTracking('Scrolls correct amount when maxChildSize < 1.0', (WidgetTester tester) async {
const Key key = ValueKey<String>('container');
await tester.pumpWidget(boilerplateWidget(
null,
......@@ -135,7 +136,7 @@ void main() {
expect(tester.getRect(find.byKey(key)), const Rect.fromLTRB(0.0, 325.0, 800.0, 600.0));
});
testWidgets('Scrolls correct amount when maxChildSize == 1.0', (WidgetTester tester) async {
testWidgetsWithLeakTracking('Scrolls correct amount when maxChildSize == 1.0', (WidgetTester tester) async {
const Key key = ValueKey<String>('container');
await tester.pumpWidget(boilerplateWidget(
null,
......@@ -172,7 +173,7 @@ void main() {
});
group('Scroll Physics', () {
testWidgets('Can be dragged up without covering its container', (WidgetTester tester) async {
testWidgetsWithLeakTracking('Can be dragged up without covering its container', (WidgetTester tester) async {
int taps = 0;
await tester.pumpWidget(boilerplateWidget(() => taps++));
......@@ -193,7 +194,7 @@ void main() {
expect(find.text('Item 31'), findsOneWidget);
}, variant: TargetPlatformVariant.all());
testWidgets('Can be dragged down when not full height', (WidgetTester tester) async {
testWidgetsWithLeakTracking('Can be dragged down when not full height', (WidgetTester tester) async {
await tester.pumpWidget(boilerplateWidget(null));
expect(find.text('Item 1'), findsOneWidget);
expect(find.text('Item 21'), findsOneWidget);
......@@ -206,7 +207,7 @@ void main() {
expect(find.text('Item 36'), findsNothing);
}, variant: TargetPlatformVariant.all());
testWidgets('Can be dragged down when list is shorter than full height', (WidgetTester tester) async {
testWidgetsWithLeakTracking('Can be dragged down when list is shorter than full height', (WidgetTester tester) async {
await tester.pumpWidget(boilerplateWidget(null, itemCount: 30, initialChildSize: .25));
expect(find.text('Item 1').hitTestable(), findsOneWidget);
......@@ -223,7 +224,7 @@ void main() {
expect(find.text('Item 29').hitTestable(), findsNothing);
}, variant: TargetPlatformVariant.all());
testWidgets('Can be dragged up and cover its container and scroll in single motion, and then dragged back down', (WidgetTester tester) async {
testWidgetsWithLeakTracking('Can be dragged up and cover its container and scroll in single motion, and then dragged back down', (WidgetTester tester) async {
int taps = 0;
await tester.pumpWidget(boilerplateWidget(() => taps++));
......@@ -252,7 +253,7 @@ void main() {
expect(find.text('Item 36'), findsNothing);
}, variant: TargetPlatformVariant.all());
testWidgets('Can be flung up gently', (WidgetTester tester) async {
testWidgetsWithLeakTracking('Can be flung up gently', (WidgetTester tester) async {
int taps = 0;
await tester.pumpWidget(boilerplateWidget(() => taps++));
......@@ -275,7 +276,7 @@ void main() {
expect(find.text('Item 70'), findsNothing);
}, variant: TargetPlatformVariant.all());
testWidgets('Can be flung up', (WidgetTester tester) async {
testWidgetsWithLeakTracking('Can be flung up', (WidgetTester tester) async {
int taps = 0;
await tester.pumpWidget(boilerplateWidget(() => taps++));
......@@ -301,7 +302,7 @@ void main() {
}
}, variant: TargetPlatformVariant.all());
testWidgets('Can be flung down when not full height', (WidgetTester tester) async {
testWidgetsWithLeakTracking('Can be flung down when not full height', (WidgetTester tester) async {
await tester.pumpWidget(boilerplateWidget(null));
expect(find.text('Item 1'), findsOneWidget);
expect(find.text('Item 21'), findsOneWidget);
......@@ -314,7 +315,7 @@ void main() {
expect(find.text('Item 36'), findsNothing);
}, variant: TargetPlatformVariant.all());
testWidgets('Can be flung up and then back down', (WidgetTester tester) async {
testWidgetsWithLeakTracking('Can be flung up and then back down', (WidgetTester tester) async {
int taps = 0;
await tester.pumpWidget(boilerplateWidget(() => taps++));
......@@ -358,7 +359,7 @@ void main() {
expect(find.text('Item 70'), findsNothing);
}, variant: TargetPlatformVariant.all());
testWidgets('Ballistic animation on fling can be interrupted', (WidgetTester tester) async {
testWidgetsWithLeakTracking('Ballistic animation on fling can be interrupted', (WidgetTester tester) async {
int taps = 0;
await tester.pumpWidget(boilerplateWidget(() => taps++));
......@@ -392,7 +393,7 @@ void main() {
expect(find.text('Item 70'), findsNothing);
}, variant: TargetPlatformVariant.all());
testWidgets('Ballistic animation on fling should not leak Ticker', (WidgetTester tester) async {
testWidgetsWithLeakTracking('Ballistic animation on fling should not leak Ticker', (WidgetTester tester) async {
// Regression test for https://github.com/flutter/flutter/issues/101061
await tester.pumpWidget(
Directionality(
......@@ -447,7 +448,7 @@ void main() {
});
});
testWidgets('Does not snap away from initial child on build', (WidgetTester tester) async {
testWidgetsWithLeakTracking('Does not snap away from initial child on build', (WidgetTester tester) async {
const Key containerKey = ValueKey<String>('container');
const Key stackKey = ValueKey<String>('stack');
await tester.pumpWidget(boilerplateWidget(null,
......@@ -467,10 +468,11 @@ void main() {
}, variant: TargetPlatformVariant.all());
for (final bool useActuator in <bool>[false, true]) {
testWidgets('Does not snap away from initial child on ${useActuator ? 'actuator' : 'controller'}.reset()', (WidgetTester tester) async {
testWidgetsWithLeakTracking('Does not snap away from initial child on ${useActuator ? 'actuator' : 'controller'}.reset()', (WidgetTester tester) async {
const Key containerKey = ValueKey<String>('container');
const Key stackKey = ValueKey<String>('stack');
final DraggableScrollableController controller = DraggableScrollableController();
addTearDown(controller.dispose);
await tester.pumpWidget(boilerplateWidget(
null,
controller: controller,
......@@ -504,7 +506,7 @@ void main() {
}
for (final Duration? snapAnimationDuration in <Duration?>[null, const Duration(seconds: 2)]) {
testWidgets(
testWidgetsWithLeakTracking(
'Zero velocity drag snaps to nearest snap target with '
'snapAnimationDuration: $snapAnimationDuration',
(WidgetTester tester) async {
......@@ -563,7 +565,7 @@ void main() {
}
for (final List<double>? snapSizes in <List<double>?>[null, <double>[]]) {
testWidgets('Setting snapSizes to $snapSizes resolves to min and max', (WidgetTester tester) async {
testWidgetsWithLeakTracking('Setting snapSizes to $snapSizes resolves to min and max', (WidgetTester tester) async {
const Key stackKey = ValueKey<String>('stack');
const Key containerKey = ValueKey<String>('container');
await tester.pumpWidget(boilerplateWidget(null,
......@@ -591,7 +593,7 @@ void main() {
}, variant: TargetPlatformVariant.all());
}
testWidgets('Min and max are implicitly added to snapSizes', (WidgetTester tester) async {
testWidgetsWithLeakTracking('Min and max are implicitly added to snapSizes', (WidgetTester tester) async {
const Key stackKey = ValueKey<String>('stack');
const Key containerKey = ValueKey<String>('container');
await tester.pumpWidget(boilerplateWidget(null,
......@@ -618,7 +620,7 @@ void main() {
);
}, variant: TargetPlatformVariant.all());
testWidgets('Changes to widget parameters are propagated', (WidgetTester tester) async {
testWidgetsWithLeakTracking('Changes to widget parameters are propagated', (WidgetTester tester) async {
const Key stackKey = ValueKey<String>('stack');
const Key containerKey = ValueKey<String>('container');
await tester.pumpWidget(boilerplateWidget(
......@@ -726,7 +728,7 @@ void main() {
);
}, variant: TargetPlatformVariant.all());
testWidgets('Fling snaps in direction of momentum', (WidgetTester tester) async {
testWidgetsWithLeakTracking('Fling snaps in direction of momentum', (WidgetTester tester) async {
const Key stackKey = ValueKey<String>('stack');
const Key containerKey = ValueKey<String>('container');
await tester.pumpWidget(boilerplateWidget(null,
......@@ -754,7 +756,7 @@ void main() {
}, variant: TargetPlatformVariant.all());
testWidgets("Changing parameters with an un-listened controller doesn't throw", (WidgetTester tester) async {
testWidgetsWithLeakTracking("Changing parameters with an un-listened controller doesn't throw", (WidgetTester tester) async {
await tester.pumpWidget(boilerplateWidget(
null,
snap: true,
......@@ -769,7 +771,7 @@ void main() {
await tester.pumpAndSettle();
}, variant: TargetPlatformVariant.all());
testWidgets('Transitioning between scrollable children sharing a scroll controller will not throw', (WidgetTester tester) async {
testWidgetsWithLeakTracking('Transitioning between scrollable children sharing a scroll controller will not throw', (WidgetTester tester) async {
int s = 0;
await tester.pumpWidget(MaterialApp(
home: StatefulBuilder(
......@@ -826,7 +828,7 @@ void main() {
// Completes without throwing
});
testWidgets('ScrollNotification correctly dispatched when flung without covering its container', (WidgetTester tester) async {
testWidgetsWithLeakTracking('ScrollNotification correctly dispatched when flung without covering its container', (WidgetTester tester) async {
final List<Type> notificationTypes = <Type>[];
await tester.pumpWidget(boilerplateWidget(
null,
......@@ -847,7 +849,7 @@ void main() {
expect(notificationTypes, equals(types));
});
testWidgets('ScrollNotification correctly dispatched when flung with contents scroll', (WidgetTester tester) async {
testWidgetsWithLeakTracking('ScrollNotification correctly dispatched when flung with contents scroll', (WidgetTester tester) async {
final List<Type> notificationTypes = <Type>[];
await tester.pumpWidget(boilerplateWidget(
null,
......@@ -870,7 +872,7 @@ void main() {
expect(notificationTypes, types);
});
testWidgets('Emits DraggableScrollableNotification with shouldCloseOnMinExtent set to non-default value', (WidgetTester tester) async {
testWidgetsWithLeakTracking('Emits DraggableScrollableNotification with shouldCloseOnMinExtent set to non-default value', (WidgetTester tester) async {
DraggableScrollableNotification? receivedNotification;
await tester.pumpWidget(boilerplateWidget(
null,
......@@ -886,7 +888,7 @@ void main() {
expect(receivedNotification!.shouldCloseOnMinExtent, isFalse);
});
testWidgets('Do not crash when remove the tree during animation.', (WidgetTester tester) async {
testWidgetsWithLeakTracking('Do not crash when remove the tree during animation.', (WidgetTester tester) async {
// Regression test for https://github.com/flutter/flutter/issues/89214
await tester.pumpWidget(boilerplateWidget(
null,
......@@ -905,10 +907,11 @@ void main() {
});
for (final bool shouldAnimate in <bool>[true, false]) {
testWidgets('Can ${shouldAnimate ? 'animate' : 'jump'} to arbitrary positions', (WidgetTester tester) async {
testWidgetsWithLeakTracking('Can ${shouldAnimate ? 'animate' : 'jump'} to arbitrary positions', (WidgetTester tester) async {
const Key stackKey = ValueKey<String>('stack');
const Key containerKey = ValueKey<String>('container');
final DraggableScrollableController controller = DraggableScrollableController();
addTearDown(controller.dispose);
await tester.pumpWidget(boilerplateWidget(
null,
controller: controller,
......@@ -980,10 +983,11 @@ void main() {
});
}
testWidgets('Can animateTo with a nonlinear curve', (WidgetTester tester) async {
testWidgetsWithLeakTracking('Can animateTo with a nonlinear curve', (WidgetTester tester) async {
const Key stackKey = ValueKey<String>('stack');
const Key containerKey = ValueKey<String>('container');
final DraggableScrollableController controller = DraggableScrollableController();
addTearDown(controller.dispose);
await tester.pumpWidget(boilerplateWidget(
null,
controller: controller,
......@@ -1027,10 +1031,11 @@ void main() {
);
});
testWidgets('Can animateTo with a Curves.easeInOutBack curve begin min-size', (WidgetTester tester) async {
testWidgetsWithLeakTracking('Can animateTo with a Curves.easeInOutBack curve begin min-size', (WidgetTester tester) async {
const Key stackKey = ValueKey<String>('stack');
const Key containerKey = ValueKey<String>('container');
final DraggableScrollableController controller = DraggableScrollableController();
addTearDown(controller.dispose);
await tester.pumpWidget(boilerplateWidget(
null,
initialChildSize: 0.25,
......@@ -1050,10 +1055,11 @@ void main() {
);
});
testWidgets('Can reuse a controller after the old controller is disposed', (WidgetTester tester) async {
testWidgetsWithLeakTracking('Can reuse a controller after the old controller is disposed', (WidgetTester tester) async {
const Key stackKey = ValueKey<String>('stack');
const Key containerKey = ValueKey<String>('container');
final DraggableScrollableController controller = DraggableScrollableController();
addTearDown(controller.dispose);
await tester.pumpWidget(boilerplateWidget(
null,
controller: controller,
......@@ -1080,10 +1086,11 @@ void main() {
);
});
testWidgets('animateTo interrupts other animations', (WidgetTester tester) async {
testWidgetsWithLeakTracking('animateTo interrupts other animations', (WidgetTester tester) async {
const Key stackKey = ValueKey<String>('stack');
const Key containerKey = ValueKey<String>('container');
final DraggableScrollableController controller = DraggableScrollableController();
addTearDown(controller.dispose);
await tester.pumpWidget(Directionality(
textDirection: TextDirection.ltr,
child: boilerplateWidget(
......@@ -1116,10 +1123,11 @@ void main() {
expect(find.text('Item 1'), findsOneWidget);
});
testWidgets('Other animations interrupt animateTo', (WidgetTester tester) async {
testWidgetsWithLeakTracking('Other animations interrupt animateTo', (WidgetTester tester) async {
const Key stackKey = ValueKey<String>('stack');
const Key containerKey = ValueKey<String>('container');
final DraggableScrollableController controller = DraggableScrollableController();
addTearDown(controller.dispose);
await tester.pumpWidget(Directionality(
textDirection: TextDirection.ltr,
child: boilerplateWidget(
......@@ -1149,10 +1157,11 @@ void main() {
);
});
testWidgets('animateTo can be interrupted by other animateTo or jumpTo', (WidgetTester tester) async {
testWidgetsWithLeakTracking('animateTo can be interrupted by other animateTo or jumpTo', (WidgetTester tester) async {
const Key stackKey = ValueKey<String>('stack');
const Key containerKey = ValueKey<String>('container');
final DraggableScrollableController controller = DraggableScrollableController();
addTearDown(controller.dispose);
await tester.pumpWidget(Directionality(
textDirection: TextDirection.ltr,
child: boilerplateWidget(
......@@ -1191,10 +1200,11 @@ void main() {
);
});
testWidgets('Can get size and pixels', (WidgetTester tester) async {
testWidgetsWithLeakTracking('Can get size and pixels', (WidgetTester tester) async {
const Key stackKey = ValueKey<String>('stack');
const Key containerKey = ValueKey<String>('container');
final DraggableScrollableController controller = DraggableScrollableController();
addTearDown(controller.dispose);
await tester.pumpWidget(boilerplateWidget(
null,
controller: controller,
......@@ -1223,6 +1233,7 @@ void main() {
testWidgets('Cannot attach a controller to multiple sheets', (WidgetTester tester) async {
final DraggableScrollableController controller = DraggableScrollableController();
addTearDown(controller.dispose);
await tester.pumpWidget(Directionality(
textDirection: TextDirection.ltr,
child: Stack(
......@@ -1241,11 +1252,12 @@ void main() {
expect(tester.takeException(), isAssertionError);
});
testWidgets('Can listen for changes in sheet size', (WidgetTester tester) async {
testWidgetsWithLeakTracking('Can listen for changes in sheet size', (WidgetTester tester) async {
const Key stackKey = ValueKey<String>('stack');
const Key containerKey = ValueKey<String>('container');
final List<double> loggedSizes = <double>[];
final DraggableScrollableController controller = DraggableScrollableController();
addTearDown(controller.dispose);
controller.addListener(() {
loggedSizes.add(controller.size);
});
......@@ -1289,11 +1301,12 @@ void main() {
loggedSizes.clear();
});
testWidgets('Listener does not fire on parameter change and persists after change', (WidgetTester tester) async {
testWidgetsWithLeakTracking('Listener does not fire on parameter change and persists after change', (WidgetTester tester) async {
const Key stackKey = ValueKey<String>('stack');
const Key containerKey = ValueKey<String>('container');
final List<double> loggedSizes = <double>[];
final DraggableScrollableController controller = DraggableScrollableController();
addTearDown(controller.dispose);
controller.addListener(() {
loggedSizes.add(controller.size);
});
......@@ -1331,11 +1344,12 @@ void main() {
loggedSizes.clear();
});
testWidgets('Listener fires if a parameter change forces a change in size', (WidgetTester tester) async {
testWidgetsWithLeakTracking('Listener fires if a parameter change forces a change in size', (WidgetTester tester) async {
const Key stackKey = ValueKey<String>('stack');
const Key containerKey = ValueKey<String>('container');
final List<double> loggedSizes = <double>[];
final DraggableScrollableController controller = DraggableScrollableController();
addTearDown(controller.dispose);
controller.addListener(() {
loggedSizes.add(controller.size);
});
......@@ -1382,8 +1396,9 @@ void main() {
loggedSizes.clear();
});
testWidgets('Invalid controller interactions throw assertion errors', (WidgetTester tester) async {
testWidgetsWithLeakTracking('Invalid controller interactions throw assertion errors', (WidgetTester tester) async {
final DraggableScrollableController controller = DraggableScrollableController();
addTearDown(controller.dispose);
// Can't use a controller before attaching it.
expect(() => controller.jumpTo(.1), throwsAssertionError);
......@@ -1414,8 +1429,9 @@ void main() {
expect(() => controller.animateTo(.5, duration: Duration.zero, curve: Curves.linear), throwsAssertionError);
});
testWidgets('DraggableScrollableController must be attached before using any of its parameters', (WidgetTester tester) async {
testWidgetsWithLeakTracking('DraggableScrollableController must be attached before using any of its parameters', (WidgetTester tester) async {
final DraggableScrollableController controller = DraggableScrollableController();
addTearDown(controller.dispose);
expect(controller.isAttached, false);
expect(()=>controller.size, throwsAssertionError);
final Widget boilerplate = boilerplateWidget(
......@@ -1428,8 +1444,9 @@ void main() {
expect(controller.size, isNotNull);
});
testWidgets('DraggableScrollableController.animateTo after detach', (WidgetTester tester) async {
testWidgetsWithLeakTracking('DraggableScrollableController.animateTo after detach', (WidgetTester tester) async {
final DraggableScrollableController controller = DraggableScrollableController();
addTearDown(controller.dispose);
await tester.pumpWidget(boilerplateWidget(() {}, controller: controller));
controller.animateTo(0.0, curve: Curves.linear, duration: const Duration(milliseconds: 200));
......@@ -1442,11 +1459,12 @@ void main() {
expect(tester.takeException(), isNull);
});
testWidgets('DraggableScrollableSheet should not reset programmatic drag on rebuild', (WidgetTester tester) async {
testWidgetsWithLeakTracking('DraggableScrollableSheet should not reset programmatic drag on rebuild', (WidgetTester tester) async {
// Regression test for https://github.com/flutter/flutter/issues/101114
const Key stackKey = ValueKey<String>('stack');
const Key containerKey = ValueKey<String>('container');
final DraggableScrollableController controller = DraggableScrollableController();
addTearDown(controller.dispose);
await tester.pumpWidget(boilerplateWidget(
null,
controller: controller,
......@@ -1508,9 +1526,10 @@ void main() {
);
});
testWidgets('DraggableScrollableSheet should respect NeverScrollableScrollPhysics', (WidgetTester tester) async {
testWidgetsWithLeakTracking('DraggableScrollableSheet should respect NeverScrollableScrollPhysics', (WidgetTester tester) async {
// Regression test for https://github.com/flutter/flutter/issues/121021
final DraggableScrollableController controller = DraggableScrollableController();
addTearDown(controller.dispose);
Widget buildFrame(ScrollPhysics? physics) {
return MaterialApp(
home: Scaffold(
......@@ -1553,7 +1572,7 @@ void main() {
expect(controller.pixels, initPixels + 300.0);
});
testWidgets('DraggableScrollableSheet should not rebuild every frame while dragging', (WidgetTester tester) async {
testWidgetsWithLeakTracking('DraggableScrollableSheet should not rebuild every frame while dragging', (WidgetTester tester) async {
// Regression test for https://github.com/flutter/flutter/issues/67219
int buildCount = 0;
await tester.pumpWidget(MaterialApp(
......@@ -1600,9 +1619,11 @@ void main() {
expect(buildCount, 2);
});
testWidgets('DraggableScrollableSheet controller can be changed', (WidgetTester tester) async {
testWidgetsWithLeakTracking('DraggableScrollableSheet controller can be changed', (WidgetTester tester) async {
final DraggableScrollableController controller1 = DraggableScrollableController();
addTearDown(controller1.dispose);
final DraggableScrollableController controller2 = DraggableScrollableController();
addTearDown(controller2.dispose);
final List<double> loggedSizes = <double>[];
DraggableScrollableController controller = controller1;
......@@ -1658,7 +1679,7 @@ void main() {
expect(loggedSizes, <double>[1.0].map((double v) => closeTo(v, precisionErrorTolerance)));
});
testWidgets('DraggableScrollableSheet controller can be changed while animating', (WidgetTester tester) async {
testWidgetsWithLeakTracking('DraggableScrollableSheet controller can be changed while animating', (WidgetTester tester) async {
final DraggableScrollableController controller1 = DraggableScrollableController();
final DraggableScrollableController controller2 = DraggableScrollableController();
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment