Some clean-up of the Heroes code (#9428)

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

@@ -28,7 +28,7 @@ enum _HeroFlightType {
 }
 
 // The bounding box for context in global coordinates.
-Rect _globalRect(BuildContext context) {
+Rect _globalBoundingBoxFor(BuildContext context) {
   final RenderBox box = context.findRenderObject();
   assert(box != null && box.hasSize);
   return MatrixUtils.transformRect(box.getTransformTo(null), Offset.zero & box.size);
@@ -170,7 +170,7 @@ class _HeroState extends State<Hero> {
   }
 }
 
-// Everything known about a hero flight that's to be started or restarted.
+// Everything known about a hero flight that's to be started or diverted.
 class _HeroFlightManifest {
   _HeroFlightManifest({
     @required this.type,
@@ -317,9 +317,9 @@ class _HeroFlight {
     manifest.fromHero.startFlight();
     manifest.toHero.startFlight();
 
-    heroRect = new RectTween(
-      begin: _globalRect(manifest.fromHero.context),
-      end: _globalRect(manifest.toHero.context),
+    heroRect = _doCreateRectTween(
+      _globalBoundingBoxFor(manifest.fromHero.context),
+      _globalBoundingBoxFor(manifest.toHero.context),
     );
 
     overlayEntry = new OverlayEntry(builder: _buildOverlay);
@@ -328,7 +328,7 @@ class _HeroFlight {
 
   // While this flight's hero was in transition a push or a pop occurred for
   // routes with the same hero. Redirect the in-flight hero to the new toRoute.
-  void restart(_HeroFlightManifest newManifest) {
+  void divert(_HeroFlightManifest newManifest) {
     assert(manifest.tag == newManifest.tag);
 
     if (manifest.type == _HeroFlightType.push && newManifest.type == _HeroFlightType.pop) {
@@ -341,10 +341,7 @@ class _HeroFlight {
 
       _proxyAnimation.parent = new ReverseAnimation(newManifest.animation);
 
-      heroRect = new RectTween(
-        begin: heroRect.end,
-        end: heroRect.begin,
-      );
+      heroRect = _doCreateRectTween(heroRect.end, heroRect.begin);
     } else if (manifest.type == _HeroFlightType.pop && newManifest.type == _HeroFlightType.push) {
       // A pop flight was interrupted by a push.
       assert(newManifest.animation.status == AnimationStatus.forward);
@@ -359,15 +356,9 @@ class _HeroFlight {
       if (manifest.fromHero != newManifest.toHero) {
         manifest.fromHero.endFlight();
         newManifest.toHero.startFlight();
-        heroRect = new RectTween(
-          begin: heroRect.end,
-          end: _globalRect(newManifest.toHero.context),
-        );
+        heroRect = _doCreateRectTween(heroRect.end, _globalBoundingBoxFor(newManifest.toHero.context));
       } else {
-        heroRect = new RectTween(
-          begin: heroRect.end,
-          end: heroRect.begin,
-        );
+        heroRect = _doCreateRectTween(heroRect.end, heroRect.begin);
       }
     } else {
       // A push or a pop flight is heading to a new route, i.e.
@@ -376,10 +367,7 @@ class _HeroFlight {
       assert(manifest.fromHero != newManifest.fromHero);
       assert(manifest.toHero != newManifest.toHero);
 
-      heroRect = new RectTween(
-        begin: heroRect.evaluate(_proxyAnimation),
-        end: _globalRect(newManifest.toHero.context),
-      );
+      heroRect = _doCreateRectTween(heroRect.evaluate(_proxyAnimation), _globalBoundingBoxFor(newManifest.toHero.context));
 
       if (newManifest.type == _HeroFlightType.pop)
         _proxyAnimation.parent = new ReverseAnimation(newManifest.animation);
@@ -464,39 +452,38 @@ class HeroController extends NavigatorObserver {
       final PageRoute<dynamic> to = toRoute;
       final Animation<double> animation = (flightType == _HeroFlightType.push) ? to.animation : from.animation;
 
-      // A "user" gesture may have already completed the pop.
+      // A user gesture may have already completed the pop.
       if (flightType == _HeroFlightType.pop && animation.status == AnimationStatus.dismissed)
         return;
 
-      // Putting a route offstage changes its animation value to 1.0.
-      // Once this frame completes, we'll know where the heroes in the toRoute
-      // are going to end up, and the toRoute will go back on stage.
-      to.offstage = animation.value == 0.0 || animation.value == 1.0;
+      // Putting a route offstage changes its animation value to 1.0. Once this
+      // frame completes, we'll know where the heroes in the `to` route are
+      // going to end up, and the `to` route will go back onstage.
+      to.offstage = to.animation.value == 0.0;
 
-      WidgetsBinding.instance.addPostFrameCallback((Duration _) {
+      WidgetsBinding.instance.addPostFrameCallback((Duration value) {
         _startHeroTransition(from, to, flightType);
       });
     }
   }
 
   // Find the matching pairs of heros in from and to and either start or a new
-  // hero flight, or restart an existing one.
+  // hero flight, or divert an existing one.
   void _startHeroTransition(PageRoute<dynamic> from, PageRoute<dynamic> to, _HeroFlightType flightType) {
-    // The navigator or one of the routes subtrees was removed before this
-    // end-of-frame callback was called then don't actually start a transition.
-    // TBD: need to generate tests for these cases
+    // If the navigator or one of the routes subtrees was removed before this
+    // end-of-frame callback was called, then don't actually start a transition.
     if (navigator == null || from.subtreeContext == null || to.subtreeContext == null) {
-      to.offstage = false; // TBD: only do this if to.subtreeContext != null?
+      to.offstage = false; // in case we set this in _maybeStartHeroTransition
       return;
     }
 
-    final Rect navigatorRect = _globalRect(navigator.context);
+    final Rect navigatorRect = _globalBoundingBoxFor(navigator.context);
 
     // At this point the toHeroes may have been built and laid out for the first time.
     final Map<Object, _HeroState> fromHeroes = Hero._allHeroesFor(from.subtreeContext);
     final Map<Object, _HeroState> toHeroes = Hero._allHeroesFor(to.subtreeContext);
 
-    // If the to route was offstage, then we're implicitly restoring its
+    // If the `to` route was offstage, then we're implicitly restoring its
     // animation value back to what it was before it was "moved" offstage.
     to.offstage = false;
 
@@ -512,9 +499,8 @@ class HeroController extends NavigatorObserver {
           toHero: toHeroes[tag],
           createRectTween: createRectTween,
         );
-
         if (_flights[tag] != null)
-            _flights[tag].restart(manifest);
+          _flights[tag].divert(manifest);
         else
           _flights[tag] = new _HeroFlight(_handleFlightEnded)..start(manifest);
       } else if (_flights[tag] != null) {