Unverified Commit e1e8ad80 authored by Dan Field's avatar Dan Field Committed by GitHub

Fix crasher in DragableScrollableSheet when controller is animating and switching widgets (#125721)

We were failing to dispose of animation controllers created by `animateTo` when `didUpdateWidget` happens and we null out the `_attachedController`. This would cause the listener added here to fail on a null assertion:

https://github.com/flutter/flutter/blob/fef41cfce0e3582907f6846cf2385470bb17ecd1/packages/flutter/lib/src/widgets/draggable_scrollable_sheet.dart#L135

Without the code change, the test ends up throwing exceptions related to that line. I don't have a great way to verify what this does visually though, hoping for help on that from internal customer.

b/279930163

See cl/527868355 as well.

Fixes https://github.com/flutter/flutter/issues/125709
parent 9417b2a4
...@@ -206,6 +206,7 @@ class DraggableScrollableController extends ChangeNotifier { ...@@ -206,6 +206,7 @@ class DraggableScrollableController extends ChangeNotifier {
} else { } else {
_attachedController?.extent._currentSize.removeListener(notifyListeners); _attachedController?.extent._currentSize.removeListener(notifyListeners);
} }
_disposeAnimationControllers();
_attachedController = null; _attachedController = null;
} }
......
...@@ -1404,22 +1404,21 @@ void main() { ...@@ -1404,22 +1404,21 @@ void main() {
await tester.pumpWidget(boilerplate); await tester.pumpWidget(boilerplate);
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 { testWidgets('DraggableScrollableController.animateTo after detach', (WidgetTester tester) async {
// Regression test for https://github.com/flutter/flutter/issues/102483 final DraggableScrollableController controller = DraggableScrollableController();
final DraggableScrollableController controller = DraggableScrollableController(); await tester.pumpWidget(boilerplateWidget(() {}, controller: controller));
await tester.pumpWidget(boilerplateWidget(() {}, controller: controller));
controller.animateTo(0.0, curve: Curves.linear, duration: const Duration(milliseconds: 200)); controller.animateTo(0.0, curve: Curves.linear, duration: const Duration(milliseconds: 200));
await tester.pump(); await tester.pump();
// Dispose the DraggableScrollableSheet // Dispose the DraggableScrollableSheet
await tester.pumpWidget(const SizedBox.shrink()); await tester.pumpWidget(const SizedBox.shrink());
// Controller should be detached and no exception should be thrown // Controller should be detached and no exception should be thrown
expect(controller.isAttached, false); expect(controller.isAttached, false);
expect(tester.takeException(), isNull); expect(tester.takeException(), isNull);
}); });
testWidgets('DraggableScrollableSheet should not reset programmatic drag on rebuild', (WidgetTester tester) async { testWidgets('DraggableScrollableSheet should not reset programmatic drag on rebuild', (WidgetTester tester) async {
// Regression test for https://github.com/flutter/flutter/issues/101114 // Regression test for https://github.com/flutter/flutter/issues/101114
...@@ -1636,4 +1635,59 @@ void main() { ...@@ -1636,4 +1635,59 @@ void main() {
controller2.jumpTo(1.0); controller2.jumpTo(1.0);
expect(loggedSizes, <double>[1.0].map((double v) => closeTo(v, precisionErrorTolerance))); expect(loggedSizes, <double>[1.0].map((double v) => closeTo(v, precisionErrorTolerance)));
}); });
testWidgets('DraggableScrollableSheet controller can be changed while animating', (WidgetTester tester) async {
final DraggableScrollableController controller1 = DraggableScrollableController();
final DraggableScrollableController controller2 = DraggableScrollableController();
DraggableScrollableController controller = controller1;
await tester.pumpWidget(MaterialApp(
home: StatefulBuilder(
builder: (BuildContext context, StateSetter setState) => Scaffold(
body: DraggableScrollableSheet(
initialChildSize: 0.25,
snap: true,
snapSizes: const <double>[0.25, 0.5, 1.0],
controller: controller,
builder: (BuildContext context, ScrollController scrollController) {
return ListView(
controller: scrollController,
children: <Widget>[
ElevatedButton(
onPressed: () => setState(() {
controller = controller2;
}),
child: const Text('Switch controller'),
),
Container(
height: 10000,
color: Colors.blue,
),
],
);
},
),
),
),
));
expect(controller1.isAttached, true);
expect(controller2.isAttached, false);
controller1.animateTo(0.5, curve: Curves.linear, duration: const Duration(milliseconds: 200));
await tester.pump();
await tester.tap(find.text('Switch controller'));
await tester.pump();
expect(controller1.isAttached, false);
expect(controller2.isAttached, true);
controller2.animateTo(1.0, curve: Curves.linear, duration: const Duration(milliseconds: 200));
await tester.pump();
await tester.pumpWidget(const SizedBox.shrink());
expect(controller1.isAttached, false);
expect(controller2.isAttached, false);
});
} }
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