From 25ab5555b70b09fadb85949aef534bf3b40ef3ff Mon Sep 17 00:00:00 2001
From: Adam Barth <abarth@chromium.org>
Date: Fri, 26 Feb 2016 15:41:23 -0800
Subject: [PATCH] Elide fewer AnimationStatus callbacks

Previously we would elide forward and reverse callbacks that canceled each
other out, which broke the expected state machine. Now we synchronously deliver
status callbacks when start an animation.

Fixes #1913
---
 .../src/animation/animation_controller.dart   | 13 ++--
 packages/flutter/lib/src/widgets/overlay.dart |  2 +-
 .../animation/animation_controller_test.dart  | 65 +++++++++++++++++++
 3 files changed, 74 insertions(+), 6 deletions(-)

diff --git a/packages/flutter/lib/src/animation/animation_controller.dart b/packages/flutter/lib/src/animation/animation_controller.dart
index 12cc1b767f..db0461095d 100644
--- a/packages/flutter/lib/src/animation/animation_controller.dart
+++ b/packages/flutter/lib/src/animation/animation_controller.dart
@@ -146,6 +146,7 @@ class AnimationController extends Animation<double>
     stop();
     if (simulationDuration == Duration.ZERO) {
       assert(value == target);
+      _checkStatusChanged();
       return new Future.value();
     }
     assert(simulationDuration > Duration.ZERO);
@@ -180,7 +181,9 @@ class AnimationController extends Animation<double>
     assert(!isAnimating);
     _simulation = simulation;
     _value = simulation.x(0.0).clamp(lowerBound, upperBound);
-    return _ticker.start();
+    Future result = _ticker.start();
+    _checkStatusChanged();
+    return result;
   }
 
   /// Stops running this animation.
@@ -194,13 +197,13 @@ class AnimationController extends Animation<double>
     stop();
   }
 
-  AnimationStatus _lastStatus = AnimationStatus.dismissed;
+  AnimationStatus _lastReportedStatus = AnimationStatus.dismissed;
   void _checkStatusChanged() {
     AnimationStatus newStatus = status;
-    AnimationStatus oldStatus = _lastStatus;
-    _lastStatus = newStatus;
-    if (oldStatus != newStatus)
+    if (_lastReportedStatus != newStatus) {
+      _lastReportedStatus = newStatus;
       notifyStatusListeners(newStatus);
+    }
   }
 
   void _tick(Duration elapsed) {
diff --git a/packages/flutter/lib/src/widgets/overlay.dart b/packages/flutter/lib/src/widgets/overlay.dart
index 9225413412..015248b7f6 100644
--- a/packages/flutter/lib/src/widgets/overlay.dart
+++ b/packages/flutter/lib/src/widgets/overlay.dart
@@ -22,9 +22,9 @@ class OverlayEntry {
   bool get opaque => _opaque;
   bool _opaque;
   void set opaque (bool value) {
-    assert(_overlay != null);
     if (_opaque == value)
       return;
+    assert(_overlay != null);
     _overlay.setState(() {
       _opaque = value;
     });
diff --git a/packages/flutter/test/animation/animation_controller_test.dart b/packages/flutter/test/animation/animation_controller_test.dart
index cb283f5612..f7f1903f18 100644
--- a/packages/flutter/test/animation/animation_controller_test.dart
+++ b/packages/flutter/test/animation/animation_controller_test.dart
@@ -36,5 +36,70 @@ void main() {
     Scheduler.instance.handleBeginFrame(const Duration(seconds: 2));
     expect(didComplete, isTrue);
     expect(didDismiss, isTrue);
+
+    controller.stop();
+  });
+
+  test("Receives status callbacks for forward and reverse", () {
+    WidgetFlutterBinding.ensureInitialized();
+    AnimationController controller = new AnimationController(
+      duration: const Duration(milliseconds: 100)
+    );
+    List<double> valueLog = <double>[];
+    List<AnimationStatus> log = <AnimationStatus>[];
+    controller
+      ..addStatusListener((AnimationStatus status) {
+        log.add(status);
+      })
+      ..addListener(() {
+        valueLog.add(controller.value);
+      });
+
+    expect(log, equals([]));
+    expect(valueLog, equals([]));
+
+    controller.forward();
+
+    expect(log, equals([AnimationStatus.forward]));
+    expect(valueLog, equals([]));
+
+    controller.reverse();
+
+    expect(log, equals([AnimationStatus.forward, AnimationStatus.dismissed]));
+    expect(valueLog, equals([]));
+
+    controller.reverse();
+
+    expect(log, equals([AnimationStatus.forward, AnimationStatus.dismissed]));
+    expect(valueLog, equals([]));
+
+    log.clear();
+    controller.forward();
+
+    expect(log, equals([AnimationStatus.forward]));
+    expect(valueLog, equals([]));
+
+    controller.forward();
+
+    expect(log, equals([AnimationStatus.forward]));
+    expect(valueLog, equals([]));
+
+    controller.reverse();
+    log.clear();
+
+    Scheduler.instance.handleBeginFrame(const Duration(seconds: 10));
+    expect(log, equals([]));
+    expect(valueLog, equals([]));
+    Scheduler.instance.handleBeginFrame(const Duration(seconds: 20));
+    expect(log, equals([]));
+    expect(valueLog, equals([]));
+    Scheduler.instance.handleBeginFrame(const Duration(seconds: 30));
+    expect(log, equals([]));
+    expect(valueLog, equals([]));
+    Scheduler.instance.handleBeginFrame(const Duration(seconds: 40));
+    expect(log, equals([]));
+    expect(valueLog, equals([]));
+
+    controller.stop();
   });
 }