From e55e3d5b9bf055a4dcae4d6b139295a26409f11c Mon Sep 17 00:00:00 2001 From: Matt Gibson Date: Wed, 15 May 2024 17:41:04 -0400 Subject: [PATCH] [PM-8159] [PM-8158] [PM-8156] Swallow multiple offscreen document errors (#9195) * Swallow multiple offscreen document errors The API has race issues with determining if an offscreen document exists (https://groups.google.com/a/chromium.org/g/chromium-extensions/c/s2Wp55bjySE/m/SnjJu1MdAAAJ). However, there are no negative effects of attempting to open multiple other than this throw. * Resolve circular dependency --- .../browser/src/background/main.background.ts | 5 ++-- .../offscreen-document.service.spec.ts | 7 +++++- .../offscreen-document.service.ts | 24 ++++++++++++++----- .../src/popup/services/services.module.ts | 10 ++++---- 4 files changed, 33 insertions(+), 13 deletions(-) diff --git a/apps/browser/src/background/main.background.ts b/apps/browser/src/background/main.background.ts index d5e8fe1da7..41f300270e 100644 --- a/apps/browser/src/background/main.background.ts +++ b/apps/browser/src/background/main.background.ts @@ -380,7 +380,8 @@ export default class MainBackground { const logoutCallback = async (expired: boolean, userId?: UserId) => await this.logout(expired, userId); - this.logService = new ConsoleLogService(false); + const isDev = process.env.ENV === "development"; + this.logService = new ConsoleLogService(isDev); this.cryptoFunctionService = new WebCryptoFunctionService(self); this.keyGenerationService = new KeyGenerationService(this.cryptoFunctionService); this.storageService = new BrowserLocalStorageService(); @@ -399,7 +400,7 @@ export default class MainBackground { ), ); - this.offscreenDocumentService = new DefaultOffscreenDocumentService(); + this.offscreenDocumentService = new DefaultOffscreenDocumentService(this.logService); this.platformUtilsService = new BackgroundPlatformUtilsService( this.messagingService, diff --git a/apps/browser/src/platform/offscreen-document/offscreen-document.service.spec.ts b/apps/browser/src/platform/offscreen-document/offscreen-document.service.spec.ts index d6be0a924e..c9bdd823a5 100644 --- a/apps/browser/src/platform/offscreen-document/offscreen-document.service.spec.ts +++ b/apps/browser/src/platform/offscreen-document/offscreen-document.service.spec.ts @@ -1,3 +1,7 @@ +import { mock } from "jest-mock-extended"; + +import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; + import { DefaultOffscreenDocumentService } from "./offscreen-document.service"; class TestCase { @@ -21,6 +25,7 @@ describe.each([ new TestCase("synchronous callback", () => 42), new TestCase("asynchronous callback", () => Promise.resolve(42)), ])("DefaultOffscreenDocumentService %s", (testCase) => { + const logService = mock(); let sut: DefaultOffscreenDocumentService; const reasons = [chrome.offscreen.Reason.TESTING]; const justification = "justification is testing"; @@ -37,7 +42,7 @@ describe.each([ callback = testCase.callback; chrome.offscreen = api; - sut = new DefaultOffscreenDocumentService(); + sut = new DefaultOffscreenDocumentService(logService); }); afterEach(() => { diff --git a/apps/browser/src/platform/offscreen-document/offscreen-document.service.ts b/apps/browser/src/platform/offscreen-document/offscreen-document.service.ts index da0ca38269..a260e3ca6c 100644 --- a/apps/browser/src/platform/offscreen-document/offscreen-document.service.ts +++ b/apps/browser/src/platform/offscreen-document/offscreen-document.service.ts @@ -1,7 +1,9 @@ +import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; + export class DefaultOffscreenDocumentService implements DefaultOffscreenDocumentService { private workerCount = 0; - constructor() {} + constructor(private logService: LogService) {} async withDocument( reasons: chrome.offscreen.Reason[], @@ -24,11 +26,21 @@ export class DefaultOffscreenDocumentService implements DefaultOffscreenDocument } private async create(reasons: chrome.offscreen.Reason[], justification: string): Promise { - await chrome.offscreen.createDocument({ - url: "offscreen-document/index.html", - reasons, - justification, - }); + try { + await chrome.offscreen.createDocument({ + url: "offscreen-document/index.html", + reasons, + justification, + }); + } catch (e) { + // gobble multiple offscreen document creation errors + // TODO: remove this when the offscreen document service is fixed PM-8014 + if (e.message === "Only a single offscreen document may be created.") { + this.logService.info("Ignoring offscreen document creation error."); + return; + } + throw e; + } } private async close(): Promise { diff --git a/apps/browser/src/popup/services/services.module.ts b/apps/browser/src/popup/services/services.module.ts index 0b9c8f6fe6..32d4adae4a 100644 --- a/apps/browser/src/popup/services/services.module.ts +++ b/apps/browser/src/popup/services/services.module.ts @@ -195,9 +195,11 @@ const safeProviders: SafeProvider[] = [ }), safeProvider({ provide: LogService, - useFactory: (platformUtilsService: PlatformUtilsService) => - new ConsoleLogService(platformUtilsService.isDev()), - deps: [PlatformUtilsService], + useFactory: () => { + const isDev = process.env.ENV === "development"; + return new ConsoleLogService(isDev); + }, + deps: [], }), safeProvider({ provide: EnvironmentService, @@ -286,7 +288,7 @@ const safeProviders: SafeProvider[] = [ safeProvider({ provide: OffscreenDocumentService, useClass: DefaultOffscreenDocumentService, - deps: [], + deps: [LogService], }), safeProvider({ provide: PlatformUtilsService,