Skip to content

Commit

Permalink
[PM-5880] Refactor browser platform utils service to remove window
Browse files Browse the repository at this point in the history
…references (#7885)

* [PM-5880] Refactor Browser Platform Utils Service to Remove Window Service

* [PM-5880] Implementing BrowserClipboardService to handle clipboard logic between the BrowserPlatformUtils and offscreen document

* [PM-5880] Adjusting how readText is handled within BrowserClipboardService

* [PM-5880] Adjusting how readText is handled within BrowserClipboardService

* [PM-5880] Working through implementation of chrome offscreen API usage

* [PM-5880] Implementing jest tests for the methods added to the BrowserApi class

* [PM-5880] Implementing jest tests for the OffscreenDocument class

* [PM-5880] Working through jest tests for BrowserClipboardService

* [PM-5880] Adding typing information to the clipboard methods present within the BrowserPlatformUtilsService

* [PM-5880] Working on adding ServiceWorkerGlobalScope typing information

* [PM-5880] Updating window references when calling BrowserPlatformUtils methods

* [PM-5880] Finishing out jest tests for the BrowserClipboardService

* [PM-5880] Finishing out jest tests for the BrowserClipboardService

* [PM-5880] Implementing jest tests to validate the changes within BrowserApi

* [PM-5880] Implementing jest tests to ensure coverage within OffscreenDocument

* [PM-5880] Implementing jest tests for the BrowserPlatformUtilsService

* [PM-5880] Removing unused catch statements

* [PM-5880] Implementing jest tests for the BrowserPlatformUtilsService

* [PM-5880] Implementing jest tests for the BrowserPlatformUtilsService

* [PM-5880] Fixing broken tests
  • Loading branch information
cagonzalezcs authored Mar 6, 2024
1 parent 940fd21 commit 51f482d
Show file tree
Hide file tree
Showing 25 changed files with 936 additions and 139 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1259,7 +1259,7 @@ describe("OverlayBackground", () => {
});
await flushPromises();

expect(copyToClipboardSpy).toHaveBeenCalledWith("totp-code", { window });
expect(copyToClipboardSpy).toHaveBeenCalledWith("totp-code");
});
});

Expand Down
2 changes: 1 addition & 1 deletion apps/browser/src/autofill/background/overlay.background.ts
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ class OverlayBackground implements OverlayBackgroundInterface {
});

if (totpCode) {
this.platformUtilsService.copyToClipboard(totpCode, { window });
this.platformUtilsService.copyToClipboard(totpCode);
}

