Merge branch 'reintroduce-foreground-background-derived' into autofill/pm-8519-inline-menu-fails-to-update-credentials-after-saving-credentials-when-unlocking-extension

apps/browser/src/background/main.background.ts

@@ -1,4 +1,4 @@
-import { Subject, filter, firstValueFrom, map, merge, timeout } from "rxjs";
+import { Subject, filter, firstValueFrom, identity, map, merge, timeout } from "rxjs";
 
 import {
   PinServiceAbstraction,
@@ -132,7 +132,6 @@ import { DefaultActiveUserStateProvider } from "@bitwarden/common/platform/state
 import { DefaultGlobalStateProvider } from "@bitwarden/common/platform/state/implementations/default-global-state.provider";
 import { DefaultSingleUserStateProvider } from "@bitwarden/common/platform/state/implementations/default-single-user-state.provider";
 import { DefaultStateProvider } from "@bitwarden/common/platform/state/implementations/default-state.provider";
-import { InlineDerivedStateProvider } from "@bitwarden/common/platform/state/implementations/inline-derived-state";
 import { StateEventRegistrarService } from "@bitwarden/common/platform/state/state-event-registrar.service";
 /* eslint-enable import/no-restricted-paths */
 import { DefaultThemeStateService } from "@bitwarden/common/platform/theming/theme-state.service";
@@ -225,6 +224,8 @@ import I18nService from "../platform/services/i18n.service";
 import { LocalBackedSessionStorageService } from "../platform/services/local-backed-session-storage.service";
 import { BackgroundPlatformUtilsService } from "../platform/services/platform-utils/background-platform-utils.service";
 import { BrowserPlatformUtilsService } from "../platform/services/platform-utils/browser-platform-utils.service";
+import { BackgroundDerivedStateProvider } from "../platform/state/background-derived-state.provider";
+import { ForegroundDerivedStateProvider } from "../platform/state/foreground-derived-state.provider";
 import { BackgroundMemoryStorageService } from "../platform/storage/background-memory-storage.service";
 import { BrowserStorageServiceProvider } from "../platform/storage/browser-storage-service.provider";
 import { ForegroundMemoryStorageService } from "../platform/storage/foreground-memory-storage.service";
@@ -499,7 +500,9 @@ export default class MainBackground {
       this.accountService,
       this.singleUserStateProvider,
     );
-    this.derivedStateProvider = new InlineDerivedStateProvider();
+    this.derivedStateProvider = this.popupOnlyContext
+      ? new ForegroundDerivedStateProvider(identity) // Can't give the NgZone to this version
+      : new BackgroundDerivedStateProvider();
     this.stateProvider = new DefaultStateProvider(
       this.activeUserStateProvider,
       this.singleUserStateProvider,

apps/browser/src/platform/state/background-derived-state.provider.ts

@@ -0,0 +1,23 @@
+import { Observable } from "rxjs";
+
+import { DeriveDefinition, DerivedState } from "@bitwarden/common/platform/state";
+// eslint-disable-next-line import/no-restricted-paths -- extending this class for this client
+import { DefaultDerivedStateProvider } from "@bitwarden/common/platform/state/implementations/default-derived-state.provider";
+import { DerivedStateDependencies } from "@bitwarden/common/src/types/state";
+
+import { BackgroundDerivedState } from "./background-derived-state";
+
+export class BackgroundDerivedStateProvider extends DefaultDerivedStateProvider {
+  override buildDerivedState<TFrom, TTo, TDeps extends DerivedStateDependencies>(
+    parentState$: Observable<TFrom>,
+    deriveDefinition: DeriveDefinition<TFrom, TTo, TDeps>,
+    dependencies: TDeps,
+  ): DerivedState<TTo> {
+    return new BackgroundDerivedState(
+      parentState$,
+      deriveDefinition,
+      deriveDefinition.buildCacheKey(),
+      dependencies,
+    );
+  }
+}

apps/browser/src/platform/state/background-derived-state.ts

@@ -0,0 +1,102 @@
+import { Observable, Subscription, tap } from "rxjs";
+import { Jsonify } from "type-fest";
+
+import { Utils } from "@bitwarden/common/platform/misc/utils";
+import { DeriveDefinition } from "@bitwarden/common/platform/state";
+// eslint-disable-next-line import/no-restricted-paths -- extending this class for this client
+import { DefaultDerivedState } from "@bitwarden/common/platform/state/implementations/default-derived-state";
+import { DerivedStateDependencies } from "@bitwarden/common/types/state";
+
+import { BrowserApi } from "../browser/browser-api";
+
+export class BackgroundDerivedState<
+  TFrom,
+  TTo,
+  TDeps extends DerivedStateDependencies,
+> extends DefaultDerivedState<TFrom, TTo, TDeps> {
+  private portSubscriptions: Map<chrome.runtime.Port, Subscription> = new Map();
+
+  constructor(
+    parentState$: Observable<TFrom>,
+    deriveDefinition: DeriveDefinition<TFrom, TTo, TDeps>,
+    portName: string,
+    dependencies: TDeps,
+  ) {
+    super(parentState$, deriveDefinition, dependencies);
+
+    // listen for foreground derived states to connect
+    BrowserApi.addListener(chrome.runtime.onConnect, (port) => {
+      if (port.name !== portName) {
+        return;
+      }
+
+      const listenerCallback = this.onMessageFromForeground.bind(this);
+      port.onDisconnect.addListener(() => {
+        this.portSubscriptions.get(port)?.unsubscribe();
+        this.portSubscriptions.delete(port);
+        port.onMessage.removeListener(listenerCallback);
+      });
+      port.onMessage.addListener(listenerCallback);
+
+      const stateSubscription = this.state$
+        .pipe(
+          tap((state) => {
+            this.sendMessage(
+              {
+                action: "nextState",
+                data: JSON.stringify(state),
+                id: Utils.newGuid(),
+              },
+              port,
+            );
+          }),
+        )
+        .subscribe();
+
+      this.portSubscriptions.set(port, stateSubscription);
+    });
+  }
+
+  private async onMessageFromForeground(message: DerivedStateMessage, port: chrome.runtime.Port) {
+    if (message.originator === "background") {
+      return;
+    }
+
+    switch (message.action) {
+      case "nextState": {
+        const dataObj = JSON.parse(message.data) as Jsonify<TTo>;
+        const data = this.deriveDefinition.deserialize(dataObj);
+        await this.forceValue(data);
+        this.sendResponse(
+          message,
+          {
+            action: "resolve",
+          },
+          port,
+        );
+        break;
+      }
+    }
+  }
+
+  private sendResponse(
+    originalMessage: DerivedStateMessage,
+    response: Omit<DerivedStateMessage, "originator" | "id">,
+    port: chrome.runtime.Port,
+  ) {
+    this.sendMessage(
+      {
+        ...response,
+        id: originalMessage.id,
+      },
+      port,
+    );
+  }
+
+  private sendMessage(message: Omit<DerivedStateMessage, "originator">, port: chrome.runtime.Port) {
+    port.postMessage({
+      ...message,
+      originator: "background",
+    });
+  }
+}

apps/browser/src/platform/state/derived-state-interactions.spec.ts

@@ -0,0 +1,107 @@
+/**
+ * need to update test environment so structuredClone works appropriately
+ * @jest-environment ../../libs/shared/test.environment.ts
+ */
+
+import { Subject, firstValueFrom, identity } from "rxjs";
+
+import { DeriveDefinition } from "@bitwarden/common/platform/state";
+// eslint-disable-next-line import/no-restricted-paths -- needed to define a derive definition
+import { StateDefinition } from "@bitwarden/common/platform/state/state-definition";
+import { awaitAsync, trackEmissions, ObservableTracker } from "@bitwarden/common/spec";
+
+import { mockPorts } from "../../../spec/mock-port.spec-util";
+
+import { BackgroundDerivedState } from "./background-derived-state";
+import { ForegroundDerivedState } from "./foreground-derived-state";
+
+const stateDefinition = new StateDefinition("test", "memory");
+const deriveDefinition = new DeriveDefinition(stateDefinition, "test", {
+  derive: (dateString: string) => (dateString == null ? null : new Date(dateString)),
+  deserializer: (dateString: string) => (dateString == null ? null : new Date(dateString)),
+  cleanupDelayMs: 1000,
+});
+
+describe("foreground background derived state interactions", () => {
+  let foreground: ForegroundDerivedState<Date>;
+  let background: BackgroundDerivedState<string, Date, Record<string, unknown>>;
+  let parentState$: Subject<string>;
+  const initialParent = "2020-01-01";
+  const portName = "testPort";
+
+  beforeEach(() => {
+    mockPorts();
+    parentState$ = new Subject<string>();
+
+    background = new BackgroundDerivedState(parentState$, deriveDefinition, portName, {});
+    foreground = new ForegroundDerivedState<Date>(deriveDefinition, portName, identity);
+  });
+
+  afterEach(() => {
+    parentState$.complete();
+    jest.resetAllMocks();
+  });
+
+  it("should connect between foreground and background", async () => {
+    const foregroundEmissions = trackEmissions(foreground.state$);
+    const backgroundEmissions = trackEmissions(background.state$);
+
+    parentState$.next(initialParent);
+    await awaitAsync(10);
+
+    expect(backgroundEmissions).toEqual([new Date(initialParent)]);
+    expect(foregroundEmissions).toEqual([new Date(initialParent)]);
+  });
+
+  it("should initialize a late-connected foreground", async () => {
+    const newForeground = new ForegroundDerivedState(deriveDefinition, portName, identity);
+    const backgroundTracker = new ObservableTracker(background.state$);
+    parentState$.next(initialParent);
+    const foregroundTracker = new ObservableTracker(newForeground.state$);
+
+    expect(await backgroundTracker.expectEmission()).toEqual(new Date(initialParent));
+    expect(await foregroundTracker.expectEmission()).toEqual(new Date(initialParent));
+  });
+
+  describe("forceValue", () => {
+    it("should force the value to the background", async () => {
+      const dateString = "2020-12-12";
+      const emissions = trackEmissions(background.state$);
+
+      await foreground.forceValue(new Date(dateString));
+      await awaitAsync();
+
+      expect(emissions).toEqual([new Date(dateString)]);
+    });
+
+    it("should not create new ports if already connected", async () => {
+      // establish port with subscription
+      trackEmissions(foreground.state$);
+
+      const connectMock = chrome.runtime.connect as jest.Mock;
+      const initialConnectCalls = connectMock.mock.calls.length;
+
+      expect(foreground["port"]).toBeDefined();
+      const newDate = new Date();
+      await foreground.forceValue(newDate);
+      await awaitAsync();
+
+      expect(connectMock.mock.calls.length).toBe(initialConnectCalls);
+      expect(await firstValueFrom(background.state$)).toEqual(newDate);
+    });
+
+    it("should create a port if not connected", async () => {
+      const connectMock = chrome.runtime.connect as jest.Mock;
+      const initialConnectCalls = connectMock.mock.calls.length;
+
+      expect(foreground["port"]).toBeUndefined();
+      const newDate = new Date();
+      await foreground.forceValue(newDate);
+      await awaitAsync();
+
+      expect(connectMock.mock.calls.length).toBe(initialConnectCalls + 1);
+      expect(foreground["port"]).toBeNull();
+      expect(await firstValueFrom(background.state$)).toEqual(newDate);
+    });
+  });
+});

apps/browser/src/platform/state/foreground-derived-state.provider.ts

@@ -0,0 +1,25 @@
+import { MonoTypeOperatorFunction, Observable } from "rxjs";
+
+import { DeriveDefinition, DerivedState } from "@bitwarden/common/platform/state";
+// eslint-disable-next-line import/no-restricted-paths -- extending this class for this client
+import { DefaultDerivedStateProvider } from "@bitwarden/common/platform/state/implementations/default-derived-state.provider";
+import { DerivedStateDependencies } from "@bitwarden/common/src/types/state";
+
+import { ForegroundDerivedState } from "./foreground-derived-state";
+
+export class ForegroundDerivedStateProvider extends DefaultDerivedStateProvider {
+  constructor(private pipeCustomizer: MonoTypeOperatorFunction<unknown>) {
+    super();
+  }
+  override buildDerivedState<TFrom, TTo, TDeps extends DerivedStateDependencies>(
+    _parentState$: Observable<TFrom>,
+    deriveDefinition: DeriveDefinition<TFrom, TTo, TDeps>,
+    _dependencies: TDeps,
+  ): DerivedState<TTo> {
+    return new ForegroundDerivedState(
+      deriveDefinition,
+      deriveDefinition.buildCacheKey(),
+      this.pipeCustomizer as MonoTypeOperatorFunction<TTo>,
+    );
+  }
+}

apps/browser/src/platform/state/foreground-derived-state.spec.ts

@@ -0,0 +1,52 @@
+import { awaitAsync } from "@bitwarden/common/../spec";
+import { identity } from "rxjs";
+
+import { DeriveDefinition } from "@bitwarden/common/platform/state";
+// eslint-disable-next-line import/no-restricted-paths -- needed to define a derive definition
+import { StateDefinition } from "@bitwarden/common/platform/state/state-definition";
+
+import { mockPorts } from "../../../spec/mock-port.spec-util";
+
+import { ForegroundDerivedState } from "./foreground-derived-state";
+
+const stateDefinition = new StateDefinition("test", "memory");
+const deriveDefinition = new DeriveDefinition(stateDefinition, "test", {
+  derive: (dateString: string) => (dateString == null ? null : new Date(dateString)),
+  deserializer: (dateString: string) => (dateString == null ? null : new Date(dateString)),
+  cleanupDelayMs: 1,
+});
+
+describe("ForegroundDerivedState", () => {
+  let sut: ForegroundDerivedState<Date>;
+  const portName = "testPort";
+
+  beforeEach(() => {
+    mockPorts();
+    sut = new ForegroundDerivedState<Date>(deriveDefinition, portName, identity);
+  });
+
+  afterEach(() => {
+    jest.resetAllMocks();
+  });
+
+  it("should not connect a port until subscribed", async () => {
+    expect(sut["port"]).toBeUndefined();
+    const subscription = sut.state$.subscribe();
+
+    expect(sut["port"]).toBeDefined();
+    subscription.unsubscribe();
+  });
+
+  it("should disconnect its port when unsubscribed", async () => {
+    const subscription = sut.state$.subscribe();
+
+    expect(sut["port"]).toBeDefined();
+    const disconnectSpy = jest.spyOn(sut["port"], "disconnect");
+    subscription.unsubscribe();
+    // wait for the cleanup delay
+    await awaitAsync(deriveDefinition.cleanupDelayMs * 2);
+
+    expect(disconnectSpy).toHaveBeenCalled();
+    expect(sut["port"]).toBeNull();
+  });
+});

apps/browser/src/platform/state/foreground-derived-state.ts

@@ -0,0 +1,114 @@
+import {
+  MonoTypeOperatorFunction,
+  Observable,
+  ReplaySubject,
+  defer,
+  filter,
+  firstValueFrom,
+  map,
+  of,
+  share,
+  switchMap,
+  tap,
+  timer,
+} from "rxjs";
+import { Jsonify } from "type-fest";
+
+import { Utils } from "@bitwarden/common/platform/misc/utils";
+import { DeriveDefinition, DerivedState } from "@bitwarden/common/platform/state";
+import { DerivedStateDependencies } from "@bitwarden/common/types/state";
+
+import { fromChromeEvent } from "../browser/from-chrome-event";
+
+export class ForegroundDerivedState<TTo> implements DerivedState<TTo> {
+  private port: chrome.runtime.Port;
+  private backgroundResponses$: Observable<DerivedStateMessage>;
+  state$: Observable<TTo>;
+
+  constructor(
+    private deriveDefinition: DeriveDefinition<unknown, TTo, DerivedStateDependencies>,
+    private portName: string,
+    private pipeCustomizer: MonoTypeOperatorFunction<TTo>,
+  ) {
+    const latestValueFromPort$ = (port: chrome.runtime.Port) => {
+      return fromChromeEvent(port.onMessage).pipe(
+        map(([message]) => message as DerivedStateMessage),
+        filter((message) => message.originator === "background" && message.action === "nextState"),
+        map((message) => {
+          const json = JSON.parse(message.data) as Jsonify<TTo>;
+          return this.deriveDefinition.deserialize(json);
+        }),
+      );
+    };
+
+    this.state$ = defer(() => of(this.initializePort())).pipe(
+      switchMap(() => latestValueFromPort$(this.port)),
+      share({
+        connector: () => new ReplaySubject<TTo>(1),
+        resetOnRefCountZero: () =>
+          timer(this.deriveDefinition.cleanupDelayMs).pipe(tap(() => this.tearDownPort())),
+      }),
+      this.pipeCustomizer,
+    );
+  }
+
+  async forceValue(value: TTo): Promise<TTo> {
+    let cleanPort = false;
+    if (this.port == null) {
+      this.initializePort();
+      cleanPort = true;
+    }
+    await this.delegateToBackground("nextState", value);
+    if (cleanPort) {
+      this.tearDownPort();
+    }
+    return value;
+  }
+
+  private initializePort() {
+    if (this.port != null) {
+      return;
+    }
+
+    this.port = chrome.runtime.connect({ name: this.portName });
+
+    this.backgroundResponses$ = fromChromeEvent(this.port.onMessage).pipe(
+      map(([message]) => message as DerivedStateMessage),
+      filter((message) => message.originator === "background"),
+    );
+    return this.backgroundResponses$;
+  }
+
+  private async delegateToBackground(action: DerivedStateActions, data: TTo): Promise<void> {
+    const id = Utils.newGuid();
+    // listen for response before request
+    const response = firstValueFrom(
+      this.backgroundResponses$.pipe(filter((message) => message.id === id)),
+    );
+
+    this.sendMessage({
+      id,
+      action,
+      data: JSON.stringify(data),
+    });
+
+    await response;
+  }
+
+  private sendMessage(message: Omit<DerivedStateMessage, "originator">) {
+    this.port.postMessage({
+      ...message,
+      originator: "foreground",
+    });
+  }
+
+  private tearDownPort() {
+    if (this.port == null) {
+      return;
+    }
+
+    this.port.disconnect();
+    this.port = null;
+    this.backgroundResponses$ = null;
+  }
+}

apps/browser/src/popup/services/services.module.ts

@@ -79,8 +79,6 @@ import {
   GlobalStateProvider,
   StateProvider,
 } from "@bitwarden/common/platform/state";
-// eslint-disable-next-line import/no-restricted-paths -- Used for dependency injection
-import { InlineDerivedStateProvider } from "@bitwarden/common/platform/state/implementations/inline-derived-state";
 import { VaultTimeoutStringType } from "@bitwarden/common/types/vault-timeout.type";
 import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service";
 import { CollectionService } from "@bitwarden/common/vault/abstractions/collection.service";
@@ -114,6 +112,7 @@ import { BrowserScriptInjectorService } from "../../platform/services/browser-sc
 import { DefaultBrowserStateService } from "../../platform/services/default-browser-state.service";
 import I18nService from "../../platform/services/i18n.service";
 import { ForegroundPlatformUtilsService } from "../../platform/services/platform-utils/foreground-platform-utils.service";
+import { ForegroundDerivedStateProvider } from "../../platform/state/foreground-derived-state.provider";
 import { BrowserStorageServiceProvider } from "../../platform/storage/browser-storage-service.provider";
 import { ForegroundMemoryStorageService } from "../../platform/storage/foreground-memory-storage.service";
 import { fromChromeRuntimeMessaging } from "../../platform/utils/from-chrome-runtime-messaging";
@@ -509,8 +508,8 @@ const safeProviders: SafeProvider[] = [
   }),
   safeProvider({
     provide: DerivedStateProvider,
-    useClass: InlineDerivedStateProvider,
-    deps: [],
+    useFactory: (ngZone: NgZone) => new ForegroundDerivedStateProvider(runInsideAngular(ngZone)),
+    deps: [NgZone],
   }),
   safeProvider({
     provide: AutofillSettingsServiceAbstraction,

libs/common/src/vault/services/collection.service.ts

@@ -33,7 +33,7 @@ const DECRYPTED_COLLECTION_DATA_KEY = DeriveDefinition.from<
   CollectionView[],
   { collectionService: CollectionService }
 >(ENCRYPTED_COLLECTION_DATA_KEY, {
-  deserializer: (obj) => obj.map((collection) => CollectionView.fromJSON(collection)),
+  deserializer: (obj) => obj?.map((collection) => CollectionView.fromJSON(collection)),
   derive: async (collections: Record<CollectionId, CollectionData>, { collectionService }) => {
     const data: Collection[] = [];
     for (const id in collections ?? {}) {