1
0
mirror of https://github.com/bitwarden/browser.git synced 2024-11-29 12:55:21 +01:00

[PM-3530] browser extension view cache (#10437)

Introduces a way to store temporary component state, for the purposes of persisting views between extension popup open and close.
This commit is contained in:
Will Martin 2024-08-26 16:09:33 -04:00 committed by GitHub
parent e242d7d2d5
commit 6918606677
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
9 changed files with 532 additions and 3 deletions

View File

@ -577,6 +577,7 @@ export default class MainBackground {
);
this.popupViewCacheBackgroundService = new PopupViewCacheBackgroundService(
messageListener,
this.globalStateProvider,
);

View File

@ -0,0 +1,136 @@
import {
DestroyRef,
effect,
inject,
Injectable,
Injector,
signal,
WritableSignal,
} from "@angular/core";
import { takeUntilDestroyed } from "@angular/core/rxjs-interop";
import { FormGroup } from "@angular/forms";
import { NavigationEnd, Router } from "@angular/router";
import { filter, firstValueFrom, skip } from "rxjs";
import { Jsonify } from "type-fest";
import {
FormCacheOptions,
SignalCacheOptions,
ViewCacheService,
} from "@bitwarden/angular/platform/abstractions/view-cache.service";
import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum";
import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service";
import { MessageSender } from "@bitwarden/common/platform/messaging";
import { GlobalStateProvider } from "@bitwarden/common/platform/state";
import {
ClEAR_VIEW_CACHE_COMMAND,
POPUP_VIEW_CACHE_KEY,
SAVE_VIEW_CACHE_COMMAND,
} from "../../services/popup-view-cache-background.service";
/**
* Popup implementation of {@link ViewCacheService}.
*
* Persists user changes between popup open and close
*/
@Injectable({
providedIn: "root",
})
export class PopupViewCacheService implements ViewCacheService {
private configService = inject(ConfigService);
private globalStateProvider = inject(GlobalStateProvider);
private messageSender = inject(MessageSender);
private router = inject(Router);
private featureEnabled: boolean;
private _cache: Record<string, string>;
private get cache(): Record<string, string> {
if (!this._cache) {
throw new Error("Dirty View Cache not initialized");
}
return this._cache;
}
/**
* Initialize the service. This should only be called once.
*/
async init() {
this.featureEnabled = await this.configService.getFeatureFlag(FeatureFlag.PersistPopupView);
const initialState = this.featureEnabled
? await firstValueFrom(this.globalStateProvider.get(POPUP_VIEW_CACHE_KEY).state$)
: {};
this._cache = Object.freeze(initialState ?? {});
this.router.events
.pipe(
filter((e) => e instanceof NavigationEnd),
/** Skip the first navigation triggered by `popupRouterCacheGuard` */
skip(1),
)
.subscribe(() => this.clearState());
}
/**
* @see {@link ViewCacheService.signal}
*/
signal<T>(options: SignalCacheOptions<T>): WritableSignal<T> {
const {
deserializer = (v: Jsonify<T>): T => v as T,
key,
injector = inject(Injector),
initialValue,
} = options;
const cachedValue = this.cache[key] ? deserializer(JSON.parse(this.cache[key])) : initialValue;
const _signal = signal(cachedValue);
effect(
() => {
this.updateState(key, JSON.stringify(_signal()));
},
{ injector },
);
return _signal;
}
/**
* @see {@link ViewCacheService.formGroup}
*/
formGroup<TFormGroup extends FormGroup>(options: FormCacheOptions<TFormGroup>): TFormGroup {
const { control, injector } = options;
const _signal = this.signal({
...options,
initialValue: control.getRawValue(),
});
const value = _signal();
if (value !== undefined && JSON.stringify(value) !== JSON.stringify(control.getRawValue())) {
control.setValue(value);
control.markAsDirty();
}
control.valueChanges.pipe(takeUntilDestroyed(injector?.get(DestroyRef))).subscribe(() => {
_signal.set(control.getRawValue());
});
return control;
}
private updateState(key: string, value: string) {
if (!this.featureEnabled) {
return;
}
this.messageSender.send(SAVE_VIEW_CACHE_COMMAND, {
key,
value,
});
}
private clearState() {
this.messageSender.send(ClEAR_VIEW_CACHE_COMMAND, {});
}
}

View File

@ -0,0 +1,224 @@
import { Component, inject, Injector } from "@angular/core";
import { TestBed } from "@angular/core/testing";
import { FormControl, FormGroup } from "@angular/forms";
import { Router } from "@angular/router";
import { RouterTestingModule } from "@angular/router/testing";
import { MockProxy, mock } from "jest-mock-extended";
import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service";
import { MessageSender } from "@bitwarden/common/platform/messaging";
import { GlobalStateProvider } from "@bitwarden/common/platform/state";
import { FakeGlobalState, FakeGlobalStateProvider } from "@bitwarden/common/spec";
import {
ClEAR_VIEW_CACHE_COMMAND,
POPUP_VIEW_CACHE_KEY,
SAVE_VIEW_CACHE_COMMAND,
} from "../../services/popup-view-cache-background.service";
import { PopupViewCacheService } from "./popup-view-cache.service";
@Component({ template: "" })
export class EmptyComponent {}
@Component({ template: "" })
export class TestComponent {
private viewCacheService = inject(PopupViewCacheService);
formGroup = this.viewCacheService.formGroup({
key: "test-form-cache",
control: new FormGroup({
name: new FormControl("initial name"),
}),
});
signal = this.viewCacheService.signal({
key: "test-signal",
initialValue: "initial signal",
});
}
describe("popup view cache", () => {
const configServiceMock = mock<ConfigService>();
let testBed: TestBed;
let service: PopupViewCacheService;
let fakeGlobalState: FakeGlobalState<Record<string, string>>;
let messageSenderMock: MockProxy<MessageSender>;
let router: Router;
const initServiceWithState = async (state: Record<string, string>) => {
await fakeGlobalState.update(() => state);
await service.init();
};
beforeEach(async () => {
jest.spyOn(configServiceMock, "getFeatureFlag").mockResolvedValue(true);
messageSenderMock = mock<MessageSender>();
const fakeGlobalStateProvider = new FakeGlobalStateProvider();
fakeGlobalState = fakeGlobalStateProvider.getFake(POPUP_VIEW_CACHE_KEY);
testBed = TestBed.configureTestingModule({
imports: [
RouterTestingModule.withRoutes([
{ path: "a", component: EmptyComponent },
{ path: "b", component: EmptyComponent },
]),
],
providers: [
{ provide: GlobalStateProvider, useValue: fakeGlobalStateProvider },
{ provide: MessageSender, useValue: messageSenderMock },
{ provide: ConfigService, useValue: configServiceMock },
],
});
await testBed.compileComponents();
router = testBed.inject(Router);
service = testBed.inject(PopupViewCacheService);
});
it("should initialize signal when ran within an injection context", async () => {
await initServiceWithState({});
const signal = TestBed.runInInjectionContext(() =>
service.signal({
key: "foo-123",
initialValue: "foo",
}),
);
expect(signal()).toBe("foo");
});
it("should initialize signal when provided an injector", async () => {
await initServiceWithState({});
const injector = TestBed.inject(Injector);
const signal = service.signal({
key: "foo-123",
initialValue: "foo",
injector,
});
expect(signal()).toBe("foo");
});
it("should initialize signal from state", async () => {
await initServiceWithState({ "foo-123": JSON.stringify("bar") });
const injector = TestBed.inject(Injector);
const signal = service.signal({
key: "foo-123",
initialValue: "foo",
injector,
});
expect(signal()).toBe("bar");
});
it("should initialize form from state", async () => {
await initServiceWithState({ "test-form-cache": JSON.stringify({ name: "baz" }) });
const fixture = TestBed.createComponent(TestComponent);
const component = fixture.componentRef.instance;
expect(component.formGroup.value.name).toBe("baz");
expect(component.formGroup.dirty).toBe(true);
});
it("should not modify form when empty", async () => {
await initServiceWithState({});
const fixture = TestBed.createComponent(TestComponent);
const component = fixture.componentRef.instance;
expect(component.formGroup.value.name).toBe("initial name");
expect(component.formGroup.dirty).toBe(false);
});
it("should utilize deserializer", async () => {
await initServiceWithState({ "foo-123": JSON.stringify("bar") });
const injector = TestBed.inject(Injector);
const signal = service.signal({
key: "foo-123",
initialValue: "foo",
injector,
deserializer: (jsonValue) => "test",
});
expect(signal()).toBe("test");
});
it("should not utilize deserializer when empty", async () => {
await initServiceWithState({});
const injector = TestBed.inject(Injector);
const signal = service.signal({
key: "foo-123",
initialValue: "foo",
injector,
deserializer: (jsonValue) => "test",
});
expect(signal()).toBe("foo");
});
it("should send signal updates to message sender", async () => {
await initServiceWithState({});
const fixture = TestBed.createComponent(TestComponent);
const component = fixture.componentRef.instance;
component.signal.set("Foobar");
fixture.detectChanges();
expect(messageSenderMock.send).toHaveBeenCalledWith(SAVE_VIEW_CACHE_COMMAND, {
key: "test-signal",
value: JSON.stringify("Foobar"),
});
});
it("should send form updates to message sender", async () => {
await initServiceWithState({});
const fixture = TestBed.createComponent(TestComponent);
const component = fixture.componentRef.instance;
component.formGroup.controls.name.setValue("Foobar");
fixture.detectChanges();
expect(messageSenderMock.send).toHaveBeenCalledWith(SAVE_VIEW_CACHE_COMMAND, {
key: "test-form-cache",
value: JSON.stringify({ name: "Foobar" }),
});
});
it("should clear on 2nd navigation", async () => {
await initServiceWithState({});
await router.navigate(["a"]);
expect(messageSenderMock.send).toHaveBeenCalledTimes(0);
await router.navigate(["b"]);
expect(messageSenderMock.send).toHaveBeenCalledWith(ClEAR_VIEW_CACHE_COMMAND, {});
});
it("should ignore cached values when feature flag is off", async () => {
jest.spyOn(configServiceMock, "getFeatureFlag").mockResolvedValue(false);
await initServiceWithState({ "foo-123": JSON.stringify("bar") });
const injector = TestBed.inject(Injector);
const signal = service.signal({
key: "foo-123",
initialValue: "foo",
injector,
});
// The cached state is ignored
expect(signal()).toBe("foo");
});
});

View File

@ -1,5 +1,6 @@
import { switchMap, merge, delay, filter, map } from "rxjs";
import { switchMap, merge, delay, filter, concatMap, map } from "rxjs";
import { CommandDefinition, MessageListener } from "@bitwarden/common/platform/messaging";
import {
POPUP_VIEW_MEMORY,
KeyDefinition,
@ -11,6 +12,15 @@ import { fromChromeEvent } from "../browser/from-chrome-event";
const popupClosedPortName = "new_popup";
/** We cannot use `UserKeyDefinition` because we must be able to store state when there is no active user. */
export const POPUP_VIEW_CACHE_KEY = KeyDefinition.record<string>(
POPUP_VIEW_MEMORY,
"popup-view-cache",
{
deserializer: (jsonValue) => jsonValue,
},
);
export const POPUP_ROUTE_HISTORY_KEY = new KeyDefinition<string[]>(
POPUP_VIEW_MEMORY,
"popup-route-history",
@ -19,12 +29,35 @@ export const POPUP_ROUTE_HISTORY_KEY = new KeyDefinition<string[]>(
},
);
export const SAVE_VIEW_CACHE_COMMAND = new CommandDefinition<{
key: string;
value: string;
}>("save-view-cache");
export const ClEAR_VIEW_CACHE_COMMAND = new CommandDefinition("clear-view-cache");
export class PopupViewCacheBackgroundService {
private popupViewCacheState = this.globalStateProvider.get(POPUP_VIEW_CACHE_KEY);
private popupRouteHistoryState = this.globalStateProvider.get(POPUP_ROUTE_HISTORY_KEY);
constructor(private globalStateProvider: GlobalStateProvider) {}
constructor(
private messageListener: MessageListener,
private globalStateProvider: GlobalStateProvider,
) {}
startObservingTabChanges() {
this.messageListener
.messages$(SAVE_VIEW_CACHE_COMMAND)
.pipe(
concatMap(async ({ key, value }) =>
this.popupViewCacheState.update((state) => ({
...state,
[key]: value,
})),
),
)
.subscribe();
merge(
// on tab changed, excluding extension tabs
fromChromeEvent(chrome.tabs.onActivated).pipe(
@ -45,6 +78,7 @@ export class PopupViewCacheBackgroundService {
async clearState() {
return Promise.all([
this.popupViewCacheState.update(() => ({}), { shouldUpdate: this.objNotEmpty }),
this.popupRouteHistoryState.update(() => [], { shouldUpdate: this.objNotEmpty }),
]);
}

View File

@ -1,4 +1,4 @@
import { ChangeDetectorRef, Component, NgZone, OnDestroy, OnInit } from "@angular/core";
import { ChangeDetectorRef, Component, NgZone, OnDestroy, OnInit, inject } from "@angular/core";
import { NavigationEnd, Router, RouterOutlet } from "@angular/router";
import { Subject, takeUntil, firstValueFrom, concatMap, filter, tap } from "rxjs";
@ -21,6 +21,7 @@ import {
} from "@bitwarden/components";
import { BrowserApi } from "../platform/browser/browser-api";
import { PopupViewCacheService } from "../platform/popup/view-cache/popup-view-cache.service";
import { initPopupClosedListener } from "../platform/services/popup-view-cache-background.service";
import { BrowserSendStateService } from "../tools/popup/services/browser-send-state.service";
import { VaultBrowserStateService } from "../vault/services/vault-browser-state.service";
@ -37,6 +38,8 @@ import { DesktopSyncVerificationDialogComponent } from "./components/desktop-syn
</div>`,
})
export class AppComponent implements OnInit, OnDestroy {
private viewCacheService = inject(PopupViewCacheService);
private lastActivity: Date;
private activeUserId: UserId;
private recordActivitySubject = new Subject<void>();
@ -64,6 +67,7 @@ export class AppComponent implements OnInit, OnDestroy {
async ngOnInit() {
initPopupClosedListener();
await this.viewCacheService.init();
// Component states must not persist between closing and reopening the popup, otherwise they become dead objects
// Clear them aggressively to make sure this doesn't occur

View File

@ -1,6 +1,7 @@
import { APP_INITIALIZER, NgModule, NgZone } from "@angular/core";
import { Subject, merge, of } from "rxjs";
import { ViewCacheService } from "@bitwarden/angular/platform/abstractions/view-cache.service";
import { AngularThemingService } from "@bitwarden/angular/platform/services/theming/angular-theming.service";
import { SafeProvider, safeProvider } from "@bitwarden/angular/platform/utils/safe-provider";
import {
@ -102,6 +103,7 @@ import { OffscreenDocumentService } from "../../platform/offscreen-document/abst
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 { PopupViewCacheService } from "../../platform/popup/view-cache/popup-view-cache.service";
import { ScriptInjectorService } from "../../platform/services/abstractions/script-injector.service";
import { BrowserCryptoService } from "../../platform/services/browser-crypto.service";
import { BrowserEnvironmentService } from "../../platform/services/browser-environment.service";
@ -305,6 +307,11 @@ const safeProviders: SafeProvider[] = [
provide: AutofillServiceAbstraction,
useExisting: AutofillService,
}),
safeProvider({
provide: ViewCacheService,
useExisting: PopupViewCacheService,
deps: [],
}),
safeProvider({
provide: AutofillService,
deps: [

View File

@ -0,0 +1,83 @@
import { Injector, WritableSignal } from "@angular/core";
import type { FormGroup } from "@angular/forms";
import type { Jsonify, JsonValue } from "type-fest";
type Deserializer<T> = {
/**
* A function to use to safely convert your type from json to your expected type.
*
* @param jsonValue The JSON object representation of your state.
* @returns The fully typed version of your state.
*/
readonly deserializer?: (jsonValue: Jsonify<T>) => T;
};
type BaseCacheOptions<T> = {
/** A unique key for saving the cached value to state */
key: string;
/** An optional injector. Required if the method is called outside of an injection context. */
injector?: Injector;
} & (T extends JsonValue ? Deserializer<T> : Required<Deserializer<T>>);
export type SignalCacheOptions<T> = BaseCacheOptions<T> & {
/** The initial value for the signal. */
initialValue: T;
};
/** Extract the value type from a FormGroup */
type FormValue<TFormGroup extends FormGroup> = TFormGroup["value"];
export type FormCacheOptions<TFormGroup extends FormGroup> = BaseCacheOptions<
FormValue<TFormGroup>
> & {
control: TFormGroup;
};
/**
* Cache for temporary component state
*
* #### Implementations
* - browser extension popup: used to persist UI between popup open and close
* - all other clients: noop
*/
export abstract class ViewCacheService {
/**
* Create a signal from a previously cached value. Whenever the signal is updated, the new value is saved to the cache.
*
* Non browser extension implementations are noop and return a normal signal.
*
* @returns the created signal
*
* @example
* ```ts
* const mySignal = this.viewCacheService.signal({
* key: "popup-search-text"
* initialValue: ""
* });
* ```
*/
abstract signal<T>(options: SignalCacheOptions<T>): WritableSignal<T>;
/**
* - Initialize a form from a cached value
* - Save form value to cache when it changes
* - The form is marked dirty if the restored value is not `undefined`.
*
* Non browser extension implementations are noop and return the original form group.
*
* @example
* ```ts
* this.loginDetailsForm = this.viewCacheService.formGroup({
* key: "vault-login-details-form",
* control: this.formBuilder.group({
* username: [""],
* email: [""],
* })
* });
* ```
**/
abstract formGroup<TFormGroup extends FormGroup>(
options: FormCacheOptions<TFormGroup>,
): TFormGroup;
}

View File

@ -0,0 +1,33 @@
import { Injectable, signal, WritableSignal } from "@angular/core";
import type { FormGroup } from "@angular/forms";
import {
FormCacheOptions,
SignalCacheOptions,
ViewCacheService,
} from "../abstractions/view-cache.service";
/**
* The functionality of the {@link ViewCacheService} is only needed in the browser extension popup,
* yet is provided to all clients to make sharing components easier.
*
* Non-extension clients use this noop implementation.
* */
@Injectable({
providedIn: "root",
})
export class NoopViewCacheService implements ViewCacheService {
/**
* Return a normal signal.
*/
signal<T>(options: SignalCacheOptions<T>): WritableSignal<T> {
return signal(options.initialValue);
}
/**
* Return the original form group.
**/
formGroup<TFormGroup extends FormGroup>(options: FormCacheOptions<TFormGroup>): TFormGroup {
return options.control;
}
}

View File

@ -268,8 +268,10 @@ import {
} from "@bitwarden/vault-export-core";
import { FormValidationErrorsService as FormValidationErrorsServiceAbstraction } from "../platform/abstractions/form-validation-errors.service";
import { ViewCacheService } from "../platform/abstractions/view-cache.service";
import { FormValidationErrorsService } from "../platform/services/form-validation-errors.service";
import { LoggingErrorHandler } from "../platform/services/logging-error-handler";
import { NoopViewCacheService } from "../platform/services/noop-view-cache.service";
import { AngularThemingService } from "../platform/services/theming/angular-theming.service";
import { AbstractThemingService } from "../platform/services/theming/theming.service.abstraction";
import { safeProvider, SafeProvider } from "../platform/utils/safe-provider";
@ -1290,6 +1292,11 @@ const safeProviders: SafeProvider[] = [
useClass: DefaultRegistrationFinishService,
deps: [CryptoServiceAbstraction, AccountApiServiceAbstraction],
}),
safeProvider({
provide: ViewCacheService,
useExisting: NoopViewCacheService,
deps: [],
}),
];
@NgModule({