fix(*): detect external changes in history.state

Previously, `$browser.$$checkUrlChange()` (which was run before each `$digest`)
would only detect an external change (i.e. not via `$location`) to the browser
URL. External changes to `history.state` would not be detected and propagated to
`$location`.

This would not be a problem if changes were followed by a `popstate` or
`hashchange` event (which would call `cacheStateAndFireUrlChange()`). But since
`history.pushState()/replaceState()` do not fire any events, calling these
methods manually would result in `$location` getting out-of-sync with the actual
history state.

This was not detected in tests, because the mocked `window.history` would
incorrectly trigger `popstate` when calling `pushState()/replaceState()`, which
"covered" the bug.

This commit fixes it by always calling `cacheState()`, before looking for and
propagating a URL/state change.

Author: gkalpak

src/ng/browser.js

@@ -96,7 +96,6 @@ function Browser(window, document, $log, $sniffer) {
       };
 
   cacheState();
-  lastHistoryState = cachedState;
 
   /**
    * @name $browser#url
@@ -150,8 +149,6 @@ function Browser(window, document, $log, $sniffer) {
       if ($sniffer.history && (!sameBase || !sameState)) {
         history[replace ? 'replaceState' : 'pushState'](state, '', url);
         cacheState();
-        // Do the assignment again so that those two variables are referentially identical.
-        lastHistoryState = cachedState;
       } else {
         if (!sameBase) {
           pendingLocation = url;
@@ -200,8 +197,7 @@ function Browser(window, document, $log, $sniffer) {
 
   function cacheStateAndFireUrlChange() {
     pendingLocation = null;
-    cacheState();
-    fireUrlChange();
+    fireStateOrUrlChange();
   }
 
   // This variable should be used *only* inside the cacheState function.
@@ -215,11 +211,16 @@ function Browser(window, document, $log, $sniffer) {
     if (equals(cachedState, lastCachedState)) {
       cachedState = lastCachedState;
     }
+
     lastCachedState = cachedState;
+    lastHistoryState = cachedState;
   }
 
-  function fireUrlChange() {
-    if (lastBrowserUrl === self.url() && lastHistoryState === cachedState) {
+  function fireStateOrUrlChange() {
+    var prevLastHistoryState = lastHistoryState;
+    cacheState();
+
+    if (lastBrowserUrl === self.url() && prevLastHistoryState === cachedState) {
       return;
     }
 
@@ -285,7 +286,7 @@ function Browser(window, document, $log, $sniffer) {
    * Needs to be exported to be able to check for changes that have been done in sync,
    * as hashchange/popstate events fire in async.
    */
-  self.$$checkUrlChange = fireUrlChange;
+  self.$$checkUrlChange = fireStateOrUrlChange;
 
   //////////////////////////////////////////////////////////////
   // Misc API

test/ng/locationSpec.js

@@ -1039,11 +1039,21 @@ describe('$location', function() {
     });
 
     it('should update $location when browser state changes', function() {
-      initService({html5Mode:true, supportHistory: true});
-      mockUpBrowser({initialUrl:'http://new.com/a/b/', baseHref:'/a/b/'});
-      inject(function($location, $window) {
+      initService({html5Mode: true, supportHistory: true});
+      mockUpBrowser({initialUrl: 'http://new.com/a/b/', baseHref: '/a/b/'});
+      inject(function($location, $rootScope, $window) {
         $window.history.pushState({b: 3});
+        $rootScope.$digest();
+
         expect($location.state()).toEqual({b: 3});
+
+        $window.history.pushState({b: 4}, null, $window.location.href + 'c?d=e#f');
+        $rootScope.$digest();
+
+        expect($location.path()).toBe('/c');
+        expect($location.search()).toEqual({d: 'e'});
+        expect($location.hash()).toBe('f');
+        expect($location.state()).toEqual({b: 4});
       });
     });
 
@@ -2666,12 +2676,10 @@ describe('$location', function() {
           replaceState: function(state, title, url) {
             win.history.state = copy(state);
             if (url) win.location.href = url;
-            jqLite(win).triggerHandler('popstate');
           },
           pushState: function(state, title, url) {
             win.history.state = copy(state);
             if (url) win.location.href = url;
-            jqLite(win).triggerHandler('popstate');
           }
         };
         win.addEventListener = angular.noop;