refactor(router): restore history in a consistent way on canceled navigations (#43651)

The Router code currently has special-case handling around when and how
the internal state is reset. Specifically, it only resets the internal
tracking of the state when an error is thrown, which does not happen
when guards reject or resolvers return `EMPTY`. Other than the
navigation URL not matching a config, guards rejecting would be the main
cause of a navigation being turned down.

This change updates the router code to always reset the internal state
in the same way, regardless of the reason for navigation cancellation.

In the end, this will only affect _very_ specific use-cases with
`UrlHandlingStrategy`. Because the internal state is not updated until
the end of the transition pipe, the state reset generally doesn't do
anything at all. However, because the `rawUrlTree` is reset by calling
`urlHandlingStrategy.merge` with the _attempted_ `rawUrl` that failed,
the resulting browser URL reset could be different than before (but will
now be consistent with how the URL is reset in other scenarios, like a
URL not matching a `Route` config).

PR Close #43651

packages/router/src/router.ts

@@ -952,7 +952,7 @@ export class Router {
                            // This is only applicable with initial navigation, so setting
                            // `navigated` only when not redirecting resolves this scenario.
                            this.navigated = true;
-                           this.restoreHistory(t, true);
+                           this.restoreHistory(t);
                          }
                          const navCancel = new NavigationCancel(
                              t.id, this.serializeUrl(t.extractedUrl), e.message);
@@ -989,7 +989,7 @@ export class Router {
                          /* All other errors should reset to the router's internal URL reference to
                           * the pre-error state. */
                        } else {
-                         this.restoreHistory(t, true);
+                         this.restoreHistory(t);
                          const navError =
                              new NavigationError(t.id, this.serializeUrl(t.extractedUrl), e);
                          eventsSubject.next(navError);
@@ -1454,7 +1454,7 @@ export class Router {
    * Performs the necessary rollback action to restore the browser URL to the
    * state before the transition.
    */
-  private restoreHistory(t: NavigationTransition, restoringFromCaughtError = false) {
+  private restoreHistory(t: NavigationTransition) {
     if (this.canceledNavigationResolution === 'computed') {
       const targetPagePosition = this.currentPageId - t.targetPageId;
       // The navigator change the location before triggered the browser event,
@@ -1476,20 +1476,12 @@ export class Router {
         // TODO(atscott): resetting the `browserUrlTree` should really be done in `resetState`.
         // Investigate if this can be done by running TGP.
         this.browserUrlTree = t.currentUrlTree;
-        this.resetUrlToCurrentUrlTree();
       } else {
         // The browser URL and router state was not updated before the navigation cancelled so
         // there's no restoration needed.
       }
     } else if (this.canceledNavigationResolution === 'replace') {
-      // TODO(atscott): It seems like we should _always_ reset the state here. It would be a no-op
-      // for `deferred` navigations that haven't change the internal state yet because guards
-      // reject. For 'eager' navigations, it seems like we also really should reset the state
-      // because the navigation was cancelled. Investigate if this can be done by running TGP.
-      if (restoringFromCaughtError) {
-        this.resetState(t);
-      }
-      this.resetUrlToCurrentUrlTree();
+      this.resetState(t);
     }
   }
 
@@ -1502,6 +1494,7 @@ export class Router {
     // addition, the URLHandlingStrategy may be configured to specifically preserve parts of the URL
     // when merging, such as the query params so they are not lost on a refresh.
     this.rawUrlTree = this.urlHandlingStrategy.merge(this.currentUrlTree, t.rawUrl);
+    this.resetUrlToCurrentUrlTree();
   }
 
   private resetUrlToCurrentUrlTree(): void {