[PM-5540] DesktopSettingsService (#8369)

* WIP: First Try at making DesktopSettingsService

Does not work, migrations are ran in renderer but the values are read in main.

* Update window$ retrieval

* Fix DesktopSettings

* Rename Migration

* Add Migration to Builder

* Cleanup

* Update Comments

* Update `migrate.ts`

* Catch Unawaited Promises

* Remove Comments

* Update Tests

* Rename Migration

* Add `alwaysOnTop`

* Make `init` async

* Fix Desktop Build

apps/desktop/src/app/accounts/settings.component.ts

@@ -120,8 +120,8 @@ export class SettingsComponent implements OnInit {
     private domainSettingsService: DomainSettingsService,
     private dialogService: DialogService,
     private userVerificationService: UserVerificationServiceAbstraction,
-    private biometricStateService: BiometricStateService,
     private desktopSettingsService: DesktopSettingsService,
+    private biometricStateService: BiometricStateService,
   ) {
     const isMac = this.platformUtilsService.getDevice() === DeviceType.MacOsDesktop;
 
@@ -253,12 +253,12 @@ export class SettingsComponent implements OnInit {
       clearClipboard: await firstValueFrom(this.autofillSettingsService.clearClipboardDelay$),
       minimizeOnCopyToClipboard: await this.stateService.getMinimizeOnCopyToClipboard(),
       enableFavicons: await firstValueFrom(this.domainSettingsService.showFavicons$),
-      enableTray: await this.stateService.getEnableTray(),
-      enableMinToTray: await this.stateService.getEnableMinimizeToTray(),
-      enableCloseToTray: await this.stateService.getEnableCloseToTray(),
-      startToTray: await this.stateService.getEnableStartToTray(),
-      openAtLogin: await this.stateService.getOpenAtLogin(),
-      alwaysShowDock: await this.stateService.getAlwaysShowDock(),
+      enableTray: await firstValueFrom(this.desktopSettingsService.trayEnabled$),
+      enableMinToTray: await firstValueFrom(this.desktopSettingsService.minimizeToTray$),
+      enableCloseToTray: await firstValueFrom(this.desktopSettingsService.closeToTray$),
+      startToTray: await firstValueFrom(this.desktopSettingsService.startToTray$),
+      openAtLogin: await firstValueFrom(this.desktopSettingsService.openAtLogin$),
+      alwaysShowDock: await firstValueFrom(this.desktopSettingsService.alwaysShowDock$),
       enableBrowserIntegration: await this.stateService.getEnableBrowserIntegration(),
       enableBrowserIntegrationFingerprint:
         await this.stateService.getEnableBrowserIntegrationFingerprint(),
@@ -507,16 +507,16 @@ export class SettingsComponent implements OnInit {
   }
 
   async saveMinToTray() {
-    await this.stateService.setEnableMinimizeToTray(this.form.value.enableMinToTray);
+    await this.desktopSettingsService.setMinimizeToTray(this.form.value.enableMinToTray);
   }
 
   async saveCloseToTray() {
     if (this.requireEnableTray) {
       this.form.controls.enableTray.setValue(true);
-      await this.stateService.setEnableTray(this.form.value.enableTray);
+      await this.desktopSettingsService.setTrayEnabled(this.form.value.enableTray);
     }
 
-    await this.stateService.setEnableCloseToTray(this.form.value.enableCloseToTray);
+    await this.desktopSettingsService.setCloseToTray(this.form.value.enableCloseToTray);
   }
 
   async saveTray() {
@@ -533,9 +533,9 @@ export class SettingsComponent implements OnInit {
 
       if (confirm) {
         this.form.controls.startToTray.setValue(false, { emitEvent: false });
-        await this.stateService.setEnableStartToTray(this.form.value.startToTray);
+        await this.desktopSettingsService.setStartToTray(this.form.value.startToTray);
         this.form.controls.enableCloseToTray.setValue(false, { emitEvent: false });
-        await this.stateService.setEnableCloseToTray(this.form.value.enableCloseToTray);
+        await this.desktopSettingsService.setCloseToTray(this.form.value.enableCloseToTray);
       } else {
         this.form.controls.enableTray.setValue(true);
       }
@@ -543,17 +543,18 @@ export class SettingsComponent implements OnInit {
       return;
     }
 
-    await this.stateService.setEnableTray(this.form.value.enableTray);
+    await this.desktopSettingsService.setTrayEnabled(this.form.value.enableTray);
+    // TODO: Ideally the DesktopSettingsService.trayEnabled$ could be subscribed to instead of using messaging.
     this.messagingService.send(this.form.value.enableTray ? "showTray" : "removeTray");
   }
 
   async saveStartToTray() {
     if (this.requireEnableTray) {
       this.form.controls.enableTray.setValue(true);
-      await this.stateService.setEnableTray(this.form.value.enableTray);
+      await this.desktopSettingsService.setTrayEnabled(this.form.value.enableTray);
     }
 
-    await this.stateService.setEnableStartToTray(this.form.value.startToTray);
+    await this.desktopSettingsService.setStartToTray(this.form.value.startToTray);
   }
 
   async saveLocale() {
@@ -573,13 +574,12 @@ export class SettingsComponent implements OnInit {
   }
 
   async saveAlwaysShowDock() {
-    await this.stateService.setAlwaysShowDock(this.form.value.alwaysShowDock);
+    await this.desktopSettingsService.setAlwaysShowDock(this.form.value.alwaysShowDock);
   }
 
   async saveOpenAtLogin() {
-    // 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
-    this.stateService.setOpenAtLogin(this.form.value.openAtLogin);
+    await this.desktopSettingsService.setOpenAtLogin(this.form.value.openAtLogin);
+    // TODO: Ideally DesktopSettingsService.openAtLogin$ could be subscribed to directly rather than sending a message
     this.messagingService.send(
       this.form.value.openAtLogin ? "addOpenAtLogin" : "removeOpenAtLogin",
     );

apps/desktop/src/main.ts

@@ -57,8 +57,8 @@ export class Main {
   environmentService: DefaultEnvironmentService;
   mainCryptoFunctionService: MainCryptoFunctionService;
   desktopCredentialStorageListener: DesktopCredentialStorageListener;
-  migrationRunner: MigrationRunner;
   desktopSettingsService: DesktopSettingsService;
+  migrationRunner: MigrationRunner;
   tokenService: TokenServiceAbstraction;
 
   windowMain: WindowMain;
@@ -179,6 +179,8 @@ export class Main {
       false, // Do not use disk caching because this will get out of sync with the renderer service
     );
 
+    this.desktopSettingsService = new DesktopSettingsService(stateProvider);
+
     const biometricStateService = new DefaultBiometricStateService(stateProvider);
 
     this.windowMain = new WindowMain(
@@ -186,13 +188,13 @@ export class Main {
       biometricStateService,
       this.logService,
       this.storageService,
+      this.desktopSettingsService,
       (arg) => this.processDeepLink(arg),
       (win) => this.trayMain.setupWindowListeners(win),
     );
-    this.messagingMain = new MessagingMain(this, this.stateService);
+    this.messagingMain = new MessagingMain(this, this.stateService, this.desktopSettingsService);
     this.updaterMain = new UpdaterMain(this.i18nService, this.windowMain);
-    this.trayMain = new TrayMain(this.windowMain, this.i18nService, this.stateService);
-    this.desktopSettingsService = new DesktopSettingsService(stateProvider);
+    this.trayMain = new TrayMain(this.windowMain, this.i18nService, this.desktopSettingsService);
 
     this.messagingService = new ElectronMainMessagingService(this.windowMain, (message) => {
       this.messagingMain.onMessage(message);
@@ -244,7 +246,7 @@ export class Main {
         await this.toggleHardwareAcceleration();
         await this.windowMain.init();
         await this.i18nService.init();
-        this.messagingMain.init();
+        await this.messagingMain.init();
         // 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
         this.menuMain.init();
@@ -256,10 +258,8 @@ export class Main {
             click: () => this.messagingService.send("lockVault"),
           },
         ]);
-        if (await this.stateService.getEnableStartToTray()) {
-          // 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
-          this.trayMain.hideToTray();
+        if (await firstValueFrom(this.desktopSettingsService.startToTray$)) {
+          await this.trayMain.hideToTray();
         }
         this.powerMonitorMain.init();
         await this.updaterMain.init();

apps/desktop/src/main/messaging.main.ts

@@ -6,6 +6,7 @@ import { app, ipcMain } from "electron";
 import { StateService } from "@bitwarden/common/platform/abstractions/state.service";
 
 import { Main } from "../main";
+import { DesktopSettingsService } from "../platform/services/desktop-settings.service";
 
 import { MenuUpdateRequest } from "./menu/menu.updater";
 
@@ -17,19 +18,16 @@ export class MessagingMain {
   constructor(
     private main: Main,
     private stateService: StateService,
+    private desktopSettingsService: DesktopSettingsService,
   ) {}
 
-  init() {
+  async init() {
     this.scheduleNextSync();
     if (process.platform === "linux") {
-      // 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
-      this.stateService.setOpenAtLogin(fs.existsSync(this.linuxStartupFile()));
+      await this.desktopSettingsService.setOpenAtLogin(fs.existsSync(this.linuxStartupFile()));
     } else {
       const loginSettings = app.getLoginItemSettings();
-      // 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
-      this.stateService.setOpenAtLogin(loginSettings.openAtLogin);
+      await this.desktopSettingsService.setOpenAtLogin(loginSettings.openAtLogin);
     }
     ipcMain.on("messagingService", async (event: any, message: any) => this.onMessage(message));
   }

apps/desktop/src/main/tray.main.ts

@@ -1,9 +1,11 @@
 import * as path from "path";
 
 import { app, BrowserWindow, Menu, MenuItemConstructorOptions, nativeImage, Tray } from "electron";
+import { firstValueFrom } from "rxjs";
 
 import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
-import { StateService } from "@bitwarden/common/platform/abstractions/state.service";
+
+import { DesktopSettingsService } from "../platform/services/desktop-settings.service";
 
 import { WindowMain } from "./window.main";
 
@@ -18,7 +20,7 @@ export class TrayMain {
   constructor(
     private windowMain: WindowMain,
     private i18nService: I18nService,
-    private stateService: StateService,
+    private desktopSettingsService: DesktopSettingsService,
   ) {
     if (process.platform === "win32") {
       this.icon = path.join(__dirname, "/images/icon.ico");
@@ -54,14 +56,14 @@ export class TrayMain {
     }
 
     this.contextMenu = Menu.buildFromTemplate(menuItemOptions);
-    if (await this.stateService.getEnableTray()) {
+    if (await firstValueFrom(this.desktopSettingsService.trayEnabled$)) {
       this.showTray();
     }
   }
 
   setupWindowListeners(win: BrowserWindow) {
     win.on("minimize", async (e: Event) => {
-      if (await this.stateService.getEnableMinimizeToTray()) {
+      if (await firstValueFrom(this.desktopSettingsService.minimizeToTray$)) {
         e.preventDefault();
         // 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
@@ -70,7 +72,7 @@ export class TrayMain {
     });
 
     win.on("close", async (e: Event) => {
-      if (await this.stateService.getEnableCloseToTray()) {
+      if (await firstValueFrom(this.desktopSettingsService.closeToTray$)) {
         if (!this.windowMain.isQuitting) {
           e.preventDefault();
           // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling.
@@ -81,7 +83,7 @@ export class TrayMain {
     });
 
     win.on("show", async () => {
-      const enableTray = await this.stateService.getEnableTray();
+      const enableTray = await firstValueFrom(this.desktopSettingsService.trayEnabled$);
       if (!enableTray) {
         setTimeout(() => this.removeTray(false), 100);
       }
@@ -106,7 +108,7 @@ export class TrayMain {
     if (this.windowMain.win != null) {
       this.windowMain.win.hide();
     }
-    if (this.isDarwin() && !(await this.stateService.getAlwaysShowDock())) {
+    if (this.isDarwin() && !(await firstValueFrom(this.desktopSettingsService.alwaysShowDock$))) {
       this.hideDock();
     }
   }
@@ -176,7 +178,7 @@ export class TrayMain {
     }
     if (this.windowMain.win.isVisible()) {
       this.windowMain.win.hide();
-      if (this.isDarwin() && !(await this.stateService.getAlwaysShowDock())) {
+      if (this.isDarwin() && !(await firstValueFrom(this.desktopSettingsService.alwaysShowDock$))) {
         this.hideDock();
       }
     } else {

apps/desktop/src/main/window.main.ts

@@ -3,13 +3,15 @@ import * as path from "path";
 import * as url from "url";
 
 import { app, BrowserWindow, ipcMain, nativeTheme, screen, session } from "electron";
+import { firstValueFrom } from "rxjs";
 
-import { WindowState } from "@bitwarden/common/models/domain/window-state";
 import { LogService } from "@bitwarden/common/platform/abstractions/log.service";
 import { StateService } from "@bitwarden/common/platform/abstractions/state.service";
 import { AbstractStorageService } from "@bitwarden/common/platform/abstractions/storage.service";
 import { BiometricStateService } from "@bitwarden/common/platform/biometrics/biometric-state.service";
 
+import { WindowState } from "../platform/models/domain/window-state";
+import { DesktopSettingsService } from "../platform/services/desktop-settings.service";
 import {
   cleanUserAgent,
   isDev,
@@ -40,6 +42,7 @@ export class WindowMain {
     private biometricStateService: BiometricStateService,
     private logService: LogService,
     private storageService: AbstractStorageService,
+    private desktopSettingsService: DesktopSettingsService,
     private argvCallback: (argv: string[]) => void = null,
     private createWindowCallback: (win: BrowserWindow) => void,
   ) {}
@@ -121,7 +124,7 @@ export class WindowMain {
         app.on("activate", async () => {
           // On OS X it's common to re-create a window in the app when the
           // dock icon is clicked and there are no other windows open.
-          if (this.win === null) {
+          if (this.win == null) {
             await this.createWindow();
           } else {
             // Show the window when clicking on Dock icon
@@ -141,7 +144,7 @@ export class WindowMain {
       this.defaultWidth,
       this.defaultHeight,
     );
-    this.enableAlwaysOnTop = await this.stateService.getEnableAlwaysOnTop();
+    this.enableAlwaysOnTop = await firstValueFrom(this.desktopSettingsService.alwaysOnTop$);
 
     this.session = session.fromPartition("persist:bitwarden", { cache: false });
 
@@ -265,7 +268,7 @@ export class WindowMain {
   async toggleAlwaysOnTop() {
     this.enableAlwaysOnTop = !this.win.isAlwaysOnTop();
     this.win.setAlwaysOnTop(this.enableAlwaysOnTop);
-    await this.stateService.setEnableAlwaysOnTop(this.enableAlwaysOnTop);
+    await this.desktopSettingsService.setAlwaysOnTop(this.enableAlwaysOnTop);
   }
 
   private windowStateChangeHandler(configKey: string, win: BrowserWindow) {
@@ -284,7 +287,7 @@ export class WindowMain {
       const bounds = win.getBounds();
 
       if (this.windowStates[configKey] == null) {
-        this.windowStates[configKey] = await this.stateService.getWindow();
+        this.windowStates[configKey] = await firstValueFrom(this.desktopSettingsService.window$);
         if (this.windowStates[configKey] == null) {
           this.windowStates[configKey] = <WindowState>{};
         }
@@ -304,14 +307,14 @@ export class WindowMain {
         this.windowStates[configKey].zoomFactor = win.webContents.zoomFactor;
       }
 
-      await this.stateService.setWindow(this.windowStates[configKey]);
+      await this.desktopSettingsService.setWindow(this.windowStates[configKey]);
     } catch (e) {
       this.logService.error(e);
     }
   }
 
   private async getWindowState(defaultWidth: number, defaultHeight: number) {
-    const state = await this.stateService.getWindow();
+    const state = await firstValueFrom(this.desktopSettingsService.window$);
 
     const isValid = state != null && (this.stateHasBounds(state) || state.isMaximized);
     let displayBounds: Electron.Rectangle = null;

apps/desktop/src/platform/models/domain/window-state.ts

@@ -0,0 +1,11 @@
+export class WindowState {
+  width?: number;
+  height?: number;
+  isMaximized?: boolean;
+  // TODO: displayBounds is an Electron.Rectangle.
+  // We need to establish some kind of client-specific global state, similar to the way we already extend a base Account.
+  displayBounds: Electron.Rectangle;
+  x?: number;
+  y?: number;
+  zoomFactor?: number;
+}

apps/desktop/src/platform/services/desktop-settings.service.ts

@@ -1,4 +1,4 @@
-import { map } from "rxjs";
+import { Observable, map } from "rxjs";
 
 import {
   DESKTOP_SETTINGS_DISK,
@@ -6,6 +6,8 @@ import {
   StateProvider,
 } from "@bitwarden/common/platform/state";
 
+import { WindowState } from "../models/domain/window-state";
+
 export const HARDWARE_ACCELERATION = new KeyDefinition<boolean>(
   DESKTOP_SETTINGS_DISK,
   "hardwareAcceleration",
@@ -14,13 +16,165 @@ export const HARDWARE_ACCELERATION = new KeyDefinition<boolean>(
   },
 );
 
+const WINDOW_KEY = new KeyDefinition<WindowState | null>(DESKTOP_SETTINGS_DISK, "window", {
+  deserializer: (s) => s,
+});
+
+const CLOSE_TO_TRAY_KEY = new KeyDefinition<boolean>(DESKTOP_SETTINGS_DISK, "closeToTray", {
+  deserializer: (b) => b,
+});
+
+const MINIMIZE_TO_TRAY_KEY = new KeyDefinition<boolean>(DESKTOP_SETTINGS_DISK, "minimizeToTray", {
+  deserializer: (b) => b,
+});
+
+const START_TO_TRAY_KEY = new KeyDefinition<boolean>(DESKTOP_SETTINGS_DISK, "startToTray", {
+  deserializer: (b) => b,
+});
+
+const TRAY_ENABLED_KEY = new KeyDefinition<boolean>(DESKTOP_SETTINGS_DISK, "trayEnabled", {
+  deserializer: (b) => b,
+});
+
+const OPEN_AT_LOGIN_KEY = new KeyDefinition<boolean>(DESKTOP_SETTINGS_DISK, "openAtLogin", {
+  deserializer: (b) => b,
+});
+
+const ALWAYS_SHOW_DOCK_KEY = new KeyDefinition<boolean>(DESKTOP_SETTINGS_DISK, "alwaysShowDock", {
+  deserializer: (b) => b,
+});
+
+const ALWAYS_ON_TOP_KEY = new KeyDefinition<boolean>(DESKTOP_SETTINGS_DISK, "alwaysOnTop", {
+  deserializer: (b) => b,
+});
+
+/**
+ * Various settings for controlling application behavior specific to the desktop client.
+ */
 export class DesktopSettingsService {
   private hwState = this.stateProvider.getGlobal(HARDWARE_ACCELERATION);
   hardwareAcceleration$ = this.hwState.state$.pipe(map((v) => v ?? true));
 
-  constructor(private stateProvider: StateProvider) {}
+  private readonly windowState = this.stateProvider.getGlobal(WINDOW_KEY);
+
+  private readonly closeToTrayState = this.stateProvider.getGlobal(CLOSE_TO_TRAY_KEY);
+  /**
+   * Tha applications setting for whether or not to close the application into the system tray.
+   */
+  closeToTray$ = this.closeToTrayState.state$.pipe(map((value) => value ?? false));
+
+  private readonly minimizeToTrayState = this.stateProvider.getGlobal(MINIMIZE_TO_TRAY_KEY);
+  /**
+   * The application setting for whether or not to minimize the applicaiton into the system tray.
+   */
+  minimizeToTray$ = this.minimizeToTrayState.state$.pipe(map((value) => value ?? false));
+
+  private readonly startToTrayState = this.stateProvider.getGlobal(START_TO_TRAY_KEY);
+  /**
+   * The application setting for whether or not to start the application into the system tray.
+   */
+  startToTray$ = this.startToTrayState.state$.pipe(map((value) => value ?? false));
+
+  private readonly trayEnabledState = this.stateProvider.getGlobal(TRAY_ENABLED_KEY);
+  /**
+   * Whether or not the system tray has been enabled.
+   */
+  trayEnabled$ = this.trayEnabledState.state$.pipe(map((value) => value ?? false));
+
+  private readonly openAtLoginState = this.stateProvider.getGlobal(OPEN_AT_LOGIN_KEY);
+  /**
+   * The application setting for whether or not the application should open at system login.
+   */
+  openAtLogin$ = this.openAtLoginState.state$.pipe(map((value) => value ?? false));
+
+  private readonly alwaysShowDockState = this.stateProvider.getGlobal(ALWAYS_SHOW_DOCK_KEY);
+  /**
+   * The application setting for whether or not the application should show up in the dock.
+   */
+  alwaysShowDock$ = this.alwaysShowDockState.state$.pipe(map((value) => value ?? false));
+
+  private readonly alwaysOnTopState = this.stateProvider.getGlobal(ALWAYS_ON_TOP_KEY);
+
+  alwaysOnTop$ = this.alwaysOnTopState.state$.pipe(map((value) => value ?? false));
+
+  constructor(private stateProvider: StateProvider) {
+    this.window$ = this.windowState.state$.pipe(
+      map((window) =>
+        window != null && Object.keys(window).length > 0 ? window : new WindowState(),
+      ),
+    );
+  }
 
   async setHardwareAcceleration(enabled: boolean) {
     await this.hwState.update(() => enabled);
   }
+
+  /**
+   * The applications current window state.
+   */
+  window$: Observable<WindowState>;
+
+  /**
+   * Updates the window state of the application so that the application can reopen in the same place as it was closed from.
+   * @param windowState The window state to set.
+   */
+  async setWindow(windowState: WindowState) {
+    await this.windowState.update(() => windowState);
+  }
+
+  /**
+   * Sets the setting for whether or not the application should go into the system tray when closed.
+   * @param value `true` if the application should go into the system tray when closed, `false` if it should not.
+   */
+  async setCloseToTray(value: boolean) {
+    await this.closeToTrayState.update(() => value);
+  }
+
+  /**
+   * Sets the setting for whether or not the application should go into the tray when minimized.
+   * @param value `true` if the application should minimize into the system tray, `false` if it should not.
+   */
+  async setMinimizeToTray(value: boolean) {
+    await this.minimizeToTrayState.update(() => value);
+  }
+
+  /**
+   * Sets the setting for whether or not the application should be started into the system tray.
+   * @param value `true` if the application should be started to the tray`, `false` if it should not.
+   */
+  async setStartToTray(value: boolean) {
+    await this.startToTrayState.update(() => value);
+  }
+
+  /**
+   * Sets the setting for whether or not the application be shown in the system tray.
+   * @param value `true` if the application should show in the tray, `false` if it should not.
+   */
+  async setTrayEnabled(value: boolean) {
+    await this.trayEnabledState.update(() => value);
+  }
+
+  /**
+   * Sets the setting for whether or not the application should open at login of the computer.
+   * @param value `true` if the application should open at login, `false` if it should not.
+   */
+  async setOpenAtLogin(value: boolean) {
+    await this.openAtLoginState.update(() => value);
+  }
+
+  /**
+   * Sets the setting for whether or not the application should be shown in the dock.
+   * @param value `true` if the application should should in the dock, `false` if it should not.
+   */
+  async setAlwaysShowDock(value: boolean) {
+    await this.alwaysShowDockState.update(() => value);
+  }
+
+  /**
+   * Sets the setting for whether or not the application should stay on top of all other windows.
+   * @param value `true` if the application should stay on top, `false` if it should not.
+   */
+  async setAlwaysOnTop(value: boolean) {
+    await this.alwaysOnTopState.update(() => value);
+  }
 }