[PM-8289] Inline menu content script does not update when user updates setting (#9279)

* [PM-8289] Inline menu content script does not update whne user updates setting

* [PM-8289] Fixing issue present within Jest tests

* [PM-8289] Triggering a reload of autofill scripts when a user logs into their account

apps/browser/src/autofill/popup/settings/autofill.component.ts

@@ -105,11 +105,7 @@ export class AutofillComponent implements OnInit {
   }
 
   async updateAutoFillOverlayVisibility() {
-    const previousAutoFillOverlayVisibility = await firstValueFrom(
-      this.autofillSettingsService.inlineMenuVisibility$,
-    );
     await this.autofillSettingsService.setInlineMenuVisibility(this.autoFillOverlayVisibility);
-    await this.handleUpdatingAutofillOverlayContentScripts(previousAutoFillOverlayVisibility);
     await this.requestPrivacyPermission();
   }
 
@@ -181,27 +177,6 @@ export class AutofillComponent implements OnInit {
     BrowserApi.createNewTab(this.disablePasswordManagerLink);
   }
 
-  private async handleUpdatingAutofillOverlayContentScripts(
-    previousAutoFillOverlayVisibility: number,
-  ) {
-    const autofillOverlayPreviouslyDisabled =
-      previousAutoFillOverlayVisibility === AutofillOverlayVisibility.Off;
-    const autofillOverlayCurrentlyDisabled =
-      this.autoFillOverlayVisibility === AutofillOverlayVisibility.Off;
-
-    if (!autofillOverlayPreviouslyDisabled && !autofillOverlayCurrentlyDisabled) {
-      const tabs = await BrowserApi.tabsQuery({});
-      tabs.forEach((tab) =>
-        BrowserApi.tabSendMessageData(tab, "updateAutofillOverlayVisibility", {
-          autofillOverlayVisibility: this.autoFillOverlayVisibility,
-        }),
-      );
-      return;
-    }
-
-    await this.autofillService.reloadAutofillScripts();
-  }
-
   async requestPrivacyPermission() {
     if (
       this.autoFillOverlayVisibility === AutofillOverlayVisibility.Off ||

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

@@ -1,5 +1,5 @@
-import { mock, mockReset } from "jest-mock-extended";
+import { mock, mockReset, MockProxy } from "jest-mock-extended";
-import { of } from "rxjs";
+import { BehaviorSubject, of } from "rxjs";
 
 import { UserVerificationService } from "@bitwarden/common/auth/services/user-verification/user-verification.service";
 import { AutofillOverlayVisibility } from "@bitwarden/common/autofill/constants";
@@ -8,6 +8,7 @@ import {
   DefaultDomainSettingsService,
   DomainSettingsService,
 } from "@bitwarden/common/autofill/services/domain-settings.service";
+import { InlineMenuVisibilitySetting } from "@bitwarden/common/autofill/types";
 import { BillingAccountProfileStateService } from "@bitwarden/common/billing/abstractions/account/billing-account-profile-state.service";
 import { EventType } from "@bitwarden/common/enums";
 import { UriMatchStrategy } from "@bitwarden/common/models/domain/domain-service";
@@ -45,7 +46,7 @@ import {
   createChromeTabMock,
   createGenerateFillScriptOptionsMock,
 } from "../spec/autofill-mocks";
-import { triggerTestFailure } from "../spec/testing-utils";
+import { flushPromises, triggerTestFailure } from "../spec/testing-utils";
 
 import {
   AutoFillOptions,
@@ -64,7 +65,8 @@ const mockEquivalentDomains = [
 describe("AutofillService", () => {
   let autofillService: AutofillService;
   const cipherService = mock<CipherService>();
-  const autofillSettingsService = mock<AutofillSettingsService>();
+  let inlineMenuVisibilityMock$!: BehaviorSubject<InlineMenuVisibilitySetting>;
+  let autofillSettingsService: MockProxy<AutofillSettingsService>;
   const mockUserId = Utils.newGuid() as UserId;
   const accountService: FakeAccountService = mockAccountServiceWith(mockUserId);
   const fakeStateProvider: FakeStateProvider = new FakeStateProvider(accountService);
@@ -79,6 +81,9 @@ describe("AutofillService", () => {
 
   beforeEach(() => {
     scriptInjectorService = new BrowserScriptInjectorService(platformUtilsService, logService);
+    inlineMenuVisibilityMock$ = new BehaviorSubject(AutofillOverlayVisibility.OnFieldFocus);
+    autofillSettingsService = mock<AutofillSettingsService>();
+    (autofillSettingsService as any).inlineMenuVisibility$ = inlineMenuVisibilityMock$;
     autofillService = new AutofillService(
       cipherService,
       autofillSettingsService,
@@ -142,17 +147,92 @@ describe("AutofillService", () => {
       // eslint-disable-next-line no-restricted-syntax
       expect(chrome.runtime.onConnect.addListener).toHaveBeenCalledWith(expect.any(Function));
     });
+
+    describe("handle inline menu visibility change", () => {
+      beforeEach(async () => {
+        await autofillService.loadAutofillScriptsOnInstall();
+        jest.spyOn(BrowserApi, "tabsQuery").mockResolvedValue([tab1, tab2]);
+        jest.spyOn(BrowserApi, "tabSendMessageData").mockImplementation();
+        jest.spyOn(autofillService, "reloadAutofillScripts").mockImplementation();
+      });
+
+      it("returns early if the setting is being initialized", async () => {
+        await flushPromises();
+
+        expect(BrowserApi.tabsQuery).toHaveBeenCalledTimes(1);
+        expect(BrowserApi.tabSendMessageData).not.toHaveBeenCalled();
+      });
+
+      it("returns early if the previous setting is equivalent to the new setting", async () => {
+        inlineMenuVisibilityMock$.next(AutofillOverlayVisibility.OnFieldFocus);
+        await flushPromises();
+
+        expect(BrowserApi.tabsQuery).toHaveBeenCalledTimes(1);
+        expect(BrowserApi.tabSendMessageData).not.toHaveBeenCalled();
+      });
+
+      describe("updates the inline menu visibility setting", () => {
+        it("when changing the inline menu from on focus of field to on button click", async () => {
+          inlineMenuVisibilityMock$.next(AutofillOverlayVisibility.OnButtonClick);
+          await flushPromises();
+
+          expect(BrowserApi.tabSendMessageData).toHaveBeenCalledWith(
+            tab1,
+            "updateAutofillOverlayVisibility",
+            { autofillOverlayVisibility: AutofillOverlayVisibility.OnButtonClick },
+          );
+          expect(BrowserApi.tabSendMessageData).toHaveBeenCalledWith(
+            tab2,
+            "updateAutofillOverlayVisibility",
+            { autofillOverlayVisibility: AutofillOverlayVisibility.OnButtonClick },
+          );
+        });
+
+        it("when changing the inline menu from button click to field focus", async () => {
+          inlineMenuVisibilityMock$.next(AutofillOverlayVisibility.OnButtonClick);
+          inlineMenuVisibilityMock$.next(AutofillOverlayVisibility.OnFieldFocus);
+          await flushPromises();
+
+          expect(BrowserApi.tabSendMessageData).toHaveBeenCalledWith(
+            tab1,
+            "updateAutofillOverlayVisibility",
+            { autofillOverlayVisibility: AutofillOverlayVisibility.OnFieldFocus },
+          );
+          expect(BrowserApi.tabSendMessageData).toHaveBeenCalledWith(
+            tab2,
+            "updateAutofillOverlayVisibility",
+            { autofillOverlayVisibility: AutofillOverlayVisibility.OnFieldFocus },
+          );
+        });
+      });
+
+      describe("reloads the autofill scripts", () => {
+        it("when changing the inline menu from a disabled setting to an enabled setting", async () => {
+          inlineMenuVisibilityMock$.next(AutofillOverlayVisibility.Off);
+          inlineMenuVisibilityMock$.next(AutofillOverlayVisibility.OnFieldFocus);
+          await flushPromises();
+
+          expect(autofillService.reloadAutofillScripts).toHaveBeenCalled();
+        });
+
+        it("when changing the inline menu from a enabled setting to a disabled setting", async () => {
+          inlineMenuVisibilityMock$.next(AutofillOverlayVisibility.OnFieldFocus);
+          inlineMenuVisibilityMock$.next(AutofillOverlayVisibility.Off);
+          await flushPromises();
+
+          expect(autofillService.reloadAutofillScripts).toHaveBeenCalled();
+        });
+      });
+    });
   });
 
   describe("reloadAutofillScripts", () => {
-    it("disconnects and removes all autofill script ports", () => {
+    it("re-injects the autofill scripts in all tabs and disconnects all connected ports", () => {
-      const port1 = mock<chrome.runtime.Port>({
+      const port1 = mock<chrome.runtime.Port>();
-        disconnect: jest.fn(),
+      const port2 = mock<chrome.runtime.Port>();
-      });
-      const port2 = mock<chrome.runtime.Port>({
-        disconnect: jest.fn(),
-      });
       autofillService["autofillScriptPortsSet"] = new Set([port1, port2]);
+      jest.spyOn(autofillService as any, "injectAutofillScriptsInAllTabs");
+      jest.spyOn(autofillService, "getAutofillOnPageLoad").mockResolvedValue(true);
 
       // 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
@@ -161,17 +241,6 @@ describe("AutofillService", () => {
       expect(port1.disconnect).toHaveBeenCalled();
       expect(port2.disconnect).toHaveBeenCalled();
       expect(autofillService["autofillScriptPortsSet"].size).toBe(0);
-    });
-
-    it("re-injects the autofill scripts in all tabs", () => {
-      autofillService["autofillScriptPortsSet"] = new Set([mock<chrome.runtime.Port>()]);
-      jest.spyOn(autofillService as any, "injectAutofillScriptsInAllTabs");
-      jest.spyOn(autofillService, "getAutofillOnPageLoad").mockResolvedValue(true);
-
-      // 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
-      autofillService.reloadAutofillScripts();
-
       expect(autofillService["injectAutofillScriptsInAllTabs"]).toHaveBeenCalled();
     });
   });

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

@@ -1,4 +1,5 @@
-import { firstValueFrom } from "rxjs";
+import { firstValueFrom, startWith } from "rxjs";
+import { pairwise } from "rxjs/operators";
 
 import { EventCollectionService } from "@bitwarden/common/abstractions/event/event-collection.service";
 import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
@@ -70,10 +71,12 @@ export default class AutofillService implements AutofillServiceInterface {
    */
   async loadAutofillScriptsOnInstall() {
     BrowserApi.addListener(chrome.runtime.onConnect, this.handleInjectedScriptPortConnection);
-
+    void this.injectAutofillScriptsInAllTabs();
-    // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling.
+    this.autofillSettingsService.inlineMenuVisibility$
-    // eslint-disable-next-line @typescript-eslint/no-floating-promises
+      .pipe(startWith(undefined), pairwise())
-    this.injectAutofillScriptsInAllTabs();
+      .subscribe(([previousSetting, currentSetting]) =>
+        this.handleInlineMenuVisibilityChange(previousSetting, currentSetting),
+      );
   }
 
   /**
@@ -2090,4 +2093,34 @@ export default class AutofillService implements AutofillServiceInterface {
       }
     }
   }
+
+  /**
+   * Updates the autofill inline menu visibility setting in all active tabs
+   * when the InlineMenuVisibilitySetting observable is updated.
+   *
+   * @param previousSetting - The previous setting value
+   * @param currentSetting - The current setting value
+   */
+  private async handleInlineMenuVisibilityChange(
+    previousSetting: InlineMenuVisibilitySetting,
+    currentSetting: InlineMenuVisibilitySetting,
+  ) {
+    if (previousSetting === undefined || previousSetting === currentSetting) {
+      return;
+    }
+
+    const inlineMenuPreviouslyDisabled = previousSetting === AutofillOverlayVisibility.Off;
+    const inlineMenuCurrentlyDisabled = currentSetting === AutofillOverlayVisibility.Off;
+    if (!inlineMenuPreviouslyDisabled && !inlineMenuCurrentlyDisabled) {
+      const tabs = await BrowserApi.tabsQuery({});
+      tabs.forEach((tab) =>
+        BrowserApi.tabSendMessageData(tab, "updateAutofillOverlayVisibility", {
+          autofillOverlayVisibility: currentSetting,
+        }),
+      );
+      return;
+    }
+
+    await this.reloadAutofillScripts();
+  }
 }

apps/browser/src/autofill/services/collect-autofill-content.service.ts

@@ -1349,6 +1349,10 @@ class CollectAutofillContentService implements CollectAutofillContentServiceInte
       }
 
       const cachedAutofillFieldElement = this.autofillFieldElements.get(formFieldElement);
+      if (!cachedAutofillFieldElement) {
+        continue;
+      }
+
       cachedAutofillFieldElement.viewable = true;
 
       void this.autofillOverlayContentService?.setupAutofillOverlayListenerOnField(

apps/browser/src/background/runtime.background.ts

@@ -188,6 +188,7 @@ export default class RuntimeBackground {
 
         if (msg.command === "loggedIn") {
           await this.sendBwInstalledMessageToVault();
+          await this.autofillService.reloadAutofillScripts();
         }
 
         if (this.lockedVaultPendingNotifications?.length > 0) {