Fix some hero observer bugs (#5633)

1: If a route is already dismissed when it's popped, there's no point
trying to animate heroes, because it's going to be gone before the
heroes code can look at it.

2: If a hero animation finishes just as a new one is starting, we
previously blew away the state for the starting one. Now we correctly
segregate the "starting up quest" variables from the "actively ongoing
quest" variables.

packages/flutter/lib/src/scheduler/binding.dart

@@ -504,6 +504,12 @@ abstract class SchedulerBinding extends BindingBase {
 
     Timeline.finishSync();
 
+    assert(() {
+      if (debugPrintEndFrameBanner)
+        debugPrint('━━━━━━━┫ End of Frame ┣━━━━━━━');
+      return true;
+    });
+
     // All frame-related callbacks have been executed. Run lower-priority tasks.
     _runTasks();
   }

packages/flutter/lib/src/scheduler/debug.dart

@@ -4,3 +4,9 @@
 
 /// Print a banner at the beginning of each frame.
 bool debugPrintBeginFrameBanner = false;
+
+/// Print a banner at the end of each frame.
+///
+/// Combined with [debugPrintBeginFrameBanner], this can be helpful for
+/// determining if code is running during a frame or between frames.
+bool debugPrintEndFrameBanner = false;

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

@@ -5,6 +5,7 @@
 import 'dart:collection';
 
 import 'package:meta/meta.dart';
+import 'package:flutter/foundation.dart';
 
 import 'basic.dart';
 import 'binding.dart';
@@ -424,10 +425,15 @@ class HeroController extends NavigatorObserver {
     );
   }
 
+  // The current party, if they're on a quest.
   _HeroParty _party;
-  Animation<double> _animation;
+
+  // The settings used to prepare the next quest.
+  // These members are only non-null between the didPush/didPop call and the
+  // corresponding _updateQuest call.
   PageRoute<dynamic> _from;
   PageRoute<dynamic> _to;
+  Animation<double> _animation;
 
   final List<OverlayEntry> _overlayEntries = new List<OverlayEntry>();
 
@@ -451,9 +457,9 @@ class HeroController extends NavigatorObserver {
     assert(route != null);
     if (route is PageRoute<dynamic>) {
       assert(route.animation != null);
-      if (previousRoute is PageRoute<dynamic>) {
-        _to = previousRoute;
+      if (route.animation.status != AnimationStatus.dismissed && previousRoute is PageRoute<dynamic>) {
         _from = route;
+        _to = previousRoute;
         _animation = route.animation;
         _checkForHeroQuest();
       }
@@ -475,16 +481,17 @@ class HeroController extends NavigatorObserver {
 
   void _checkForHeroQuest() {
     if (_from != null && _to != null && _from != _to && _questsEnabled) {
+      assert(_animation != null);
       _to.offstage = _to.animation.status != AnimationStatus.completed;
       WidgetsBinding.instance.addPostFrameCallback(_updateQuest);
+    } else {
+      // this isn't a valid quest
+      _clearPendingHeroQuest();
     }
   }
 
   void _handleQuestFinished() {
     _removeHeroesFromOverlay();
-    _from = null;
-    _to = null;
-    _animation = null;
   }
 
   Rect _getAnimationArea(BuildContext context) {
@@ -514,8 +521,11 @@ class HeroController extends NavigatorObserver {
   void _updateQuest(Duration timeStamp) {
     if (navigator == null) {
       // The navigator was removed before this end-of-frame callback was called.
+      _clearPendingHeroQuest();
       return;
     }
+    assert(_from.subtreeContext != null);
+    assert(_to.subtreeContext != null);
     Map<Object, _HeroHandle> heroesFrom = _party.isEmpty ?
         Hero._of(_from.subtreeContext) : _party.getHeroesToAnimate();
 
@@ -541,5 +551,13 @@ class HeroController extends NavigatorObserver {
         navigator.overlay
       );
     }
+
+    _clearPendingHeroQuest();
+  }
+
+  void _clearPendingHeroQuest() {
+    _from = null;
+    _to = null;
+    _animation = null;
   }
 }

packages/flutter/test/widget/heroes_test.dart

@@ -286,4 +286,64 @@ void main() {
     await tester.tap(find.text('bar'));
     expect(log, equals(<String>['bar']));
   });
+
+  testWidgets('Popping on first frame does not cause hero observer to crash', (WidgetTester tester) async {
+    await tester.pumpWidget(new MaterialApp(
+      onGenerateRoute: (RouteSettings settings) {
+        return new MaterialPageRoute<Null>(
+          settings: settings,
+          builder: (BuildContext context) => new Hero(tag: 'test', child: new Container()),
+        );
+      },
+    ));
+    await tester.pump();
+
+    Finder heroes = find.byType(Hero);
+    expect(heroes, findsOneWidget);
+
+    Navigator.pushNamed(heroes.evaluate().first, 'test');
+    await tester.pump(); // adds the new page to the tree...
+
+    Navigator.pop(heroes.evaluate().first);
+    await tester.pump(); // ...and removes it straight away (since it's already at 0.0)
+
+    // this is verifying that there's no crash
+
+    // TODO(ianh): once https://github.com/flutter/flutter/issues/5631 is fixed, remove this line:
+    await tester.pump(const Duration(hours: 1));
+  });
+
+  testWidgets('Overlapping starting and ending a hero transition works ok', (WidgetTester tester) async {
+    await tester.pumpWidget(new MaterialApp(
+      onGenerateRoute: (RouteSettings settings) {
+        return new MaterialPageRoute<Null>(
+          settings: settings,
+          builder: (BuildContext context) => new Hero(tag: 'test', child: new Container()),
+        );
+      },
+    ));
+    await tester.pump();
+
+    Finder heroes = find.byType(Hero);
+    expect(heroes, findsOneWidget);
+
+    Navigator.pushNamed(heroes.evaluate().first, 'test');
+    await tester.pump();
+    await tester.pump(const Duration(hours: 1));
+
+    Navigator.pushNamed(heroes.evaluate().first, 'test');
+    await tester.pump();
+    await tester.pump(const Duration(hours: 1));
+
+    Navigator.pop(heroes.evaluate().first);
+    await tester.pump();
+    Navigator.pop(heroes.evaluate().first);
+    await tester.pump(const Duration(hours: 1)); // so the first transition is finished, but the second hasn't started
+    await tester.pump();
+
+    // this is verifying that there's no crash
+
+    // TODO(ianh): once https://github.com/flutter/flutter/issues/5631 is fixed, remove this line:
+    await tester.pump(const Duration(hours: 1));
+  });
 }