devol
/
bitwarden-estensione-browser
mirror of https://github.com/bitwarden/browser
1
0
Fork 0
Code Issues 1 Releases Wiki Activity

[PM-6426] Reworking implementation to register handlers separately from the call to those handlers

This commit is contained in:
Cesar Gonzalez 2024-04-17 12:57:25 -05:00
parent a45974fa6d
commit 061e622875
No known key found for this signature in database
GPG Key ID: 3381A5457F8CCECF
13 changed files with 167 additions and 107 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
apps/browser/src/autofill/clipboard/generate-password-to-clipboard-command.ts
View File

@ -16,7 +16,11 @@ export class GeneratePasswordToClipboardCommand {
private passwordGenerationService: PasswordGenerationServiceAbstraction,
private autofillSettingsService: AutofillSettingsServiceAbstraction,
private taskSchedulerService: BrowserTaskSchedulerService,
) {}
) {
this.taskSchedulerService.registerTaskHandler(ScheduledTaskNames.clearClipboardTimeout, () =>
ClearClipboard.run(),
);
}
async getClearClipboard() {
return await firstValueFrom(this.autofillSettingsService.clearClipboardDelay$);
@ -39,9 +43,8 @@ export class GeneratePasswordToClipboardCommand {
timeoutId: this.clearClipboardTimeout,
});
await this.taskSchedulerService.setTimeout(
() => ClearClipboard.run(),
timeoutInMs,
ScheduledTaskNames.clearClipboardTimeout,
timeoutInMs,
);
}
}

9
apps/browser/src/background/main.background.ts
View File