this.overlayLoginCiphers = new Map([[overlayCipherId, cipher], ...this.overlayLoginCiphers]);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ export default class WebRequestBackground {
private cipherService: CipherService,
private authService: AuthService,
) {
if (BrowserApi.manifestVersion === 2) {
if (BrowserApi.isManifestVersion(2)) {
this.webRequest = (window as any).chrome.webRequest;
}
this.isFirefox = platformUtilsService.isFirefox();
Expand Down
2 changes: 1 addition & 1 deletion apps/browser/src/background/commands.background.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ export default class CommandsBackground {
private async generatePasswordToClipboard() {
const options = (await this.passwordGenerationService.getOptions())?.[0] ?? {};
const password = await this.passwordGenerationService.generatePassword(options);
this.platformUtilsService.copyToClipboard(password, { window: window });
this.platformUtilsService.copyToClipboard(password);
await this.passwordGenerationService.addHistory(password);
}

Expand Down
36 changes: 17 additions & 19 deletions apps/browser/src/background/main.background.ts
Original file line number Diff line number Diff line change
Expand Up @@ -325,7 +325,7 @@ export default class MainBackground {
popupOnlyContext: boolean;

constructor(public isPrivateMode: boolean = false) {
this.popupOnlyContext = isPrivateMode || BrowserApi.manifestVersion === 3;
this.popupOnlyContext = isPrivateMode || BrowserApi.isManifestVersion(3);

// Services
const lockedCallback = async (userId?: string) => {
Expand Down Expand Up @@ -353,20 +353,18 @@ export default class MainBackground {
this.keyGenerationService = new KeyGenerationService(this.cryptoFunctionService);
this.storageService = new BrowserLocalStorageService();
this.secureStorageService = new BrowserLocalStorageService();
this.memoryStorageService =
BrowserApi.manifestVersion === 3
? new LocalBackedSessionStorageService(
new EncryptServiceImplementation(this.cryptoFunctionService, this.logService, false),
this.keyGenerationService,
)
: new MemoryStorageService();
this.memoryStorageForStateProviders =
BrowserApi.manifestVersion === 3
? new LocalBackedSessionStorageService(
new EncryptServiceImplementation(this.cryptoFunctionService, this.logService, false),
this.keyGenerationService,
)
: new BackgroundMemoryStorageService();
this.memoryStorageService = BrowserApi.isManifestVersion(3)
? new LocalBackedSessionStorageService(
new EncryptServiceImplementation(this.cryptoFunctionService, this.logService, false),
this.keyGenerationService,
)
: new MemoryStorageService();
this.memoryStorageForStateProviders = BrowserApi.isManifestVersion(3)
? new LocalBackedSessionStorageService(
new EncryptServiceImplementation(this.cryptoFunctionService, this.logService, false),
this.keyGenerationService,
)
: new BackgroundMemoryStorageService();

const storageServiceProvider = new StorageServiceProvider(
this.storageService as BrowserLocalStorageService,
Expand Down Expand Up @@ -462,7 +460,7 @@ export default class MainBackground {
return promise.then((result) => result.response === "unlocked");
}
},
window,
self,
);
this.i18nService = new BrowserI18nService(BrowserApi.getUILanguage(), this.stateService);
this.cryptoService = new BrowserCryptoService(
Expand Down Expand Up @@ -898,11 +896,11 @@ export default class MainBackground {
);
if (!this.popupOnlyContext) {
const contextMenuClickedHandler = new ContextMenuClickedHandler(
(options) => this.platformUtilsService.copyToClipboard(options.text, { window: self }),
(options) => this.platformUtilsService.copyToClipboard(options.text),
async (_tab) => {
const options = (await this.passwordGenerationService.getOptions())?.[0] ?? {};
const password = await this.passwordGenerationService.generatePassword(options);
this.platformUtilsService.copyToClipboard(password, { window: window });
this.platformUtilsService.copyToClipboard(password);
// FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling.
// eslint-disable-next-line @typescript-eslint/no-floating-promises
this.passwordGenerationService.addHistory(password);
Expand Down Expand Up @@ -1143,7 +1141,7 @@ export default class MainBackground {
await this.reseedStorage();
}

if (BrowserApi.manifestVersion === 3) {
if (BrowserApi.isManifestVersion(3)) {
// FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling.
// eslint-disable-next-line @typescript-eslint/no-floating-promises
BrowserApi.sendMessage("updateBadge");
Expand Down
6 changes: 3 additions & 3 deletions apps/browser/src/background/runtime.background.ts
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ export default class RuntimeBackground {
msg.sender === "autofill_cmd",
);
if (totpCode != null) {
this.platformUtilsService.copyToClipboard(totpCode, { window: window });
this.platformUtilsService.copyToClipboard(totpCode);
}
break;
}
Expand Down Expand Up @@ -261,7 +261,7 @@ export default class RuntimeBackground {
});
break;
case "getClickedElementResponse":
this.platformUtilsService.copyToClipboard(msg.identifier, { window: window });
this.platformUtilsService.copyToClipboard(msg.identifier);
break;
case "triggerFido2ContentScriptInjection":
await this.fido2Service.injectFido2ContentScripts(sender);
Expand Down Expand Up @@ -319,7 +319,7 @@ export default class RuntimeBackground {
});

if (totpCode != null) {
this.platformUtilsService.copyToClipboard(totpCode, { window: window });
this.platformUtilsService.copyToClipboard(totpCode);
}

// reset
Expand Down
3 changes: 2 additions & 1 deletion apps/browser/src/manifest.v3.json
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,8 @@
"clipboardWrite",
"idle",
"alarms",
"scripting"
"scripting",
"offscreen"
],
"optional_permissions": ["nativeMessaging", "privacy"],
"host_permissions": ["http://*/*", "https://*/*"],
Expand Down
4 changes: 2 additions & 2 deletions apps/browser/src/platform/alarms/alarm-state.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ const alarmState: AlarmState = {
*/
export async function getAlarmTime(commandName: AlarmKeys): Promise<number> {
let alarmTime: number;
if (BrowserApi.manifestVersion == 3) {
if (BrowserApi.isManifestVersion(3)) {
const fromSessionStore = await chrome.storage.session.get(commandName);
alarmTime = fromSessionStore[commandName];
} else {
Expand Down Expand Up @@ -58,7 +58,7 @@ export async function clearAlarmTime(commandName: AlarmKeys): Promise<void> {
}

async function setAlarmTimeInternal(commandName: AlarmKeys, time: number): Promise<void> {
if (BrowserApi.manifestVersion == 3) {
if (BrowserApi.isManifestVersion(3)) {
await chrome.storage.session.set({ [commandName]: time });
} else {
alarmState[commandName] = time;
Expand Down
2 changes: 1 addition & 1 deletion apps/browser/src/platform/background.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import {
tabsOnUpdatedListener,
} from "./listeners";

if (BrowserApi.manifestVersion === 3) {
if (BrowserApi.isManifestVersion(3)) {
chrome.commands.onCommand.addListener(onCommandListener);
chrome.runtime.onInstalled.addListener(onInstallListener);
chrome.alarms.onAlarm.addListener(onAlarmListener);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ export function memoryStorageServiceFactory(
opts: MemoryStorageServiceInitOptions,
): Promise<AbstractMemoryStorageService> {
return factory(cache, "memoryStorageService", opts, async () => {
if (BrowserApi.manifestVersion === 3) {
if (BrowserApi.isManifestVersion(3)) {
return new LocalBackedSessionStorageService(
await encryptServiceFactory(cache, opts),
await keyGenerationServiceFactory(cache, opts),
Expand Down
94 changes: 94 additions & 0 deletions apps/browser/src/platform/browser/browser-api.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,24 @@ describe("BrowserApi", () => {
jest.clearAllMocks();
});

describe("isManifestVersion", () => {
beforeEach(() => {
jest.spyOn(BrowserApi, "manifestVersion", "get").mockReturnValue(3);
});

it("returns true if the manifest version matches the provided version", () => {
const result = BrowserApi.isManifestVersion(3);

expect(result).toBe(true);
});

it("returns false if the manifest version does not match the provided version", () => {
const result = BrowserApi.isManifestVersion(2);

expect(result).toBe(false);
});
});

describe("getWindow", () => {
it("will get the current window if a window id is not provided", () => {
// FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling.
Expand Down Expand Up @@ -106,6 +124,38 @@ describe("BrowserApi", () => {
});
});

describe("getTab", () => {
it("returns `null` if the tabId is a falsy value", async () => {
const result = await BrowserApi.getTab(null);

expect(result).toBeNull();
});

it("returns the tab within manifest v3", async () => {
const tabId = 1;
jest.spyOn(BrowserApi, "manifestVersion", "get").mockReturnValue(3);
(chrome.tabs.get as jest.Mock).mockImplementation(
(tabId) => ({ id: tabId }) as chrome.tabs.Tab,
);

const result = await BrowserApi.getTab(tabId);

expect(result).toEqual({ id: tabId });
});

it("returns the tab within manifest v2", async () => {
const tabId = 1;
jest.spyOn(BrowserApi, "manifestVersion", "get").mockReturnValue(2);
(chrome.tabs.get as jest.Mock).mockImplementation((tabId, callback) =>
callback({ id: tabId } as chrome.tabs.Tab),
);

const result = BrowserApi.getTab(tabId);

await expect(result).resolves.toEqual({ id: tabId });
});
});

describe("getBackgroundPage", () => {
it("returns a null value if the `getBackgroundPage` method is not available", () => {
chrome.extension.getBackgroundPage = undefined;
Expand Down Expand Up @@ -280,6 +330,24 @@ describe("BrowserApi", () => {
});
});

describe("getBrowserAction", () => {
it("returns the `chrome.action` API if the extension manifest is for version 3", () => {
jest.spyOn(BrowserApi, "manifestVersion", "get").mockReturnValue(3);

const result = BrowserApi.getBrowserAction();

expect(result).toEqual(chrome.action);
});

it("returns the `chrome.browserAction` API if the extension manifest is for version 2", () => {
jest.spyOn(BrowserApi, "manifestVersion", "get").mockReturnValue(2);

const result = BrowserApi.getBrowserAction();

expect(result).toEqual(chrome.browserAction);
});
});

describe("executeScriptInTab", () => {
it("calls to the extension api to execute a script within the give tabId", async () => {
const tabId = 1;
Expand Down Expand Up @@ -456,4 +524,30 @@ 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();
});
});
});
51 changes: 48 additions & 3 deletions apps/browser/src/platform/browser/browser-api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,15 @@ export class BrowserApi {
return chrome.runtime.getManifest().manifest_version;
}

/**
* Determines if the extension manifest version is the given version.
*
* @param expectedVersion - The expected manifest version to check against.
*/
static isManifestVersion(expectedVersion: 2 | 3) {
return BrowserApi.manifestVersion === expectedVersion;
}

/**
* Gets the current window or the window with the given id.
*
Expand Down Expand Up @@ -98,12 +107,17 @@ export class BrowserApi {
});
}

/**
* Gets the tab with the given id.
*
* @param tabId - The id of the tab to get.
*/
static async getTab(tabId: number): Promise<chrome.tabs.Tab> | null {
if (!tabId) {
return null;
}

if (BrowserApi.manifestVersion === 3) {
if (BrowserApi.isManifestVersion(3)) {
return await chrome.tabs.get(tabId);
}

Expand Down Expand Up @@ -453,8 +467,11 @@ export class BrowserApi {
});
}

/**
* Returns the supported BrowserAction API based on the manifest version.
*/
static getBrowserAction() {
return BrowserApi.manifestVersion === 3 ? chrome.action : chrome.browserAction;
return BrowserApi.isManifestVersion(3) ? chrome.action : chrome.browserAction;
}

static getSidebarAction(
Expand Down Expand Up @@ -488,7 +505,7 @@ export class BrowserApi {
world: chrome.scripting.ExecutionWorld;
},
): Promise<unknown> {
if (BrowserApi.manifestVersion === 3) {
if (BrowserApi.isManifestVersion(3)) {
return chrome.scripting.executeScript({
target: {
tabId: tabId,
Expand Down Expand Up @@ -546,4 +563,32 @@ export class BrowserApi {
chrome.privacy.services.autofillCreditCardEnabled.set({ value });
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();
}
});
}
}
Loading

0 comments on commit 51f482d

Please sign in to comment.