From 59aa7ebc5716ef08177e54594c9eb1dd918e7878 Mon Sep 17 00:00:00 2001
From: Will Martin <contact@willmartian.com>
Date: Tue, 20 Aug 2024 09:41:55 -0400
Subject: [PATCH] [PM-10940] misc popout history cache bug fixes (#10619)

---
 .../view-cache/popup-router-cache.service.ts  | 22 +++++++++++++------
 .../view-cache/popup-router-cache.spec.ts     | 10 +++++++++
 .../popup-view-cache-background.service.ts    |  3 ++-
 3 files changed, 27 insertions(+), 8 deletions(-)

diff --git a/apps/browser/src/platform/popup/view-cache/popup-router-cache.service.ts b/apps/browser/src/platform/popup/view-cache/popup-router-cache.service.ts
index 52b08ac09a..8876ac44d5 100644
--- a/apps/browser/src/platform/popup/view-cache/popup-router-cache.service.ts
+++ b/apps/browser/src/platform/popup/view-cache/popup-router-cache.service.ts
@@ -7,7 +7,7 @@ import {
   Router,
   UrlSerializer,
 } from "@angular/router";
-import { filter, first, firstValueFrom, map, Observable, of, switchMap } from "rxjs";
+import { filter, first, firstValueFrom, map, Observable, of, switchMap, tap } from "rxjs";
 
 import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum";
 import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service";
@@ -29,16 +29,25 @@ export class PopupRouterCacheService {
   private state = inject(GlobalStateProvider).get(POPUP_ROUTE_HISTORY_KEY);
   private location = inject(Location);
 
+  private hasNavigated = false;
+
   constructor() {
     // init history with existing state
     this.history$()
       .pipe(first())
-      .subscribe((history) => history.forEach((location) => this.location.go(location)));
+      .subscribe(
+        (history) =>
+          Array.isArray(history) && history.forEach((location) => this.location.go(location)),
+      );
 
     // update state when route change occurs
     this.router.events
       .pipe(
         filter((event) => event instanceof NavigationEnd),
+        tap(() => {
+          // `Location.back()` can now be called successfully
+          this.hasNavigated = true;
+        }),
         filter((_event: NavigationEnd) => {
           const state: ActivatedRouteSnapshot = this.router.routerState.snapshot.root;
 
@@ -77,7 +86,7 @@ export class PopupRouterCacheService {
   /**
    * If in browser popup, push new route onto history stack
    */
-  private async push(url: string): Promise<boolean> {
+  private async push(url: string) {
     if (!BrowserPopupUtils.inPopup(window) || url === (await firstValueFrom(this.last$()))) {
       return;
     }
@@ -88,11 +97,10 @@ export class PopupRouterCacheService {
    * Navigate back in history
    */
   async back() {
-    await this.state.update((prevState) => prevState.slice(0, -1));
+    await this.state.update((prevState) => (prevState ? prevState.slice(0, -1) : []));
 
-    const url = this.router.url;
-    this.location.back();
-    if (url !== this.router.url) {
+    if (this.hasNavigated) {
+      this.location.back();
       return;
     }
 
diff --git a/apps/browser/src/platform/popup/view-cache/popup-router-cache.spec.ts b/apps/browser/src/platform/popup/view-cache/popup-router-cache.spec.ts
index dde7f10500..465a6e6c69 100644
--- a/apps/browser/src/platform/popup/view-cache/popup-router-cache.spec.ts
+++ b/apps/browser/src/platform/popup/view-cache/popup-router-cache.spec.ts
@@ -65,6 +65,16 @@ describe("Popup router cache guard", () => {
     expect(response).toBe(true);
   });
 
+  it("returns true if the history stack is null", async () => {
+    await service.setHistory(null);
+
+    const response = await firstValueFrom(
+      testBed.runInInjectionContext(() => popupRouterCacheGuard()),
+    );
+
+    expect(response).toBe(true);
+  });
+
   it("redirects to the latest stored route", async () => {
     await router.navigate(["a"]);
     await router.navigate(["b"]);
diff --git a/apps/browser/src/platform/services/popup-view-cache-background.service.ts b/apps/browser/src/platform/services/popup-view-cache-background.service.ts
index 3427a02d3f..09099b08c8 100644
--- a/apps/browser/src/platform/services/popup-view-cache-background.service.ts
+++ b/apps/browser/src/platform/services/popup-view-cache-background.service.ts
@@ -6,6 +6,7 @@ import {
   GlobalStateProvider,
 } from "@bitwarden/common/platform/state";
 
+import { BrowserApi } from "../browser/browser-api";
 import { fromChromeEvent } from "../browser/from-chrome-event";
 
 const popupClosedPortName = "new_popup";
@@ -27,7 +28,7 @@ export class PopupViewCacheBackgroundService {
     merge(
       // on tab changed, excluding extension tabs
       fromChromeEvent(chrome.tabs.onActivated).pipe(
-        switchMap(([tabInfo]) => chrome.tabs.get(tabInfo.tabId)),
+        switchMap(([tabInfo]) => BrowserApi.getTab(tabInfo.tabId)),
         map((tab) => tab.url || tab.pendingUrl),
         filter((url) => !url.startsWith(chrome.runtime.getURL(""))),
       ),