refactor: reduce service switcher tearing

We render the location bar and notification banners separately for each
service and keep track of the BrowserView size separately for each
service to reduce the tearing that appears when people switch services.
The tearing cannot be eliminated completely, because it comes from the
separation between the main and renderer processes. But we can at least
try and reduce the IPC round-tripping and layout calculations required
to accurately position the services.

This approach has an overhead compared to the single
BrowserViewPlaceholder approach, because the renderer process has to
layout the location bar and notification for all services, not only the
selected one. The number of IPC messages during windows resize is also
increased. To compensate, we increase the throttle interval for resize
IPC messages and let electron itself resize the BrowserView between IPC
updates. (We must still keep pumping IPC messages during window resize,
because, e.g., changes in notification banner size due to re-layouting
will still affect the required BrowserView size).

If further reduction of IPC traffic is needed, we could implement
batching for resize IPC messages and more intelligent throttling via a
token bucker mechanism.

Signed-off-by: Kristóf Marussy <kristof@marussy.com>

6 files changed, 47 insertions, 33 deletions

diff --git a/packages/main/src/infrastructure/electron/impl/ElectronMainWindow.ts b/packages/main/src/infrastructure/electron/impl/ElectronMainWindow.ts
index edc6592..b0db115 100644
--- a/packages/main/src/infrastructure/electron/impl/ElectronMainWindow.ts
+++ b/packages/main/src/infrastructure/electron/impl/ElectronMainWindow.ts
@@ -153,6 +153,9 @@ export default class ElectronMainWindow implements MainWindow {
     }
     if (serviceView instanceof ElectronServiceView) {
       this.browserWindow.setBrowserView(serviceView.browserView);
+      // If this `BrowserView` hasn't been attached previously,
+      // we must update its bounds _after_ attaching for the resizing to take effect.
+      serviceView.updateBounds();
       return;
     }
     throw new TypeError(
diff --git a/packages/main/src/infrastructure/electron/impl/ElectronServiceView.ts b/packages/main/src/infrastructure/electron/impl/ElectronServiceView.ts
index 2e64269..3118efc 100644
--- a/packages/main/src/infrastructure/electron/impl/ElectronServiceView.ts
+++ b/packages/main/src/infrastructure/electron/impl/ElectronServiceView.ts
@@ -18,7 +18,6 @@
  * SPDX-License-Identifier: AGPL-3.0-only
  */
 
-import type { BrowserViewBounds } from '@sophie/shared';
 import { BrowserView } from 'electron';
 
 import type Service from '../../../stores/Service';
@@ -39,7 +38,7 @@ export default class ElectronServiceView implements ServiceView {
   readonly browserView: BrowserView;
 
   constructor(
-    service: Service,
+    private readonly service: Service,
     resources: Resources,
     partition: ElectronPartition,
     private readonly parent: ElectronViewFactory,
@@ -56,6 +55,13 @@ export default class ElectronServiceView implements ServiceView {
     });
 
     this.browserView.setBackgroundColor('#fff');
+    this.browserView.setAutoResize({
+      width: true,
+      height: true,
+    });
+    // Util we first attach `browserView` to a `BrowserWindow`,
+    // `setBounds` calls will be ignored, so there's no point in callind `updateBounds` here.
+    // It will be called by `ElectronMainWindow` when we first attach this service to it.
 
     const { webContents } = this.browserView;
 
@@ -191,13 +197,17 @@ export default class ElectronServiceView implements ServiceView {
     this.browserView.webContents.toggleDevTools();
   }
 
-  setBounds(bounds: BrowserViewBounds): void {
-    this.browserView.setBounds(bounds);
+  updateBounds(): void {
+    const { x, y, width, height, hasBounds } = this.service;
+    if (!hasBounds) {
+      return;
+    }
+    this.browserView.setBounds({ x, y, width, height });
   }
 
   dispose(): void {
-    this.parent.unregisterServiceView(this.webContentsId);
     setImmediate(() => {
+      this.parent.unregisterServiceView(this.webContentsId);
       // Undocumented electron API, see e.g., https://github.com/electron/electron/issues/29626
       (
         this.browserView.webContents as unknown as { destroy(): void }
diff --git a/packages/main/src/infrastructure/electron/types.ts b/packages/main/src/infrastructure/electron/types.ts
index 1321048..92ca9ad 100644
--- a/packages/main/src/infrastructure/electron/types.ts
+++ b/packages/main/src/infrastructure/electron/types.ts
@@ -18,8 +18,6 @@
  * SPDX-License-Identifier: AGPL-3.0-only
  */
 
-import type { BrowserViewBounds } from '@sophie/shared';
-
 import type MainStore from '../../stores/MainStore';
 import type Profile from '../../stores/Profile';
 import type Service from '../../stores/Service';
@@ -67,7 +65,7 @@ export interface ServiceView {
 
   toggleDeveloperTools(): void;
 
-  setBounds(bounds: BrowserViewBounds): void;
+  updateBounds(): void;
 
   dispose(): void;
 }
diff --git a/packages/main/src/reactions/loadServices.ts b/packages/main/src/reactions/loadServices.ts
index 4ef6131..f56ac62 100644
--- a/packages/main/src/reactions/loadServices.ts
+++ b/packages/main/src/reactions/loadServices.ts
@@ -18,7 +18,7 @@
  * SPDX-License-Identifier: AGPL-3.0-only
  */
 
-import { autorun, reaction } from 'mobx';
+import { reaction } from 'mobx';
 import { addDisposer } from 'mobx-state-tree';
 
 import type {
@@ -94,7 +94,6 @@ export default function loadServices(
             throw new Error(`Missing Partition ${profileId}`);
           }
           view = viewFactory.createServiceView(service, partition);
-          view.setBounds(store.browserViewBounds);
           servicesToViews.set(serviceId, view);
           service.setServiceView(view);
           const { urlToLoad } = service;
@@ -133,12 +132,7 @@ export default function loadServices(
     },
   );
 
-  const resizeDisposer = autorun(() => {
-    store.visibleService?.serviceView?.setBounds(store.browserViewBounds);
-  });
-
   addDisposer(store, () => {
-    resizeDisposer();
     disposer();
     store.mainWindow?.setServiceView(undefined);
     servicesToViews.forEach((serviceView, serviceId) => {
diff --git a/packages/main/src/stores/MainStore.ts b/packages/main/src/stores/MainStore.ts
index d717bed..9affbd0 100644
--- a/packages/main/src/stores/MainStore.ts
+++ b/packages/main/src/stores/MainStore.ts
@@ -18,9 +18,9 @@
  * SPDX-License-Identifier: AGPL-3.0-only
  */
 
-import type { Action, BrowserViewBounds } from '@sophie/shared';
+import type { Action } from '@sophie/shared';
 import type { ResourceKey } from 'i18next';
-import { applySnapshot, flow, Instance, types } from 'mobx-state-tree';
+import { flow, Instance, types } from 'mobx-state-tree';
 
 import type I18nStore from '../i18n/I18nStore';
 import type { UseTranslationResult } from '../i18n/I18nStore';
@@ -37,15 +37,6 @@ const log = getLogger('MainStore');
 
 const MainStore = types
   .model('MainStore', {
-    browserViewBounds: types.optional(
-      types.model('BrowserViewBounds', {
-        x: 0,
-        y: 0,
-        width: 0,
-        height: 0,
-      }),
-      {},
-    ),
     shared: types.optional(SharedStore, {}),
     i18n: types.frozen<I18nStore | undefined>(),
   })
@@ -83,9 +74,6 @@ const MainStore = types
     }),
   )
   .actions((self) => ({
-    setBrowserViewBounds(bounds: BrowserViewBounds): void {
-      applySnapshot(self.browserViewBounds, bounds);
-    },
     setMainWindow(mainWindow: MainWindow | undefined): void {
       self.mainWindow = mainWindow;
     },
@@ -121,9 +109,6 @@ const MainStore = types
   .actions((self) => ({
     dispatch(action: Action): void {
       switch (action.action) {
-        case 'set-browser-view-bounds':
-          self.setBrowserViewBounds(action.browserViewBounds);
-          break;
         case 'set-selected-service-id':
           self.settings.setSelectedServiceId(action.serviceId);
           break;
diff --git a/packages/main/src/stores/Service.ts b/packages/main/src/stores/Service.ts
index 1d46dc9..8ba8098 100644
--- a/packages/main/src/stores/Service.ts
+++ b/packages/main/src/stores/Service.ts
@@ -25,6 +25,7 @@ import {
   defineServiceModel,
   ServiceAction,
   type ServiceStateSnapshotIn,
+  type BrowserViewBounds,
 } from '@sophie/shared';
 import { type Instance, getSnapshot, cast, flow } from 'mobx-state-tree';
 
@@ -41,8 +42,18 @@ const Service = defineServiceModel(ServiceSettings)
   .volatile(
     (): {
       serviceView: ServiceView | undefined;
+      x: number;
+      y: number;
+      width: number;
+      height: number;
+      hasBounds: boolean;
     } => ({
       serviceView: undefined,
+      x: 0,
+      y: 0,
+      width: 0,
+      height: 0,
+      hasBounds: false,
     }),
   )
   .views((self) => ({
@@ -59,10 +70,20 @@ const Service = defineServiceModel(ServiceSettings)
   }))
   .views((self) => ({
     get shouldBeVisible(): boolean {
-      return self.shouldBeLoaded && !self.hasError;
+      // Do not attach service views for which we don't know the appropriate frame size,
+      // because they will just appear in a random location until the frame size is determined.
+      return self.shouldBeLoaded && !self.hasError && self.hasBounds;
     },
   }))
   .actions((self) => ({
+    setBrowserViewBounds(bounds: BrowserViewBounds): void {
+      self.x = bounds.x;
+      self.y = bounds.y;
+      self.width = bounds.width;
+      self.height = bounds.height;
+      self.hasBounds = true;
+      self.serviceView?.updateBounds();
+    },
     setServiceView(serviceView: ServiceView | undefined): void {
       self.serviceView = serviceView;
     },
@@ -263,6 +284,9 @@ const Service = defineServiceModel(ServiceSettings)
   .actions((self) => ({
     dispatch(action: ServiceAction): void {
       switch (action.action) {
+        case 'set-browser-view-bounds':
+          self.setBrowserViewBounds(action.browserViewBounds);
+          break;
         case 'back':
           self.goBack();
           break;