Skip to content
This repository was archived by the owner on Feb 22, 2023. It is now read-only.

Commit 750ad32

Browse files
authored
Fix DraggableScrollableSheet leaks Ticker (#102916)
1 parent 077e7e1 commit 750ad32

File tree

2 files changed

+74
-17
lines changed

2 files changed

+74
-17
lines changed

packages/flutter/lib/src/widgets/draggable_scrollable_sheet.dart

+21-16
Original file line numberDiff line numberDiff line change
@@ -795,8 +795,7 @@ class _DraggableScrollableSheetScrollController extends ScrollController {
795795
/// See also:
796796
///
797797
/// * [_DraggableScrollableSheetScrollController], which uses this as its [ScrollPosition].
798-
class _DraggableScrollableSheetScrollPosition
799-
extends ScrollPositionWithSingleContext {
798+
class _DraggableScrollableSheetScrollPosition extends ScrollPositionWithSingleContext {
800799
_DraggableScrollableSheetScrollPosition({
801800
required super.physics,
802801
required super.context,
@@ -805,16 +804,18 @@ class _DraggableScrollableSheetScrollPosition
805804
});
806805

807806
VoidCallback? _dragCancelCallback;
808-
VoidCallback? _ballisticCancelCallback;
809807
final _DraggableSheetExtent Function() getExtent;
808+
final Set<AnimationController> _ballisticControllers = <AnimationController>{};
810809
bool get listShouldScroll => pixels > 0.0;
811810

812811
_DraggableSheetExtent get extent => getExtent();
813812

814813
@override
815814
void beginActivity(ScrollActivity? newActivity) {
816-
// Cancel the running ballistic simulation, if there is one.
817-
_ballisticCancelCallback?.call();
815+
// Cancel the running ballistic simulations
816+
for (final AnimationController ballisticController in _ballisticControllers) {
817+
ballisticController.stop();
818+
}
818819
super.beginActivity(newActivity);
819820
}
820821

@@ -852,8 +853,10 @@ class _DraggableScrollableSheetScrollPosition
852853

853854
@override
854855
void dispose() {
855-
// Stop the animation before dispose.
856-
_ballisticCancelCallback?.call();
856+
for (final AnimationController ballisticController in _ballisticControllers) {
857+
ballisticController.dispose();
858+
}
859+
_ballisticControllers.clear();
857860
super.dispose();
858861
}
859862

@@ -873,10 +876,11 @@ class _DraggableScrollableSheetScrollPosition
873876
if (extent.snap) {
874877
// Snap is enabled, simulate snapping instead of clamping scroll.
875878
simulation = _SnappingSimulation(
876-
position: extent.currentPixels,
877-
initialVelocity: velocity,
878-
pixelSnapSize: extent.pixelSnapSizes,
879-
tolerance: physics.tolerance);
879+
position: extent.currentPixels,
880+
initialVelocity: velocity,
881+
pixelSnapSize: extent.pixelSnapSizes,
882+
tolerance: physics.tolerance,
883+
);
880884
} else {
881885
// The iOS bouncing simulation just isn't right here - once we delegate
882886
// the ballistic back to the ScrollView, it will use the right simulation.
@@ -892,9 +896,8 @@ class _DraggableScrollableSheetScrollPosition
892896
debugLabel: objectRuntimeType(this, '_DraggableScrollableSheetPosition'),
893897
vsync: context.vsync,
894898
);
895-
// Stop the ballistic animation if a new activity starts.
896-
// See: [beginActivity].
897-
_ballisticCancelCallback = ballisticController.stop;
899+
_ballisticControllers.add(ballisticController);
900+
898901
double lastPosition = extent.currentPixels;
899902
void tick() {
900903
final double delta = ballisticController.value - lastPosition;
@@ -916,8 +919,10 @@ class _DraggableScrollableSheetScrollPosition
916919
..addListener(tick)
917920
..animateWith(simulation).whenCompleteOrCancel(
918921
() {
919-
_ballisticCancelCallback = null;
920-
ballisticController.dispose();
922+
if (_ballisticControllers.contains(ballisticController)) {
923+
_ballisticControllers.remove(ballisticController);
924+
ballisticController.dispose();
925+
}
921926
},
922927
);
923928
}

packages/flutter/test/widgets/draggable_scrollable_sheet_test.dart

+53-1
Original file line numberDiff line numberDiff line change
@@ -326,7 +326,59 @@ void main() {
326326
expect(find.text('Item 70'), findsNothing);
327327
}, variant: TargetPlatformVariant.all());
328328

329-
debugDefaultTargetPlatformOverride = null;
329+
testWidgets('Ballistic animation on fling should not leak Ticker', (WidgetTester tester) async {
330+
// Regression test for https://github.com/flutter/flutter/issues/101061
331+
await tester.pumpWidget(
332+
Directionality(
333+
textDirection: TextDirection.ltr,
334+
child: MediaQuery(
335+
data: const MediaQueryData(),
336+
child: Align(
337+
alignment: Alignment.bottomCenter,
338+
child: DraggableScrollableSheet(
339+
initialChildSize: 0.8,
340+
minChildSize: 0.2,
341+
maxChildSize: 0.9,
342+
expand: false,
343+
builder: (_, ScrollController scrollController) {
344+
return ListView.separated(
345+
physics: const BouncingScrollPhysics(),
346+
controller: scrollController,
347+
separatorBuilder: (_, __) => const Divider(),
348+
itemCount: 100,
349+
itemBuilder: (_, int index) => SizedBox(
350+
height: 100,
351+
child: ColoredBox(
352+
color: Colors.primaries[index % Colors.primaries.length],
353+
child: Text('Item $index'),
354+
),
355+
),
356+
);
357+
},
358+
),
359+
),
360+
),
361+
),
362+
);
363+
364+
await tester.flingFrom(
365+
tester.getCenter(find.text('Item 1')),
366+
const Offset(0, 50),
367+
10000,
368+
);
369+
370+
// Pumps several times to let the DraggableScrollableSheet react to scroll position changes.
371+
const int numberOfPumpsBeforeError = 22;
372+
for (int i = 0; i < numberOfPumpsBeforeError; i++) {
373+
await tester.pump(const Duration(milliseconds: 10));
374+
}
375+
376+
// Dispose the DraggableScrollableSheet
377+
await tester.pumpWidget(const SizedBox.shrink());
378+
379+
// When a Ticker leaks an exception is thrown
380+
expect(tester.takeException(), isNull);
381+
});
330382
});
331383

332384
testWidgets('Does not snap away from initial child on build', (WidgetTester tester) async {

0 commit comments

Comments
 (0)