fix(desktop): keep hidden previews paintable during screenshots

apps/desktop/src/ipc/channels.ts

@@ -125,3 +125,5 @@ export const RECEIVE_PROVIDER_AUTH_CALLBACK_CHANNEL = "desktop:receive-provider-
 export const CANCEL_PROVIDER_AUTH_CALLBACK_CHANNEL = "desktop:cancel-provider-auth-callback";
 export const TAKE_LEGACY_LOCAL_STORAGE_CHANNEL = "desktop:take-legacy-local-storage";
 export const COMPLETE_LEGACY_LOCAL_STORAGE_CHANNEL = "desktop:complete-legacy-local-storage";
+
+export const PREVIEW_PREPARE_CAPTURE_CHANNEL = "desktop:preview-prepare-capture";

apps/desktop/src/preload.ts

@@ -409,6 +409,28 @@ contextBridge.exposeInMainWorld("desktopBridge", {
           ipcRenderer.removeListener(IpcChannels.PREVIEW_RECORDING_FRAME_CHANNEL, wrappedListener);
       },
     },
+    onPrepareCapture: (listener) => {
+      const pending = new Set<() => void>();
+      const wrappedListener = (event: Electron.IpcRendererEvent, tabId: unknown) => {
+        const port = event.ports[0];
+        if (!port || typeof tabId !== "string") return;
+        const release = listener(tabId);
+        const close = () => {
+          pending.delete(close);
+          release?.();
+          port.close();
+        };
+        pending.add(close);
+        port.addEventListener("message", close, { once: true });
+        port.start();
+        port.postMessage(release ? "ready" : "missing");
+      };
+      ipcRenderer.on(IpcChannels.PREVIEW_PREPARE_CAPTURE_CHANNEL, wrappedListener);
+      return () => {
+        ipcRenderer.removeListener(IpcChannels.PREVIEW_PREPARE_CAPTURE_CHANNEL, wrappedListener);
+        for (const close of pending) close();
+      };
+    },
     onStateChange: (listener) => {
       const wrappedListener = (
         _event: Electron.IpcRendererEvent,

apps/desktop/src/preview/DesktopBrowserHost.test.ts

@@ -30,20 +30,24 @@ const command = (
     }),
   );
 
