fix(common): update $locationShim to notify onChange listeners before emitting AngularJS events (#32037)

The $locationShim has onChange listeners to allow for synchronization logic between
AngularJS and Angular. When the AngularJS routing events are emitted first, this can
cause Angular code to be out of sync. Notifying the listeners earlier solves the
problem.

PR Close #32037

packages/common/upgrade/src/location_shim.ts

@@ -144,6 +144,7 @@ export class $locationShim {
         this.$$parse(oldUrl);
         this.state(oldState);
         this.setBrowserUrlWithFallback(oldUrl, false, oldState);
+        this.$$notifyChangeListeners(this.url(), this.$$state, oldUrl, oldState);
       } else {
         this.initalizing = false;
         $rootScope.$broadcast('$locationChangeSuccess', newUrl, oldUrl, newState, oldState);
@@ -199,6 +200,9 @@ export class $locationShim {
               }
               $rootScope.$broadcast(
                   '$locationChangeSuccess', newUrl, oldUrl, this.$$state, oldState);
+              if (urlOrStateChanged) {
+                this.$$notifyChangeListeners(this.url(), this.$$state, oldUrl, oldState);
+              }
             }
           });
         }
@@ -415,7 +419,6 @@ export class $locationShim {
       // state object; this makes possible quick checking if the state changed in the digest
       // loop. Checking deep equality would be too expensive.
       this.$$state = this.browserState();
-      this.$$notifyChangeListeners(url, state, oldUrl, oldState);
     } catch (e) {
       // Restore old values if pushState fails
       this.url(oldUrl);

packages/common/upgrade/test/upgrade.spec.ts

@@ -628,6 +628,7 @@ describe('$location.onChange()', () => {
 
   let $location: $locationShim;
   let upgradeModule: UpgradeModule;
+  let mock$rootScope: $rootScopeMock;
 
   beforeEach(() => {
     TestBed.configureTestingModule({
@@ -640,6 +641,7 @@ describe('$location.onChange()', () => {
 
     upgradeModule = TestBed.get(UpgradeModule);
     upgradeModule.$injector = {get: injectorFactory()};
+    mock$rootScope = upgradeModule.$injector.get('$rootScope');
   });
 
   beforeEach(inject([$locationShim], (loc: $locationShim) => { $location = loc; }));
@@ -674,17 +676,44 @@ describe('$location.onChange()', () => {
 
     $location.onChange(changeListener);
 
-    // Mock out setting browserUrl
-    ($location as any).browserUrl = (url: string, replace: boolean, state: unknown) => {};
-
     const newState = {foo: 'bar'};
-    ($location as any).setBrowserUrlWithFallback('/newUrl', false, newState);
+    $location.state(newState);
+    $location.path('/newUrl');
+    mock$rootScope.runWatchers();
+
     expect(onChangeVals.url).toBe('/newUrl');
-    expect(onChangeVals.state).toBe(newState);
-    expect(onChangeVals.oldUrl).toBe('/');
+    expect(onChangeVals.state).toEqual(newState);
+    expect(onChangeVals.oldUrl).toBe('http://host.com');
     expect(onChangeVals.oldState).toBe(null);
   });
 
+  it('should call changeListeners after $locationChangeSuccess', () => {
+
+    let changeListenerCalled = false;
+    let locationChangeSuccessEmitted = false;
+
+    function changeListener(url: string, state: unknown, oldUrl: string, oldState: unknown) {
+      changeListenerCalled = true;
+    }
+
+    $location.onChange(changeListener);
+
+    mock$rootScope.$on('$locationChangeSuccess', () => {
+      // Ensure that the changeListener hasn't been called yet
+      expect(changeListenerCalled).toBe(false);
+      locationChangeSuccessEmitted = true;
+    });
+
+    // Update state and run watchers
+    const stateValue = {foo: 'bar'};
+    $location.state(stateValue);
+    mock$rootScope.runWatchers();
+
+    // Ensure that change listeners are called and location events are emitted
+    expect(changeListenerCalled).toBe(true);
+    expect(locationChangeSuccessEmitted).toBe(true);
+  });
+
   it('should call forward errors to error handler', () => {
 
     let error !: Error;
@@ -696,10 +725,8 @@ describe('$location.onChange()', () => {
 
     $location.onChange(changeListener, errorHandler);
 
-    // Mock out setting browserUrl
-    ($location as any).browserUrl = (url: string, replace: boolean, state: unknown) => {};
-
-    ($location as any).setBrowserUrlWithFallback('/newUrl');
+    $location.url('/newUrl');
+    mock$rootScope.runWatchers();
     expect(error.message).toBe('Handle error');
   });