From 3da2777e96d2a0c5c5d73552f6b739a8704cb510 Mon Sep 17 00:00:00 2001 From: Cesar Gonzalez Date: Tue, 19 Mar 2024 08:48:01 -0500 Subject: [PATCH] [PM-6921] Optimize methodology for storing page details within inline menu background processes --- .../background/overlay.background.spec.ts | 77 +++++++++++++++---- .../autofill/background/overlay.background.ts | 19 +++-- 2 files changed, 73 insertions(+), 23 deletions(-) diff --git a/apps/browser/src/autofill/background/overlay.background.spec.ts b/apps/browser/src/autofill/background/overlay.background.spec.ts index ebad626d8df..0cd074a1fb4 100644 --- a/apps/browser/src/autofill/background/overlay.background.spec.ts +++ b/apps/browser/src/autofill/background/overlay.background.spec.ts @@ -117,7 +117,8 @@ describe("OverlayBackground", () => { describe("removePageDetails", () => { it("removes the page details for a specific tab from the pageDetailsForTab object", () => { const tabId = 1; - overlayBackground["pageDetailsForTab"][tabId] = [createPageDetailMock()]; + const frameId = 2; + overlayBackground["pageDetailsForTab"][tabId] = new Map([[frameId, createPageDetailMock()]]); overlayBackground.removePageDetails(tabId); expect(overlayBackground["pageDetailsForTab"][tabId]).toBeUndefined(); @@ -856,29 +857,40 @@ describe("OverlayBackground", () => { sender, ); - expect(overlayBackground["pageDetailsForTab"][sender.tab.id]).toStrictEqual([ - { frameId: sender.frameId, tab: sender.tab, details: pageDetails1 }, - ]); + expect(overlayBackground["pageDetailsForTab"][sender.tab.id]).toStrictEqual( + new Map([ + [sender.frameId, { frameId: sender.frameId, tab: sender.tab, details: pageDetails1 }], + ]), + ); }); it("updates the page details for a tab that already has a set of page details stored ", () => { - overlayBackground["pageDetailsForTab"][sender.tab.id] = [ - { - frameId: sender.frameId, - tab: sender.tab, - details: pageDetails1, - }, - ]; + const secondFrameSender = mock({ + tab: { id: 1 }, + frameId: 3, + }); + overlayBackground["pageDetailsForTab"][sender.tab.id] = new Map([ + [sender.frameId, { frameId: sender.frameId, tab: sender.tab, details: pageDetails1 }], + ]); sendExtensionRuntimeMessage( { command: "collectPageDetailsResponse", details: pageDetails2 }, - sender, + secondFrameSender, ); - expect(overlayBackground["pageDetailsForTab"][sender.tab.id]).toStrictEqual([ - { frameId: sender.frameId, tab: sender.tab, details: pageDetails1 }, - { frameId: sender.frameId, tab: sender.tab, details: pageDetails2 }, - ]); + expect(overlayBackground["pageDetailsForTab"][sender.tab.id]).toStrictEqual( + new Map([ + [sender.frameId, { frameId: sender.frameId, tab: sender.tab, details: pageDetails1 }], + [ + secondFrameSender.frameId, + { + frameId: secondFrameSender.frameId, + tab: secondFrameSender.tab, + details: pageDetails2, + }, + ], + ]), + ); }); }); @@ -1188,6 +1200,10 @@ describe("OverlayBackground", () => { let getLoginCiphersSpy: jest.SpyInstance; let isPasswordRepromptRequiredSpy: jest.SpyInstance; let doAutoFillSpy: jest.SpyInstance; + let sender: chrome.runtime.MessageSender; + const pageDetails = createAutofillPageDetailsMock({ + login: { username: "username1", password: "password1" }, + }); beforeEach(() => { getLoginCiphersSpy = jest.spyOn(overlayBackground["overlayLoginCiphers"], "get"); @@ -1196,6 +1212,7 @@ describe("OverlayBackground", () => { "isPasswordRepromptRequired", ); doAutoFillSpy = jest.spyOn(overlayBackground["autofillService"], "doAutoFill"); + sender = mock({ tab: { id: 1 } }); }); it("ignores the fill request if the overlay cipher id is not provided", async () => { @@ -1207,12 +1224,27 @@ describe("OverlayBackground", () => { expect(doAutoFillSpy).not.toHaveBeenCalled(); }); + it("ignores the fill request if the tab does not contain any identified page details", async () => { + sendPortMessage(listPortSpy, { + command: "fillSelectedListItem", + overlayCipherId: "overlay-cipher-1", + }); + await flushPromises(); + + expect(getLoginCiphersSpy).not.toHaveBeenCalled(); + expect(isPasswordRepromptRequiredSpy).not.toHaveBeenCalled(); + expect(doAutoFillSpy).not.toHaveBeenCalled(); + }); + it("ignores the fill request if a master password reprompt is required", async () => { const cipher = mock({ reprompt: CipherRepromptType.Password, type: CipherType.Login, }); overlayBackground["overlayLoginCiphers"] = new Map([["overlay-cipher-1", cipher]]); + overlayBackground["pageDetailsForTab"][sender.tab.id] = new Map([ + [sender.frameId, { frameId: sender.frameId, tab: sender.tab, details: pageDetails }], + ]); getLoginCiphersSpy = jest.spyOn(overlayBackground["overlayLoginCiphers"], "get"); isPasswordRepromptRequiredSpy.mockResolvedValue(true); @@ -1239,6 +1271,14 @@ describe("OverlayBackground", () => { ["overlay-cipher-2", cipher2], ["overlay-cipher-3", cipher3], ]); + const pageDetailsForTab = { + frameId: sender.frameId, + tab: sender.tab, + details: pageDetails, + }; + overlayBackground["pageDetailsForTab"][sender.tab.id] = new Map([ + [sender.frameId, pageDetailsForTab], + ]); isPasswordRepromptRequiredSpy.mockResolvedValue(false); sendPortMessage(listPortSpy, { @@ -1254,7 +1294,7 @@ describe("OverlayBackground", () => { expect(doAutoFillSpy).toHaveBeenCalledWith({ tab: listPortSpy.sender.tab, cipher: cipher2, - pageDetails: undefined, + pageDetails: [pageDetailsForTab], fillNewPassword: true, allowTotpAutofill: true, }); @@ -1270,6 +1310,9 @@ describe("OverlayBackground", () => { it("copies the cipher's totp code to the clipboard after filling", async () => { const cipher1 = mock({ id: "overlay-cipher-1" }); overlayBackground["overlayLoginCiphers"] = new Map([["overlay-cipher-1", cipher1]]); + overlayBackground["pageDetailsForTab"][sender.tab.id] = new Map([ + [sender.frameId, { frameId: sender.frameId, tab: sender.tab, details: pageDetails }], + ]); isPasswordRepromptRequiredSpy.mockResolvedValue(false); const copyToClipboardSpy = jest .spyOn(overlayBackground["platformUtilsService"], "copyToClipboard") diff --git a/apps/browser/src/autofill/background/overlay.background.ts b/apps/browser/src/autofill/background/overlay.background.ts index 6c3aea9dc71..580194922a3 100644 --- a/apps/browser/src/autofill/background/overlay.background.ts +++ b/apps/browser/src/autofill/background/overlay.background.ts @@ -47,7 +47,10 @@ class OverlayBackground implements OverlayBackgroundInterface { private readonly openViewVaultItemPopout = openViewVaultItemPopout; private readonly openAddEditVaultItemPopout = openAddEditVaultItemPopout; private overlayLoginCiphers: Map = new Map(); - private pageDetailsForTab: Record = {}; + private pageDetailsForTab: Record< + chrome.runtime.MessageSender["tab"]["id"], + Map + > = {}; private userAuthStatus: AuthenticationStatus = AuthenticationStatus.LoggedOut; private overlayButtonPort: chrome.runtime.Port; private overlayListPort: chrome.runtime.Port; @@ -203,12 +206,15 @@ class OverlayBackground implements OverlayBackgroundInterface { details: message.details, }; - if (this.pageDetailsForTab[sender.tab.id]?.length) { - this.pageDetailsForTab[sender.tab.id].push(pageDetails); + const pageDetailsMap = this.pageDetailsForTab[sender.tab.id]; + if (!pageDetailsMap) { + this.pageDetailsForTab[sender.tab.id] = new Map([[sender.frameId, pageDetails]]); return; } - this.pageDetailsForTab[sender.tab.id] = [pageDetails]; + if (!pageDetailsMap.has(sender.frameId)) { + pageDetailsMap.set(sender.frameId, pageDetails); + } } /** @@ -222,7 +228,8 @@ class OverlayBackground implements OverlayBackgroundInterface { { overlayCipherId }: OverlayPortMessage, { sender }: chrome.runtime.Port, ) { - if (!overlayCipherId) { + const pageDetails = this.pageDetailsForTab[sender.tab.id]; + if (!overlayCipherId || !pageDetails?.size) { return; } @@ -234,7 +241,7 @@ class OverlayBackground implements OverlayBackgroundInterface { const totpCode = await this.autofillService.doAutoFill({ tab: sender.tab, cipher: cipher, - pageDetails: this.pageDetailsForTab[sender.tab.id], + pageDetails: Array.from(pageDetails.values()), fillNewPassword: true, allowTotpAutofill: true, });