Skip to content

Commit 57cddce

Browse files
authored
Fix race condition causing crash when interacting with an animating scrollable (#164392)
Fix flutter/flutter#163297 Fix flutter/flutter#14452 Partial patch: flutter/flutter#37267 ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [ ] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. <!-- Links --> [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md
1 parent 86e4c12 commit 57cddce

File tree

2 files changed

+57
-1
lines changed

2 files changed

+57
-1
lines changed

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -872,7 +872,10 @@ class ScrollableState extends State<Scrollable>
872872
assert(_drag == null);
873873
_drag = position.drag(details, _disposeDrag);
874874
assert(_drag != null);
875-
assert(_hold == null);
875+
// _hold might be non-null if the scroll position is currently animating.
876+
if (_hold != null) {
877+
_disposeHold();
878+
}
876879
}
877880

878881
void _handleDragUpdate(DragUpdateDetails details) {

packages/flutter/test/widgets/scrollable_animations_test.dart

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// Use of this source code is governed by a BSD-style license that can be
33
// found in the LICENSE file.
44

5+
import 'package:flutter/gestures.dart';
56
import 'package:flutter/scheduler.dart';
67
import 'package:flutter/widgets.dart';
78
import 'package:flutter_test/flutter_test.dart';
@@ -103,6 +104,58 @@ void main() {
103104
reason: 'Expected an animation.',
104105
);
105106
});
107+
108+
testWidgets('HoldActivity can interrupt ScrollPosition.animateTo', (WidgetTester tester) async {
109+
const double animationExtent = 100.0;
110+
const double dragExtent = 30.0;
111+
final ScrollController controller = ScrollController();
112+
addTearDown(controller.dispose);
113+
await tester.pumpWidget(
114+
Directionality(
115+
textDirection: TextDirection.ltr,
116+
child: NotificationListener<ScrollNotification>(
117+
onNotification: (ScrollNotification notification) {
118+
controller.position.animateTo(
119+
animationExtent,
120+
duration: const Duration(seconds: 1),
121+
curve: Curves.linear,
122+
);
123+
return true;
124+
},
125+
child: ListView(
126+
controller: controller,
127+
dragStartBehavior: DragStartBehavior.down,
128+
children: List<Widget>.generate(
129+
80,
130+
(int i) => Text('$i', textDirection: TextDirection.ltr),
131+
),
132+
),
133+
),
134+
),
135+
);
136+
137+
expectNoAnimation();
138+
139+
// Drag to initiate the scroll animation.
140+
await tester.drag(find.byType(Scrollable), const Offset(0.0, 1.0));
141+
await tester.pump();
142+
143+
// Pump to halfway through the animation.
144+
await tester.pump(const Duration(milliseconds: 500));
145+
expect(controller.position.pixels, animationExtent / 2);
146+
147+
// Interrupt the scroll animation.
148+
final TestGesture gesture = await tester.startGesture(
149+
tester.getCenter(find.byType(Scrollable)),
150+
);
151+
await gesture.moveBy(const Offset(0.0, dragExtent));
152+
await gesture.up();
153+
154+
await tester.pump(const Duration(milliseconds: 500));
155+
156+
// The drag stops the animation, and the drag extent is respected.
157+
expect(controller.position.pixels, (animationExtent / 2) - dragExtent);
158+
});
106159
}
107160

108161
void expectNoAnimation() {

0 commit comments

Comments
 (0)