Use a service to track when to open and close offscreen document (#8977)

* Use a service to track when to open and close offscreen document

There some strangeness around maintaining the offscreen document for more callbacks, that need not have the same reasons and justifications as the original.

We'd need to test, but perhaps the intent is something closer to maintaining a work queue ourselves and creating the offscreen page for only a single reason as it comes in, then waiting for that page to close before opening another.

* Prefer builtin promise flattening

* Await anything and everything

---------

Co-authored-by: Cesar Gonzalez <cesar.a.gonzalezcs@gmail.com>

apps/browser/src/background/main.background.ts

@@ -212,6 +212,8 @@ import { UpdateBadge } from "../platform/listeners/update-badge";
 /* eslint-disable no-restricted-imports */
 import { ChromeMessageSender } from "../platform/messaging/chrome-message.sender";
 /* eslint-enable no-restricted-imports */
+import { OffscreenDocumentService } from "../platform/offscreen-document/abstractions/offscreen-document";
+import { DefaultOffscreenDocumentService } from "../platform/offscreen-document/offscreen-document.service";
 import { BrowserStateService as StateServiceAbstraction } from "../platform/services/abstractions/browser-state.service";
 import { BrowserCryptoService } from "../platform/services/browser-crypto.service";
 import { BrowserEnvironmentService } from "../platform/services/browser-environment.service";
@@ -336,6 +338,7 @@ export default class MainBackground {
   userAutoUnlockKeyService: UserAutoUnlockKeyService;
   scriptInjectorService: BrowserScriptInjectorService;
   kdfConfigService: kdfConfigServiceAbstraction;
+  offscreenDocumentService: OffscreenDocumentService;
 
   onUpdatedRan: boolean;
   onReplacedRan: boolean;
@@ -393,11 +396,14 @@ export default class MainBackground {
       ),
     );
 
+    this.offscreenDocumentService = new DefaultOffscreenDocumentService();
+
     this.platformUtilsService = new BackgroundPlatformUtilsService(
       this.messagingService,
       (clipboardValue, clearMs) => this.clearClipboard(clipboardValue, clearMs),
       async () => this.biometricUnlock(),
       self,
+      this.offscreenDocumentService,
     );
 
     // Creates a session key for mv3 storage of large memory items

apps/browser/src/platform/background/service-factories/platform-utils-service.factory.ts

@@ -30,6 +30,7 @@ export function platformUtilsServiceFactory(
         opts.platformUtilsServiceOptions.clipboardWriteCallback,
         opts.platformUtilsServiceOptions.biometricCallback,
         opts.platformUtilsServiceOptions.win,
+        null,
       ),
   );
 }

apps/browser/src/platform/browser/browser-api.spec.ts

@@ -525,32 +525,6 @@ describe("BrowserApi", () => {
     });
   });
 
