Do not schedule animation if already at the target value (#11503)

* Do not schedule animation if already at the target value

* Partially fixes https://github.com/flutter/flutter/issues/11495.
* Also includes a fix for Cupertino button to always run the tap animation even if the finger is immediately lifted from the screen (uncovered by a test failure).

* refactorings

* more tests

* test clearifications

* remove Listener

* fix lints

* fix async issue

packages/flutter/lib/src/animation/animation_controller.dart

@@ -335,6 +335,9 @@ class AnimationController extends Animation<double>
       final double range = upperBound - lowerBound;
       final double remainingFraction = range.isFinite ? (target - _value).abs() / range : 1.0;
       simulationDuration = this.duration * remainingFraction;
+    } else if (target == value) {
+      // Already at target, don't animate.
+      simulationDuration = Duration.ZERO;
     }
     stop();
     if (simulationDuration == Duration.ZERO) {

packages/flutter/lib/src/cupertino/button.dart

@@ -2,6 +2,8 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+import 'dart:async';
+
 import 'package:flutter/foundation.dart';
 import 'package:flutter/widgets.dart';
 
@@ -110,7 +112,7 @@ class CupertinoButton extends StatefulWidget {
 class _CupertinoButtonState extends State<CupertinoButton> with SingleTickerProviderStateMixin {
   // Eyeballed values. Feel free to tweak.
   static const Duration kFadeOutDuration = const Duration(milliseconds: 10);
-  static const Duration kFadeInDuration = const Duration(milliseconds: 350);
+  static const Duration kFadeInDuration = const Duration(milliseconds: 100);
   Tween<double> _opacityTween;
 
   AnimationController _animationController;
@@ -146,16 +148,40 @@ class _CupertinoButtonState extends State<CupertinoButton> with SingleTickerProv
     _setTween();
   }
 
-  void _handleTapDown(PointerDownEvent event) {
+  bool _buttonHeldDown = false;
-    _animationController.animateTo(1.0, duration: kFadeOutDuration);
+
+  void _handleTapDown(TapDownDetails event) {
+    if (!_buttonHeldDown) {
+      _buttonHeldDown = true;
+      _animate();
+    }
   }
 
-  void _handleTapUp(PointerUpEvent event) {
+  void _handleTapUp(TapUpDetails event) {
-    _animationController.animateTo(0.0, duration: kFadeInDuration);
+    if (_buttonHeldDown) {
+      _buttonHeldDown = false;
+      _animate();
+    }
   }
 
-  void _handleTapCancel(PointerCancelEvent event) {
+  void _handleTapCancel() {
-    _animationController.animateTo(0.0, duration: kFadeInDuration);
+    if (_buttonHeldDown) {
+      _buttonHeldDown = false;
+      _animate();
+    }
+  }
+
+  void _animate() {
+    if (_animationController.isAnimating)
+      return;
+    final bool wasHeldDown = _buttonHeldDown;
+    final Future<Null> ticker = _buttonHeldDown
+        ? _animationController.animateTo(1.0, duration: kFadeOutDuration)
+        : _animationController.animateTo(0.0, duration: kFadeInDuration);
+    ticker.then((Null value) {
+      if (mounted && wasHeldDown != _buttonHeldDown)
+        _animate();
+    });
   }
 
   @override
@@ -163,11 +189,10 @@ class _CupertinoButtonState extends State<CupertinoButton> with SingleTickerProv
     final bool enabled = widget.enabled;
     final Color backgroundColor = widget.color;
 
-    return new Listener(
+    return new GestureDetector(
-      onPointerDown: enabled ? _handleTapDown : null,
+      onTapDown: enabled ? _handleTapDown : null,
-      onPointerUp: enabled ? _handleTapUp : null,
+      onTapUp: enabled ? _handleTapUp : null,
-      onPointerCancel: enabled ? _handleTapCancel : null,
+      onTapCancel: enabled ? _handleTapCancel : null,
-      child: new GestureDetector(
       onTap: widget.onPressed,
       child: new ConstrainedBox(
         constraints: widget.minSize == null
@@ -208,7 +233,6 @@ class _CupertinoButtonState extends State<CupertinoButton> with SingleTickerProv
           ),
         ),
       ),
-      ),
     );
   }
 }

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

@@ -174,6 +174,12 @@ class ScrollPositionWithSingleContext extends ScrollPosition implements ScrollAc
     @required Duration duration,
     @required Curve curve,
   }) {
+    if (nearEqual(to, pixels, physics.tolerance.distance)) {
+      // Skip the animation, go straight to the position as we are already close.
+      jumpTo(to);
+      return new Future<Null>.value();
+    }
+
     final DrivenScrollActivity activity = new DrivenScrollActivity(
       this,
       from: pixels,

packages/flutter/test/animation/animation_controller_test.dart

@@ -292,4 +292,93 @@ void main() {
     expect((){ controller.repeat(period: null); }, throwsFlutterError);
   });
 
+  test('Do not animate if already at target', () {
+    final List<AnimationStatus> statusLog = <AnimationStatus>[];
+
+    final AnimationController controller = new AnimationController(
+      value: 0.5,
+      vsync: const TestVSync(),
+    )..addStatusListener(statusLog.add);
+
+    expect(controller.value, equals(0.5));
+    controller.animateTo(0.5, duration: const Duration(milliseconds: 100));
+    expect(statusLog, equals(<AnimationStatus>[ AnimationStatus.completed ]));
+    expect(controller.value, equals(0.5));
+  });
+
+  test('Do not animate to upperBound if already at upperBound', () {
+    final List<AnimationStatus> statusLog = <AnimationStatus>[];
+
+    final AnimationController controller = new AnimationController(
+      value: 1.0,
+      upperBound: 1.0,
+      lowerBound: 0.0,
+      vsync: const TestVSync(),
+    )..addStatusListener(statusLog.add);
+
+    expect(controller.value, equals(1.0));
+    controller.animateTo(1.0, duration: const Duration(milliseconds: 100));
+    expect(statusLog, equals(<AnimationStatus>[ AnimationStatus.completed ]));
+    expect(controller.value, equals(1.0));
+  });
+
+  test('Do not animate to lowerBound if already at lowerBound', () {
+    final List<AnimationStatus> statusLog = <AnimationStatus>[];
+
+    final AnimationController controller = new AnimationController(
+      value: 0.0,
+      upperBound: 1.0,
+      lowerBound: 0.0,
+      vsync: const TestVSync(),
+    )..addStatusListener(statusLog.add);
+
+    expect(controller.value, equals(0.0));
+    controller.animateTo(0.0, duration: const Duration(milliseconds: 100));
+    expect(statusLog, equals(<AnimationStatus>[ AnimationStatus.completed ]));
+    expect(controller.value, equals(0.0));
+  });
+
+  test('Do not animate if already at target mid-flight (forward)', () {
+    final List<AnimationStatus> statusLog = <AnimationStatus>[];
+    final AnimationController controller = new AnimationController(
+      value: 0.0,
+      duration: const Duration(milliseconds: 1000),
+      vsync: const TestVSync(),
+    )..addStatusListener(statusLog.add);
+
+    expect(controller.value, equals(0.0));
+
+    controller.forward();
+    tick(const Duration(milliseconds: 0));
+    tick(const Duration(milliseconds: 500));
+    expect(controller.value, inInclusiveRange(0.4, 0.6));
+    expect(statusLog, equals(<AnimationStatus>[ AnimationStatus.forward ]));
+
+    final double currentValue = controller.value;
+    controller.animateTo(currentValue, duration: const Duration(milliseconds: 100));
+    expect(statusLog, equals(<AnimationStatus>[ AnimationStatus.forward, AnimationStatus.completed ]));
+    expect(controller.value, currentValue);
+  });
+
+  test('Do not animate if already at target mid-flight (reverse)', () {
+    final List<AnimationStatus> statusLog = <AnimationStatus>[];
+    final AnimationController controller = new AnimationController(
+      value: 1.0,
+      duration: const Duration(milliseconds: 1000),
+      vsync: const TestVSync(),
+    )..addStatusListener(statusLog.add);
+
+    expect(controller.value, equals(1.0));
+
+    controller.reverse();
+    tick(const Duration(milliseconds: 0));
+    tick(const Duration(milliseconds: 500));
+    expect(controller.value, inInclusiveRange(0.4, 0.6));
+    expect(statusLog, equals(<AnimationStatus>[ AnimationStatus.reverse ]));
+
+    final double currentValue = controller.value;
+    controller.animateTo(currentValue, duration: const Duration(milliseconds: 100));
+    expect(statusLog, equals(<AnimationStatus>[ AnimationStatus.reverse, AnimationStatus.dismissed ]));
+    expect(controller.value, currentValue);
+  });
 }

packages/flutter/test/widgets/scrollable_animations_test.dart

@@ -0,0 +1,72 @@
+// Copyright 2017 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/scheduler.dart';
+import 'package:flutter/widgets.dart';
+
+void main() {
+  testWidgets('Does not animate if already at target position', (WidgetTester tester) async {
+    final List<Widget> textWidgets = <Widget>[];
+    for (int i = 0; i < 80; i++)
+      textWidgets.add(new Text('$i'));
+    final ScrollController controller = new ScrollController();
+    await tester.pumpWidget(new ListView(
+      children: textWidgets,
+      controller: controller,
+    ));
+
+    expectNoAnimation();
+    final double currentPosition = controller.position.pixels;
+    controller.position.animateTo(currentPosition, duration: const Duration(seconds: 10), curve: Curves.linear);
+
+    expectNoAnimation();
+    expect(controller.position.pixels, currentPosition);
+  });
+
+  testWidgets('Does not animate if already at target position within tolerance', (WidgetTester tester) async {
+    final List<Widget> textWidgets = <Widget>[];
+    for (int i = 0; i < 80; i++)
+      textWidgets.add(new Text('$i'));
+    final ScrollController controller = new ScrollController();
+    await tester.pumpWidget(new ListView(
+      children: textWidgets,
+      controller: controller,
+    ));
+
+    expectNoAnimation();
+
+    final double halfTolerance = controller.position.physics.tolerance.distance / 2;
+    expect(halfTolerance, isNonZero);
+    final double targetPosition = controller.position.pixels + halfTolerance;
+    controller.position.animateTo(targetPosition, duration: const Duration(seconds: 10), curve: Curves.linear);
+
+    expectNoAnimation();
+    expect(controller.position.pixels, targetPosition);
+  });
+
+  testWidgets('Animates if going to a position outside of tolerance', (WidgetTester tester) async {
+    final List<Widget> textWidgets = <Widget>[];
+    for (int i = 0; i < 80; i++)
+      textWidgets.add(new Text('$i'));
+    final ScrollController controller = new ScrollController();
+    await tester.pumpWidget(new ListView(
+      children: textWidgets,
+      controller: controller,
+    ));
+
+    expectNoAnimation();
+
+    final double doubleTolerance = controller.position.physics.tolerance.distance * 2;
+    expect(doubleTolerance, isNonZero);
+    final double targetPosition = controller.position.pixels + doubleTolerance;
+    controller.position.animateTo(targetPosition, duration: const Duration(seconds: 10), curve: Curves.linear);
+
+    expect(SchedulerBinding.instance.transientCallbackCount, equals(1), reason: 'Expected an animation.');
+  });
+}
+
+void expectNoAnimation() {
+  expect(SchedulerBinding.instance.transientCallbackCount, equals(0), reason: 'Expected no animation.');
+}