feat(account-switching) [PM-5594]: Enabling account switching causes performance issues on safari (#18339)

* refactor(account-switching) [PM-5594]: Move account switching enabled flag to AccountSwitcherService to accommodate server-side feature flag.

* test(account-switching) [PM-5594]: Update tests to include ConfigService dependency for feature flag.

* refactor(account-switching) [PM-5594]: Remove compile-time account switching flags from browser.

* refactor(account-switching) [PM-5594]: Move initialization to ctor for strict.

apps/browser/config/base.json

@@ -1,7 +1,6 @@
 {
   "devFlags": {},
   "flags": {
-    "accountSwitching": false,
     "sdk": true
   }
 }

apps/browser/config/development.json

@@ -4,8 +4,5 @@
       "base": "https://localhost:8080"
     },
     "skipWelcomeOnInstall": true
-  },
-  "flags": {
-    "accountSwitching": true
   }
 }

apps/browser/config/production.json

@@ -1,5 +0,0 @@
-{
-  "flags": {
-    "accountSwitching": true
-  }
-}

apps/browser/src/auth/popup/account-switching/account-switcher.component.html

@@ -7,13 +7,16 @@
   </popup-header>
 
   <ng-container *ngIf="availableAccounts$ | async as availableAccounts">
-    <bit-section [disableMargin]="!enableAccountSwitching">
+    <bit-section [disableMargin]="!(enableAccountSwitching$ | async)">
       <ng-container *ngFor="let availableAccount of availableAccounts; first as isFirst">
-        <div *ngIf="availableAccount.isActive" [ngClass]="{ 'tw-mb-6': enableAccountSwitching }">
+        <div
+          *ngIf="availableAccount.isActive"
+          [ngClass]="{ 'tw-mb-6': enableAccountSwitching$ | async }"
+        >
           <auth-account [account]="availableAccount" (loading)="loading = $event"></auth-account>
         </div>
 
-        <ng-container *ngIf="enableAccountSwitching">
+        <ng-container *ngIf="enableAccountSwitching$ | async">
           <bit-section-header *ngIf="isFirst">
             <h2 bitTypography="h6">{{ "availableAccounts" | i18n }}</h2>
           </bit-section-header>

apps/browser/src/auth/popup/account-switching/account-switcher.component.ts

@@ -1,7 +1,7 @@
 import { CommonModule, Location } from "@angular/common";
 import { Component, OnDestroy, OnInit } from "@angular/core";
 import { Router } from "@angular/router";
-import { Subject, firstValueFrom, map, of, startWith, switchMap } from "rxjs";
+import { Observable, Subject, firstValueFrom, map, of, startWith, switchMap } from "rxjs";
 
 import { JslibModule } from "@bitwarden/angular/jslib.module";
 import { LockService, LogoutService } from "@bitwarden/auth/common";
@@ -24,7 +24,6 @@ import {
   TypographyModule,
 } from "@bitwarden/components";
 
-import { enableAccountSwitching } from "../../../platform/flags";
 import { PopOutComponent } from "../../../platform/popup/components/pop-out.component";
 import { PopupHeaderComponent } from "../../../platform/popup/layout/popup-header.component";
 import { PopupPageComponent } from "../../../platform/popup/layout/popup-page.component";