-  describe("createOffscreenDocument", () => {
-    it("creates the offscreen document with the supplied reasons and justification", async () => {
-      const reasons = [chrome.offscreen.Reason.CLIPBOARD];
-      const justification = "justification";
-
-      await BrowserApi.createOffscreenDocument(reasons, justification);
-
-      expect(chrome.offscreen.createDocument).toHaveBeenCalledWith({
-        url: "offscreen-document/index.html",
-        reasons,
-        justification,
-      });
-    });
-  });
-
-  describe("closeOffscreenDocument", () => {
-    it("closes the offscreen document", () => {
-      const callbackMock = jest.fn();
-
-      BrowserApi.closeOffscreenDocument(callbackMock);
-
-      expect(chrome.offscreen.closeDocument).toHaveBeenCalled();
-      expect(callbackMock).toHaveBeenCalled();
-    });
-  });
-
   describe("registerContentScriptsMv2", () => {
     const details: browser.contentScripts.RegisteredContentScriptOptions = {
       matches: ["<all_urls>"],

apps/browser/src/platform/browser/browser-api.ts

@@ -558,34 +558,6 @@ export class BrowserApi {
     chrome.privacy.services.passwordSavingEnabled.set({ value });
   }
 
-  /**
-   * Opens the offscreen document with the given reasons and justification.
-   *
-   * @param reasons - List of reasons for opening the offscreen document.
-   * @see https://developer.chrome.com/docs/extensions/reference/api/offscreen#type-Reason
-   * @param justification - Custom written justification for opening the offscreen document.
-   */
-  static async createOffscreenDocument(reasons: chrome.offscreen.Reason[], justification: string) {
-    await chrome.offscreen.createDocument({
-      url: "offscreen-document/index.html",
-      reasons,
-      justification,
-    });
-  }
-
-  /**
-   * Closes the offscreen document.
-   *
-   * @param callback - Optional callback to execute after the offscreen document is closed.
-   */
-  static closeOffscreenDocument(callback?: () => void) {
-    chrome.offscreen.closeDocument(() => {
-      if (callback) {
-        callback();
-      }
-    });
-  }
-
   /**
    * Handles registration of static content scripts within manifest v2.
    *

apps/browser/src/platform/offscreen-document/abstractions/offscreen-document.ts

@@ -1,4 +1,4 @@
-type OffscreenDocumentExtensionMessage = {
+export type OffscreenDocumentExtensionMessage = {
   [key: string]: any;
   command: string;
   text?: string;
@@ -9,18 +9,20 @@ type OffscreenExtensionMessageEventParams = {
   sender: chrome.runtime.MessageSender;
 };
 
-type OffscreenDocumentExtensionMessageHandlers = {
+export type OffscreenDocumentExtensionMessageHandlers = {
   [key: string]: ({ message, sender }: OffscreenExtensionMessageEventParams) => any;
   offscreenCopyToClipboard: ({ message }: OffscreenExtensionMessageEventParams) => any;
   offscreenReadFromClipboard: () => any;
 };
 
-interface OffscreenDocument {
+export interface OffscreenDocument {
   init(): void;
 }
 
-export {
-  OffscreenDocumentExtensionMessage,
-  OffscreenDocumentExtensionMessageHandlers,
-  OffscreenDocument,
-};
+export abstract class OffscreenDocumentService {
+  abstract withDocument<T>(
+    reasons: chrome.offscreen.Reason[],
+    justification: string,
+    callback: () => Promise<T> | T,
+  ): Promise<T>;
+}

apps/browser/src/platform/offscreen-document/offscreen-document.service.spec.ts

@@ -0,0 +1,101 @@
+import { DefaultOffscreenDocumentService } from "./offscreen-document.service";
+
+class TestCase {
+  synchronicity: string;
+  private _callback: () => Promise<any> | any;
+  get callback() {
+    return jest.fn(this._callback);
+  }
+
+  constructor(synchronicity: string, callback: () => Promise<any> | any) {
+    this.synchronicity = synchronicity;
+    this._callback = callback;
+  }
+
+  toString() {
+    return this.synchronicity;
+  }
+}
+
+describe.each([
+  new TestCase("synchronous callback", () => 42),
+  new TestCase("asynchronous callback", () => Promise.resolve(42)),
+])("DefaultOffscreenDocumentService %s", (testCase) => {
+  let sut: DefaultOffscreenDocumentService;
+  const reasons = [chrome.offscreen.Reason.TESTING];
+  const justification = "justification is testing";
+  const url = "offscreen-document/index.html";
+  const api = {
+    createDocument: jest.fn(),
+    closeDocument: jest.fn(),
+    hasDocument: jest.fn().mockResolvedValue(false),
+    Reason: chrome.offscreen.Reason,
+  };
+  let callback: jest.Mock<() => Promise<number> | number>;
+
+  beforeEach(() => {
+    callback = testCase.callback;
+    chrome.offscreen = api;
+
+    sut = new DefaultOffscreenDocumentService();
+  });
+
+  afterEach(() => {
+    jest.resetAllMocks();
+  });
+
+  describe("withDocument", () => {
+    it("creates a document when none exists", async () => {
+      await sut.withDocument(reasons, justification, () => {});
+
+      expect(chrome.offscreen.createDocument).toHaveBeenCalledWith({
+        url,
+        reasons,
+        justification,
+      });
+    });
+
+    it("does not create a document when one exists", async () => {
+      api.hasDocument.mockResolvedValue(true);
+
+      await sut.withDocument(reasons, justification, callback);
+
+      expect(chrome.offscreen.createDocument).not.toHaveBeenCalled();
+    });
+
+    describe.each([true, false])("hasDocument returns %s", (hasDocument) => {
+      beforeEach(() => {
+        api.hasDocument.mockResolvedValue(hasDocument);
+      });
+
+      it("calls the callback", async () => {
+        await sut.withDocument(reasons, justification, callback);
+
+        expect(callback).toHaveBeenCalled();
+      });
+
+      it("returns the callback result", async () => {
+        const result = await sut.withDocument(reasons, justification, callback);
+
+        expect(result).toBe(42);
+      });
+
+      it("closes the document when the callback completes and no other callbacks are running", async () => {
+        await sut.withDocument(reasons, justification, callback);
+
+        expect(chrome.offscreen.closeDocument).toHaveBeenCalled();
+      });
+
+      it("does not close the document when the callback completes and other callbacks are running", async () => {
+        await Promise.all([
+          sut.withDocument(reasons, justification, callback),
+          sut.withDocument(reasons, justification, callback),
+          sut.withDocument(reasons, justification, callback),
+          sut.withDocument(reasons, justification, callback),
+        ]);
+
+        expect(chrome.offscreen.closeDocument).toHaveBeenCalledTimes(1);
+      });
+    });
+  });
+});

apps/browser/src/platform/offscreen-document/offscreen-document.service.ts

@@ -0,0 +1,41 @@
+export class DefaultOffscreenDocumentService implements DefaultOffscreenDocumentService {
+  private workerCount = 0;
+
+  constructor() {}
+
+  async withDocument<T>(
+    reasons: chrome.offscreen.Reason[],
+    justification: string,
+    callback: () => Promise<T> | T,
+  ): Promise<T> {
+    this.workerCount++;
+    try {
+      if (!(await this.documentExists())) {
+        await this.create(reasons, justification);
+      }
+
+      return await callback();
+    } finally {
+      this.workerCount--;
+      if (this.workerCount === 0) {
+        await this.close();
+      }
+    }
+  }
+
+  private async create(reasons: chrome.offscreen.Reason[], justification: string): Promise<void> {
+    await chrome.offscreen.createDocument({
+      url: "offscreen-document/index.html",
+      reasons,
+      justification,
+    });
+  }
+
+  private async close(): Promise<void> {
+    await chrome.offscreen.closeDocument();
+  }
+
+  private async documentExists(): Promise<boolean> {
+    return await chrome.offscreen.hasDocument();
+  }
+}

apps/browser/src/platform/services/platform-utils/background-platform-utils.service.ts

@@ -1,5 +1,7 @@
 import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service";
 
+import { OffscreenDocumentService } from "../../offscreen-document/abstractions/offscreen-document";
+
 import { BrowserPlatformUtilsService } from "./browser-platform-utils.service";
 
 export class BackgroundPlatformUtilsService extends BrowserPlatformUtilsService {
@@ -8,8 +10,9 @@ export class BackgroundPlatformUtilsService extends BrowserPlatformUtilsService
     clipboardWriteCallback: (clipboardValue: string, clearMs: number) => void,
     biometricCallback: () => Promise<boolean>,
     win: Window & typeof globalThis,
+    offscreenDocumentService: OffscreenDocumentService,
   ) {
-    super(clipboardWriteCallback, biometricCallback, win);
+    super(clipboardWriteCallback, biometricCallback, win, offscreenDocumentService);
   }
 
   override showToast(

apps/browser/src/platform/services/platform-utils/browser-platform-utils.service.spec.ts

@@ -1,15 +1,22 @@
+import { MockProxy, mock } from "jest-mock-extended";
+
 import { DeviceType } from "@bitwarden/common/enums";
 
 import { flushPromises } from "../../../autofill/spec/testing-utils";
 import { SafariApp } from "../../../browser/safariApp";
 import { BrowserApi } from "../../browser/browser-api";
+import { OffscreenDocumentService } from "../../offscreen-document/abstractions/offscreen-document";
 import BrowserClipboardService from "../browser-clipboard.service";
 
 import { BrowserPlatformUtilsService } from "./browser-platform-utils.service";
 
 class TestBrowserPlatformUtilsService extends BrowserPlatformUtilsService {
-  constructor(clipboardSpy: jest.Mock, win: Window & typeof globalThis) {
-    super(clipboardSpy, null, win);
+  constructor(
+    clipboardSpy: jest.Mock,
+    win: Window & typeof globalThis,
+    offscreenDocumentService: OffscreenDocumentService,
+  ) {
+    super(clipboardSpy, null, win, offscreenDocumentService);
   }
 
   showToast(
@@ -24,13 +31,16 @@ class TestBrowserPlatformUtilsService extends BrowserPlatformUtilsService {
 
 describe("Browser Utils Service", () => {
   let browserPlatformUtilsService: BrowserPlatformUtilsService;
+  let offscreenDocumentService: MockProxy<OffscreenDocumentService>;
   const clipboardWriteCallbackSpy = jest.fn();
 
   beforeEach(() => {
+    offscreenDocumentService = mock();
     (window as any).matchMedia = jest.fn().mockReturnValueOnce({});
     browserPlatformUtilsService = new TestBrowserPlatformUtilsService(
       clipboardWriteCallbackSpy,
       window,
+      offscreenDocumentService,
     );
   });
 
@@ -223,23 +233,23 @@ describe("Browser Utils Service", () => {
         .spyOn(browserPlatformUtilsService, "getDevice")
         .mockReturnValue(DeviceType.ChromeExtension);
       getManifestVersionSpy.mockReturnValue(3);
-      jest.spyOn(BrowserApi, "createOffscreenDocument");
-      jest.spyOn(BrowserApi, "sendMessageWithResponse").mockResolvedValue(undefined);
-      jest.spyOn(BrowserApi, "closeOffscreenDocument");
 
       browserPlatformUtilsService.copyToClipboard(text);
       await flushPromises();
 
       expect(triggerOffscreenCopyToClipboardSpy).toHaveBeenCalledWith(text);
       expect(clipboardServiceCopySpy).not.toHaveBeenCalled();
-      expect(BrowserApi.createOffscreenDocument).toHaveBeenCalledWith(
+      expect(offscreenDocumentService.withDocument).toHaveBeenCalledWith(
         [chrome.offscreen.Reason.CLIPBOARD],
         "Write text to the clipboard.",
+        expect.any(Function),
       );
+
+      const callback = offscreenDocumentService.withDocument.mock.calls[0][2];
+      await callback();
       expect(BrowserApi.sendMessageWithResponse).toHaveBeenCalledWith("offscreenCopyToClipboard", {
         text,
       });
-      expect(BrowserApi.closeOffscreenDocument).toHaveBeenCalled();
     });
 
     it("skips the clipboardWriteCallback if the clipboard is clearing", async () => {
@@ -298,18 +308,21 @@ describe("Browser Utils Service", () => {
         .spyOn(browserPlatformUtilsService, "getDevice")
         .mockReturnValue(DeviceType.ChromeExtension);
       getManifestVersionSpy.mockReturnValue(3);
-      jest.spyOn(BrowserApi, "createOffscreenDocument");
-      jest.spyOn(BrowserApi, "sendMessageWithResponse").mockResolvedValue("test");
-      jest.spyOn(BrowserApi, "closeOffscreenDocument");
+      offscreenDocumentService.withDocument.mockImplementationOnce((_, __, callback) =>
+        Promise.resolve("test"),
+      );
 
       await browserPlatformUtilsService.readFromClipboard();
 
-      expect(BrowserApi.createOffscreenDocument).toHaveBeenCalledWith(
+      expect(offscreenDocumentService.withDocument).toHaveBeenCalledWith(
         [chrome.offscreen.Reason.CLIPBOARD],
         "Read text from the clipboard.",
+        expect.any(Function),
       );
+
+      const callback = offscreenDocumentService.withDocument.mock.calls[0][2];
+      await callback();
       expect(BrowserApi.sendMessageWithResponse).toHaveBeenCalledWith("offscreenReadFromClipboard");
-      expect(BrowserApi.closeOffscreenDocument).toHaveBeenCalled();
     });
 
     it("returns an empty string from the offscreen document if the response is not of type string", async () => {
@@ -317,9 +330,10 @@ describe("Browser Utils Service", () => {
         .spyOn(browserPlatformUtilsService, "getDevice")
         .mockReturnValue(DeviceType.ChromeExtension);
       getManifestVersionSpy.mockReturnValue(3);
-      jest.spyOn(BrowserApi, "createOffscreenDocument");
       jest.spyOn(BrowserApi, "sendMessageWithResponse").mockResolvedValue(1);
-      jest.spyOn(BrowserApi, "closeOffscreenDocument");
+      offscreenDocumentService.withDocument.mockImplementationOnce((_, __, callback) =>
+        Promise.resolve(1),
+      );
 
       const result = await browserPlatformUtilsService.readFromClipboard();
 

apps/browser/src/platform/services/platform-utils/browser-platform-utils.service.ts

@@ -6,6 +6,7 @@ import {
 
 import { SafariApp } from "../../../browser/safariApp";
 import { BrowserApi } from "../../browser/browser-api";
+import { OffscreenDocumentService } from "../../offscreen-document/abstractions/offscreen-document";
 import BrowserClipboardService from "../browser-clipboard.service";
 
 export abstract class BrowserPlatformUtilsService implements PlatformUtilsService {
@@ -15,6 +16,7 @@ export abstract class BrowserPlatformUtilsService implements PlatformUtilsServic
     private clipboardWriteCallback: (clipboardValue: string, clearMs: number) => void,
     private biometricCallback: () => Promise<boolean>,
     private globalContext: Window | ServiceWorkerGlobalScope,
+    private offscreenDocumentService: OffscreenDocumentService,
   ) {}
 
   static getDevice(globalContext: Window | ServiceWorkerGlobalScope): DeviceType {
@@ -316,24 +318,26 @@ export abstract class BrowserPlatformUtilsService implements PlatformUtilsServic
    * Triggers the offscreen document API to copy the text to the clipboard.
    */
   private async triggerOffscreenCopyToClipboard(text: string) {
-    await BrowserApi.createOffscreenDocument(
+    await this.offscreenDocumentService.withDocument(
       [chrome.offscreen.Reason.CLIPBOARD],
       "Write text to the clipboard.",
+      async () => {
+        await BrowserApi.sendMessageWithResponse("offscreenCopyToClipboard", { text });
+      },
     );
-    await BrowserApi.sendMessageWithResponse("offscreenCopyToClipboard", { text });
-    BrowserApi.closeOffscreenDocument();
   }
 
   /**
    * Triggers the offscreen document API to read the text from the clipboard.
    */
   private async triggerOffscreenReadFromClipboard() {
-    await BrowserApi.createOffscreenDocument(
+    const response = await this.offscreenDocumentService.withDocument(
       [chrome.offscreen.Reason.CLIPBOARD],
       "Read text from the clipboard.",
+      async () => {
+        return await BrowserApi.sendMessageWithResponse("offscreenReadFromClipboard");
+      },
     );
-    const response = await BrowserApi.sendMessageWithResponse("offscreenReadFromClipboard");
-    BrowserApi.closeOffscreenDocument();
     if (typeof response === "string") {
       return response;
     }

apps/browser/src/platform/services/platform-utils/foreground-platform-utils.service.ts

@@ -1,5 +1,7 @@
 import { ToastService } from "@bitwarden/components";
 
+import { OffscreenDocumentService } from "../../offscreen-document/abstractions/offscreen-document";
+
 import { BrowserPlatformUtilsService } from "./browser-platform-utils.service";
 
 export class ForegroundPlatformUtilsService extends BrowserPlatformUtilsService {
@@ -8,8 +10,9 @@ export class ForegroundPlatformUtilsService extends BrowserPlatformUtilsService
     clipboardWriteCallback: (clipboardValue: string, clearMs: number) => void,
     biometricCallback: () => Promise<boolean>,
     win: Window & typeof globalThis,
+    offscreenDocumentService: OffscreenDocumentService,
   ) {
-    super(clipboardWriteCallback, biometricCallback, win);
+    super(clipboardWriteCallback, biometricCallback, win, offscreenDocumentService);
   }
 
   override showToast(

apps/browser/src/popup/services/services.module.ts

@@ -100,6 +100,8 @@ import { runInsideAngular } from "../../platform/browser/run-inside-angular.oper
 /* eslint-disable no-restricted-imports */
 import { ChromeMessageSender } from "../../platform/messaging/chrome-message.sender";
 /* eslint-enable no-restricted-imports */
+import { OffscreenDocumentService } from "../../platform/offscreen-document/abstractions/offscreen-document";
+import { DefaultOffscreenDocumentService } from "../../platform/offscreen-document/offscreen-document.service";
 import BrowserPopupUtils from "../../platform/popup/browser-popup-utils";
 import { BrowserFileDownloadService } from "../../platform/popup/services/browser-file-download.service";
 import { BrowserStateService as StateServiceAbstraction } from "../../platform/services/abstractions/browser-state.service";
@@ -287,9 +289,17 @@ const safeProviders: SafeProvider[] = [
     useFactory: getBgService<DevicesServiceAbstraction>("devicesService"),
     deps: [],
   }),
+  safeProvider({
+    provide: OffscreenDocumentService,
+    useClass: DefaultOffscreenDocumentService,
+    deps: [],
+  }),
   safeProvider({
     provide: PlatformUtilsService,
-    useFactory: (toastService: ToastService) => {
+    useFactory: (
+      toastService: ToastService,
+      offscreenDocumentService: OffscreenDocumentService,
+    ) => {
       return new ForegroundPlatformUtilsService(
         toastService,
         (clipboardValue: string, clearMs: number) => {
@@ -306,9 +316,10 @@ const safeProviders: SafeProvider[] = [
           return response.result;
         },
         window,
+        offscreenDocumentService,
       );
     },
-    deps: [ToastService],
+    deps: [ToastService, OffscreenDocumentService],
   }),
   safeProvider({
     provide: PasswordGenerationServiceAbstraction,