Commit 112f2cc3 authored by Ian Hickson's avatar Ian Hickson

Reset _simulation at the end of a fling (#3435)

Also a bit of code cleanup.

The key part of this patch is the addition in `_endScroll` to reset
`_simulation`. It seems like this was the one place where it's possible
for us to end the animation but not reset our state. Since we assert
that are state is coherent, we were hitting asserts when a fling
finished and then you interacted with the widget again.
parent 140bf64a
...@@ -126,7 +126,7 @@ class AnimationController extends Animation<double> ...@@ -126,7 +126,7 @@ class AnimationController extends Animation<double>
/// The amount of time that has passed between the time the animation started and the most recent tick of the animation. /// The amount of time that has passed between the time the animation started and the most recent tick of the animation.
/// ///
/// If the controller is not animating, the last elapsed duration is null; /// If the controller is not animating, the last elapsed duration is null.
Duration get lastElapsedDuration => _lastElapsedDuration; Duration get lastElapsedDuration => _lastElapsedDuration;
Duration _lastElapsedDuration; Duration _lastElapsedDuration;
...@@ -212,7 +212,7 @@ class AnimationController extends Animation<double> ...@@ -212,7 +212,7 @@ class AnimationController extends Animation<double>
assert(simulation != null); assert(simulation != null);
assert(!isAnimating); assert(!isAnimating);
_simulation = simulation; _simulation = simulation;
_lastElapsedDuration = const Duration(); _lastElapsedDuration = Duration.ZERO;
_value = simulation.x(0.0).clamp(lowerBound, upperBound); _value = simulation.x(0.0).clamp(lowerBound, upperBound);
Future<Null> result = _ticker.start(); Future<Null> result = _ticker.start();
_checkStatusChanged(); _checkStatusChanged();
......
...@@ -223,17 +223,17 @@ abstract class ScrollableState<T extends Scrollable> extends State<T> { ...@@ -223,17 +223,17 @@ abstract class ScrollableState<T extends Scrollable> extends State<T> {
@override @override
void initState() { void initState() {
super.initState(); super.initState();
_controller = new AnimationController.unbounded()..addListener(_handleAnimationChanged); _controller = new AnimationController.unbounded()
..addListener(_handleAnimationChanged);
_scrollOffset = PageStorage.of(context)?.readState(context) ?? config.initialScrollOffset ?? 0.0; _scrollOffset = PageStorage.of(context)?.readState(context) ?? config.initialScrollOffset ?? 0.0;
} }
Simulation _simulation; Simulation _simulation; // if we're flinging, then this is the animation with which we're doing it
AnimationController _controller; AnimationController _controller;
@override @override
void dispose() { void dispose() {
_simulation = null; _stop();
_controller.stop();
super.dispose(); super.dispose();
} }
...@@ -360,8 +360,7 @@ abstract class ScrollableState<T extends Scrollable> extends State<T> { ...@@ -360,8 +360,7 @@ abstract class ScrollableState<T extends Scrollable> extends State<T> {
return new Future<Null>.value(); return new Future<Null>.value();
if (duration == null) { if (duration == null) {
_simulation = null; _stop();
_controller.stop();
_setScrollOffset(newScrollOffset); _setScrollOffset(newScrollOffset);
return new Future<Null>.value(); return new Future<Null>.value();
} }
...@@ -371,8 +370,7 @@ abstract class ScrollableState<T extends Scrollable> extends State<T> { ...@@ -371,8 +370,7 @@ abstract class ScrollableState<T extends Scrollable> extends State<T> {
} }
Future<Null> _animateTo(double newScrollOffset, Duration duration, Curve curve) { Future<Null> _animateTo(double newScrollOffset, Duration duration, Curve curve) {
_simulation = null; _stop();
_controller.stop();
_controller.value = scrollOffset; _controller.value = scrollOffset;
_startScroll(); _startScroll();
return _controller.animateTo(newScrollOffset, duration: duration, curve: curve).then(_endScroll); return _controller.animateTo(newScrollOffset, duration: duration, curve: curve).then(_endScroll);
...@@ -388,6 +386,7 @@ abstract class ScrollableState<T extends Scrollable> extends State<T> { ...@@ -388,6 +386,7 @@ abstract class ScrollableState<T extends Scrollable> extends State<T> {
/// If there are no in-progress scrolling physics, this function scrolls to /// If there are no in-progress scrolling physics, this function scrolls to
/// the given offset instead. /// the given offset instead.
void didUpdateScrollBehavior(double newScrollOffset) { void didUpdateScrollBehavior(double newScrollOffset) {
assert(_controller.isAnimating || _simulation == null);
if (_numberOfInProgressScrolls > 0) { if (_numberOfInProgressScrolls > 0) {
if (_simulation != null) { if (_simulation != null) {
double dx = _simulation.dx(_controller.lastElapsedDuration.inMicroseconds / Duration.MICROSECONDS_PER_SECOND); double dx = _simulation.dx(_controller.lastElapsedDuration.inMicroseconds / Duration.MICROSECONDS_PER_SECOND);
...@@ -420,8 +419,7 @@ abstract class ScrollableState<T extends Scrollable> extends State<T> { ...@@ -420,8 +419,7 @@ abstract class ScrollableState<T extends Scrollable> extends State<T> {
} }
Future<Null> _startToEndAnimation(double scrollVelocity) { Future<Null> _startToEndAnimation(double scrollVelocity) {
_simulation = null; _stop();
_controller.stop();
_simulation = _createSnapSimulation(scrollVelocity) ?? _createFlingSimulation(scrollVelocity); _simulation = _createSnapSimulation(scrollVelocity) ?? _createFlingSimulation(scrollVelocity);
if (_simulation == null) if (_simulation == null)
return new Future<Null>.value(); return new Future<Null>.value();
...@@ -498,8 +496,13 @@ abstract class ScrollableState<T extends Scrollable> extends State<T> { ...@@ -498,8 +496,13 @@ abstract class ScrollableState<T extends Scrollable> extends State<T> {
} }
void _handleDragDown(_) { void _handleDragDown(_) {
_simulation = null; _stop();
}
void _stop() {
assert(_controller.isAnimating || _simulation == null);
_controller.stop(); _controller.stop();
_simulation = null;
} }
void _handleDragStart(_) { void _handleDragStart(_) {
...@@ -533,8 +536,10 @@ abstract class ScrollableState<T extends Scrollable> extends State<T> { ...@@ -533,8 +536,10 @@ abstract class ScrollableState<T extends Scrollable> extends State<T> {
Null _endScroll([Null _]) { Null _endScroll([Null _]) {
_numberOfInProgressScrolls -= 1; _numberOfInProgressScrolls -= 1;
if (_numberOfInProgressScrolls == 0) if (_numberOfInProgressScrolls == 0) {
_simulation = null;
dispatchOnScrollEnd(); dispatchOnScrollEnd();
}
return null; return null;
} }
......
// Copyright 2015 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
import 'package:flutter_test/flutter_test.dart';
import 'package:flutter/widgets.dart';
import 'package:test/test.dart';
void main() {
test('Scroll flings twice in a row does not crash', () {
testWidgets((WidgetTester tester) {
tester.pumpWidget(new Block(
children: <Widget>[
new Container(height: 100000.0)
]
));
ScrollableState<ScrollableViewport> scrollable =
tester.stateOf/*<ScrollableState<ScrollableViewport>>*/(find.byType(ScrollableViewport));
expect(scrollable.scrollOffset, equals(0.0));
tester.flingFrom(new Point(200.0, 300.0), new Offset(0.0, -200.0), 500.0);
tester.pump();
tester.pump(const Duration(seconds: 5));
expect(scrollable.scrollOffset, greaterThan(0.0));
double oldOffset = scrollable.scrollOffset;
tester.flingFrom(new Point(200.0, 300.0), new Offset(0.0, -200.0), 500.0);
tester.pump();
tester.pump(const Duration(seconds: 5));
expect(scrollable.scrollOffset, greaterThan(oldOffset));
});
});
}
...@@ -126,7 +126,7 @@ class WidgetTester { ...@@ -126,7 +126,7 @@ class WidgetTester {
} }
/// Finds the first state object, searching in the depth-first traversal order. /// Finds the first state object, searching in the depth-first traversal order.
State stateOf(Finder finder) { State/*=T*/ stateOf/*<T extends State>*/(Finder finder) {
Element element = finder.findFirst(this); Element element = finder.findFirst(this);
Widget widget = element.widget; Widget widget = element.widget;
......
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