@@ -59,7 +58,7 @@ export class AccountSwitcherComponent implements OnInit, OnDestroy {
 
   loading = false;
   activeUserCanLock = false;
-  enableAccountSwitching = true;
+  enableAccountSwitching$: Observable<boolean>;
 
   constructor(
     private accountSwitcherService: AccountSwitcherService,
@@ -72,7 +71,9 @@ export class AccountSwitcherComponent implements OnInit, OnDestroy {
     private authService: AuthService,
     private lockService: LockService,
     private logoutService: LogoutService,
-  ) {}
+  ) {
+    this.enableAccountSwitching$ = this.accountSwitcherService.accountSwitchingEnabled$();
+  }
 
   get accountLimit() {
     return this.accountSwitcherService.ACCOUNT_LIMIT;
@@ -97,19 +98,21 @@ export class AccountSwitcherComponent implements OnInit, OnDestroy {
     switchMap((accounts) => {
       // If account switching is disabled, don't show the lock all button
       // as only one account should be shown.
-      if (!enableAccountSwitching()) {
-        return of(false);
-      }
+      return this.accountSwitcherService.accountSwitchingEnabled$().pipe(
+        switchMap((enabled) => {
+          if (!enabled) {
+            return of(false);
+          }
 
-      // When there are an inactive accounts provide the option to lock all accounts
-      // Note: "Add account" is counted as an inactive account, so check for more than one account
-      return of(accounts.length > 1);
+          // When there are inactive accounts provide the option to lock all accounts
+          // Note: "Add account" is counted as an inactive account, so check for more than one account
+          return of(accounts.length > 1);
+        }),
+      );
     }),
   );
 
   async ngOnInit() {
-    this.enableAccountSwitching = enableAccountSwitching();
-
     const availableVaultTimeoutActions = await firstValueFrom(
       this.vaultTimeoutSettingsService.availableVaultTimeoutActions$(),
     );

apps/browser/src/auth/popup/account-switching/services/account-switcher.service.spec.ts

@@ -9,6 +9,7 @@ import {
 import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service";
 import { AvatarService } from "@bitwarden/common/auth/abstractions/avatar.service";
 import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status";
+import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service";
 import {
   Environment,
   EnvironmentService,
@@ -37,6 +38,7 @@ describe("AccountSwitcherService", () => {
   const environmentService = mock<EnvironmentService>();
   const logService = mock<LogService>();
   const authService = mock<AuthService>();
+  const configService = mock<ConfigService>();
 
   let accountSwitcherService: AccountSwitcherService;
 
@@ -60,6 +62,7 @@ describe("AccountSwitcherService", () => {
       messagingService,
       environmentService,
       logService,
+      configService,
       authService,
     );
   });

apps/browser/src/auth/popup/account-switching/services/account-switcher.service.ts

@@ -7,6 +7,7 @@ import {
   filter,
   firstValueFrom,
   map,
+  of,
   switchMap,
   throwError,
   timeout,
@@ -17,11 +18,14 @@ import { AccountService } from "@bitwarden/common/auth/abstractions/account.serv
 import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service";
 import { AvatarService } from "@bitwarden/common/auth/abstractions/avatar.service";
 import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status";
+import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum";
+import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service";
 import { EnvironmentService } from "@bitwarden/common/platform/abstractions/environment.service";
 import { LogService } from "@bitwarden/common/platform/abstractions/log.service";
 import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service";
 import { UserId } from "@bitwarden/common/types/guid";
 
+import { BrowserApi } from "../../../../platform/browser/browser-api";
 import { fromChromeEvent } from "../../../../platform/browser/from-chrome-event";
 
 export type AvailableAccount = {
@@ -52,6 +56,7 @@ export class AccountSwitcherService {
     private messagingService: MessagingService,
     private environmentService: EnvironmentService,
     private logService: LogService,
+    private configService: ConfigService,
     authService: AuthService,
   ) {
     this.availableAccounts$ = combineLatest([
@@ -123,6 +128,19 @@ export class AccountSwitcherService {
     );
   }
 
+  /*
+   * PM-5594: This was a compile-time flag (default true) which made an exception for Safari in platform/flags.
+   * The truthiness of AccountSwitching has been enshrined at this point, so those compile-time flags have been removed
+   * in favor of this method to allow easier access to the config service for controlling Safari. Unwinding the Safari
+   * flag should be more straightforward from this consolidation.
+   */
+  accountSwitchingEnabled$(): Observable<boolean> {
+    if (BrowserApi.isSafariApi) {
+      return this.configService.getFeatureFlag$(FeatureFlag.SafariAccountSwitching);
+    }
+    return of(true);
+  }
+
   get specialAccountAddId() {
     return this.SPECIAL_ADD_ACCOUNT_ID;
   }

apps/browser/src/autofill/popup/settings/excluded-domains.component.html

@@ -8,7 +8,9 @@
   <div class="tw-bg-background-alt">
     <p>
       {{
-        accountSwitcherEnabled ? ("excludedDomainsDescAlt" | i18n) : ("excludedDomainsDesc" | i18n)
+        (accountSwitcherEnabled$ | async)
+          ? ("excludedDomainsDescAlt" | i18n)
+          : ("excludedDomainsDesc" | i18n)
       }}
     </p>
     <bit-section *ngIf="!isLoading">

apps/browser/src/autofill/popup/settings/excluded-domains.component.ts

@@ -15,7 +15,7 @@ import {
   FormArray,
 } from "@angular/forms";
 import { RouterModule } from "@angular/router";
-import { Subject, takeUntil } from "rxjs";
+import { Observable, Subject, takeUntil } from "rxjs";
 
 import { JslibModule } from "@bitwarden/angular/jslib.module";
 import { DomainSettingsService } from "@bitwarden/common/autofill/services/domain-settings.service";
@@ -35,7 +35,7 @@ import {
   TypographyModule,
 } from "@bitwarden/components";
 
-import { enableAccountSwitching } from "../../../platform/flags";
+import { AccountSwitcherService } from "../../../auth/popup/account-switching/services/account-switcher.service";
 import { PopOutComponent } from "../../../platform/popup/components/pop-out.component";
 import { PopupFooterComponent } from "../../../platform/popup/layout/popup-footer.component";
 import { PopupHeaderComponent } from "../../../platform/popup/layout/popup-header.component";
@@ -74,7 +74,8 @@ export class ExcludedDomainsComponent implements AfterViewInit, OnDestroy {
   @ViewChildren("uriInput") uriInputElements: QueryList<ElementRef<HTMLInputElement>> =
     new QueryList();
 
-  accountSwitcherEnabled = false;
+  readonly accountSwitcherEnabled$: Observable<boolean> =
+    this.accountSwitcherService.accountSwitchingEnabled$();
   dataIsPristine = true;
   isLoading = false;
   excludedDomainsState: string[] = [];
@@ -95,9 +96,8 @@ export class ExcludedDomainsComponent implements AfterViewInit, OnDestroy {
     private toastService: ToastService,
     private formBuilder: FormBuilder,
     private popupRouterCacheService: PopupRouterCacheService,
-  ) {
-    this.accountSwitcherEnabled = enableAccountSwitching();
-  }
+    private accountSwitcherService: AccountSwitcherService,
+  ) {}
 
   get domainForms() {
     return this.domainListForm.get("domains") as FormArray;

apps/browser/src/platform/flags.ts

@@ -8,12 +8,8 @@ import {
 
 import { GroupPolicyEnvironment } from "../admin-console/types/group-policy-environment";
 
-import { BrowserApi } from "./browser/browser-api";
-
 // required to avoid linting errors when there are no flags
-export type Flags = {
-  accountSwitching?: boolean;
-} & SharedFlags;
+export type Flags = SharedFlags;
 
 // required to avoid linting errors when there are no flags
 export type DevFlags = {
@@ -31,14 +27,3 @@ export function devFlagEnabled(flag: keyof DevFlags) {
 export function devFlagValue(flag: keyof DevFlags) {
   return baseDevFlagValue(flag);
 }
-
-/** Helper method to sync flag specifically for account switching, which as platform-based values.
- * If this pattern needs to be repeated, it's better handled by increasing complexity of webpack configurations
- * Not by expanding these flag getters.
- */
-export function enableAccountSwitching(): boolean {
-  if (BrowserApi.isSafariApi) {
-    return false;
-  }
-  return flagEnabled("accountSwitching");
-}

libs/common/src/enums/feature-flag.enum.ts

@@ -18,6 +18,7 @@ export enum FeatureFlag {
 
   /* Auth */
   PM23801_PrefetchPasswordPrelogin = "pm-23801-prefetch-password-prelogin",
+  SafariAccountSwitching = "pm-5594-safari-account-switching",
 
   /* Autofill */
   MacOsNativeCredentialSync = "macos-native-credential-sync",
@@ -134,6 +135,7 @@ export const DefaultFeatureFlagValue = {
 
   /* Auth */
   [FeatureFlag.PM23801_PrefetchPasswordPrelogin]: FALSE,
+  [FeatureFlag.SafariAccountSwitching]: FALSE,
 
   /* Billing */
   [FeatureFlag.TrialPaymentOptional]: FALSE,