-const makeDebuggee = () => {
+const makeDebuggee = (captureFails = false) => {
   const emitter = new NodeEvents.EventEmitter();
   const pending: Array<() => void> = [];
   const sent: Array<unknown> = [];
   const sendWaiters: Array<() => void> = [];
   const debuggee = Object.assign(emitter, {
     sendCommand: (method: string, params?: { id?: number }) =>
-      new Promise((resolve) => {
+      new Promise((resolve, reject) => {
         if (method === "Target.getTargetInfo") {
           resolve({ targetInfo: { targetId: "GUEST" } });
           return;
         }
         sent.push(params?.id);
-        pending.push(() => resolve({ method }));
+        pending.push(() => {
+          if (captureFails && method === "Page.captureScreenshot")
+            reject(new Error("Capture failed"));
+          else resolve({ method });
+        });
         sendWaiters.shift()?.();
       }),
   });
@@ -131,6 +135,95 @@ describe("DesktopBrowserHost", () => {
     }).pipe(Effect.scoped),
   );
 
+  it.effect("waits for a paintable guest before capturing and releases it afterwards", () =>
+    Effect.gen(function* () {
+      const host = yield* DesktopBrowserHost.make;
+      const debuggee = makeDebuggee();
+      let ready: (() => void) | undefined;
+      let released = 0;
+      const prepared = new Promise<() => void>((resolve) => {
+        ready = () =>
+          resolve(() => {
+            released += 1;
+          });
+      });
+      host.attach(key, { ...debuggee.tab, prepareScreenshot: () => prepared });
+      const replies = yield* watchReplies(host);
+
+      yield* command(host, 1, "Page.captureScreenshot");
+      expect(debuggee.sent).toEqual([]);
+      const sent = debuggee.nextSend();
+      ready?.();
+      yield* Effect.promise(() => sent);
+      expect(debuggee.sent).toEqual([1]);
+      expect(released).toBe(0);
+      debuggee.releaseFirst();
+      expect(yield* Queue.take(replies)).toBe(1);
+      expect(released).toBe(1);
+    }).pipe(Effect.scoped),
+  );
+
+  it.effect("releases a paintable guest when capture fails and continues the queue", () =>
+    Effect.gen(function* () {
+      const host = yield* DesktopBrowserHost.make;
+      const debuggee = makeDebuggee(true);
+      let released = 0;
+      host.attach(key, {
+        ...debuggee.tab,
+        prepareScreenshot: async () => () => {
+          released += 1;
+        },
+      });
+      const replies = yield* watchReplies(host);
+      const firstSent = debuggee.nextSend();
+      yield* command(host, 1, "Page.captureScreenshot");
+      yield* command(host, 2, "Page.captureScreenshot");
+      yield* Effect.promise(() => firstSent);
+      expect(debuggee.sent).toEqual([1]);
+      const secondSent = debuggee.nextSend();
+      debuggee.releaseFirst();
+      expect(yield* Queue.take(replies)).toBe(1);
+      yield* Effect.promise(() => secondSent);
+      expect(released).toBe(1);
+      expect(debuggee.sent).toEqual([1, 2]);
+      debuggee.releaseFirst();
+      expect(yield* Queue.take(replies)).toBe(2);
+      expect(released).toBe(2);
+    }).pipe(Effect.scoped),
+  );
+
+  it.effect("does not start a stale capture after navigation finishes its preparation", () =>
+    Effect.gen(function* () {
+      const host = yield* DesktopBrowserHost.make;
+      const debuggee = makeDebuggee();
+      let ready: (() => void) | undefined;
+      let preparations = 0;
+      let released = 0;
+      const release = () => {
+        released += 1;
+      };
+      const prepared = new Promise<() => void>((resolve) => {
+        ready = () => resolve(release);
+      });
+      host.attach(key, {
+        ...debuggee.tab,
+        prepareScreenshot: () => (++preparations === 1 ? prepared : Promise.resolve(release)),
+      });
+      const replies = yield* watchReplies(host);
+      yield* command(host, 1, "Page.captureScreenshot");
+      yield* command(host, 2, "Page.captureScreenshot");
+      const sent = debuggee.nextSend();
+      debuggee.navigate();
+      yield* Effect.promise(() => sent);
+      ready?.();
+      expect(yield* Queue.take(replies)).toBe(1);
+      expect(debuggee.sent).toEqual([2]);
+      debuggee.releaseFirst();
+      expect(yield* Queue.take(replies)).toBe(2);
+      expect(released).toBe(2);
+    }).pipe(Effect.scoped),
+  );
+
   it.effect("sends a page one screenshot at a time", () =>
     Effect.gen(function* () {
       const host = yield* DesktopBrowserHost.make;
@@ -149,27 +242,53 @@ describe("DesktopBrowserHost", () => {
     }).pipe(Effect.scoped),
   );
 
-  it.effect("stops waiting on a screenshot the page navigated away from", () =>
+  it.effect("releases an abandoned screenshot's rendering activity and preserves the queue", () =>
     Effect.gen(function* () {
       const host = yield* DesktopBrowserHost.make;
       const debuggee = makeDebuggee();
-      host.attach(key, debuggee.tab);
+      const released: Array<number> = [];
+      let preparations = 0;
+      host.attach(key, {
+        ...debuggee.tab,
+        prepareScreenshot: async () => {
+          const preparation = ++preparations;
+          return () => {
+            released.push(preparation);
+          };
+        },
+      });
       const replies = yield* watchReplies(host);
 
+      const firstSent = debuggee.nextSend();
       yield* command(host, 1, "Page.captureScreenshot");
       yield* command(host, 2, "Page.captureScreenshot");
       yield* command(host, 3, "Page.captureScreenshot");
+      yield* Effect.promise(() => firstSent);
+      expect(debuggee.sent).toEqual([1]);
+      expect(released).toEqual([]);
+
+      const secondSent = debuggee.nextSend();
       debuggee.navigate();
+      expect(released).toEqual([1]);
+      yield* Effect.promise(() => secondSent);
       expect(debuggee.sent).toEqual([1, 2]);
 
       // The superseded screenshot answering late must not start a third beside the second.
       debuggee.releaseFirst();
       expect(yield* Queue.take(replies)).toBe(1);
+      expect(released).toEqual([1]);
       expect(debuggee.sent).toEqual([1, 2]);
 
+      const thirdSent = debuggee.nextSend();
       debuggee.releaseFirst();
       expect(yield* Queue.take(replies)).toBe(2);
+      yield* Effect.promise(() => thirdSent);
+      expect(released).toEqual([1, 2]);
       expect(debuggee.sent).toEqual([1, 2, 3]);
+
+      debuggee.releaseFirst();
+      expect(yield* Queue.take(replies)).toBe(3);
+      expect(released).toEqual([1, 2, 3]);
     }).pipe(Effect.scoped),
   );
 

apps/desktop/src/preview/DesktopBrowserHost.ts

@@ -35,15 +35,20 @@ export interface DesktopBrowserTabKey {
 export interface DesktopBrowserTabDebugger {
   readonly webContents: Electron.WebContents;
   readonly debugger: Electron.Debugger;
+  readonly prepareScreenshot?: () => Promise<() => void>;
 }
 
 const keyOf = ({ threadId, tabId }: DesktopBrowserTabKey) => `${threadId}\u0000${tabId}`;
 
+interface ScreenshotTurn {
+  release?: () => void;
+}
+
 interface AttachedTab {
   readonly key: DesktopBrowserTabKey;
   readonly debuggee: DesktopBrowserTabDebugger;
   relay: CdpRelayConnection | null;
-  screenshotTurn: object | null;
+  screenshotTurn: ScreenshotTurn | null;
   readonly waitingScreenshots: Array<() => void>;
   downloadDirectory: string | null;
   pendingDownloadGuid: string | null;
@@ -88,6 +93,7 @@ export const make = Effect.gen(function* () {
   const emit = (event: DesktopBrowserEventType) => runFork(PubSub.publish(outbox, event));
 
   const nextScreenshot = (tab: AttachedTab) => {
+    tab.screenshotTurn?.release?.();
     tab.screenshotTurn = null;
     tab.waitingScreenshots.shift()?.();
   };
@@ -110,9 +116,29 @@ export const make = Effect.gen(function* () {
           nextScreenshot(tab);
           return;
         }
-        const turn = {};
+        const turn: ScreenshotTurn = {};
         tab.screenshotTurn = turn;
-        capture()
+        const prepare = tab.debuggee.prepareScreenshot;
+        const capturing = prepare
+          ? prepare().then(async (acquired) => {
+              let released = false;
+              const release = () => {
+                if (released) return;
+                released = true;
+                acquired();
+              };
+              turn.release = release;
+              try {
+                if (tab.screenshotTurn !== turn || tab.relay !== relay) {
+                  throw new Error("The preview changed while preparing its screenshot.");
+                }
+                return await capture();
+              } finally {
+                release();
+              }
+            })
+          : capture();
+        capturing
           .finally(() => {
             if (tab.screenshotTurn === turn) nextScreenshot(tab);
           })

apps/desktop/src/preview/Manager.ts

@@ -65,6 +65,7 @@ import * as DesktopEnvironment from "../app/DesktopEnvironment.ts";
 import * as DesktopRendererHistory from "../telemetry/DesktopRendererHistory.ts";
 import { MENU_ACTION_CHANNEL, PREVIEW_PICTURE_IN_PICTURE_FRAME_CHANNEL } from "../ipc/channels.ts";
 import * as DesktopBrowserHost from "./DesktopBrowserHost.ts";
+import { prepareGuestCapture } from "./prepareGuestCapture.ts";
 import * as PreviewPasskeys from "./Passkeys.ts";
 import * as BrowserSession from "./BrowserSession.ts";
 import {
@@ -560,6 +561,7 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function
   const passkeys = yield* PreviewPasskeys.PreviewPasskeys;
   const context = yield* Effect.context<never>();
   const runFork = Effect.runForkWith(context);
+  const runPromise = Effect.runPromiseWith(context);
   const resolvedArtifactDirectory = path.resolve(artifactDirectory);
 
   let forwardedShortcuts: ReadonlyArray<PreviewForwardedShortcut> = [];
@@ -2387,7 +2389,11 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function
       }
       if (afterAttach.serverTab) {
         yield* listenForAgentPointers;
-        browserHost.attach(afterAttach.serverTab, { webContents: wc, debugger: control.debugger });
+        browserHost.attach(afterAttach.serverTab, {
+          webContents: wc,
+          debugger: control.debugger,
+          prepareScreenshot: () => runPromise(prepareGuestCapture(wc, tabId)),
+        });
       }
       if (afterAttach.colorScheme !== "system") {
         yield* attemptPromise({ operation: "applyColorScheme", tabId, webContentsId: wc.id }, () =>

apps/desktop/src/preview/prepareGuestCapture.test.ts

@@ -0,0 +1,89 @@
+import { describe, expect, it } from "@effect/vitest";
+import * as Effect from "effect/Effect";
+import * as Fiber from "effect/Fiber";
+import * as TestClock from "effect/testing/TestClock";
+import { vi } from "vite-plus/test";
+
+const { channels } = vi.hoisted(() => ({
+  channels: [] as Array<{
+    reply: (data: string) => void;
+    released: Array<unknown>;
+    closed: boolean;
+  }>,
+}));
+
+vi.mock("electron", () => ({
+  MessageChannelMain: class {
+    port1;
+    port2 = {};
+    constructor() {
+      let listener: (event: { data: string }) => void = () => undefined;
+      const channel = {
+        reply: (data: string) => listener({ data }),
+        released: [] as Array<unknown>,
+        closed: false,
+      };
+      channels.push(channel);
+      this.port1 = {
+        on: (_event: string, handle: typeof listener) => {
+          listener = handle;
+        },
+        start: () => undefined,
+        postMessage: (value: unknown) => channel.released.push(value),
+        close: () => {
+          channel.closed = true;
+        },
+      };
+    }
+  },
+}));
+
+import { prepareGuestCapture } from "./prepareGuestCapture.ts";
+
+const fixture = () => {
+  let requested: ((tabId: string) => void) | undefined;
+  const request = new Promise<string>((resolve) => {
+    requested = resolve;
+  });
+  const guest = {
+    hostWebContents: {
+      isDestroyed: () => false,
+      postMessage: (_channel: string, tabId: string) => requested?.(tabId),
+    },
+  } as unknown as Electron.WebContents;
+  return { guest, request };
+};
+
+describe("prepareGuestCapture", () => {
+  it.effect("holds the renderer's activity until the capture releases it", () =>
+    Effect.gen(function* () {
+      const { guest, request } = fixture();
+      const preparing = yield* prepareGuestCapture(guest, "background-tab").pipe(Effect.forkScoped);
+      expect(yield* Effect.promise(() => request)).toBe("background-tab");
+      const channel = channels.at(-1)!;
+      expect(channel.released).toEqual([]);
+      channel.reply("ready");
+      const release = yield* Fiber.join(preparing);
+      expect(channel.closed).toBe(false);
+      release();
+      expect(channel.released).toEqual(["release"]);
+      expect(channel.closed).toBe(true);
+    }).pipe(Effect.scoped),
+  );
+
+  it.effect("releases a request when its renderer does not acknowledge it", () =>
+    Effect.gen(function* () {
+      const { guest, request } = fixture();
+      const preparing = yield* prepareGuestCapture(guest, "background-tab").pipe(
+        Effect.result,
+        Effect.forkScoped,
+      );
+      yield* Effect.promise(() => request);
+      const channel = channels.at(-1)!;
+      yield* TestClock.adjust(2_000);
+      expect((yield* Fiber.join(preparing))._tag).toBe("Failure");
+      expect(channel.released).toEqual(["release"]);
+      expect(channel.closed).toBe(true);
+    }).pipe(Effect.scoped),
+  );
+});