[PM-8510] Implement collect page details observable (#9452)

* Working through a POC of a collectPageDetails observable

* Implementing collect page details observable

* [PM-8510] Implement collectPageDetails observable

* [PM-8510] Adding documentation to newly created collectPageDetailsFromTab method

* [PM-8510] Removing unnecessary file

* [PM-8510] Implementing Jest tests for the collectPageDetailsFromTab$ method

* [PM-8510] Implementing Jest tests for the collectPageDetailsFromTab$ method

* [PM-8510] Implementing Jest tests for the collectPageDetailsFromTab$ method

* [PM-8510] Implementing Jest tests for the collectPageDetailsFromTab$ method

* [PM-8510] Removing unnecessary property

* [PM-8510] Adding subscription reference to current tab component

* [PM-8510] Fixing jest tests

apps/browser/src/autofill/commands/autofill-tab-command.ts

@@ -1,71 +0,0 @@
-import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view";
-
-import AutofillPageDetails from "../models/autofill-page-details";
-import { AutofillService } from "../services/abstractions/autofill.service";
-
-export class AutofillTabCommand {
-  constructor(private autofillService: AutofillService) {}
-
-  async doAutofillTabCommand(tab: chrome.tabs.Tab) {
-    if (!tab.id) {
-      throw new Error("Tab does not have an id, cannot complete autofill.");
-    }
-
-    const details = await this.collectPageDetails(tab.id);
-    await this.autofillService.doAutoFillOnTab(
-      [
-        {
-          frameId: 0,
-          tab: tab,
-          details: details,
-        },
-      ],
-      tab,
-      true,
-    );
-  }
-
-  async doAutofillTabWithCipherCommand(tab: chrome.tabs.Tab, cipher: CipherView) {
-    if (!tab.id) {
-      throw new Error("Tab does not have an id, cannot complete autofill.");
-    }
-
-    const details = await this.collectPageDetails(tab.id);
-    await this.autofillService.doAutoFill({
-      tab: tab,
-      cipher: cipher,
-      pageDetails: [
-        {
-          frameId: 0,
-          tab: tab,
-          details: details,
-        },
-      ],
-      skipLastUsed: false,
-      skipUsernameOnlyFill: false,
-      onlyEmptyFields: false,
-      onlyVisibleFields: false,
-      fillNewPassword: true,
-      allowTotpAutofill: true,
-    });
-  }
-
-  private async collectPageDetails(tabId: number): Promise<AutofillPageDetails> {
-    return new Promise((resolve, reject) => {
-      chrome.tabs.sendMessage(
-        tabId,
-        {
-          command: "collectPageDetailsImmediately",
-        },
-        (response: AutofillPageDetails) => {
-          if (chrome.runtime.lastError) {
-            reject(chrome.runtime.lastError);
-            return;
-          }
-
-          resolve(response);
-        },
-      );
-    });
-  }
-}

apps/browser/src/autofill/enums/autofill-message.enums.ts

@@ -0,0 +1,14 @@
+export const AutofillMessageCommand = {
+  collectPageDetails: "collectPageDetails",
+  collectPageDetailsResponse: "collectPageDetailsResponse",
+} as const;
+
+export type AutofillMessageCommandType =
+  (typeof AutofillMessageCommand)[keyof typeof AutofillMessageCommand];
+
+export const AutofillMessageSender = {
+  collectPageDetailsFromTabObservable: "collectPageDetailsFromTabObservable",
+} as const;
+
+export type AutofillMessageSenderType =
+  (typeof AutofillMessageSender)[keyof typeof AutofillMessageSender];

apps/browser/src/autofill/services/abstractions/autofill.service.ts

@@ -1,7 +1,11 @@
+import { Observable } from "rxjs";
+
 import { UriMatchStrategySetting } from "@bitwarden/common/models/domain/domain-service";
+import { CommandDefinition } from "@bitwarden/common/platform/messaging";
 import { CipherType } from "@bitwarden/common/vault/enums";
 import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view";
 
+import { AutofillMessageCommand } from "../../enums/autofill-message.enums";
 import AutofillField from "../../models/autofill-field";
 import AutofillForm from "../../models/autofill-form";
 import AutofillPageDetails from "../../models/autofill-page-details";
@@ -44,7 +48,20 @@ export interface GenerateFillScriptOptions {
   defaultUriMatch: UriMatchStrategySetting;
 }
 
+export type CollectPageDetailsResponseMessage = {
+  tab: chrome.tabs.Tab;
+  details: AutofillPageDetails;
+  sender?: string;
+  webExtSender: chrome.runtime.MessageSender;
+};
+
+export const COLLECT_PAGE_DETAILS_RESPONSE_COMMAND =
+  new CommandDefinition<CollectPageDetailsResponseMessage>(
+    AutofillMessageCommand.collectPageDetailsResponse,
+  );
+
 export abstract class AutofillService {
+  collectPageDetailsFromTab$: (tab: chrome.tabs.Tab) => Observable<PageDetail[]>;
   loadAutofillScriptsOnInstall: () => Promise<void>;
   reloadAutofillScripts: () => Promise<void>;
   injectAutofillScripts: (

apps/browser/src/autofill/services/autofill.service.spec.ts

@@ -1,5 +1,5 @@
 import { mock, mockReset, MockProxy } from "jest-mock-extended";
-import { BehaviorSubject, of } from "rxjs";
+import { BehaviorSubject, of, Subject } from "rxjs";
 
 import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service";
 import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status";
@@ -16,12 +16,14 @@ import { EventType } from "@bitwarden/common/enums";
 import { UriMatchStrategy } from "@bitwarden/common/models/domain/domain-service";
 import { LogService } from "@bitwarden/common/platform/abstractions/log.service";
 import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service";
+import { MessageListener } from "@bitwarden/common/platform/messaging";
 import { Utils } from "@bitwarden/common/platform/misc/utils";
 import { EventCollectionService } from "@bitwarden/common/services/event/event-collection.service";
 import {
   FakeStateProvider,
   FakeAccountService,
   mockAccountServiceWith,
+  subscribeTo,
 } from "@bitwarden/common/spec";
 import { UserId } from "@bitwarden/common/types/guid";
 import { FieldType, LinkedIdType, LoginLinkedId, CipherType } from "@bitwarden/common/vault/enums";
@@ -37,6 +39,7 @@ import { TotpService } from "@bitwarden/common/vault/services/totp.service";
 
 import { BrowserApi } from "../../platform/browser/browser-api";
 import { BrowserScriptInjectorService } from "../../platform/services/browser-script-injector.service";
+import { AutofillMessageCommand, AutofillMessageSender } from "../enums/autofill-message.enums";
 import { AutofillPort } from "../enums/autofill-port.enums";
 import AutofillField from "../models/autofill-field";
 import AutofillPageDetails from "../models/autofill-page-details";
@@ -52,6 +55,7 @@ import { flushPromises, triggerTestFailure } from "../spec/testing-utils";
 
 import {
   AutoFillOptions,
+  CollectPageDetailsResponseMessage,
   GenerateFillScriptOptions,
   PageDetail,
 } from "./abstractions/autofill.service";
@@ -82,6 +86,7 @@ describe("AutofillService", () => {
   const platformUtilsService = mock<PlatformUtilsService>();
   let activeAccountStatusMock$: BehaviorSubject<AuthenticationStatus>;
   let authService: MockProxy<AuthService>;
+  let messageListener: MockProxy<MessageListener>;
 
   beforeEach(() => {
     scriptInjectorService = new BrowserScriptInjectorService(platformUtilsService, logService);
@@ -91,6 +96,7 @@ describe("AutofillService", () => {
     activeAccountStatusMock$ = new BehaviorSubject(AuthenticationStatus.Unlocked);
     authService = mock<AuthService>();
     authService.activeAccountStatus$ = activeAccountStatusMock$;
+    messageListener = mock<MessageListener>();
     autofillService = new AutofillService(
       cipherService,
       autofillSettingsService,
@@ -103,10 +109,11 @@ describe("AutofillService", () => {
       scriptInjectorService,
       accountService,
       authService,
+      messageListener,
     );
-
     domainSettingsService = new DefaultDomainSettingsService(fakeStateProvider);
     domainSettingsService.equivalentDomains$ = of(mockEquivalentDomains);
+    jest.spyOn(BrowserApi, "tabSendMessage");
   });
 
   afterEach(() => {
@@ -114,6 +121,84 @@ describe("AutofillService", () => {
     mockReset(cipherService);
   });
 
+  describe("collectPageDetailsFromTab$", () => {
+    const tab = mock<chrome.tabs.Tab>({ id: 1 });
+    const messages = new Subject<CollectPageDetailsResponseMessage>();
+
+    function mockCollectPageDetailsResponseMessage(
+      tab: chrome.tabs.Tab,
+      webExtSender: chrome.runtime.MessageSender = mock<chrome.runtime.MessageSender>(),
+      sender: string = AutofillMessageSender.collectPageDetailsFromTabObservable,
+    ): CollectPageDetailsResponseMessage {
+      return mock<CollectPageDetailsResponseMessage>({
+        tab,
+        webExtSender,
+        sender,
+      });
+    }
+
+    beforeEach(() => {
+      messageListener.messages$.mockReturnValue(messages.asObservable());
+    });
+
+    it("sends a `collectPageDetails` message to the passed tab", () => {
+      autofillService.collectPageDetailsFromTab$(tab);
+
+      expect(BrowserApi.tabSendMessage).toHaveBeenCalledWith(tab, {
+        command: AutofillMessageCommand.collectPageDetails,
+        sender: AutofillMessageSender.collectPageDetailsFromTabObservable,
+        tab,
+      });
+    });
+
+    it("builds an array of page details from received `collectPageDetailsResponse` messages", async () => {
+      const topLevelSender = mock<chrome.runtime.MessageSender>({ tab, frameId: 0 });
+      const subFrameSender = mock<chrome.runtime.MessageSender>({ tab, frameId: 1 });
+
+      const tracker = subscribeTo(autofillService.collectPageDetailsFromTab$(tab));
+      const pausePromise = tracker.pauseUntilReceived(2);
+
+      messages.next(mockCollectPageDetailsResponseMessage(tab, topLevelSender));
+      messages.next(mockCollectPageDetailsResponseMessage(tab, subFrameSender));
+
+      await pausePromise;
+
+      expect(tracker.emissions[1].length).toBe(2);
+    });
+
+    it("ignores messages from a different tab", async () => {
+      const otherTab = mock<chrome.tabs.Tab>({ id: 2 });
+
+      const tracker = subscribeTo(autofillService.collectPageDetailsFromTab$(tab));
+      const pausePromise = tracker.pauseUntilReceived(1);
+
+      messages.next(mockCollectPageDetailsResponseMessage(tab));
+      messages.next(mockCollectPageDetailsResponseMessage(otherTab));
+
+      await pausePromise;
+
+      expect(tracker.emissions[1]).toBeUndefined();
+    });
+
+    it("ignores messages from a different sender", async () => {
+      const tracker = subscribeTo(autofillService.collectPageDetailsFromTab$(tab));
+      const pausePromise = tracker.pauseUntilReceived(1);
+
+      messages.next(mockCollectPageDetailsResponseMessage(tab));
+      messages.next(
+        mockCollectPageDetailsResponseMessage(
+          tab,
+          mock<chrome.runtime.MessageSender>(),
+          "some-other-sender",
+        ),
+      );
+
+      await pausePromise;
+
+      expect(tracker.emissions[1]).toBeUndefined();
+    });
+  });
+
   describe("loadAutofillScriptsOnInstall", () => {
     let tab1: chrome.tabs.Tab;
     let tab2: chrome.tabs.Tab;

apps/browser/src/autofill/services/autofill.service.ts

@@ -1,4 +1,4 @@
-import { firstValueFrom, startWith } from "rxjs";
+import { filter, firstValueFrom, Observable, scan, startWith } from "rxjs";
 import { pairwise } from "rxjs/operators";
 
 import { EventCollectionService } from "@bitwarden/common/abstractions/event/event-collection.service";
@@ -17,6 +17,7 @@ import {
   UriMatchStrategy,
 } from "@bitwarden/common/models/domain/domain-service";
 import { LogService } from "@bitwarden/common/platform/abstractions/log.service";
+import { MessageListener } from "@bitwarden/common/platform/messaging";
 import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service";
 import { TotpService } from "@bitwarden/common/vault/abstractions/totp.service";
 import { FieldType, CipherType } from "@bitwarden/common/vault/enums";
@@ -27,6 +28,7 @@ import { FieldView } from "@bitwarden/common/vault/models/view/field.view";
 import { BrowserApi } from "../../platform/browser/browser-api";
 import { ScriptInjectorService } from "../../platform/services/abstractions/script-injector.service";
 import { openVaultItemPasswordRepromptPopout } from "../../vault/popup/utils/vault-popout-window";
+import { AutofillMessageCommand, AutofillMessageSender } from "../enums/autofill-message.enums";
 import { AutofillPort } from "../enums/autofill-port.enums";
 import AutofillField from "../models/autofill-field";
 import AutofillPageDetails from "../models/autofill-page-details";
@@ -35,6 +37,7 @@ import AutofillScript from "../models/autofill-script";
 import {
   AutoFillOptions,
   AutofillService as AutofillServiceInterface,
+  COLLECT_PAGE_DETAILS_RESPONSE_COMMAND,
   FormData,
   GenerateFillScriptOptions,
   PageDetail,
@@ -64,8 +67,47 @@ export default class AutofillService implements AutofillServiceInterface {
     private scriptInjectorService: ScriptInjectorService,
     private accountService: AccountService,
     private authService: AuthService,
+    private messageListener: MessageListener,
   ) {}
 
+  /**
+   * Collects page details from the specific tab. This method returns an observable that can
+   * be subscribed to in order to build the results from all collectPageDetailsResponse
+   * messages from the given tab.
+   *
+   * @param tab The tab to collect page details from
+   */
+  collectPageDetailsFromTab$(tab: chrome.tabs.Tab): Observable<PageDetail[]> {
+    const pageDetailsFromTab$ = this.messageListener
+      .messages$(COLLECT_PAGE_DETAILS_RESPONSE_COMMAND)
+      .pipe(
+        filter(
+          (message) =>
+            message.tab.id === tab.id &&
+            message.sender === AutofillMessageSender.collectPageDetailsFromTabObservable,
+        ),
+        scan(
+          (acc, message) => [
+            ...acc,
+            {
+              frameId: message.webExtSender.frameId,
+              tab: message.tab,
+              details: message.details,
+            },
+          ],
+          [] as PageDetail[],
+        ),
+      );
+
+    void BrowserApi.tabSendMessage(tab, {
+      tab: tab,
+      command: AutofillMessageCommand.collectPageDetails,
+      sender: AutofillMessageSender.collectPageDetailsFromTabObservable,
+    });
+
+    return pageDetailsFromTab$;
+  }
+
   /**
    * Triggers on installation of the extension Handles injecting
    * content scripts into all tabs that are currently open, and

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

@@ -889,6 +889,7 @@ export default class MainBackground {
       this.scriptInjectorService,
       this.accountService,
       this.authService,
+      messageListener,
     );
     this.auditService = new AuditService(this.cryptoFunctionService, this.apiService);
 

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

@@ -342,6 +342,7 @@ const safeProviders: SafeProvider[] = [
       ScriptInjectorService,
       AccountServiceAbstraction,
       AuthService,
+      MessageListener,
     ],
   }),
   safeProvider({

apps/browser/src/vault/popup/components/vault/current-tab.component.ts

@@ -1,6 +1,6 @@
 import { ChangeDetectorRef, Component, NgZone, OnDestroy, OnInit } from "@angular/core";
 import { Router } from "@angular/router";
-import { Subject, firstValueFrom, from } from "rxjs";
+import { Subject, firstValueFrom, from, Subscription } from "rxjs";
 import { debounceTime, switchMap, takeUntil } from "rxjs/operators";
 
 import { UnassignedItemsBannerService } from "@bitwarden/angular/services/unassigned-items-banner.service";
@@ -51,12 +51,12 @@ export class CurrentTabComponent implements OnInit, OnDestroy {
   autofillCalloutText: string;
   protected search$ = new Subject<void>();
   private destroy$ = new Subject<void>();
+  private collectPageDetailsSubscription: Subscription;
 
   private totpCode: string;
   private totpTimeout: number;
   private loadedTimeout: number;
   private searchTimeout: number;
-  private initPageDetailsTimeout: number;
 
   protected unassignedItemsBannerEnabled$ = this.configService.getFeatureFlag$(
     FeatureFlag.UnassignedItemsBanner,
@@ -100,15 +100,6 @@ export class CurrentTabComponent implements OnInit, OnDestroy {
               }, 500);
             }
             break;
-          case "collectPageDetailsResponse":
-            if (message.sender === BroadcasterSubscriptionId) {
-              this.pageDetails.push({
-                frameId: message.webExtSender.frameId,
-                tab: message.tab,
-                details: message.details,
-              });
-            }
-            break;
           default:
             break;
         }
@@ -266,6 +257,7 @@ export class CurrentTabComponent implements OnInit, OnDestroy {
   protected async load() {
     this.isLoading = false;
     this.tab = await BrowserApi.getTabFromCurrentWindow();
+
     if (this.tab != null) {
       this.url = this.tab.url;
     } else {
@@ -274,8 +266,14 @@ export class CurrentTabComponent implements OnInit, OnDestroy {
       return;
     }
 
-    this.hostname = Utils.getHostname(this.url);
     this.pageDetails = [];
+    this.collectPageDetailsSubscription?.unsubscribe();
+    this.collectPageDetailsSubscription = this.autofillService
+      .collectPageDetailsFromTab$(this.tab)
+      .pipe(takeUntil(this.destroy$))
+      .subscribe((pageDetails) => (this.pageDetails = pageDetails));
+
+    this.hostname = Utils.getHostname(this.url);
     const otherTypes: CipherType[] = [];
     const dontShowCards = !(await firstValueFrom(this.vaultSettingsService.showCardsCurrentTab$));
     const dontShowIdentities = !(await firstValueFrom(
@@ -323,7 +321,6 @@ export class CurrentTabComponent implements OnInit, OnDestroy {
     }
 
     this.isLoading = this.loaded = true;
-    this.collectTabPageDetails();
   }
 
   async goToSettings() {
@@ -361,19 +358,4 @@ export class CurrentTabComponent implements OnInit, OnDestroy {
       this.autofillCalloutText = this.i18nService.t("autofillSelectInfoWithoutCommand");
     }
   }
-
-  private collectTabPageDetails() {
-    void BrowserApi.tabSendMessage(this.tab, {
-      command: "collectPageDetails",
-      tab: this.tab,
-      sender: BroadcasterSubscriptionId,
-    });
-
-    window.clearTimeout(this.initPageDetailsTimeout);
-    this.initPageDetailsTimeout = window.setTimeout(() => {
-      if (this.pageDetails.length === 0) {
-        this.collectTabPageDetails();
-      }
-    }, 250);
-  }
 }

apps/browser/src/vault/popup/components/vault/view.component.ts

@@ -1,7 +1,7 @@
 import { DatePipe, Location } from "@angular/common";
 import { ChangeDetectorRef, Component, NgZone } from "@angular/core";
 import { ActivatedRoute, Router } from "@angular/router";
-import { Subject, firstValueFrom, takeUntil } from "rxjs";
+import { Subject, firstValueFrom, takeUntil, Subscription } from "rxjs";
 import { first } from "rxjs/operators";
 
 import { ViewComponent as BaseViewComponent } from "@bitwarden/angular/vault/components/view.component";
@@ -68,6 +68,7 @@ export class ViewComponent extends BaseViewComponent {
   inPopout = false;
   cipherType = CipherType;
   private fido2PopoutSessionData$ = fido2PopoutSessionData$();
+  private collectPageDetailsSubscription: Subscription;
 
   private destroy$ = new Subject<void>();
 
@@ -152,15 +153,6 @@ export class ViewComponent extends BaseViewComponent {
       // eslint-disable-next-line @typescript-eslint/no-floating-promises
       this.ngZone.run(async () => {
         switch (message.command) {
-          case "collectPageDetailsResponse":
-            if (message.sender === BroadcasterSubscriptionId) {
-              this.pageDetails.push({
-                frameId: message.webExtSender.frameId,
-                tab: message.tab,
-                details: message.details,
-              });
-            }
-            break;
           case "tabChanged":
           case "windowChanged":
             if (this.loadPageDetailsTimeout != null) {
@@ -337,6 +329,7 @@ export class ViewComponent extends BaseViewComponent {
   }
 
   private async loadPageDetails() {
+    this.collectPageDetailsSubscription?.unsubscribe();
     this.pageDetails = [];
     this.tab = this.senderTabId
       ? await BrowserApi.getTab(this.senderTabId)
@@ -346,13 +339,10 @@ export class ViewComponent extends BaseViewComponent {
       return;
     }
 
-    // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling.
+    this.collectPageDetailsSubscription = this.autofillService
-    // eslint-disable-next-line @typescript-eslint/no-floating-promises
+      .collectPageDetailsFromTab$(this.tab)
-    BrowserApi.tabSendMessage(this.tab, {
+      .pipe(takeUntil(this.destroy$))
-      command: "collectPageDetails",
+      .subscribe((pageDetails) => (this.pageDetails = pageDetails));
-      tab: this.tab,
-      sender: BroadcasterSubscriptionId,
-    });
   }
 
   private async doAutofill() {