Bugfix for router preleave and browser back/foward interaction

lib/client.dart

@@ -450,6 +450,7 @@ class Router {
   final bool sortRoutes;
   bool _listen = false;
   WindowClickHandler _clickHandler;
+  int _historyLength = 0;
 
   /**
    * [useFragment] determines whether this Router uses pure paths with
@@ -472,6 +473,9 @@ class Router {
             : useFragment,
         _window = (windowImpl == null) ? window : windowImpl,
         root = new RouteImpl._new() {
+    if (_window != null && _window.history != null) {
+      _historyLength = _window.history.length;
+    }
     if (clickHandler == null) {
       if (linkMatcher == null) {
         linkMatcher = new DefaultRouterLinkMatcher();
@@ -788,6 +792,24 @@ class Router {
         [kvPair.substring(0, splitPoint), kvPair.substring(splitPoint + 1)];
   }
 
+  void _handleRouteResponse(bool allowed) {
+    int newHistoryLength = _window.history.length;
+    // if not allowed, we need to restore the browser location
+    if (!allowed) {
+      if (_historyLength < 50) {
+        if (newHistoryLength > _historyLength) {
+          _window.history.back();
+        } else {
+          _window.history.forward();
+        }
+      } else {
+        window.location.replace(_oldLocation);
+      }
+    }
+    _historyLength = _window.history.length;
+    _oldLocation = window.location.href;
+  }
+
   /**
    * Listens for window history events and invokes the router. On older
    * browsers the hashChange event is used instead.
@@ -799,26 +821,16 @@ class Router {
     }
     _listen = true;
     if (_useFragment) {
+
       _window.onHashChange.listen((_) {
-        route(_normalizeHash(_window.location.hash)).then((allowed) {
-          // if not allowed, we need to restore the browser location
-          if (!allowed) {
-            _window.history.back();
-          }
-        });
+        route(_normalizeHash(_window.location.hash)).then(_handleRouteResponse);
       });
       route(_normalizeHash(_window.location.hash));
     } else {
       String getPath() =>
           '${_window.location.pathname}${_window.location.hash}';
-
       _window.onPopState.listen((_) {
-        route(getPath()).then((allowed) {
-          // if not allowed, we need to restore the browser location
-          if (!allowed) {
-            _window.history.back();
-          }
-        });
+        route(getPath()).then(_handleRouteResponse);
       });
       route(getPath());
     }

test/client_test.dart

@@ -602,7 +602,9 @@ main() {
         });
       });
     });
+  });
 
+  group('preLeave', () {
     void _testAllowLeave(bool allowLeave) {
       var completer = new Completer<bool>();
       bool barEntered = false;
@@ -635,6 +637,190 @@ main() {
     test('should veto navigation', () {
       _testAllowLeave(false);
     });
+
+    MockWindow mockWindow;
+    StreamController<Event> hashChangeController;
+    StreamController<PopStateEvent> popStateEventController;
+    Router router;
+    int historyCount;
+
+    void _setUpPreleave({bool useFragment}) {
+      Completer<bool> completer = new Completer<bool>();
+      completer.complete(false);
+      historyCount = 0;
+
+      mockWindow = new MockWindow();
+      hashChangeController = new StreamController<Event>.broadcast(onListen: () {},
+          onCancel:() {}, sync:false);
+
+      popStateEventController = new StreamController<PopStateEvent>.broadcast(onListen: () {},
+          onCancel:() {}, sync:false);
+
+      mockWindow.when(callsTo('get onHashChange'))
+          .alwaysReturn(hashChangeController.stream);
+
+      mockWindow.when(callsTo('get onPopState'))
+          .alwaysReturn(popStateEventController.stream);
+
+      mockWindow.history.when(callsTo('get length')).alwaysCall(() => historyCount);
+
+      mockWindow.location.when(callsTo('get hash')).alwaysReturn('#/foo');
+      router = new Router(useFragment: true, windowImpl: mockWindow);
+      router.root
+        ..addRoute(name: 'foo', path: '/foo',
+            mount: (Route child) => child
+              ..addRoute(name: 'noLeave', path: '/noLeave',
+                  enter: (RouteEnterEvent e) {},
+                  preLeave: (RoutePreLeaveEvent e) => e.allowLeave(completer.future))
+              ..addRoute(name: 'freeLeave1', path: '/freeLeave1',
+                  enter: (RouteEnterEvent e) {}));
+      router.listen(ignoreClick: true);
+    }
+
+    void changeWindowLocation() {
+      historyCount++;
+      hashChangeController.add(new Event.eventType('KeyboardEvent', 'keyup'));
+    }
+
+    void windowBackAction() {
+      historyCount--;
+      hashChangeController.add(new Event.eventType('KeyboardEvent', 'keyup'));
+    }
+
+    void windowForwardAction() {
+      historyCount++;
+      hashChangeController.add(new Event.eventType('KeyboardEvent', 'keyup'));
+    }
+
+    void expectBack(int count) {
+      mockWindow.history.getLogs(callsTo('back'))
+          .verify(happenedExactly(count));
+    }
+
+    void expectForward(int count) {
+      mockWindow.history.getLogs(callsTo('forward'))
+          .verify(happenedExactly(count));
+    }
+
+    test('should handle location change cancel with useFragment', () {
+      _setUpPreleave(useFragment:true);
+
+      router.route('/foo/noLeave').then(expectAsync((_) {
+        expectBack(0);
+        expectForward(0);
+        changeWindowLocation();
+
+        hashChangeController.close().then(expectAsync((_) {
+          expectBack(1);
+        }));
+      }));
+    });
+
+    test('should handle history.back cancel with useFragment', () {
+      _setUpPreleave(useFragment:true);
+
+      router.route('/foo/freeLeave1').then(expectAsync((_) {
+        expectBack(0);
+        expectForward(0);
+        changeWindowLocation();
+
+        router.route('/foo/noLeave').then(expectAsync((_) {
+          expectBack(0);
+          expectForward(0);
+          windowBackAction();
+          hashChangeController.close().then(expectAsync((_) {
+            expectBack(0);
+            expectForward(1);
+          }));
+        }));
+      }));
+    });
+
+    test('should handle history.forward cancel with useFragment', () {
+      _setUpPreleave(useFragment:true);
+
+      router.route('/foo/freeLeave1').then(expectAsync((_) {
+        expectBack(0);
+        expectForward(0);
+        changeWindowLocation();
+
+        router.route('/foo/freeLeave1').then(expectAsync((_) {
+          expectBack(0);
+          expectForward(0);
+          windowBackAction();
+
+          router.route('/foo/noLeave').then(expectAsync((_) {
+            expectBack(0);
+            expectForward(0);
+            windowForwardAction();
+            hashChangeController.close().then(expectAsync((_) {
+              expectBack(1);
+              expectForward(0);
+            }));
+          }));
+        }));
+      }));
+    });
+
+    test('should handle location change cancel without useFragment', () {
+      _setUpPreleave(useFragment:false);
+
+      router.route('/foo/noLeave').then(expectAsync((_) {
+        expectBack(0);
+        expectForward(0);
+        changeWindowLocation();
+
+        hashChangeController.close().then(expectAsync((_) {
+          expectBack(1);
+        }));
+      }));
+    });
+
+    test('should handle history.back cancel without useFragment', () {
+      _setUpPreleave(useFragment:false);
+
+      router.route('/foo/freeLeave1').then(expectAsync((_) {
+        expectBack(0);
+        expectForward(0);
+        changeWindowLocation();
+
+        router.route('/foo/noLeave').then(expectAsync((_) {
+          expectBack(0);
+          expectForward(0);
+          windowBackAction();
+          hashChangeController.close().then(expectAsync((_) {
+            expectBack(0);
+            expectForward(1);
+          }));
+        }));
+      }));
+    });
+
+    test('should handle history.forward cancel without useFragment', () {
+      _setUpPreleave(useFragment:false);
+
+      router.route('/foo/freeLeave1').then(expectAsync((_) {
+        expectBack(0);
+        expectForward(0);
+        changeWindowLocation();
+
+        router.route('/foo/freeLeave1').then(expectAsync((_) {
+          expectBack(0);
+          expectForward(0);
+          windowBackAction();
+
+          router.route('/foo/noLeave').then(expectAsync((_) {
+            expectBack(0);
+            expectForward(0);
+            windowForwardAction();
+            hashChangeController.close().then(expectAsync((_) {
+              expectBack(1);
+              expectForward(0);
+            }));
+          }));
+        }));
+      }));
+    });
   });
 
   group('preEnter', () {
@@ -1788,3 +1974,4 @@ main() {
 
 /// An alias for Router.root.findRoute(path)
 r(Router router, String path) => router.root.findRoute(path);
+