@ -1098,10 +1098,13 @@ export default class MainBackground {
}
await this.fullSync(true);
await this.taskSchedulerService.setInterval(
() => this.fullSync(),
5 * 60 * 1000, // check every 5 minutes
this.taskSchedulerService.registerTaskHandler(
ScheduledTaskNames.scheduleNextSyncInterval,
() => this.fullSync(),
);
await this.taskSchedulerService.setInterval(
ScheduledTaskNames.scheduleNextSyncInterval,
5 * 60 * 1000, // check every 5 minutes
);
setTimeout(() => this.notificationsService.init(), 2500);
resolve();

95
apps/browser/src/platform/services/browser-task-scheduler.service.spec.ts
View File

@ -108,11 +108,14 @@ describe("BrowserTaskSchedulerService", () => {
const callback = jest.fn();
const delayInMs = 999;
jest.spyOn(globalThis, "setTimeout");
browserTaskSchedulerService.registerTaskHandler(
ScheduledTaskNames.loginStrategySessionTimeout,
callback,
);
await browserTaskSchedulerService.setTimeout(
callback,
delayInMs,
ScheduledTaskNames.loginStrategySessionTimeout,
delayInMs,
);
expect(globalThis.setTimeout).toHaveBeenCalledWith(expect.any(Function), delayInMs);
@ -129,11 +132,14 @@ describe("BrowserTaskSchedulerService", () => {
ScheduledTaskNames.loginStrategySessionTimeout,
);
const callback = jest.fn();
browserTaskSchedulerService.registerTaskHandler(
ScheduledTaskNames.loginStrategySessionTimeout,
callback,
);
await browserTaskSchedulerService.setTimeout(
callback,
60 * 1000,
ScheduledTaskNames.loginStrategySessionTimeout,
60 * 1000,
);
expect(callback).toHaveBeenCalled();
@ -147,11 +153,14 @@ describe("BrowserTaskSchedulerService", () => {
it("creates a timeout alarm", async () => {
const callback = jest.fn();
const delayInMinutes = 2;
browserTaskSchedulerService.registerTaskHandler(
ScheduledTaskNames.loginStrategySessionTimeout,
callback,
);
await browserTaskSchedulerService.setTimeout(
callback,
delayInMinutes * 60 * 1000,
ScheduledTaskNames.loginStrategySessionTimeout,
delayInMinutes * 60 * 1000,
);
expect(chrome.alarms.create).toHaveBeenCalledWith(
@ -170,27 +179,30 @@ describe("BrowserTaskSchedulerService", () => {
}),
);
jest.spyOn(browserTaskSchedulerService, "createAlarm");
browserTaskSchedulerService.registerTaskHandler(
ScheduledTaskNames.loginStrategySessionTimeout,
callback,
);
await browserTaskSchedulerService.setTimeout(
callback,
delayInMinutes * 60 * 1000,
ScheduledTaskNames.loginStrategySessionTimeout,
delayInMinutes * 60 * 1000,
);
expect(browserTaskSchedulerService.createAlarm).not.toHaveBeenCalled();
});
it("logs a warning if a duplicate handler is registered when creating an alarm", () => {
const callback = jest.fn();
const name = ScheduledTaskNames.loginStrategySessionTimeout;
browserTaskSchedulerService["onAlarmHandlers"][name] = jest.fn();
browserTaskSchedulerService["registerAlarmHandler"](name, callback);
expect(logService.warning).toHaveBeenCalledWith(
`Alarm handler for ${name} already exists. Overwriting.`,
);
});
// it("logs a warning if a duplicate handler is registered when creating an alarm", () => {
// const callback = jest.fn();
// const name = ScheduledTaskNames.loginStrategySessionTimeout;
// browserTaskSchedulerService["onAlarmHandlers"][name] = jest.fn();
//
// browserTaskSchedulerService["registerAlarmHandler"](name, callback);
//
// expect(logService.warning).toHaveBeenCalledWith(
// `Alarm handler for ${name} already exists. Overwriting.`,
// );
// });
});
describe("setInterval", () => {
@ -198,11 +210,14 @@ describe("BrowserTaskSchedulerService", () => {
const callback = jest.fn();
const intervalInMs = 999;
jest.spyOn(globalThis, "setInterval");
browserTaskSchedulerService.registerTaskHandler(
ScheduledTaskNames.loginStrategySessionTimeout,
callback,
);
await browserTaskSchedulerService.setInterval(
callback,
intervalInMs,
ScheduledTaskNames.loginStrategySessionTimeout,
intervalInMs,
);
expect(globalThis.setInterval).toHaveBeenCalledWith(expect.any(Function), intervalInMs);
@ -220,11 +235,14 @@ describe("BrowserTaskSchedulerService", () => {
ScheduledTaskNames.loginStrategySessionTimeout,
);
const callback = jest.fn();
browserTaskSchedulerService.registerTaskHandler(
ScheduledTaskNames.loginStrategySessionTimeout,
callback,
);
await browserTaskSchedulerService.setInterval(
callback,
periodInMinutes * 60 * 1000,
ScheduledTaskNames.loginStrategySessionTimeout,
periodInMinutes * 60 * 1000,
);
expect(callback).toHaveBeenCalled();
@ -239,11 +257,14 @@ describe("BrowserTaskSchedulerService", () => {
const callback = jest.fn();
const periodInMinutes = 2;
const initialDelayInMs = 1000;
browserTaskSchedulerService.registerTaskHandler(
ScheduledTaskNames.loginStrategySessionTimeout,
callback,
);
await browserTaskSchedulerService.setInterval(
callback,
periodInMinutes * 60 * 1000,
ScheduledTaskNames.loginStrategySessionTimeout,
periodInMinutes * 60 * 1000,
initialDelayInMs,
);
@ -304,22 +325,22 @@ describe("BrowserTaskSchedulerService", () => {
expect(browserTaskSchedulerService.clearAllAlarms).toHaveBeenCalled();
expect(browserTaskSchedulerService["updateActiveAlarms"]).toHaveBeenCalledWith([]);
expect(browserTaskSchedulerService["onAlarmHandlers"]).toEqual({});
// expect(browserTaskSchedulerService["onAlarmHandlers"]).toEqual({});
expect(browserTaskSchedulerService["recoveredAlarms"].size).toBe(0);
});
});
describe("handleOnAlarm", () => {
it("triggers the alarm", async () => {
const alarm = mock<chrome.alarms.Alarm>({ name: ScheduledTaskNames.eventUploadsInterval });
const callback = jest.fn();
browserTaskSchedulerService["onAlarmHandlers"][alarm.name] = callback;
await browserTaskSchedulerService["handleOnAlarm"](alarm);
expect(callback).toHaveBeenCalled();
});
});
// describe("handleOnAlarm", () => {
// it("triggers the alarm", async () => {
// const alarm = mock<chrome.alarms.Alarm>({ name: ScheduledTaskNames.eventUploadsInterval });
// const callback = jest.fn();
// browserTaskSchedulerService["onAlarmHandlers"][alarm.name] = callback;
//
// await browserTaskSchedulerService["handleOnAlarm"](alarm);
//
// expect(callback).toHaveBeenCalled();
// });
// });
describe("clearAlarm", () => {
it("uses the browser.alarms API if it is available", async () => {

47
apps/browser/src/platform/services/browser-task-scheduler.service.ts
View File

@ -29,13 +29,12 @@ export class BrowserTaskSchedulerService
private activeAlarmsState: GlobalState<ActiveAlarm[]>;
readonly activeAlarms$: Observable<ActiveAlarm[]>;
private recoveredAlarms: Set<string> = new Set();
private onAlarmHandlers: Record<string, () => void> = {};
constructor(
private logService: LogService,
logService: LogService,
private stateProvider: StateProvider,
) {
super();
super(logService);
this.activeAlarmsState = this.stateProvider.getGlobal(ACTIVE_ALARMS);
this.activeAlarms$ = this.activeAlarmsState.state$.pipe(
@ -51,21 +50,18 @@ export class BrowserTaskSchedulerService
* than 1 minute, it will use the global setTimeout. Otherwise, it will
* create a browser extension alarm to handle the delay.
*
* @param callback - The function to be called after the delay.
* @param delayInMs - The delay in milliseconds.
* @param taskName - The name of the task, used in defining the alarm.
* @param delayInMs - The delay in milliseconds.
*/
async setTimeout(
callback: () => void,
delayInMs: number,
taskName: ScheduledTaskName,
delayInMs: number,
): Promise<number | NodeJS.Timeout> {
const delayInMinutes = delayInMs / 1000 / 60;
if (delayInMinutes < 1) {
return super.setTimeout(callback, delayInMs);
return super.setTimeout(taskName, delayInMs);
}
this.registerAlarmHandler(taskName, callback);
if (this.recoveredAlarms.has(taskName)) {
await this.triggerRecoveredAlarm(taskName);
return;
@ -79,23 +75,20 @@ export class BrowserTaskSchedulerService
* less than 1 minute, it will use the global setInterval. Otherwise, it will
* create a browser extension alarm to handle the interval.
*
* @param callback - The function to be called at each interval.
* @param intervalInMs - The interval in milliseconds.
* @param taskName - The name of the task, used in defining the alarm.
* @param intervalInMs - The interval in milliseconds.
* @param initialDelayInMs - The initial delay in milliseconds.
*/
async setInterval(
callback: () => void,
intervalInMs: number,
taskName: ScheduledTaskName,
intervalInMs: number,
initialDelayInMs?: number,
): Promise<number | NodeJS.Timeout> {
const intervalInMinutes = intervalInMs / 1000 / 60;
if (intervalInMinutes < 1) {
return super.setInterval(callback, intervalInMs);
return super.setInterval(taskName, intervalInMs);
}
this.registerAlarmHandler(taskName, callback);
if (this.recoveredAlarms.has(taskName)) {
await this.triggerRecoveredAlarm(taskName, intervalInMinutes);
}
@ -136,7 +129,6 @@ export class BrowserTaskSchedulerService
async clearAllScheduledTasks(): Promise<void> {
await this.clearAllAlarms();
await this.updateActiveAlarms([]);
this.onAlarmHandlers = {};
this.recoveredAlarms.clear();
}
@ -160,20 +152,6 @@ export class BrowserTaskSchedulerService
await this.setActiveAlarm({ name, startTime: Date.now(), createInfo });
}
/**
* Registers an alarm handler for the given name.
*
* @param name - The name of the alarm.
* @param handler - The alarm handler.
*/
private registerAlarmHandler(name: ScheduledTaskName, handler: CallableFunction): void {
if (this.onAlarmHandlers[name]) {
this.logService.warning(`Alarm handler for ${name} already exists. Overwriting.`);
}
this.onAlarmHandlers[name] = () => handler();
}
/**
* Verifies the state of the active alarms by checking if
* any alarms have been missed or need to be created.
@ -223,7 +201,6 @@ export class BrowserTaskSchedulerService
* @param name - The name of the active alarm to delete.
*/
private async deleteActiveAlarm(name: ScheduledTaskName): Promise<void> {
delete this.onAlarmHandlers[name];
const activeAlarms = await firstValueFrom(this.activeAlarms$);
const filteredAlarms = activeAlarms?.filter((alarm) => alarm.name !== name);
await this.updateActiveAlarms(filteredAlarms || []);
@ -249,7 +226,7 @@ export class BrowserTaskSchedulerService
periodInMinutes?: number,
): Promise<void> {
this.recoveredAlarms.delete(name);
await this.triggerAlarm(name, periodInMinutes);
await this.triggerTask(name, periodInMinutes);
}
/**
@ -266,7 +243,7 @@ export class BrowserTaskSchedulerService
*/
private handleOnAlarm = async (alarm: chrome.alarms.Alarm): Promise<void> => {
const { name, periodInMinutes } = alarm;
await this.triggerAlarm(name as ScheduledTaskName, periodInMinutes);
await this.triggerTask(name as ScheduledTaskName, periodInMinutes);
};
/**
@ -276,8 +253,8 @@ export class BrowserTaskSchedulerService
* @param name - The name of the alarm to trigger.
* @param periodInMinutes - The period in minutes of an interval alarm.
*/
private async triggerAlarm(name: ScheduledTaskName, periodInMinutes?: number): Promise<void> {
const handler = this.onAlarmHandlers[name];
protected async triggerTask(name: ScheduledTaskName, periodInMinutes?: number): Promise<void> {
const handler = this.taskHandlers.get(name);
if (!periodInMinutes) {
await this.deleteActiveAlarm(name);
}

2
apps/desktop/src/app/services/services.module.ts
View File

@ -255,7 +255,7 @@ const safeProviders: SafeProvider[] = [
safeProvider({
provide: TaskSchedulerService,
useClass: DefaultTaskSchedulerService,
deps: [],
deps: [LogServiceAbstraction],
}),
];

2
libs/angular/src/services/jslib-services.module.ts
View File

@ -1130,7 +1130,7 @@ const safeProviders: SafeProvider[] = [
safeProvider({
provide: TaskSchedulerService,
useClass: DefaultTaskSchedulerService,
deps: [],
deps: [LogService],
}),
safeProvider({
provide: ProviderApiServiceAbstraction,

7
libs/auth/src/common/services/login-strategies/login-strategy.service.ts
View File

@ -115,6 +115,10 @@ export class LoginStrategyService implements LoginStrategyServiceAbstraction {
this.authRequestPushNotificationState = this.stateProvider.get(
AUTH_REQUEST_PUSH_NOTIFICATION_KEY,
);
this.taskSchedulerService.registerTaskHandler(
ScheduledTaskNames.loginStrategySessionTimeout,
() => this.clearCache(),
);
this.currentAuthType$ = this.currentAuthnTypeState.state$;
this.loginStrategy$ = this.currentAuthnTypeState.state$.pipe(
@ -309,9 +313,8 @@ export class LoginStrategyService implements LoginStrategyServiceAbstraction {
(_) => new Date(Date.now() + sessionTimeoutLength),
);
this.sessionTimeout = await this.taskSchedulerService.setTimeout(
() => this.clearCache(),
sessionTimeoutLength,
ScheduledTaskNames.loginStrategySessionTimeout,
sessionTimeoutLength,
);
}

20
libs/common/src/platform/abstractions/task-scheduler.service.ts
View File

@ -1,5 +1,7 @@
import { ScheduledTaskName } from "../enums/scheduled-task-name.enum";
import { LogService } from "./log.service";
export type TaskIdentifier = {
taskName?: ScheduledTaskName;
timeoutId?: number | NodeJS.Timeout;
@ -7,16 +9,26 @@ export type TaskIdentifier = {
};
export abstract class TaskSchedulerService {
protected taskHandlers: Map<string, () => void>;
constructor(protected logService: LogService) {}
abstract registerTaskHandler(taskName: ScheduledTaskName, handler: () => void): void;
abstract unregisterTaskHandler(taskName: ScheduledTaskName): void;
abstract setTimeout(
callback: () => void,
taskName: ScheduledTaskName,
delayInMs: number,
taskName?: ScheduledTaskName,
): Promise<number | NodeJS.Timeout>;
abstract setInterval(
callback: () => void,
taskName: ScheduledTaskName,
intervalInMs: number,
taskName?: ScheduledTaskName,
initialDelayInMs?: number,
): Promise<number | NodeJS.Timeout>;
abstract clearScheduledTask(taskIdentifier: TaskIdentifier): Promise<void>;
protected abstract triggerTask(taskName: ScheduledTaskName, periodInMinutes?: number): void;
}

44
libs/common/src/platform/services/default-task-scheduler.service.ts
View File

@ -1,37 +1,61 @@
import { LogService } from "../abstractions/log.service";
import { TaskIdentifier, TaskSchedulerService } from "../abstractions/task-scheduler.service";
import { ScheduledTaskName } from "../enums/scheduled-task-name.enum";
export class DefaultTaskSchedulerService extends TaskSchedulerService {
constructor(logService: LogService) {
super(logService);
this.taskHandlers = new Map();
}
registerTaskHandler(taskName: ScheduledTaskName, handler: () => void): void {
const existingHandler = this.taskHandlers.get(taskName);
if (existingHandler) {
this.logService.warning(`Task handler for ${taskName} already exists. Overwriting.`);
this.unregisterTaskHandler(taskName);
}
this.taskHandlers.set(taskName, handler);
}
unregisterTaskHandler(taskName: ScheduledTaskName): void {
this.taskHandlers.delete(taskName);
}
protected triggerTask(taskName: ScheduledTaskName, _periodInMinutes?: number): void {
const handler = this.taskHandlers.get(taskName);
if (handler) {
handler();
}
}
/**
* Sets a timeout and returns the timeout id.
*
* @param callback - The function to be called after the delay.
* @param taskName - The name of the task. Unused in the base implementation.
* @param delayInMs - The delay in milliseconds.
* @param _taskName - The name of the task. Unused in the base implementation.
*/
async setTimeout(
callback: () => void,
taskName: ScheduledTaskName,
delayInMs: number,
_taskName?: ScheduledTaskName,
): Promise<number | NodeJS.Timeout> {
return globalThis.setTimeout(() => callback(), delayInMs);
return globalThis.setTimeout(() => this.triggerTask(taskName), delayInMs);
}
/**
* Sets an interval and returns the interval id.
*
* @param callback - The function to be called at each interval.
* @param taskName - The name of the task. Unused in the base implementation.
* @param intervalInMs - The interval in milliseconds.
* @param _taskName - The name of the task. Unused in the base implementation.
* @param _initialDelayInMs - The initial delay in milliseconds. Unused in the base implementation.
*/
async setInterval(
callback: () => void,
taskName: ScheduledTaskName,
intervalInMs: number,
_taskName?: ScheduledTaskName,
_initialDelayInMs?: number,
): Promise<number | NodeJS.Timeout> {
return globalThis.setInterval(() => callback(), intervalInMs);
return globalThis.setInterval(() => this.triggerTask(taskName), intervalInMs);
}
/**

10
libs/common/src/platform/services/system.service.ts
View File

@ -28,7 +28,12 @@ export class SystemService implements SystemServiceAbstraction {
private vaultTimeoutSettingsService: VaultTimeoutSettingsService,
private biometricStateService: BiometricStateService,
private taskSchedulerService: TaskSchedulerService,
) {}
) {
this.taskSchedulerService.registerTaskHandler(
ScheduledTaskNames.systemClearClipboardTimeout,
() => this.clearPendingClipboard(),
);
}
async startProcessReload(authService: AuthService): Promise<void> {
const accounts = await firstValueFrom(this.stateService.accounts$);
@ -126,9 +131,8 @@ export class SystemService implements SystemServiceAbstraction {
};
this.clearClipboardTimeout = this.taskSchedulerService.setTimeout(
() => this.clearPendingClipboard(),
taskTimeoutInMs,
ScheduledTaskNames.systemClearClipboardTimeout,
taskTimeoutInMs,
);
}

9
libs/common/src/services/event/event-upload.service.ts
View File

@ -22,7 +22,11 @@ export class EventUploadService implements EventUploadServiceAbstraction {
private logService: LogService,
private authService: AuthService,
private taskSchedulerService: TaskSchedulerService,
) {}
) {
this.taskSchedulerService.registerTaskHandler(ScheduledTaskNames.eventUploadsInterval, () =>
this.uploadEvents(),
);
}
init(checkOnInterval: boolean) {
if (this.inited) {
@ -33,9 +37,8 @@ export class EventUploadService implements EventUploadServiceAbstraction {
if (checkOnInterval) {
void this.uploadEvents();
void this.taskSchedulerService.setInterval(
() => this.uploadEvents(),
60 * 1000, // check every 60 seconds
ScheduledTaskNames.eventUploadsInterval,
60 * 1000, // check every 60 seconds
);
}
}

9
libs/common/src/services/notifications.service.ts
View File

@ -31,6 +31,7 @@ export class NotificationsService implements NotificationsServiceAbstraction {
private inited = false;
private inactive = false;
private reconnectTimer: number | NodeJS.Timeout = null;
private isSyncingOnReconnect = true;
constructor(
private logService: LogService,
@ -45,6 +46,10 @@ export class NotificationsService implements NotificationsServiceAbstraction {
private messagingService: MessagingService,
private taskSchedulerService: TaskSchedulerService,
) {
this.taskSchedulerService.registerTaskHandler(
ScheduledTaskNames.notificationsReconnectTimeout,
() => this.reconnect(this.isSyncingOnReconnect),
);
this.environmentService.environment$.subscribe(() => {
if (!this.inited) {
return;
@ -244,10 +249,10 @@ export class NotificationsService implements NotificationsServiceAbstraction {
}
if (!this.connected) {
this.isSyncingOnReconnect = sync;
this.reconnectTimer = await this.taskSchedulerService.setTimeout(
() => this.reconnect(sync),
this.random(120000, 300000),
ScheduledTaskNames.notificationsReconnectTimeout,
this.random(120000, 300000),
);
}
}

11
libs/common/src/vault/services/fido2/fido2-client.service.ts
View File

@ -40,6 +40,7 @@ import { Fido2Utils } from "./fido2-utils";
* It is highly recommended that the W3C specification is used a reference when reading this code.
*/
export class Fido2ClientService implements Fido2ClientServiceAbstraction {
private timeoutAbortController: AbortController;
private readonly TIMEOUTS = {
NO_VERIFICATION: {
DEFAULT: 120000,
@ -61,7 +62,11 @@ export class Fido2ClientService implements Fido2ClientServiceAbstraction {
private domainSettingsService: DomainSettingsService,
private taskSchedulerService: TaskSchedulerService,
private logService?: LogService,
) {}
) {
this.taskSchedulerService.registerTaskHandler(ScheduledTaskNames.fido2ClientAbortTimeout, () =>
this.timeoutAbortController?.abort(),
);
}
async isFido2FeatureEnabled(hostname: string, origin: string): Promise<boolean> {
const userEnabledPasskeys = await firstValueFrom(this.vaultSettingsService.enablePasskeys$);
@ -354,10 +359,10 @@ export class Fido2ClientService implements Fido2ClientServiceAbstraction {
clampedTimeout = Math.max(NO_VERIFICATION.MIN, Math.min(timeout, NO_VERIFICATION.MAX));
}
this.timeoutAbortController = abortController;
return await this.taskSchedulerService.setTimeout(
() => abortController.abort(),
clampedTimeout,
ScheduledTaskNames.fido2ClientAbortTimeout,
clampedTimeout,
);
};
}