Change how navigator prevents redundant operations (#4769)

* Change how navigator prevents redundant operations

Instead of requiring transactions, we now cancel all active pointers that are
interacting with the navigator and absorb future pointers until we get a chance
to build. This approach isn't perfect (e.g., events that trigger off the
cancelled pointers could still interact with the navigator), but it should be
better than the current transaction-based approach.

Fixes #4716

* Remove openTransaction

* test

* fixup

examples/flutter_gallery/lib/demo/full_screen_dialog_demo.dart

@@ -128,10 +128,9 @@ class FullScreenDialogDemoState extends State<FullScreenDialogDemo> {
           new FlatButton(
             child: new Text('DISCARD'),
             onPressed: () {
-              Navigator.openTransaction(context, (NavigatorTransaction transaction) {
-                transaction.pop(DismissDialogAction.discard); // pop the cancel/discard dialog
-                transaction.pop(null); // pop this route
-              });
+              Navigator.of(context)
+                ..pop(DismissDialogAction.discard) // pop the cancel/discard dialog
+                ..pop(); // pop this route
             }
           )
         ]

examples/flutter_gallery/lib/demo/pesto_demo.dart

@@ -158,10 +158,9 @@ class _PestoDemoState extends State<PestoDemo> {
           new DrawerItem(
             child: new Text('Return to Gallery'),
             onPressed: () {
-              Navigator.openTransaction(context, (NavigatorTransaction transaction) {
-                transaction.pop();  // Close the Drawer
-                transaction.pop();  // Go back to the gallery
-              });
+              Navigator.of(context)
+                ..pop()  // Close the drawer.
+                ..pop();  // Go back to the gallery.
             }
           ),
         ]

packages/flutter/lib/src/gestures/binding.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 'dart:collection';
 import 'dart:typed_data';
 import 'dart:ui' as ui show window;
 
@@ -38,8 +40,25 @@ abstract class GestureBinding extends BindingBase implements HitTestable, HitTes
       0
     );
     final PointerPacket packet = PointerPacket.deserialize(message);
-    for (PointerEvent event in PointerEventConverter.expand(packet.pointers))
-      _handlePointerEvent(event);
+    _pendingPointerEvents.addAll(PointerEventConverter.expand(packet.pointers));
+    _flushPointerEventQueue();
+  }
+
+  final Queue<PointerEvent> _pendingPointerEvents = new Queue<PointerEvent>();
+
+  void _flushPointerEventQueue() {
+    while (_pendingPointerEvents.isNotEmpty)
+      _handlePointerEvent(_pendingPointerEvents.removeFirst());
+  }
+
+  /// Dispatch a [PointerCancelEvent] for the given pointer soon.
+  ///
+  /// The pointer event will be dispatch before the next pointer event and
+  /// before the end of the microtask but not within this function call.
+  void cancelPointer(int pointer) {
+    if (_pendingPointerEvents.isEmpty)
+      scheduleMicrotask(_flushPointerEventQueue);
+    _pendingPointerEvents.addFirst(new PointerCancelEvent(pointer: pointer));
   }
 
   /// A router that routes all pointer events received from the engine.
@@ -56,22 +75,21 @@ abstract class GestureBinding extends BindingBase implements HitTestable, HitTes
   Map<int, HitTestResult> _hitTests = <int, HitTestResult>{};
 
   void _handlePointerEvent(PointerEvent event) {
+    HitTestResult result;
     if (event is PointerDownEvent) {
       assert(!_hitTests.containsKey(event.pointer));
-      HitTestResult result = new HitTestResult();
+      result = new HitTestResult();
       hitTest(result, event.position);
       _hitTests[event.pointer] = result;
-    } else if (event is! PointerUpEvent && event is! PointerCancelEvent) {
-      assert(event.down == _hitTests.containsKey(event.pointer));
-      if (!event.down)
-        return; // we currently ignore add, remove, and hover move events
-    }
-    assert(_hitTests[event.pointer] != null);
-    dispatchEvent(event, _hitTests[event.pointer]);
-    if (event is PointerUpEvent || event is PointerCancelEvent) {
-      assert(_hitTests.containsKey(event.pointer));
-      _hitTests.remove(event.pointer);
+    } else if (event is PointerUpEvent || event is PointerCancelEvent) {
+      result = _hitTests.remove(event.pointer);
+    } else if (event.down) {
+      result = _hitTests[event.pointer];
+    } else {
+      return;  // We currently ignore add, remove, and hover move events.
     }
+    if (result != null)
+      dispatchEvent(event, result);
   }
 
   /// Determine which [HitTestTarget] objects are located at a given position.
@@ -160,4 +178,3 @@ class FlutterErrorDetailsForPointerEventDispatcher extends FlutterErrorDetails {
   /// the hitTestEntry object.
   final HitTestEntry hitTestEntry;
 }
-

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

