diff --git a/apps/browser/src/background/main.background.ts b/apps/browser/src/background/main.background.ts index c4c412732c9..bcceac6fb84 100644 --- a/apps/browser/src/background/main.background.ts +++ b/apps/browser/src/background/main.background.ts @@ -300,6 +300,7 @@ import { BrowserActionsService } from "../platform/actions/browser-actions.servi import { DefaultBadgeBrowserApi } from "../platform/badge/badge-browser-api"; import { BadgeService } from "../platform/badge/badge.service"; import { BrowserApi } from "../platform/browser/browser-api"; +import BrowserPopupUtils from "../platform/browser/browser-popup-utils"; import { flagEnabled } from "../platform/flags"; import { IpcBackgroundService } from "../platform/ipc/ipc-background.service"; import { IpcContentScriptManagerService } from "../platform/ipc/ipc-content-script-manager.service"; @@ -1237,6 +1238,12 @@ export default class MainBackground { const systemUtilsServiceReloadCallback = async () => { await this.taskSchedulerService.clearAllScheduledTasks(); + + // Close browser action popup before reloading to prevent zombie popup with invalidated context. + // The 'reloadProcess' message is sent by ProcessReloadService before this callback runs, + // and popups will close themselves upon receiving it. Poll to verify popup is actually closed. + await BrowserPopupUtils.waitForAllPopupsClose(); + BrowserApi.reloadExtension(); }; diff --git a/apps/browser/src/platform/browser/browser-popup-utils.spec.ts b/apps/browser/src/platform/browser/browser-popup-utils.spec.ts index 9f9a6e313c8..e4165348c6e 100644 --- a/apps/browser/src/platform/browser/browser-popup-utils.spec.ts +++ b/apps/browser/src/platform/browser/browser-popup-utils.spec.ts @@ -337,6 +337,68 @@ describe("BrowserPopupUtils", () => { }); }); + describe("waitForAllPopupsClose", () => { + beforeEach(() => { + jest.useFakeTimers(); + }); + + afterEach(() => { + jest.useRealTimers(); + }); + + it("should resolve immediately if no popups are open", async () => { + jest.spyOn(BrowserApi, "isPopupOpen").mockResolvedValue(false); + + const promise = BrowserPopupUtils.waitForAllPopupsClose(); + jest.advanceTimersByTime(100); + + await expect(promise).resolves.toBeUndefined(); + expect(BrowserApi.isPopupOpen).toHaveBeenCalledTimes(1); + }); + + it("should resolve after timeout if popup never closes when using custom timeout", async () => { + jest.spyOn(BrowserApi, "isPopupOpen").mockResolvedValue(true); + + const promise = BrowserPopupUtils.waitForAllPopupsClose(500); + + // Advance past the timeout + jest.advanceTimersByTime(600); + + await expect(promise).resolves.toBeUndefined(); + }); + + it("should resolve after timeout if popup never closes when using default timeout", async () => { + jest.spyOn(BrowserApi, "isPopupOpen").mockResolvedValue(true); + + const promise = BrowserPopupUtils.waitForAllPopupsClose(); + + // Advance past the default timeout + jest.advanceTimersByTime(1100); + + await expect(promise).resolves.toBeUndefined(); + }); + + it("should stop polling after popup closes before timeout", async () => { + let callCount = 0; + jest.spyOn(BrowserApi, "isPopupOpen").mockImplementation(async () => { + callCount++; + return callCount <= 2; + }); + + const promise = BrowserPopupUtils.waitForAllPopupsClose(1000); + + // Advance to when popup closes (300ms) + jest.advanceTimersByTime(300); + + await expect(promise).resolves.toBeUndefined(); + + // Advance further to ensure no more calls are made + jest.advanceTimersByTime(1000); + + expect(BrowserApi.isPopupOpen).toHaveBeenCalledTimes(3); + }); + }); + describe("isSingleActionPopoutOpen", () => { const windowOptions = { id: 1, diff --git a/apps/browser/src/platform/browser/browser-popup-utils.ts b/apps/browser/src/platform/browser/browser-popup-utils.ts index aebb3e92113..cd55f6361a0 100644 --- a/apps/browser/src/platform/browser/browser-popup-utils.ts +++ b/apps/browser/src/platform/browser/browser-popup-utils.ts @@ -1,5 +1,6 @@ // FIXME: Update this file to be type safe and remove this and next line // @ts-strict-ignore +import { filter, firstValueFrom, interval, of, switchMap, takeWhile, timeout } from "rxjs"; import { ScrollOptions } from "./abstractions/browser-popup-utils.abstractions"; import { BrowserApi } from "./browser-api"; @@ -212,6 +213,27 @@ export default class BrowserPopupUtils { } } + /** + * Waits for all browser action popups to close, polling up to the specified timeout. + * Used before extension reload to prevent zombie popups with invalidated contexts. + * + * @param timeoutMs - Maximum time to wait in milliseconds. Defaults to 1 second. + * @returns Promise that resolves when all popups are closed or timeout is reached. + */ + static async waitForAllPopupsClose(timeoutMs = 1000): Promise { + await firstValueFrom( + interval(100).pipe( + switchMap(() => BrowserApi.isPopupOpen()), + takeWhile((isOpen) => isOpen, true), + filter((isOpen) => !isOpen), + timeout({ + first: timeoutMs, + with: () => of(true), + }), + ), + ); + } + /** * Identifies if a single action window is open based on the passed popoutKey. * Will focus the existing window, and close any other windows that might exist diff --git a/apps/browser/src/popup/app.component.ts b/apps/browser/src/popup/app.component.ts index 998531488d3..b85da665fa0 100644 --- a/apps/browser/src/popup/app.component.ts +++ b/apps/browser/src/popup/app.component.ts @@ -36,6 +36,7 @@ import { LogoutReason, UserDecryptionOptionsServiceAbstraction, } from "@bitwarden/auth/common"; +import { BrowserApi } from "@bitwarden/browser/platform/browser/browser-api"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { AuthRequestAnsweringServiceAbstraction } from "@bitwarden/common/auth/abstractions/auth-request-answering/auth-request-answering.service.abstraction"; import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service"; @@ -58,6 +59,7 @@ import { } from "@bitwarden/components"; import { BiometricsService, BiometricStateService, KeyService } from "@bitwarden/key-management"; +import BrowserPopupUtils from "../platform/browser/browser-popup-utils"; import { PopupCompactModeService } from "../platform/popup/layout/popup-compact-mode.service"; import { PopupSizeService } from "../platform/popup/layout/popup-size.service"; import { initPopupClosedListener } from "../platform/services/popup-view-cache-background.service"; @@ -286,6 +288,13 @@ export class AppComponent implements OnInit, OnDestroy { await this.biometricStateService.updateLastProcessReload(); window.location.reload(); }, 2000); + } else { + // Close browser action popup before extension reload to prevent zombie popup with invalidated context. + // This issue occurs in Chromium-based browsers (Chrome, Vivaldi, etc.) where chrome.runtime.reload() + // invalidates extension contexts before popup can close naturally + if (BrowserPopupUtils.inPopup(window)) { + BrowserApi.closePopup(window); + } } } else if (msg.command === "reloadPopup") { // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling.