Navigator.push and Navigator.pushNamed should return Futures (#6333)

These futures complete when the route is popped off the navigator. This
generalizes and simplifies a mechanism already in place for dialogs and
menus.

Fixes #5283

examples/flutter_gallery/lib/demo/shrine/shrine_home.dart

@@ -296,18 +296,10 @@ class _ShrineHomeState extends State<ShrineHome> {
   static final GlobalKey<ScrollableState> scrollableKey = new GlobalKey<ScrollableState>();
   static final GridDelegate gridDelegate = new ShrineGridDelegate();
 
-  void handleCompletedOrder(Order completedOrder) {
-    assert(completedOrder.product != null);
-    if (completedOrder.quantity == 0)
-      _shoppingCart.remove(completedOrder.product);
-  }
-
-  void showOrderPage(Product product) {
+  Future<Null> showOrderPage(Product product) async {
     final Order order = _shoppingCart[product] ?? new Order(product: product);
-    final Completer<Order> completer = new Completer<Order>();
-    Navigator.push(context, new ShrineOrderRoute(
+    final Order completedOrder = await Navigator.push(context, new ShrineOrderRoute(
       order: order,
-      completer: completer,
       builder: (BuildContext context) {
         return new OrderPage(
           order: order,
@@ -316,7 +308,9 @@ class _ShrineHomeState extends State<ShrineHome> {
         );
       }
     ));
-    completer.future.then(handleCompletedOrder);
+    assert(completedOrder.product != null);
+    if (completedOrder.quantity == 0)
+      _shoppingCart.remove(completedOrder.product);
   }
 
   @override

examples/flutter_gallery/lib/demo/shrine/shrine_order.dart

@@ -2,8 +2,6 @@
 // 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/material.dart';
 
 import '../shrine_demo.dart' show ShrinePageRoute;
@@ -226,9 +224,8 @@ class ShrineOrderRoute extends ShrinePageRoute<Order> {
   ShrineOrderRoute({
     this.order,
     WidgetBuilder builder,
-    Completer<Order> completer,
     RouteSettings settings: const RouteSettings()
-  }) : super(builder: builder, completer: completer, settings: settings) {
+  }) : super(builder: builder, settings: settings) {
     assert(order != null);
   }
 

examples/flutter_gallery/lib/demo/shrine_demo.dart

@@ -2,8 +2,6 @@
 // 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/material.dart';
 
 import 'shrine/shrine_home.dart' show ShrineHome;
@@ -26,9 +24,8 @@ Widget buildShrine(Widget child) {
 class ShrinePageRoute<T> extends MaterialPageRoute<T> {
   ShrinePageRoute({
     WidgetBuilder builder,
-    Completer<T> completer,
     RouteSettings settings: const RouteSettings()
-  }) : super(builder: builder, completer: completer, settings: settings);
+  }) : super(builder: builder, settings: settings);
 
   @override
   Widget buildPage(BuildContext context, Animation<double> animation, Animation<double> forwardAnimation) {

packages/flutter/lib/src/material/bottom_sheet.dart

@@ -204,10 +204,9 @@ class _ModalBottomSheetState<T> extends State<_ModalBottomSheet<T>> {
 
 class _ModalBottomSheetRoute<T> extends PopupRoute<T> {
   _ModalBottomSheetRoute({
-    Completer<T> completer,
     this.builder,
     this.theme,
-  }) : super(completer: completer);
+  });
 
   final WidgetBuilder builder;
   final ThemeData theme;
@@ -260,11 +259,8 @@ class _ModalBottomSheetRoute<T> extends PopupRoute<T> {
 Future<dynamic/*=T*/> showModalBottomSheet/*<T>*/({ BuildContext context, WidgetBuilder builder }) {
   assert(context != null);
   assert(builder != null);
-  final Completer<dynamic/*=T*/> completer = new Completer<dynamic/*=T*/>();
-  Navigator.push(context, new _ModalBottomSheetRoute<dynamic/*=T*/>(
-    completer: completer,
+  return Navigator.push(context, new _ModalBottomSheetRoute<dynamic/*=T*/>(
     builder: builder,
     theme: Theme.of(context, shadowThemeOnly: true),
   ));
-  return completer.future;
 }

packages/flutter/lib/src/material/dialog.dart

@@ -278,10 +278,9 @@ class SimpleDialog extends StatelessWidget {
 
 class _DialogRoute<T> extends PopupRoute<T> {
   _DialogRoute({
-    Completer<T> completer,
     this.child,
     this.theme,
-  }) : super(completer: completer);
+  });
 
   final Widget child;
   final ThemeData theme;
@@ -324,11 +323,8 @@ class _DialogRoute<T> extends PopupRoute<T> {
 ///  * [Dialog]
 ///  * <https://www.google.com/design/spec/components/dialogs.html>
 Future<dynamic/*=T*/> showDialog/*<T>*/({ BuildContext context, Widget child }) {
-  Completer<dynamic/*=T*/> completer = new Completer<dynamic/*=T*/>();
-  Navigator.push(context, new _DialogRoute<dynamic/*=T*/>(
-    completer: completer,
+  return Navigator.push(context, new _DialogRoute<dynamic/*=T*/>(
     child: child,
     theme: Theme.of(context, shadowThemeOnly: true),
   ));
-  return completer.future;
 }

packages/flutter/lib/src/material/drop_down.dart

@@ -288,14 +288,13 @@ class _DropdownRouteResult<T> {
 
 class _DropdownRoute<T> extends PopupRoute<_DropdownRouteResult<T>> {
   _DropdownRoute({
-    Completer<_DropdownRouteResult<T>> completer,
     this.items,
     this.buttonRect,
     this.selectedIndex,
     this.elevation: 8,
     this.theme,
     TextStyle style,
-  }) : _style = style, super(completer: completer) {
+  }) : _style = style {
     assert(style != null);
   }
 
@@ -502,7 +501,6 @@ class _DropdownButtonState<T> extends State<DropdownButton<T>> {
     final Rect itemRect = itemBox.localToGlobal(Point.origin) & itemBox.size;
     final Completer<_DropdownRouteResult<T>> completer = new Completer<_DropdownRouteResult<T>>();
     _currentRoute = new _DropdownRoute<T>(
-      completer: completer,
       items: config.items,
       buttonRect: _kMenuHorizontalPadding.inflateRect(itemRect),
       selectedIndex: _selectedIndex,

packages/flutter/lib/src/material/page.dart

@@ -2,8 +2,6 @@
 // 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/widgets.dart';
 import 'material.dart';
 import 'theme.dart';
@@ -167,10 +165,9 @@ class MaterialPageRoute<T> extends PageRoute<T> {
   /// Creates a page route for use in a material design app.
   MaterialPageRoute({
     this.builder,
-    Completer<T> completer,
     RouteSettings settings: const RouteSettings(),
     this.maintainState: true,
-  }) : super(completer: completer, settings: settings) {
+  }) : super(settings: settings) {
     assert(builder != null);
     assert(opaque);
   }

packages/flutter/lib/src/material/popup_menu.dart

@@ -370,13 +370,12 @@ class _PopupMenuRouteLayout extends SingleChildLayoutDelegate {
 
 class _PopupMenuRoute<T> extends PopupRoute<T> {
   _PopupMenuRoute({
-    Completer<T> completer,
     this.position,
     this.items,
     this.initialValue,
     this.elevation,
     this.theme
-  }) : super(completer: completer);
+  });
 
   final RelativeRect position;
   final List<PopupMenuEntry<T>> items;
@@ -439,16 +438,13 @@ Future<dynamic/*=T*/> showMenu/*<T>*/({
 }) {
   assert(context != null);
   assert(items != null && items.length > 0);
-  Completer<dynamic/*=T*/> completer = new Completer<dynamic/*=T*/>();
-  Navigator.push(context, new _PopupMenuRoute<dynamic/*=T*/>(
-    completer: completer,
+  return Navigator.push(context, new _PopupMenuRoute<dynamic/*=T*/>(
     position: position,
     items: items,
     initialValue: initialValue,
     elevation: elevation,
     theme: Theme.of(context, shadowThemeOnly: true),
   ));
-  return completer.future;
 }
 
 /// A callback that is passed the value of the PopupMenuItem that caused

packages/flutter/lib/src/widgets/navigator.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/rendering.dart';
 import 'package:meta/meta.dart';
 
@@ -32,6 +34,12 @@ abstract class Route<T> {
   /// will initialize its [Focus] to this key.
   GlobalKey get focusKey => null;
 
+  /// A future that completes when this route is popped off the navigator.
+  ///
+  /// The future completes with the value given to [Navigator.pop], if any.
+  Future<T> get popped => _popCompleter.future;
+  final Completer<T> _popCompleter = new Completer<T>();
+
   /// Called when the route is inserted into the navigator.
   ///
   /// Use this to populate overlayEntries and add them to the overlay
@@ -40,9 +48,13 @@ abstract class Route<T> {
   /// responsible for _removing_ the entries and this way it's symmetric.)
   ///
   /// The overlay argument will be null if this is the first route inserted.
+  @protected
+  @mustCallSuper
   void install(OverlayEntry insertionPoint) { }
 
   /// Called after install() when the route is pushed onto the navigator.
+  @protected
+  @mustCallSuper
   void didPush() { }
 
   /// When this route is popped (see [Navigator.pop]) if the result isn't
@@ -50,6 +62,8 @@ abstract class Route<T> {
   T get currentResult => null;
 
   /// Called after install() when the route replaced another in the navigator.
+  @protected
+  @mustCallSuper
   void didReplace(Route<dynamic> oldRoute) { }
 
   /// A request was made to pop this route. If the route can handle it
@@ -59,19 +73,28 @@ abstract class Route<T> {
   ///
   /// If this is called, the Navigator will not call dispose(). It is the
   /// responsibility of the Route to later call dispose().
-  bool didPop(T result) => true;
+  @protected
+  @mustCallSuper
+  bool didPop(T result) {
+    _popCompleter.complete(result);
+    return true;
+  }
 
   /// Whether calling didPop() would return false.
   bool get willHandlePopInternally => false;
 
   /// The given route, which came after this one, has been popped off the
   /// navigator.
+  @protected
+  @mustCallSuper
   void didPopNext(Route<dynamic> nextRoute) { }
 
   /// This route's next route has changed to the given new route. This is called
   /// on a route whenever the next route changes for any reason, except for
   /// cases when didPopNext() would be called, so long as it is in the history.
   /// nextRoute will be null if there's no next route.
+  @protected
+  @mustCallSuper
   void didChangeNext(Route<dynamic> nextRoute) { }
 
   /// The route should remove its overlays and free any other resources.
@@ -79,6 +102,7 @@ abstract class Route<T> {
   /// A call to didPop() implies that the Route should call dispose() itself,
   /// but it is possible for dispose() to be called directly (e.g. if the route
   /// is replaced, or if the navigator itself is disposed).
+  @mustCallSuper
   void dispose() { }
 
   /// If the route's transition can be popped via a user gesture (e.g. the iOS
@@ -248,8 +272,10 @@ class Navigator extends StatefulWidget {
   ///
   /// The route name will be passed to that navigator's [onGenerateRoute]
   /// callback. The returned route will be pushed into the navigator.
-  static void pushNamed(BuildContext context, String routeName) {
-    Navigator.of(context).pushNamed(routeName);
+  ///
+  /// Returns a Future that completes when the pushed route is popped.
+  static Future<dynamic> pushNamed(BuildContext context, String routeName) {
+    return Navigator.of(context).pushNamed(routeName);
   }
 
   /// Push a route onto the navigator that most tightly encloses the given context.
@@ -258,8 +284,10 @@ class Navigator extends StatefulWidget {
   /// The route will have didPush() and didChangeNext() called on it; the
   /// previous route, if any, will have didChangeNext() called on it; and the
   /// Navigator observer, if any, will have didPush() called on it.
-  static void push(BuildContext context, Route<dynamic> route) {
-    Navigator.of(context).push(route);
+  ///
+  /// Returns a Future that completes when the pushed route is popped.
+  static Future<dynamic> push(BuildContext context, Route<dynamic> route) {
+    return Navigator.of(context).push(route);
   }
 
   /// Pop a route off the navigator that most tightly encloses the given context.
@@ -269,10 +297,10 @@ class Navigator extends StatefulWidget {
   /// (if any) is notified using its didPop() method, and the previous route is
   /// notified using [Route.didChangeNext].
   ///
-  /// If non-null, [result] will be used as the result of the route. Routes
+  /// If non-null, `result` will be used as the result of the route. Routes
   /// such as dialogs or popup menus typically use this mechanism to return the
   /// value selected by the user to the widget that created their route. The
-  /// type of [result], if provided, must match the type argument of the class
+  /// type of `result`, if provided, must match the type argument of the class
   /// of the current route. (In practice, this is usually "dynamic".)
   ///
   /// Returns true if a route was popped; returns false if there are no further
@@ -299,11 +327,20 @@ class Navigator extends StatefulWidget {
   }
 
   /// Executes a simple transaction that both pops the current route off and
-  /// pushes a named route into the navigator that most tightly encloses the given context.
-  static void popAndPushNamed(BuildContext context, String routeName) {
-    Navigator.of(context)
-      ..pop()
-      ..pushNamed(routeName);
+  /// pushes a named route into the navigator that most tightly encloses the
+  /// given context.
+  ///
+  /// If non-null, `result` will be used as the result of the route that is
+  /// popped. Routes such as dialogs or popup menus typically use this mechanism
+  /// to return the value selected by the user to the widget that created their
+  /// route. The type of `result`, if provided, must match the type argument of
+  /// the class of the current route. (In practice, this is usually "dynamic".)
+  ///
+  /// Returns a Future that completes when the pushed route is popped.
+  static Future<dynamic> popAndPushNamed(BuildContext context, String routeName, { dynamic result }) {
+    NavigatorState navigator = Navigator.of(context);
+    navigator.pop(result);
+    return navigator.pushNamed(routeName);
   }
 
   /// The state from the closest instance of this class that encloses the given context.
@@ -378,7 +415,9 @@ class NavigatorState extends State<Navigator> with TickerProviderStateMixin {
 
   /// Looks up the route with the given name using [Navigator.onGenerateRoute],
   /// and then [push]es that route.
-  void pushNamed(String name) {
+  ///
+  /// Returns a Future that completes when the pushed route is popped.
+  Future<dynamic> pushNamed(String name) {
     assert(!_debugLocked);
     assert(name != null);
     RouteSettings settings = new RouteSettings(name: name);
@@ -388,7 +427,7 @@ class NavigatorState extends State<Navigator> with TickerProviderStateMixin {
       route = config.onUnknownRoute(settings);
       assert(route != null);
     }
-    push(route);
+    return push(route);
   }
 
   /// Adds the given route to the navigator's history, and transitions to it.
@@ -400,7 +439,9 @@ class NavigatorState extends State<Navigator> with TickerProviderStateMixin {
   ///
   /// Ongoing gestures within the current route are canceled when a new route is
   /// pushed.
-  void push(Route<dynamic> route) {
+  ///
+  /// Returns a Future that completes when the pushed route is popped.
+  Future<dynamic> push(Route<dynamic> route) {
     assert(!_debugLocked);
     assert(() { _debugLocked = true; return true; });
     assert(route != null);
@@ -418,6 +459,7 @@ class NavigatorState extends State<Navigator> with TickerProviderStateMixin {
     });
     assert(() { _debugLocked = false; return true; });
     _cancelActivePointers();
+    return route.popped;
   }
 
   /// Replaces a route that is not currently visible with a new route.

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

@@ -2,8 +2,6 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-import 'dart:async';
-
 import 'basic.dart';
 import 'navigator.dart';
 import 'overlay.dart';
@@ -13,9 +11,8 @@ import 'routes.dart';
 abstract class PageRoute<T> extends ModalRoute<T> {
   /// Creates a modal route that replaces the entire screen.
   PageRoute({
-    Completer<T> completer,
     RouteSettings settings: const RouteSettings()
-  }) : super(completer: completer, settings: settings);
+  }) : super(settings: settings);
 
   @override
   bool get opaque => true;

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

@@ -32,21 +32,23 @@ abstract class OverlayRoute<T> extends Route<T> {
     assert(_overlayEntries.isEmpty);
     _overlayEntries.addAll(createOverlayEntries());
     navigator.overlay?.insertAll(_overlayEntries, above: insertionPoint);
+    super.install(insertionPoint);
   }
 
-  /// A request was made to pop this route. If the route can handle it
-  /// internally (e.g. because it has its own stack of internal state) then
-  /// return false, otherwise return true. Returning false will prevent the
-  /// default behavior of NavigatorState.pop().
-  ///
-  /// If this is called, the Navigator will not call dispose(). It is the
-  /// responsibility of the Route to later call dispose().
+  /// Controls whether [didPop] calls [finished].
   ///
-  /// Subclasses shouldn't call this if they want to delay the finished() call.
+  /// If true, this route removes its overlay entries during [didPop].
+  /// Subclasses can override this getter if they want to delay the [finished]
+  /// call (for example to animate the route's exit before removing it from the
+  /// overlay).
+  @protected
+  bool get finishedWhenPopped => true;
+
   @override
   bool didPop(T result) {
-    finished();
-    return true;
+    if (finishedWhenPopped)
+      finished();
+    return super.didPop(result);
   }
 
   /// Clears out the overlay entries.
@@ -56,6 +58,7 @@ abstract class OverlayRoute<T> extends Route<T> {
   /// overlay removal.
   ///
   /// Do not call this method outside of this context.
+  @protected
   void finished() {
     for (OverlayEntry entry in _overlayEntries)
       entry.remove();
@@ -65,6 +68,7 @@ abstract class OverlayRoute<T> extends Route<T> {
   @override
   void dispose() {
     finished();
+    super.dispose();
   }
 }
 
@@ -72,26 +76,20 @@ abstract class OverlayRoute<T> extends Route<T> {
 abstract class TransitionRoute<T> extends OverlayRoute<T> {
   /// Creates a route with entrance and exit transitions.
   TransitionRoute({
-    Completer<T> popCompleter,
     Completer<T> transitionCompleter
-  }) : _popCompleter = popCompleter,
-       _transitionCompleter = transitionCompleter;
+  }) : _transitionCompleter = transitionCompleter;
 
   /// The same as the default constructor but callable with mixins.
   TransitionRoute.explicit(
-    Completer<T> popCompleter,
     Completer<T> transitionCompleter
-  ) : this(popCompleter: popCompleter, transitionCompleter: transitionCompleter);
-
-  /// This future completes once the animation has been dismissed. For
-  /// ModalRoutes, this will be after the completer that's passed in, since that
-  /// one completes before the animation even starts, as soon as the route is
-  /// popped.
-  Future<T> get popped => _popCompleter?.future;
-  final Completer<T> _popCompleter;
+  ) : this(transitionCompleter: transitionCompleter);
 
   /// This future completes only once the transition itself has finished, after
   /// the overlay entries have been removed from the navigator's overlay.
+  ///
+  /// This future completes once the animation has been dismissed. That will be
+  /// after [popped], because [popped] completes before the animation even
+  /// starts, as soon as the route is popped.
   Future<T> get completed => _transitionCompleter?.future;
   final Completer<T> _transitionCompleter;
 
@@ -104,6 +102,9 @@ abstract class TransitionRoute<T> extends OverlayRoute<T> {
   /// the opaque route will not be built to save resources.
   bool get opaque;
 
+  @override
+  bool get finishedWhenPopped => false;
+
   /// The animation that drives the route's transition and the previous route's
   /// forward transition.
   Animation<double> get animation => _animation;
@@ -192,8 +193,7 @@ abstract class TransitionRoute<T> extends OverlayRoute<T> {
   bool didPop(T result) {
     _result = result;
     _controller.reverse();
-    _popCompleter?.complete(_result);
-    return true;
+    return super.didPop(result);
   }
 
   @override
@@ -455,9 +455,8 @@ class _ModalScopeState extends State<_ModalScope> {
 abstract class ModalRoute<T> extends TransitionRoute<T> with LocalHistoryRoute<T> {
   /// Creates a route that blocks interaction with previous routes.
   ModalRoute({
-    Completer<T> completer,
     this.settings: const RouteSettings()
-  }) : super.explicit(completer, null);
+  }) : super.explicit(null);
 
   // The API for general users of this class
 
@@ -678,9 +677,6 @@ abstract class ModalRoute<T> extends TransitionRoute<T> with LocalHistoryRoute<T
 
 /// A modal route that overlays a widget over the current route.
 abstract class PopupRoute<T> extends ModalRoute<T> {
-  /// Creates a modal route that overlays a widget over the current route.
-  PopupRoute({ Completer<T> completer }) : super(completer: completer);
-
   @override
   bool get opaque => false;
 

packages/flutter/test/widget/routes_test.dart

@@ -35,16 +35,19 @@ class TestRoute extends LocalHistoryRoute<String> {
     _entries.add(entry);
     navigator.overlay?.insert(entry, above: insertionPoint);
     routes.add(this);
+    super.install(insertionPoint);
   }
 
   @override
   void didPush() {
     log('didPush');
+    super.didPush();
   }
 
   @override
   void didReplace(@checked TestRoute oldRoute) {
     log('didReplace ${oldRoute.name}');
+    super.didReplace(oldRoute);
   }
 
   @override
@@ -59,11 +62,13 @@ class TestRoute extends LocalHistoryRoute<String> {
   @override
   void didPopNext(@checked TestRoute nextRoute) {
     log('didPopNext ${nextRoute.name}');
+    super.didPopNext(nextRoute);
   }
 
   @override
   void didChangeNext(@checked TestRoute nextRoute) {
     log('didChangeNext ${nextRoute?.name}');
+    super.didChangeNext(nextRoute);
   }
 
   @override
@@ -72,6 +77,7 @@ class TestRoute extends LocalHistoryRoute<String> {
     _entries.forEach((OverlayEntry entry) { entry.remove(); });
     _entries.clear();
     routes.remove(this);
+    super.dispose();
   }
 
 }