@@ -167,17 +167,15 @@ void showLicensePage({
   ImageProvider applicationIcon,
   String applicationLegalese
 }) {
-  Navigator.openTransaction(context, (NavigatorTransaction transaction) {
   // TODO(ianh): remove pop once https://github.com/flutter/flutter/issues/4667 is fixed
-    transaction.pop();
-    transaction.push(new MaterialPageRoute<Null>(
+  Navigator.pop(context);
+  Navigator.push(context, new MaterialPageRoute<Null>(
     builder: (BuildContext context) => new LicensePage(
       applicationName: applicationName,
       applicationVersion: applicationVersion,
       applicationLegalese: applicationLegalese
     )
   ));
-  });
 }
 
 /// An about box. This is a dialog box with the application's icon, name,

packages/flutter/lib/src/rendering/proxy_box.dart

@@ -1888,12 +1888,12 @@ class RenderRepaintBoundary extends RenderProxyBox {
   }
 }
 
-/// A render object that os invisible during hit testing.
+/// A render object that is invisible during hit testing.
 ///
 /// When [ignoring] is `true`, this render object (and its subtree) is invisible
 /// to hit testing. It still consumes space during layout and paints its child
-/// as usual. It just cannot be the target of located events, because it returns
-/// `false` from [hitTest].
+/// as usual. It just cannot be the target of located events, because its render
+/// object returns `false` from [hitTest].
 ///
 /// When [ignoringSemantics] is `true`, the subtree will be invisible to
 /// the semantics layer (and thus e.g. accessibility tools). If
@@ -1966,6 +1966,43 @@ class RenderIgnorePointer extends RenderProxyBox {
   }
 }
 
+/// A render object that absorbs pointers during hit testing.
+///
+/// When [absorbing] is `true`, this render object prevents its subtree from
+/// receiving pointer events by terminating hit testing at itself. It still
+/// consumes space during layout and paints its child as usual. It just prevents
+/// its children from being the target of located events, because its render
+/// object returns `true` from [hitTest].
+class RenderAbsorbPointer extends RenderProxyBox {
+  /// Creates a render object that absorbs pointers during hit testing.
+  ///
+  /// The [absorbing] argument must not be null.
+  RenderAbsorbPointer({
+    RenderBox child,
+    this.absorbing: true
+  }) : super(child) {
+    assert(absorbing != null);
+  }
+
+  /// Whether this render object absorbs pointers during hit testing.
+  ///
+  /// Regardless of whether this render object absorbs pointers during hit
+  /// testing, it will still consume space during layout and be visible during
+  /// painting.
+  bool absorbing;
+
+  @override
+  bool hitTest(HitTestResult result, { Point position }) {
+    return absorbing ? true : super.hitTest(result, position: position);
+  }
+
+  @override
+  void debugFillDescription(List<String> description) {
+    super.debugFillDescription(description);
+    description.add('absorbing: $absorbing');
+  }
+}
+
 /// Holds opaque meta data in the render tree.
 ///
 /// Useful for decorating the render tree with information that will be consumed

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

@@ -131,11 +131,7 @@ class _WidgetsAppState extends State<WidgetsApp> implements WidgetsBindingObserv
     assert(mounted);
     NavigatorState navigator = _navigator.currentState;
     assert(navigator != null);
-    bool result = false;
-    navigator.openTransaction((NavigatorTransaction transaction) {
-      result = transaction.pop();
-    });
-    return result;
+    return navigator.pop();
   }
 
   @override

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

@@ -2616,6 +2616,41 @@ class IgnorePointer extends SingleChildRenderObjectWidget {
   }
 }
 
+/// A widget that absorbs pointers during hit testing.
+///
+/// When [absorbing] is `true`, this widget prevents its subtree from receiving
+/// pointer events by terminating hit testing at itself. It still consumes space
+/// during layout and paints its child as usual. It just prevents its children
+/// from being the target of located events, because it returns `true` from
+/// [hitTest].
+class AbsorbPointer extends SingleChildRenderObjectWidget {
+  /// Creates a widget that absorbs pointers during hit testing.
+  ///
+  /// The [absorbing] argument must not be null
+  AbsorbPointer({
+    Key key,
+    this.absorbing: true,
+    Widget child
+  }) : super(key: key, child: child) {
+    assert(absorbing != null);
+  }
+
+  /// Whether this widget absorbs pointers during hit testing.
+  ///
+  /// Regardless of whether this render object absorbs pointers during hit
+  /// testing, it will still consume space during layout and be visible during
+  /// painting.
+  final bool absorbing;
+
+  @override
+  RenderAbsorbPointer createRenderObject(BuildContext context) => new RenderAbsorbPointer(absorbing: absorbing);
+
+  @override
+  void updateRenderObject(BuildContext context, RenderAbsorbPointer renderObject) {
+    renderObject.absorbing = absorbing;
+  }
+}
+
 /// Holds opaque meta data in the render tree.
 ///
 /// Useful for decorating the render tree with information that will be consumed

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

