Unverified Commit bc7d3bbc authored by Bruno Leroux's avatar Bruno Leroux Committed by GitHub

Fix DraggableScrollableController.animateTo leaks Ticker (#102504)

parent 646b910c
...@@ -52,6 +52,7 @@ typedef ScrollableWidgetBuilder = Widget Function( ...@@ -52,6 +52,7 @@ typedef ScrollableWidgetBuilder = Widget Function(
/// constraints provided to an attached sheet change. /// constraints provided to an attached sheet change.
class DraggableScrollableController extends ChangeNotifier { class DraggableScrollableController extends ChangeNotifier {
_DraggableScrollableSheetScrollController? _attachedController; _DraggableScrollableSheetScrollController? _attachedController;
final Set<AnimationController> _animationControllers = <AnimationController>{};
/// Get the current size (as a fraction of the parent height) of the attached sheet. /// Get the current size (as a fraction of the parent height) of the attached sheet.
double get size { double get size {
...@@ -115,6 +116,7 @@ class DraggableScrollableController extends ChangeNotifier { ...@@ -115,6 +116,7 @@ class DraggableScrollableController extends ChangeNotifier {
vsync: _attachedController!.position.context.vsync, vsync: _attachedController!.position.context.vsync,
value: _attachedController!.extent.currentSize, value: _attachedController!.extent.currentSize,
); );
_animationControllers.add(animationController);
_attachedController!.position.goIdle(); _attachedController!.position.goIdle();
// This disables any snapping until the next user interaction with the sheet. // This disables any snapping until the next user interaction with the sheet.
_attachedController!.extent.hasDragged = false; _attachedController!.extent.hasDragged = false;
...@@ -175,6 +177,7 @@ class DraggableScrollableController extends ChangeNotifier { ...@@ -175,6 +177,7 @@ class DraggableScrollableController extends ChangeNotifier {
assert(_attachedController == null, 'Draggable scrollable controller is already attached to a sheet.'); assert(_attachedController == null, 'Draggable scrollable controller is already attached to a sheet.');
_attachedController = scrollController; _attachedController = scrollController;
_attachedController!.extent._currentSize.addListener(notifyListeners); _attachedController!.extent._currentSize.addListener(notifyListeners);
_attachedController!.onPositionDetached = _disposeAnimationControllers;
} }
void _onExtentReplaced(_DraggableSheetExtent previousExtent) { void _onExtentReplaced(_DraggableSheetExtent previousExtent) {
...@@ -193,6 +196,13 @@ class DraggableScrollableController extends ChangeNotifier { ...@@ -193,6 +196,13 @@ class DraggableScrollableController extends ChangeNotifier {
_attachedController?.extent._currentSize.removeListener(notifyListeners); _attachedController?.extent._currentSize.removeListener(notifyListeners);
_attachedController = null; _attachedController = null;
} }
void _disposeAnimationControllers() {
for (final AnimationController animationController in _animationControllers) {
animationController.dispose();
}
_animationControllers.clear();
}
} }
/// A container for a [Scrollable] that responds to drag gestures by resizing /// A container for a [Scrollable] that responds to drag gestures by resizing
...@@ -724,6 +734,7 @@ class _DraggableScrollableSheetScrollController extends ScrollController { ...@@ -724,6 +734,7 @@ class _DraggableScrollableSheetScrollController extends ScrollController {
}) : assert(extent != null); }) : assert(extent != null);
_DraggableSheetExtent extent; _DraggableSheetExtent extent;
VoidCallback? onPositionDetached;
@override @override
_DraggableScrollableSheetScrollPosition createScrollPosition( _DraggableScrollableSheetScrollPosition createScrollPosition(
...@@ -764,6 +775,12 @@ class _DraggableScrollableSheetScrollController extends ScrollController { ...@@ -764,6 +775,12 @@ class _DraggableScrollableSheetScrollController extends ScrollController {
} }
extent.updateSize(extent.initialSize, position.context.notificationContext!); extent.updateSize(extent.initialSize, position.context.notificationContext!);
} }
@override
void detach(ScrollPosition position) {
onPositionDetached?.call();
super.detach(position);
}
} }
/// A scroll position that manages scroll activities for /// A scroll position that manages scroll activities for
......
...@@ -1209,4 +1209,19 @@ void main() { ...@@ -1209,4 +1209,19 @@ void main() {
expect(controller.isAttached, true); expect(controller.isAttached, true);
expect(controller.size, isNotNull); expect(controller.size, isNotNull);
}); });
testWidgets('DraggableScrollableController.animateTo should not leak Ticker', (WidgetTester tester) async {
// Regression test for https://github.com/flutter/flutter/issues/102483
final DraggableScrollableController controller = DraggableScrollableController();
await tester.pumpWidget(_boilerplate(() {}, controller: controller));
controller.animateTo(0.0, curve: Curves.linear, duration: const Duration(milliseconds: 200));
await tester.pump();
// Dispose the DraggableScrollableSheet
await tester.pumpWidget(const SizedBox.shrink());
// Controller should be detached and no exception should be thrown
expect(controller.isAttached, false);
expect(tester.takeException(), isNull);
});
} }
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