@@ -489,6 +489,10 @@ class HeroController extends NavigatorObserver {
   }
 
   void _updateQuest(Duration timeStamp) {
+    if (navigator == null) {
+      // The navigator has been removed for this end-of-frame callback was called.
+      return;
+    }
     Set<Key> mostValuableKeys = _getMostValuableKeys();
     Map<Object, HeroHandle> heroesFrom = _party.isEmpty ?
         Hero.of(_from.subtreeContext, mostValuableKeys) : _party.getHeroesToAnimate();

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

@@ -2,8 +2,11 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+import 'package:flutter/rendering.dart';
 import 'package:meta/meta.dart';
 
+import 'basic.dart';
+import 'binding.dart';
 import 'focus.dart';
 import 'framework.dart';
 import 'overlay.dart';
@@ -145,9 +148,6 @@ class RouteSettings {
 /// Creates a route for the given route settings.
 typedef Route<dynamic> RouteFactory(RouteSettings settings);
 
-/// A callback in during which you can perform a number of navigator operations (e.g., pop, push) that happen atomically.
-typedef void NavigatorTransactionCallback(NavigatorTransaction transaction);
-
 /// An interface for observing the behavior of a [Navigator].
 class NavigatorObserver {
   /// The navigator that the observer is observing, if any.
@@ -210,12 +210,8 @@ 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. The set of
   /// most valuable keys will be used to construct an appropriate [Hero] transition.
-  ///
-  /// Uses [openTransaction()]. Only one transaction will be executed per frame.
   static void pushNamed(BuildContext context, String routeName, { Set<Key> mostValuableKeys }) {
-    openTransaction(context, (NavigatorTransaction transaction) {
-      transaction.pushNamed(routeName, mostValuableKeys: mostValuableKeys);
-    });
+    Navigator.of(context).pushNamed(routeName, mostValuableKeys: mostValuableKeys);
   }
 
   /// Push a route onto the navigator that most tightly encloses the given context.
@@ -224,12 +220,8 @@ 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.
-  ///
-  /// Uses [openTransaction()]. Only one transaction will be executed per frame.
   static void push(BuildContext context, Route<dynamic> route) {
-    openTransaction(context, (NavigatorTransaction transaction) {
-      transaction.push(route);
-    });
+    Navigator.of(context).push(route);
   }
 
   /// Pop a route off the navigator that most tightly encloses the given context.
@@ -247,24 +239,14 @@ class Navigator extends StatefulWidget {
   ///
   /// Returns true if a route was popped; returns false if there are no further
   /// previous routes.
-  ///
-  /// Uses [openTransaction()]. Only one transaction will be executed per frame.
   static bool pop(BuildContext context, [ dynamic result ]) {
-    bool returnValue;
-    openTransaction(context, (NavigatorTransaction transaction) {
-      returnValue = transaction.pop(result);
-    });
-    return returnValue;
+    return Navigator.of(context).pop(result);
   }
 
   /// Calls pop() repeatedly until the given route is the current route.
   /// If it is already the current route, nothing happens.
-  ///
-  /// Uses [openTransaction()]. Only one transaction will be executed per frame.
   static void popUntil(BuildContext context, Route<dynamic> targetRoute) {
-    openTransaction(context, (NavigatorTransaction transaction) {
-      transaction.popUntil(targetRoute);
-    });
+    Navigator.of(context).popUntil(targetRoute);
   }
 
   /// Whether the navigator that most tightly encloses the given context can be popped.
@@ -279,33 +261,24 @@ 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.
-  ///
-  /// Uses [openTransaction()]. Only one transaction will be executed per frame.
   static void popAndPushNamed(BuildContext context, String routeName, { Set<Key> mostValuableKeys }) {
-    openTransaction(context, (NavigatorTransaction transaction) {
-      transaction.pop();
-      transaction.pushNamed(routeName, mostValuableKeys: mostValuableKeys);
-    });
+    Navigator.of(context)
+      ..pop()
+      ..pushNamed(routeName, mostValuableKeys: mostValuableKeys);
   }
 
-  /// Calls callback immediately to create a navigator transaction.
-  ///
-  /// To avoid race conditions, a navigator will execute at most one operation
-  /// per animation frame. If you wish to perform a compound change to the
-  /// navigator's state, you can use a navigator transaction to execute all the
-  /// changes atomically by making the changes inside the given callback.
-  static void openTransaction(BuildContext context, NavigatorTransactionCallback callback) {
+  static NavigatorState of(BuildContext context) {
     NavigatorState navigator = context.ancestorStateOfType(const TypeMatcher<NavigatorState>());
     assert(() {
       if (navigator == null) {
         throw new FlutterError(
-          'openTransaction called with a context that does not include a Navigator.\n'
-          'The context passed to the Navigator.openTransaction() method must be that of a widget that is a descendant of a Navigator widget.'
+          'Navigator operation requested with a context that does not include a Navigator.\n'
+          'The context used to push or pop routes from the Navigator must be that of a widget that is a descendant of a Navigator widget.'
         );
       }
       return true;
     });
-    navigator.openTransaction(callback);
+    return navigator;
   }
 
   @override
@@ -322,7 +295,7 @@ class NavigatorState extends State<Navigator> {
     super.initState();
     assert(config.observer == null || config.observer.navigator == null);
     config.observer?._navigator = this;
-    _push(config.onGenerateRoute(new RouteSettings(
+    push(config.onGenerateRoute(new RouteSettings(
       name: config.initialRoute ?? Navigator.defaultRouteName,
       isInitialRoute: true
     )));
@@ -363,7 +336,7 @@ class NavigatorState extends State<Navigator> {
 
   bool _debugLocked = false; // used to prevent re-entrant calls to push, pop, and friends
 
-  void _pushNamed(String name, { Set<Key> mostValuableKeys }) {
+  void pushNamed(String name, { Set<Key> mostValuableKeys }) {
     assert(!_debugLocked);
     assert(name != null);
     RouteSettings settings = new RouteSettings(
@@ -376,10 +349,10 @@ class NavigatorState extends State<Navigator> {
       route = config.onUnknownRoute(settings);
       assert(route != null);
     }
-    _push(route);
+    push(route);
   }
 
-  void _push(Route<dynamic> route) {
+  void push(Route<dynamic> route) {
     assert(!_debugLocked);
     assert(() { _debugLocked = true; return true; });
     assert(route != null);
@@ -396,9 +369,10 @@ class NavigatorState extends State<Navigator> {
       config.observer?.didPush(route, oldRoute);
     });
     assert(() { _debugLocked = false; return true; });
+    _cancelActivePointers();
   }
 
-  void _replace({ Route<dynamic> oldRoute, Route<dynamic> newRoute }) {
+  void replace({ Route<dynamic> oldRoute, Route<dynamic> newRoute }) {
     assert(!_debugLocked);
     assert(oldRoute != null);
     assert(newRoute != null);
@@ -427,16 +401,17 @@ class NavigatorState extends State<Navigator> {
       oldRoute._navigator = null;
     });
     assert(() { _debugLocked = false; return true; });
+    _cancelActivePointers();
   }
 
-  void _replaceRouteBefore({ Route<dynamic> anchorRoute, Route<dynamic> newRoute }) {
+  void replaceRouteBefore({ Route<dynamic> anchorRoute, Route<dynamic> newRoute }) {
     assert(anchorRoute != null);
     assert(anchorRoute._navigator == this);
     assert(_history.indexOf(anchorRoute) > 0);
-    _replace(oldRoute: _history[_history.indexOf(anchorRoute)-1], newRoute: newRoute);
+    replace(oldRoute: _history[_history.indexOf(anchorRoute)-1], newRoute: newRoute);
   }
 
-  void _removeRouteBefore(Route<dynamic> anchorRoute) {
+  void removeRouteBefore(Route<dynamic> anchorRoute) {
     assert(!_debugLocked);
     assert(() { _debugLocked = true; return true; });
     assert(anchorRoute._navigator == this);
@@ -454,9 +429,10 @@ class NavigatorState extends State<Navigator> {
       targetRoute._navigator = null;
     });
     assert(() { _debugLocked = false; return true; });
+    _cancelActivePointers();
   }
 
-  bool _pop([dynamic result]) {
+  bool pop([dynamic result]) {
     assert(!_debugLocked);
     assert(() { _debugLocked = true; return true; });
     Route<dynamic> route = _history.last;
@@ -483,13 +459,14 @@ class NavigatorState extends State<Navigator> {
       assert(!debugPredictedWouldPop);
     }
     assert(() { _debugLocked = false; return true; });
+    _cancelActivePointers();
     return true;
   }
 
-  void _popUntil(Route<dynamic> targetRoute) {
+  void popUntil(Route<dynamic> targetRoute) {
     assert(_history.contains(targetRoute));
     while (!targetRoute.isCurrent)
-      _pop();
+      pop();
   }
 
   /// Whether this navigator can be popped.
@@ -501,25 +478,23 @@ class NavigatorState extends State<Navigator> {
     return _history.length > 1 || _history[0].willHandlePopInternally;
   }
 
-  bool _hadTransaction = true;
+  final Set<int> _activePointers = new Set<int>();
 
-  /// Calls callback immediately to create a navigator transaction.
-  ///
-  /// To avoid race conditions, a navigator will execute at most one operation
-  /// per animation frame. If you wish to perform a compound change to the
-  /// navigator's state, you can use a navigator transaction to execute all the
-  /// changes atomically by making the changes inside the given callback.
-  bool openTransaction(NavigatorTransactionCallback callback) {
-    assert(callback != null);
-    if (_hadTransaction)
-      return false;
-    _hadTransaction = true;
-    NavigatorTransaction transaction = new NavigatorTransaction._(this);
-    setState(() {
-      callback(transaction);
-    });
-    assert(() { transaction._debugClose(); return true; });
-    return true;
+  void _handlePointerDown(PointerDownEvent event) {
+    _activePointers.add(event.pointer);
+  }
+
+  void _handlePointerUpOrCancel(PointerEvent event) {
+    _activePointers.remove(event.pointer);
+  }
+
+  void _cancelActivePointers() {
+    // This mechanism is far from perfect. See the issue below for more details:
+    // https://github.com/flutter/flutter/issues/4770
+    RenderAbsorbPointer absorber = _overlayKey.currentContext?.ancestorRenderObjectOfType(const TypeMatcher<RenderAbsorbPointer>());
+    absorber?.absorbing = true;
+    for (int pointer in _activePointers.toList())
+      WidgetsBinding.instance.cancelPointer(pointer);
   }
 
   // TODO(abarth): We should be able to take a focusScopeKey as configuration
@@ -530,108 +505,22 @@ class NavigatorState extends State<Navigator> {
   Widget build(BuildContext context) {
     assert(!_debugLocked);
     assert(_history.isNotEmpty);
-    _hadTransaction = false;
     final Route<dynamic> initialRoute = _history.first;
-    return new Focus(
+    return new Listener(
+      onPointerDown: _handlePointerDown,
+      onPointerUp: _handlePointerUpOrCancel,
+      onPointerCancel: _handlePointerUpOrCancel,
+      child: new AbsorbPointer(
+        absorbing: false,
+        child: new Focus(
           key: _focusScopeKey,
           initiallyFocusedScope: initialRoute.focusKey,
           child: new Overlay(
             key: _overlayKey,
             initialEntries: initialRoute.overlayEntries
           )
+        )
+      )
     );
   }
 }
-
-/// A sequence of [Navigator] operations that are executed atomically.
-class NavigatorTransaction {
-  NavigatorTransaction._(this._navigator) {
-    assert(_navigator != null);
-  }
-  NavigatorState _navigator;
-  bool _debugOpen = true;
-
-  /// The route name will be passed to the navigator's [onGenerateRoute]
-  /// callback. The returned route will be pushed into the navigator. The set of
-  /// most valuable keys will be used to construct an appropriate [Hero] transition.
-  void pushNamed(String name, { Set<Key> mostValuableKeys }) {
-    assert(_debugOpen);
-    _navigator._pushNamed(name, mostValuableKeys: mostValuableKeys);
-  }
-
-  /// Adds the given route to the Navigator's history, and transitions to it.
-  /// 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.
-  void push(Route<dynamic> route) {
-    assert(_debugOpen);
-    _navigator._push(route);
-  }
-
-  /// Replaces one given route with another. Calls install(), didReplace(), and
-  /// didChangeNext() on the new route, then dispose() on the old route. The
-  /// navigator is not informed of the replacement.
-  ///
-  /// The old route must have overlay entries, otherwise we won't know where to
-  /// insert the entries of the new route. The old route must not be currently
-  /// visible (i.e. a later route have overlay entries that are currently
-  /// opaque), otherwise the replacement would have a jarring effect.
-  ///
-  /// It is safe to call this redundantly (replacing a route with itself). Such
-  /// calls are ignored.
-  void replace({ Route<dynamic> oldRoute, Route<dynamic> newRoute }) {
-    assert(_debugOpen);
-    _navigator._replace(oldRoute: oldRoute, newRoute: newRoute);
-  }
-
-  /// Like replace(), but affects the route before the given anchorRoute rather
-  /// than the anchorRoute itself.
-  ///
-  /// If newRoute is already the route before anchorRoute, then the call is
-  /// ignored.
-  ///
-  /// The conditions described for [replace()] apply; for instance, the route
-  /// before anchorRoute must have overlay entries.
-  void replaceRouteBefore({ Route<dynamic> anchorRoute, Route<dynamic> newRoute }) {
-    assert(_debugOpen);
-    _navigator._replaceRouteBefore(anchorRoute: anchorRoute, newRoute: newRoute);
-  }
-
-  /// Removes the route prior to the given anchorRoute, and calls didChangeNext
-  /// on the route prior to that one, if any. The observer is not notified.
-  void removeRouteBefore(Route<dynamic> anchorRoute) {
-    assert(_debugOpen);
-    _navigator._removeRouteBefore(anchorRoute);
-  }
-
-  /// Tries to removes the current route, calling its didPop() method. If that
-  /// method returns false, then nothing else happens. Otherwise, the observer
-  /// (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, otherwise
-  /// the route's [Route.currentValue] will be used. 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 true if a route was popped; returns false if there are no further
-  /// previous routes.
-  bool pop([dynamic result]) {
-    assert(_debugOpen);
-    return _navigator._pop(result);
-  }
-
-  /// Calls pop() repeatedly until the given route is the current route.
-  /// If it is already the current route, nothing happens.
-  void popUntil(Route<dynamic> targetRoute) {
-    assert(_debugOpen);
-    _navigator._popUntil(targetRoute);
-  }
-
-  void _debugClose() {
-    assert(_debugOpen);
-    _debugOpen = false;
-  }
-}

packages/flutter/test/gestures/gesture_binding_test.dart

@@ -49,10 +49,34 @@ void main() {
     _binding.callback = (PointerEvent event) => events.add(event);
 
     ui.window.onPointerPacket(encoder.message.buffer);
+    expect(events.length, 2);
     expect(events[0].runtimeType, equals(PointerDownEvent));
     expect(events[1].runtimeType, equals(PointerUpEvent));
   });
 
+  test('Pointer move events', () {
+    mojo_bindings.Encoder encoder = new mojo_bindings.Encoder();
+
+    PointerPacket packet = new PointerPacket();
+    packet.pointers = <Pointer>[new Pointer(), new Pointer(), new Pointer()];
+    packet.pointers[0].type = PointerType.down;
+    packet.pointers[0].kind = PointerKind.touch;
+    packet.pointers[1].type = PointerType.move;
+    packet.pointers[1].kind = PointerKind.touch;
+    packet.pointers[2].type = PointerType.up;
+    packet.pointers[2].kind = PointerKind.touch;
+    packet.encode(encoder);
+
+    List<PointerEvent> events = <PointerEvent>[];
+    _binding.callback = (PointerEvent event) => events.add(event);
+
+    ui.window.onPointerPacket(encoder.message.buffer);
+    expect(events.length, 3);
+    expect(events[0].runtimeType, equals(PointerDownEvent));
+    expect(events[1].runtimeType, equals(PointerMoveEvent));
+    expect(events[2].runtimeType, equals(PointerUpEvent));
+  });
+
   test('Pointer cancel events', () {
     mojo_bindings.Encoder encoder = new mojo_bindings.Encoder();
 
@@ -68,6 +92,31 @@ void main() {
     _binding.callback = (PointerEvent event) => events.add(event);
 
     ui.window.onPointerPacket(encoder.message.buffer);
+    expect(events.length, 2);
+    expect(events[0].runtimeType, equals(PointerDownEvent));
+    expect(events[1].runtimeType, equals(PointerCancelEvent));
+  });
+
+  test('Can cancel pointers', () {
+    mojo_bindings.Encoder encoder = new mojo_bindings.Encoder();
+
+    PointerPacket packet = new PointerPacket();
+    packet.pointers = <Pointer>[new Pointer(), new Pointer()];
+    packet.pointers[0].type = PointerType.down;
+    packet.pointers[0].kind = PointerKind.touch;
+    packet.pointers[1].type = PointerType.up;
+    packet.pointers[1].kind = PointerKind.touch;
+    packet.encode(encoder);
+
+    List<PointerEvent> events = <PointerEvent>[];
+    _binding.callback = (PointerEvent event) {
+      events.add(event);
+      if (event is PointerDownEvent)
+        _binding.cancelPointer(event.pointer);
+    };
+
+    ui.window.onPointerPacket(encoder.message.buffer);
+    expect(events.length, 2);
     expect(events[0].runtimeType, equals(PointerDownEvent));
     expect(events[1].runtimeType, equals(PointerCancelEvent));
   });

packages/flutter/test/material/popup_menu_test.dart

+// Copyright 2016 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/material.dart';
+
+void main() {
+  testWidgets('Navigator.push works within a PopupMenuButton ', (WidgetTester tester) async {
+    await tester.pumpWidget(
+      new MaterialApp(
+        routes: <String, WidgetBuilder> {
+          '/next': (BuildContext context) {
+            return new Text('Next');
+          }
+        },
+        home: new Material(
+          child: new Center(
+            child: new Builder(
+              builder: (BuildContext context) {
+                return new PopupMenuButton<int>(
+                  onSelected: (int value) {
+                    Navigator.pushNamed(context, '/next');
+                  },
+                  itemBuilder: (BuildContext context) {
+                    return <PopupMenuItem<int>>[
+                      new PopupMenuItem<int>(
+                        value: 1,
+                        child: new Text('One')
+                      )
+                    ];
+                  }
+                );
+              }
+            )
+          )
+        )
+      )
+    );
+
+    await tester.tap(find.byType(Builder));
+    await tester.pump();
+    await tester.pump(const Duration(seconds: 1)); // finish the menu animation
+
+    expect(find.text('One'), findsOneWidget);
+    expect(find.text('Next'), findsNothing);
+
+    await tester.tap(find.text('One'));
+
+    await tester.pump();
+    await tester.pump(const Duration(seconds: 1)); // finish the menu animation
+
+    expect(find.text('One'), findsNothing);
+    expect(find.text('Next'), findsOneWidget);
+  });
+}

packages/flutter/test/widget/navigator_test.dart

@@ -56,7 +56,7 @@ class ThirdWidget extends StatelessWidget {
       key: targetKey,
       onTap: () {
         try {
-          Navigator.openTransaction(context, (_) { });
+          Navigator.of(context);
         } catch (e) {
           onException(e);
         }
@@ -98,7 +98,7 @@ void main() {
     expect(find.text('Y'), findsNothing);
   });
 
-  testWidgets('Navigator.openTransaction fails gracefully when not found in context', (WidgetTester tester) async {
+  testWidgets('Navigator.of fails gracefully when not found in context', (WidgetTester tester) async {
     Key targetKey = new Key('foo');
     dynamic exception;
     Widget widget = new ThirdWidget(
@@ -110,7 +110,7 @@ void main() {
     await tester.pumpWidget(widget);
     await tester.tap(find.byKey(targetKey));
     expect(exception, new isInstanceOf<FlutterError>());
-    expect('$exception', startsWith('openTransaction called with a context'));
+    expect('$exception', startsWith('Navigator operation requested with a context'));
   });
 
   testWidgets('Missing settings in onGenerateRoute throws exception', (WidgetTester tester) async {
@@ -124,4 +124,71 @@ void main() {
     Object exception = tester.takeException();
     expect(exception is FlutterError, isTrue);
   });
+
+  testWidgets('Gestures between push and build are ignored', (WidgetTester tester) async {
+    List<String> log = <String>[];
+    final Map<String, WidgetBuilder> routes = <String, WidgetBuilder>{
+      '/': (BuildContext context) {
+        return new Row(
+          children: <Widget>[
+            new GestureDetector(
+              onTap: () {
+                log.add('left');
+                Navigator.pushNamed(context, '/second');
+              },
+              child: new Text('left')
+            ),
+            new GestureDetector(
+              onTap: () { log.add('right'); },
+              child: new Text('right')
+            ),
+          ]
+        );
+      },
+      '/second': (BuildContext context) => new Container(),
+    };
+    await tester.pumpWidget(new MaterialApp(routes: routes));
+    expect(log, isEmpty);
+    await tester.tap(find.text('left'));
+    expect(log, equals(<String>['left']));
+    await tester.tap(find.text('right'));
+    expect(log, equals(<String>['left']));
+  });
+
+  // This test doesn't work because the testing framework uses a fake version of
+  // the pointer event dispatch loop.
+  //
+  // TODO(abarth): Test more of the real code and enable this test.
+  // See https://github.com/flutter/flutter/issues/4771.
+  //
+  // testWidgets('Pending gestures are rejected', (WidgetTester tester) async {
+  //   List<String> log = <String>[];
+  //   final Map<String, WidgetBuilder> routes = <String, WidgetBuilder>{
+  //     '/': (BuildContext context) {
+  //       return new Row(
+  //         children: <Widget>[
+  //           new GestureDetector(
+  //             onTap: () {
+  //               log.add('left');
+  //               Navigator.pushNamed(context, '/second');
+  //             },
+  //             child: new Text('left')
+  //           ),
+  //           new GestureDetector(
+  //             onTap: () { log.add('right'); },
+  //             child: new Text('right')
+  //           ),
+  //         ]
+  //       );
+  //     },
+  //     '/second': (BuildContext context) => new Container(),
+  //   };
+  //   await tester.pumpWidget(new MaterialApp(routes: routes));
+  //   TestGesture gesture = await tester.startGesture(tester.getCenter(find.text('right')), pointer: 23);
+  //   expect(log, isEmpty);
+  //   await tester.tap(find.text('left'));
+  //   expect(log, equals(<String>['left']));
+  //   await gesture.up();
+  //   expect(log, equals(<String>['left']));
+  // });
 }

packages/flutter/test/widget/page_forward_transitions_test.dart

@@ -109,7 +109,7 @@ void main() {
 
     expect(state(), equals('BC')); // transition ->1 is at 1.0
 
-    navigator.openTransaction((NavigatorTransaction transaction) => transaction.pushNamed('/2'));
+    navigator.pushNamed('/2');
     expect(state(), equals('BC')); // transition 1->2 is not yet built
     await tester.pump();
     expect(state(), equals('BCE')); // transition 1->2 is at 0.0
@@ -124,7 +124,7 @@ void main() {
     expect(state(), equals('E')); // transition 1->2 is at 1.0
 
 
-    navigator.openTransaction((NavigatorTransaction transaction) => transaction.pop());
+    navigator.pop();
     expect(state(), equals('E')); // transition 1<-2 is at 1.0, just reversed
     await tester.pump();
     expect(state(), equals('BDE')); // transition 1<-2 is at 1.0
@@ -132,7 +132,7 @@ void main() {
     await tester.pump(kFourTenthsOfTheTransitionDuration);
     expect(state(), equals('BDE')); // transition 1<-2 is at 0.6
 
-    navigator.openTransaction((NavigatorTransaction transaction) => transaction.pushNamed('/3'));
+    navigator.pushNamed('/3');
     expect(state(), equals('BDE')); // transition 1<-2 is at 0.6
     await tester.pump();
     expect(state(), equals('BDEF')); // transition 1<-2 is at 0.6, 1->3 is at 0.0
@@ -143,7 +143,7 @@ void main() {
     await tester.pump(kFourTenthsOfTheTransitionDuration);
     expect(state(), equals('BDF')); // transition 1<-2 is done, 1->3 is at 0.8
 
-    navigator.openTransaction((NavigatorTransaction transaction) => transaction.pop());
+    navigator.pop();
     expect(state(), equals('BDF')); // transition 1<-3 is at 0.8, just reversed
     await tester.pump();
     expect(state(), equals('BDF')); // transition 1<-3 is at 0.8
@@ -154,7 +154,7 @@ void main() {
     await tester.pump(kFourTenthsOfTheTransitionDuration);
     expect(state(), equals('BCF')); // transition 1<-3 is at 0.2
 
-    navigator.openTransaction((NavigatorTransaction transaction) => transaction.pushNamed('/4'));
+    navigator.pushNamed('/4');
     expect(state(), equals('BCF')); // transition 1<-3 is at 0.2, 1->4 is not yet built
     await tester.pump();
     expect(state(), equals('BCFG')); // transition 1<-3 is at 0.2, 1->4 is at 0.0

packages/flutter/test/widget/remember_scroll_position_test.dart

@@ -72,9 +72,7 @@ void main() {
     expect(find.text('16'), findsNothing);
     expect(find.text('100'), findsNothing);
 
-    navigatorKey.currentState.openTransaction(
-      (NavigatorTransaction transaction) => transaction.pushNamed('/second')
-    );
+    navigatorKey.currentState.pushNamed('/second');
     await tester.pump(); // navigating always takes two frames
     await tester.pump(new Duration(seconds: 1));
 
@@ -89,9 +87,7 @@ void main() {
     expect(find.text('10'), findsNothing);
     expect(find.text('100'), findsNothing);
 
-    navigatorKey.currentState.openTransaction(
-      (NavigatorTransaction transaction) => transaction.pop()
-    );
+    navigatorKey.currentState.pop();
     await tester.pump(); // navigating always takes two frames
     await tester.pump(new Duration(seconds: 1));
 

packages/flutter/test/widget/routes_test.dart

@@ -78,11 +78,11 @@ class TestRoute extends Route<String> {
 Future<Null> runNavigatorTest(
   WidgetTester tester,
   NavigatorState host,
-  NavigatorTransactionCallback test,
+  VoidCallback test,
   List<String> expectations
 ) async {
   expect(host, isNotNull);
-  host.openTransaction(test);
+  test();
   expect(results, equals(expectations));
   results.clear();
   await tester.pump();
@@ -99,8 +99,7 @@ void main() {
     await runNavigatorTest(
       tester,
       host,
-      (NavigatorTransaction transaction) {
-      },
+      () { },
       <String>[
         'initial: install',
         'initial: didPush',
@@ -111,9 +110,7 @@ void main() {
     await runNavigatorTest(
       tester,
       host,
-      (NavigatorTransaction transaction) {
-        transaction.push(second = new TestRoute('second'));
-      },
+      () { host.push(second = new TestRoute('second')); },
       <String>[
         'second: install',
         'second: didPush',
@@ -124,9 +121,7 @@ void main() {
     await runNavigatorTest(
       tester,
       host,
-      (NavigatorTransaction transaction) {
-        transaction.push(new TestRoute('third'));
-      },
+      () { host.push(new TestRoute('third')); },
       <String>[
         'third: install',
         'third: didPush',
@@ -137,9 +132,7 @@ void main() {
     await runNavigatorTest(
       tester,
       host,
-      (NavigatorTransaction transaction) {
-        transaction.replace(oldRoute: second, newRoute: new TestRoute('two'));
-      },
+      () { host.replace(oldRoute: second, newRoute: new TestRoute('two')); },
       <String>[
         'two: install',
         'two: didReplace second',
@@ -151,9 +144,7 @@ void main() {
     await runNavigatorTest(
       tester,
       host,
-      (NavigatorTransaction transaction) {
-        transaction.pop('hello');
-      },
+      () { host.pop('hello'); },
       <String>[
         'third: didPop hello',
         'third: dispose',
@@ -163,9 +154,7 @@ void main() {
     await runNavigatorTest(
       tester,
       host,
-      (NavigatorTransaction transaction) {
-        transaction.pop('good bye');
-      },
+      () { host.pop('good bye'); },
       <String>[
         'two: didPop good bye',
         'two: dispose',
@@ -188,8 +177,7 @@ void main() {
     await runNavigatorTest(
       tester,
       host,
-      (NavigatorTransaction transaction) {
-      },
+      () { },
       <String>[
         'first: install',
         'first: didPush',
@@ -200,9 +188,7 @@ void main() {
     await runNavigatorTest(
       tester,
       host,
-      (NavigatorTransaction transaction) {
-        transaction.push(second = new TestRoute('second'));
-      },
+      () { host.push(second = new TestRoute('second')); },
       <String>[
         'second: install',
         'second: didPush',
@@ -213,9 +199,7 @@ void main() {
     await runNavigatorTest(
       tester,
       host,
-      (NavigatorTransaction transaction) {
-        transaction.push(new TestRoute('third'));
-      },
+      () { host.push(new TestRoute('third')); },
       <String>[
         'third: install',
         'third: didPush',
@@ -226,9 +210,7 @@ void main() {
     await runNavigatorTest(
       tester,
       host,
-      (NavigatorTransaction transaction) {
-        transaction.removeRouteBefore(second);
-      },
+      () { host.removeRouteBefore(second); },
       <String>[
         'first: dispose',
       ]
@@ -236,9 +218,7 @@ void main() {
     await runNavigatorTest(
       tester,
       host,
-      (NavigatorTransaction transaction) {
-        transaction.pop('good bye');
-      },
+      () { host.pop('good bye'); },
       <String>[
         'third: didPop good bye',
         'third: dispose',
@@ -248,9 +228,7 @@ void main() {
     await runNavigatorTest(
       tester,
       host,
-      (NavigatorTransaction transaction) {
-        transaction.push(new TestRoute('three'));
-      },
+      () { host.push(new TestRoute('three')); },
       <String>[
         'three: install',
         'three: didPush',
@@ -262,9 +240,7 @@ void main() {
     await runNavigatorTest(
       tester,
       host,
-      (NavigatorTransaction transaction) {
-        transaction.push(four = new TestRoute('four'));
-      },
+      () { host.push(four = new TestRoute('four')); },
       <String>[
         'four: install',
         'four: didPush',
@@ -275,9 +251,7 @@ void main() {
     await runNavigatorTest(
       tester,
       host,
-      (NavigatorTransaction transaction) {
-        transaction.removeRouteBefore(four);
-      },
+      () { host.removeRouteBefore(four); },
       <String>[
         'second: didChangeNext four',
         'three: dispose',
@@ -286,9 +260,7 @@ void main() {
     await runNavigatorTest(
       tester,
       host,
-      (NavigatorTransaction transaction) {
-        transaction.pop('the end');
-      },
+      () { host.pop('the end'); },
       <String>[
         'four: didPop the end',
         'four: dispose',
@@ -311,8 +283,7 @@ void main() {
     await runNavigatorTest(
       tester,
       host,
-      (NavigatorTransaction transaction) {
-      },
+      () { },
       <String>[
         'A: install',
         'A: didPush',
@@ -322,9 +293,7 @@ void main() {
     await runNavigatorTest(
       tester,
       host,
-      (NavigatorTransaction transaction) {
-        transaction.push(new TestRoute('B'));
-      },
+      () { host.push(new TestRoute('B')); },
       <String>[
         'B: install',
         'B: didPush',
@@ -336,9 +305,7 @@ void main() {
     await runNavigatorTest(
       tester,
       host,
-      (NavigatorTransaction transaction) {
-        transaction.push(routeC = new TestRoute('C'));
-      },
+      () { host.push(routeC = new TestRoute('C')); },
       <String>[
         'C: install',
         'C: didPush',
@@ -350,9 +317,7 @@ void main() {
     await runNavigatorTest(
       tester,
       host,
-      (NavigatorTransaction transaction) {
-        transaction.replaceRouteBefore(anchorRoute: routeC, newRoute: routeB = new TestRoute('b'));
-      },
+      () { host.replaceRouteBefore(anchorRoute: routeC, newRoute: routeB = new TestRoute('b')); },
       <String>[
         'b: install',
         'b: didReplace B',
@@ -364,9 +329,7 @@ void main() {
     await runNavigatorTest(
       tester,
       host,
-      (NavigatorTransaction transaction) {
-        transaction.popUntil(routeB);
-      },
+      () { host.popUntil(routeB); },
       <String>[
         'C: didPop null',
         'C: dispose',