From 3d2161857af4c4dee4492f5ccf1b1726c2066890 Mon Sep 17 00:00:00 2001 From: Gregory Kolens Date: Wed, 21 Oct 2026 00:58:27 +0200 Subject: [PATCH 1/2] fix(desktop): capture background preview screenshots --- .../src/preview/DesktopBrowserHost.test.ts | 39 ++++++ .../desktop/src/preview/DesktopBrowserHost.ts | 31 ++++- apps/desktop/src/preview/Manager.ts | 37 +++++- .../src/preview/ScreenshotCapture.test.ts | 121 ++++++++++++++++++ apps/desktop/src/preview/ScreenshotCapture.ts | 67 ++++++++++ apps/web/src/browser/ElectronBrowserHost.tsx | 9 ++ .../browser/desktopScreenshotActivity.test.ts | 41 ++++++ .../src/browser/desktopScreenshotActivity.ts | 39 ++++++ packages/contracts/src/ipc.ts | 2 + 9 files changed, 379 insertions(+), 7 deletions(-) create mode 100644 apps/desktop/src/preview/ScreenshotCapture.test.ts create mode 100644 apps/desktop/src/preview/ScreenshotCapture.ts create mode 100644 apps/web/src/browser/desktopScreenshotActivity.test.ts create mode 100644 apps/web/src/browser/desktopScreenshotActivity.ts diff --git a/apps/desktop/src/preview/DesktopBrowserHost.test.ts b/apps/desktop/src/preview/DesktopBrowserHost.test.ts index 0d281c46f8605..f97312863d73f 100644 --- a/apps/desktop/src/preview/DesktopBrowserHost.test.ts +++ b/apps/desktop/src/preview/DesktopBrowserHost.test.ts @@ -38,6 +38,8 @@ const makeDebuggee = () => { tab: { webContents: webContents as unknown as Electron.WebContents, debugger: debuggee as unknown as Electron.Debugger, + captureScreenshot: (_params: Record, _signal: AbortSignal) => + debuggee.sendCommand("Page.captureScreenshot"), }, emit: (method: string, params: unknown) => emitter.emit("message", {}, method, params, ""), release: () => pending.splice(0).forEach((resolve) => resolve()), @@ -52,6 +54,43 @@ const takeEvents = (host: DesktopBrowserHost.DesktopBrowserHost["Service"], coun ); describe("DesktopBrowserHost", () => { + it.effect.each(["release", "detach", "reconnect"] as const)( + "interrupts pending screenshots on %s", + (reset) => + Effect.gen(function* () { + const host = yield* DesktopBrowserHost.make; + const debuggee = makeDebuggee(); + const signals: AbortSignal[] = []; + debuggee.tab.captureScreenshot = (_params, signal) => { + signals.push(signal); + return new Promise((_resolve, reject) => { + signal.addEventListener("abort", () => reject(new Error("capture aborted")), { + once: true, + }); + }); + }; + host.attach(key, debuggee.tab); + yield* host.handleCommandLine( + encodeJson({ + type: "cdp", + ...key, + message: encodeJson({ + id: 1, + method: "Page.captureScreenshot", + sessionId: "t3-preview-page", + }), + }), + ); + expect(signals).toHaveLength(1); + expect(signals[0]?.aborted).toBe(false); + if (reset === "release") + yield* host.handleCommandLine(encodeJson({ type: "release", ...key })); + else if (reset === "detach") host.detach(key); + else yield* takeEvents(host, 1); + expect(signals[0]?.aborted).toBe(true); + }), + ); + it.effect("announces tabs already attached to a backend that starts later", () => Effect.gen(function* () { const host = yield* DesktopBrowserHost.make; diff --git a/apps/desktop/src/preview/DesktopBrowserHost.ts b/apps/desktop/src/preview/DesktopBrowserHost.ts index cef42d4f35fc3..103a6e0471cd9 100644 --- a/apps/desktop/src/preview/DesktopBrowserHost.ts +++ b/apps/desktop/src/preview/DesktopBrowserHost.ts @@ -35,6 +35,10 @@ export interface DesktopBrowserTabKey { export interface DesktopBrowserTabDebugger { readonly webContents: Electron.WebContents; readonly debugger: Electron.Debugger; + readonly captureScreenshot: ( + params: Record, + signal: AbortSignal, + ) => Promise; } const keyOf = ({ threadId, tabId }: DesktopBrowserTabKey) => `${threadId}\u0000${tabId}`; @@ -43,6 +47,7 @@ interface AttachedTab { readonly key: DesktopBrowserTabKey; readonly debuggee: DesktopBrowserTabDebugger; relay: CdpRelayConnection | null; + readonly captures: Set; downloadDirectory: string | null; pendingDownloadGuid: string | null; agentInputAt: number; @@ -84,15 +89,29 @@ export const make = Effect.gen(function* () { const tabs = new Map(); const emit = (event: DesktopBrowserEventType) => runFork(PubSub.publish(outbox, event)); + const resetRelay = (tab: AttachedTab) => { + tab.relay = null; + for (const controller of tab.captures) controller.abort(); + tab.captures.clear(); + }; + const relayFor = (tab: AttachedTab) => { if (tab.relay) return tab.relay; const { webContents, debugger: debuggee } = tab.debuggee; const relay: CdpRelayConnection = createCdpRelayConnection( { - send: (method, params, sessionId) => - sessionId === undefined + send: (method, params, sessionId) => { + if (method === "Page.captureScreenshot" && sessionId === undefined) { + const controller = new AbortController(); + tab.captures.add(controller); + return tab.debuggee.captureScreenshot(params, controller.signal).finally(() => { + tab.captures.delete(controller); + }); + } + return sessionId === undefined ? sendPageCommand(webContents, debuggee, method, params) - : debuggee.sendCommand(method, params, sessionId), + : debuggee.sendCommand(method, params, sessionId); + }, targetId: () => debuggee .sendCommand("Target.getTargetInfo") @@ -133,6 +152,7 @@ export const make = Effect.gen(function* () { const tab = tabs.get(id); if (!tab) return; tabs.delete(id); + resetRelay(tab); tab.debuggee.debugger.off("message", tab.onMessage); emit({ type: "detached", ...key }); }; @@ -145,6 +165,7 @@ export const make = Effect.gen(function* () { key, debuggee, relay: null, + captures: new Set(), downloadDirectory: null, pendingDownloadGuid: null, agentInputAt: Number.NEGATIVE_INFINITY, @@ -173,7 +194,7 @@ export const make = Effect.gen(function* () { return; } if (command.value.type === "release") { - tab.relay = null; + resetRelay(tab); return; } if (AGENT_INPUT_COMMAND.test(command.value.message)) tab.agentInputAt = performance.now(); @@ -189,7 +210,7 @@ export const make = Effect.gen(function* () { Effect.forEach( [...tabs.values()], (tab) => { - tab.relay = null; + resetRelay(tab); return PubSub.publish(outbox, { type: "attached", ...tab.key }); }, { discard: true }, diff --git a/apps/desktop/src/preview/Manager.ts b/apps/desktop/src/preview/Manager.ts index b036a67ac8ba0..da2fc26c642c4 100644 --- a/apps/desktop/src/preview/Manager.ts +++ b/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 { captureFreshScreenshot } from "./ScreenshotCapture.ts"; import * as PreviewPasskeys from "./Passkeys.ts"; import * as BrowserSession from "./BrowserSession.ts"; import { @@ -391,7 +392,7 @@ interface ManagedListeners { readonly webContents: Electron.WebContents; } -type FrameCaptureConsumer = "picture-in-picture" | "recording"; +type FrameCaptureConsumer = "picture-in-picture" | "recording" | "screenshot"; interface FrameCaptureSession { readonly recordingInputOptions?: RecordingInputOptions; @@ -556,6 +557,7 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function const path = yield* Path.Path; const crypto = yield* Crypto.Crypto; const parentScope = yield* Scope.Scope; + const runCapture = Effect.runPromiseWith(yield* Effect.context()); const browserHost = yield* DesktopBrowserHost.DesktopBrowserHost; const passkeys = yield* PreviewPasskeys.PreviewPasskeys; const context = yield* Effect.context(); @@ -2387,7 +2389,38 @@ 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, + captureScreenshot: (params, signal) => + runCapture( + Effect.acquireUseRelease( + Effect.forkIn( + control.semaphore.withPermit( + Effect.scoped( + Effect.gen(function* () { + yield* Effect.acquireRelease(startFrameCapture(tabId, "screenshot"), () => + stopFrameCapture(tabId, "screenshot").pipe(Effect.ignore), + ); + const token = yield* crypto.randomUUIDv4(); + return yield* captureFreshScreenshot( + wc, + control.debugger, + params, + tabId, + token, + ); + }), + ), + ), + control.scope, + ), + Fiber.join, + Fiber.interrupt, + ), + { signal }, + ), + }); } if (afterAttach.colorScheme !== "system") { yield* attemptPromise({ operation: "applyColorScheme", tabId, webContentsId: wc.id }, () => diff --git a/apps/desktop/src/preview/ScreenshotCapture.test.ts b/apps/desktop/src/preview/ScreenshotCapture.test.ts new file mode 100644 index 0000000000000..ae1aa26d838e4 --- /dev/null +++ b/apps/desktop/src/preview/ScreenshotCapture.test.ts @@ -0,0 +1,121 @@ +import { describe, expect, it } from "@effect/vitest"; +import { DESKTOP_PREVIEW_SCREENSHOT_ACTIVITY } from "@t3tools/contracts"; +import * as Cause from "effect/Cause"; +import * as Effect from "effect/Effect"; +import * as Exit from "effect/Exit"; +import * as Fiber from "effect/Fiber"; +import { TestClock } from "effect/testing"; +import { vi } from "vite-plus/test"; + +import { captureFreshScreenshot } from "./ScreenshotCapture.ts"; + +const makeHarness = () => { + const reply = Promise.withResolvers(); + const copying = Promise.withResolvers(); + const copy = Promise.withResolvers(); + const activity: unknown[][] = []; + const image = { isEmpty: () => false } as Electron.NativeImage; + const globals = { + [DESKTOP_PREVIEW_SCREENSHOT_ACTIVITY]: (...args: unknown[]) => { + activity.push(args); + return true; + }, + }; + const sendCommand = vi.fn(() => reply.promise); + const capturePage = vi.fn(() => { + copying.resolve(); + return copy.promise; + }); + const contents = { + hostWebContents: { + isDestroyed: () => false, + executeJavaScript: async (script: string) => + new Function("globalThis", `return ${script}`)(globals), + }, + capturePage, + } as unknown as Electron.WebContents; + const debuggerSession = { sendCommand } as unknown as Electron.Debugger; + const capture = (params: Record = {}) => + captureFreshScreenshot(contents, debuggerSession, params, "runtime-tab", "capture-token"); + return { reply, copying, copy, image, activity, sendCommand, capturePage, capture }; +}; + +describe("captureFreshScreenshot", () => { + it.effect("drives an occluded capture without replacing the CDP image or its parameters", () => + Effect.gen(function* () { + const h = makeHarness(); + const params = { + format: "jpeg", + quality: 65, + clip: { x: 2, y: 3, width: 40, height: 50, scale: 0.5 }, + }; + const fiber = yield* h.capture(params).pipe(Effect.forkChild); + yield* Effect.promise(() => h.copying.promise); + h.reply.resolve({ data: "fresh-cdp-image" }); + expect(yield* Fiber.join(fiber)).toEqual({ data: "fresh-cdp-image" }); + h.copy.resolve(h.image); + expect(h.sendCommand).toHaveBeenCalledExactlyOnceWith("Page.captureScreenshot", params); + expect(h.activity.map((args) => args[2])).toEqual([true, false]); + expect(h.activity[0]?.slice(0, 2)).toEqual(h.activity[1]?.slice(0, 2)); + expect(h.capturePage).toHaveBeenCalledTimes(1); + }), + ); + + it.effect("retries a cold guest copy while preserving the pending CDP capture", () => + Effect.gen(function* () { + const h = makeHarness(); + const firstCopy = Promise.withResolvers(); + h.capturePage.mockImplementationOnce(() => { + firstCopy.resolve(); + return Promise.reject(new Error("UnknownVizError")); + }); + const fiber = yield* h.capture().pipe(Effect.forkChild); + yield* Effect.promise(() => firstCopy.promise); + yield* TestClock.adjust("120 millis"); + yield* Effect.promise(() => h.copying.promise); + h.reply.resolve({ data: "fresh-after-cold-start" }); + expect(yield* Fiber.join(fiber)).toEqual({ data: "fresh-after-cold-start" }); + h.copy.resolve(h.image); + expect(h.capturePage).toHaveBeenCalledTimes(2); + expect(h.sendCommand).toHaveBeenCalledTimes(1); + }), + ); + + it.effect("bounds a stalled native copy and releases renderer activity", () => + Effect.gen(function* () { + const h = makeHarness(); + const fiber = yield* h.capture().pipe(Effect.forkChild); + yield* Effect.promise(() => h.copying.promise); + yield* TestClock.adjust("3 seconds"); + expect(Exit.isFailure(yield* Fiber.await(fiber))).toBe(true); + expect(h.activity.map((args) => args[2])).toEqual([true, false]); + h.copy.resolve(h.image); + expect(h.capturePage).toHaveBeenCalledTimes(1); + }), + ); + + it.effect("releases activity when its relay is interrupted while a copy is pending", () => + Effect.gen(function* () { + const h = makeHarness(); + const fiber = yield* h.capture().pipe(Effect.forkChild); + yield* Effect.promise(() => h.copying.promise); + yield* Fiber.interrupt(fiber); + h.copy.resolve(h.image); + expect(h.activity.map((args) => args[2])).toEqual([true, false]); + expect(h.capturePage).toHaveBeenCalledTimes(1); + }), + ); + + it.effect("stops on an unavailable native surface instead of repeatedly copying it", () => + Effect.gen(function* () { + const h = makeHarness(); + h.copy.resolve({ isEmpty: () => true } as Electron.NativeImage); + const result = yield* h.capture().pipe(Effect.exit); + expect(Exit.isFailure(result)).toBe(true); + if (Exit.isFailure(result)) + expect(Cause.pretty(result.cause)).toContain("surface is unavailable"); + expect(h.activity.map((args) => args[2])).toEqual([true, false]); + expect(h.capturePage).toHaveBeenCalledTimes(1); + }), + ); +}); diff --git a/apps/desktop/src/preview/ScreenshotCapture.ts b/apps/desktop/src/preview/ScreenshotCapture.ts new file mode 100644 index 0000000000000..6143e6089d55d --- /dev/null +++ b/apps/desktop/src/preview/ScreenshotCapture.ts @@ -0,0 +1,67 @@ +import { DESKTOP_PREVIEW_SCREENSHOT_ACTIVITY } from "@t3tools/contracts"; +import * as Effect from "effect/Effect"; +import * as Schema from "effect/Schema"; +import * as Schedule from "effect/Schedule"; + +export class ScreenshotCaptureError extends Schema.TaggedError()( + "ScreenshotCaptureError", + { message: Schema.String }, +) {} + +export const captureFreshScreenshot = Effect.fn("Preview.captureFreshScreenshot")(function* ( + contents: Electron.WebContents, + debuggee: Electron.Debugger, + params: Record, + runtimeTabId: string, + token: string, +) { + const host = contents.hostWebContents; + if (!host || host.isDestroyed()) { + return yield* Effect.fail( + new ScreenshotCaptureError({ message: "The desktop preview host window is unavailable." }), + ); + } + const expression = (active: boolean) => + `globalThis[${JSON.stringify(DESKTOP_PREVIEW_SCREENSHOT_ACTIVITY)}]?.(${JSON.stringify(runtimeTabId)},${JSON.stringify(token)},${active}) === true`; + const screenshot = Effect.tryPromise(() => + debuggee.sendCommand("Page.captureScreenshot", params), + ); + // CDP requests a redraw, but an occluded guest needs page copies to present it. + // Discard those copies: only the CDP reply carries the requested clip and scale. + const copies = Effect.forever( + Effect.tryPromise(() => contents.capturePage()).pipe( + // A cold guest can reject its first copy with Chromium's UnknownVizError. + Effect.retry({ times: 2, schedule: Schedule.spaced("120 millis") }), + Effect.filterOrFail( + (image) => !image.isEmpty(), + () => + new ScreenshotCaptureError({ + message: "The desktop preview capture surface is unavailable.", + }), + ), + Effect.andThen(Effect.sleep("16 millis")), + ), + ); + return yield* Effect.scoped( + Effect.gen(function* () { + // Register cleanup before preparation: a late renderer reply must not leave + // an activity lease behind after the caller's deadline or disconnect. + yield* Effect.addFinalizer(() => + Effect.tryPromise(() => host.executeJavaScript(expression(false))).pipe( + Effect.timeout("1 second"), + Effect.ignore, + ), + ); + yield* Effect.tryPromise(() => host.executeJavaScript(expression(true))).pipe( + Effect.filterOrFail( + (prepared) => prepared === true, + () => + new ScreenshotCaptureError({ + message: "The desktop preview rendering surface is unavailable.", + }), + ), + ); + return yield* screenshot.pipe(Effect.raceFirst(copies)); + }), + ).pipe(Effect.timeout("3 seconds")); +}); diff --git a/apps/web/src/browser/ElectronBrowserHost.tsx b/apps/web/src/browser/ElectronBrowserHost.tsx index 78b4d579c71f0..53abdb64b748b 100644 --- a/apps/web/src/browser/ElectronBrowserHost.tsx +++ b/apps/web/src/browser/ElectronBrowserHost.tsx @@ -23,6 +23,7 @@ import { readPreviewAnnotationTheme } from "./annotationTheme"; import { useBrowserDefaults } from "./browserDefaults"; import { useBrowserPointerStore } from "./browserPointerStore"; import { HostedBrowserWebview } from "./HostedBrowserWebview"; +import { installDesktopScreenshotActivity } from "./desktopScreenshotActivity"; import { openUrlInPreview } from "./openFileInPreview"; import { rendersServerTabNatively } from "./previewRuntime"; import { previewRuntimeTabId } from "./previewRuntimeTabId"; @@ -97,6 +98,14 @@ export function ElectronBrowserHost() { }); }, []); + const isHostedTab = useEffectEvent((tabId: string) => + sessions.some((session) => session.runtimeTabId === tabId), + ); + useEffect(() => { + if (!isElectron) return; + return installDesktopScreenshotActivity(isHostedTab); + }, []); + const openPreview = useAtomCommand(previewEnvironment.open, { reportFailure: true }); const onOpenLink = useEffectEvent(({ tabId, url, background }: DesktopPreviewOpenLinkEvent) => { const source = sessions.find((session) => session.runtimeTabId === tabId); diff --git a/apps/web/src/browser/desktopScreenshotActivity.test.ts b/apps/web/src/browser/desktopScreenshotActivity.test.ts new file mode 100644 index 0000000000000..b9c54ac53abe2 --- /dev/null +++ b/apps/web/src/browser/desktopScreenshotActivity.test.ts @@ -0,0 +1,41 @@ +import { DESKTOP_PREVIEW_SCREENSHOT_ACTIVITY } from "@t3tools/contracts"; +import { expect, it } from "vite-plus/test"; + +import { useBrowserSurfaceStore } from "./browserSurfaceStore"; +import { installDesktopScreenshotActivity } from "./desktopScreenshotActivity"; + +it("retains overlapping screenshot and recording activity until each owner releases it", () => { + using stack = new DisposableStack(); + const recording = useBrowserSurfaceStore.getState().acquireActivity("capture-tab"); + stack.defer(recording); + stack.defer(installDesktopScreenshotActivity((tabId) => tabId === "capture-tab")); + const activity = Reflect.get(globalThis, DESKTOP_PREVIEW_SCREENSHOT_ACTIVITY); + expect(activity("capture-tab", "first", true)).toBe(true); + expect(activity("capture-tab", "second", true)).toBe(true); + expect(activity("capture-tab", "first", true)).toBe(true); + expect(useBrowserSurfaceStore.getState().activityByTabId["capture-tab"]).toBe(3); + activity("capture-tab", "first", false); + activity("capture-tab", "first", false); + expect(useBrowserSurfaceStore.getState().activityByTabId["capture-tab"]).toBe(2); + activity("capture-tab", "second", false); + expect(useBrowserSurfaceStore.getState().activityByTabId["capture-tab"]).toBe(1); +}); + +it("rejects unknown tabs and prevents one tab from releasing another tab's token", () => { + using stack = new DisposableStack(); + stack.defer(installDesktopScreenshotActivity((tabId) => tabId === "capture-tab")); + const activity = Reflect.get(globalThis, DESKTOP_PREVIEW_SCREENSHOT_ACTIVITY); + expect(activity("unknown", "first", true)).toBe(false); + expect(activity("capture-tab", "first", true)).toBe(true); + expect(activity("other-tab", "first", false)).toBe(false); + expect(useBrowserSurfaceStore.getState().activityByTabId["capture-tab"]).toBe(1); +}); + +it("releases outstanding tokens when the desktop host unmounts", () => { + const dispose = installDesktopScreenshotActivity(() => true); + const activity = Reflect.get(globalThis, DESKTOP_PREVIEW_SCREENSHOT_ACTIVITY); + activity("capture-tab", "first", true); + dispose(); + expect(useBrowserSurfaceStore.getState().activityByTabId["capture-tab"]).toBeUndefined(); + expect(Reflect.has(globalThis, DESKTOP_PREVIEW_SCREENSHOT_ACTIVITY)).toBe(false); +}); diff --git a/apps/web/src/browser/desktopScreenshotActivity.ts b/apps/web/src/browser/desktopScreenshotActivity.ts new file mode 100644 index 0000000000000..640a9b889ef18 --- /dev/null +++ b/apps/web/src/browser/desktopScreenshotActivity.ts @@ -0,0 +1,39 @@ +import { DESKTOP_PREVIEW_SCREENSHOT_ACTIVITY } from "@t3tools/contracts"; +import { flushSync } from "react-dom"; + +import { acquireBrowserSurfaceActivity } from "./browserSurfaceStore"; + +export function installDesktopScreenshotActivity(isHostedTab: (tabId: string) => boolean) { + const leases = new Map void }>(); + const activity = (tabId: unknown, token: unknown, active: unknown): boolean => { + if (typeof tabId !== "string" || typeof token !== "string" || typeof active !== "boolean") { + return false; + } + const current = leases.get(token); + if (current && current.tabId !== tabId) return false; + if (!active) { + current?.release(); + leases.delete(token); + return true; + } + if (current) return true; + if (!isHostedTab(tabId)) return false; + // The native caller needs the guest's new position committed before asking + // Chromium for a frame, including when the owning chat is not mounted. + flushSync(() => { + leases.set(token, { tabId, release: acquireBrowserSurfaceActivity(tabId) }); + }); + return true; + }; + Object.defineProperty(globalThis, DESKTOP_PREVIEW_SCREENSHOT_ACTIVITY, { + configurable: true, + value: activity, + }); + return () => { + for (const lease of leases.values()) lease.release(); + leases.clear(); + if (Reflect.get(globalThis, DESKTOP_PREVIEW_SCREENSHOT_ACTIVITY) === activity) { + Reflect.deleteProperty(globalThis, DESKTOP_PREVIEW_SCREENSHOT_ACTIVITY); + } + }; +} diff --git a/packages/contracts/src/ipc.ts b/packages/contracts/src/ipc.ts index d1f6b349397ee..20b33aeec6ed0 100644 --- a/packages/contracts/src/ipc.ts +++ b/packages/contracts/src/ipc.ts @@ -1155,6 +1155,8 @@ export interface DesktopBridge { /** Renderer callback invoked by Electron with a fresh user gesture before display-media capture. */ export const DESKTOP_PREVIEW_RECORDING_CAPTURE_TRIGGER = "__t3DesktopPreviewRecordingCapture"; +export const DESKTOP_PREVIEW_SCREENSHOT_ACTIVITY = "__t3DesktopPreviewScreenshotActivity"; + export interface DesktopPreviewBridge { setForwardedShortcuts?: (shortcuts: ReadonlyArray) => Promise; createTab: (tabId: string, defaults?: DesktopPreviewTabDefaults) => Promise; From e708ac62dbd3a41fdf9d108238dcda0c7047172d Mon Sep 17 00:00:00 2001 From: Gregory Kolens Date: Wed, 21 Oct 2026 01:20:34 +0200 Subject: [PATCH 2/2] fix(desktop): bound background capture lifecycle and preserve visibility options --- apps/desktop/src/preview/Manager.test.ts | 275 +++++++++++++++++- apps/desktop/src/preview/Manager.ts | 73 +++-- .../src/preview/ScreenshotCapture.test.ts | 1 + apps/desktop/src/preview/ScreenshotCapture.ts | 2 +- 4 files changed, 320 insertions(+), 31 deletions(-) diff --git a/apps/desktop/src/preview/Manager.test.ts b/apps/desktop/src/preview/Manager.test.ts index ad8ba85c9cbe9..fb96522843881 100644 --- a/apps/desktop/src/preview/Manager.test.ts +++ b/apps/desktop/src/preview/Manager.test.ts @@ -1,6 +1,10 @@ import { it as effectIt } from "@effect/vitest"; import * as NodeCrypto from "@effect/platform-node/NodeCrypto"; -import { DESKTOP_PREVIEW_RECORDING_CAPTURE_TRIGGER } from "@t3tools/contracts"; +import { + DesktopBrowserEvent, + DESKTOP_PREVIEW_RECORDING_CAPTURE_TRIGGER, + DESKTOP_PREVIEW_SCREENSHOT_ACTIVITY, +} from "@t3tools/contracts"; import type { DesktopPreviewRecordingFrame, DesktopPreviewRecordingInputEvent, @@ -16,6 +20,7 @@ import * as Exit from "effect/Exit"; import * as FileSystem from "effect/FileSystem"; import * as Fiber from "effect/Fiber"; import * as Queue from "effect/Queue"; +import * as Schema from "effect/Schema"; import * as Stream from "effect/Stream"; import * as Layer from "effect/Layer"; import * as Logger from "effect/Logger"; @@ -411,6 +416,100 @@ const makeTestPreviewWebContents = ( } as unknown as TestPreviewWebContents; }; +const decodeBrowserEvent = Schema.decodeUnknownSync(Schema.fromJsonString(DesktopBrowserEvent)); + +const setupAgentScreenshot = Effect.fnUntraced(function* () { + const manager = yield* PreviewManager.PreviewManager; + const host = yield* DesktopBrowserHost.DesktopBrowserHost; + const serverTab = { threadId: "thread-capture", tabId: "server-capture" }; + const replies = yield* Queue.unbounded(); + const attached = yield* Queue.unbounded(); + yield* host.events.pipe( + Stream.runForEach((line) => { + const event = decodeBrowserEvent(new TextDecoder().decode(line)); + if (event.type === "attached") return Queue.offer(attached, undefined); + return event.type === "cdp" ? Queue.offer(replies, event.message) : Effect.void; + }), + Effect.forkScoped, + ); + const preparing = Promise.withResolvers(); + const ready = Promise.withResolvers(); + const copying = Promise.withResolvers(); + const copy = Promise.withResolvers(); + const capture = Promise.withResolvers<{ data: string }>(); + const leases = new Set(); + const hostWebContents = makeTestHostWebContents(); + hostWebContents.executeJavaScript.mockImplementation(async (script: string) => { + let acquiring = false; + const prepared = new Function("globalThis", `return ${script}`)({ + [DESKTOP_PREVIEW_SCREENSHOT_ACTIVITY]: (_tabId: string, token: string, active: boolean) => { + if (active) { + acquiring = true; + leases.add(token); + preparing.resolve(); + } else { + leases.delete(token); + } + return true; + }, + }); + return acquiring ? ready.promise : prepared; + }); + const capturePage = vi.fn(() => { + copying.resolve(); + return copy.promise; + }); + const wc = Object.assign(makeTestPreviewWebContents(capturePage, 42, hostWebContents), { + isDevToolsOpened: () => false, + getUserAgent: () => "Electron", + loadURL: vi.fn(async () => {}), + }); + vi.mocked(wc.debugger.sendCommand).mockImplementation(async (method: string) => + method === "Page.captureScreenshot" ? capture.promise : {}, + ); + fromId.mockReturnValue(wc); + const setBackgroundThrottling = vi.fn(); + yield* manager.setMainWindow({ + isDestroyed: () => false, + once: vi.fn(), + webContents: Object.assign(hostWebContents, { setBackgroundThrottling }), + } as never); + yield* manager.createTab("tab_capture", { serverTab }); + yield* manager.registerWebview("tab_capture", wc.id); + yield* Queue.take(attached); + const send = (id: number) => + host.handleCommandLine( + JSON.stringify({ + type: "cdp", + ...serverTab, + message: JSON.stringify({ + id, + method: "Page.captureScreenshot", + params: { format: "png" }, + sessionId: "t3-preview-page", + }), + }), + ); + return { + manager, + host, + wc, + serverTab, + replies, + attached, + preparing, + ready, + copying, + copy, + capture, + leases, + hostWebContents, + capturePage, + setBackgroundThrottling, + send, + }; +}); + const setupRecordingRaceTabs = (manager: PreviewManager.PreviewManager["Service"]) => Effect.gen(function* () { const capturePage = vi.fn(async () => ({ @@ -3011,6 +3110,180 @@ describe("PreviewManager", () => { ), ); + effectIt.effect.each(["preparing", "capturing"] as const)( + "keeps agent capture ownership across navigation while %s and a later guest replacement", + (phase) => + Effect.gen(function* () { + const { + manager, + wc, + replies, + attached, + preparing, + ready, + copying, + copy, + capture, + leases, + hostWebContents, + capturePage, + setBackgroundThrottling, + send, + } = yield* setupAgentScreenshot(); + yield* send(1); + yield* TestClock.adjust(0); + yield* Effect.promise(() => preparing.promise); + expect(setBackgroundThrottling).toHaveBeenLastCalledWith(false); + if (phase === "capturing") { + ready.resolve(true); + yield* Effect.promise(() => copying.promise); + } + yield* manager.navigate("tab_capture", "https://example.com/next"); + expect(leases.size).toBe(1); + ready.resolve(true); + yield* Effect.promise(() => copying.promise); + capture.resolve({ data: "fresh-frame" }); + expect(JSON.parse(yield* Queue.take(replies))).toMatchObject({ + id: 1, + result: { data: "fresh-frame" }, + }); + expect(leases.size).toBe(0); + expect(capturePage).toHaveBeenCalledExactlyOnceWith(undefined, { stayHidden: true }); + expect(setBackgroundThrottling).toHaveBeenLastCalledWith(true); + const replacement = Object.assign( + makeTestPreviewWebContents(capturePage, 43, hostWebContents), + { + isDevToolsOpened: () => false, + getUserAgent: () => "Electron", + }, + ); + vi.mocked(replacement.debugger.sendCommand).mockResolvedValue({ + data: "replacement-frame", + }); + fromId.mockImplementation((id) => (id === replacement.id ? replacement : wc)); + yield* manager.registerWebview("tab_capture", replacement.id); + yield* Queue.take(attached); + yield* send(2); + expect(JSON.parse(yield* Queue.take(replies))).toMatchObject({ + id: 2, + result: { data: "replacement-frame" }, + }); + expect(leases.size).toBe(0); + const copies = capturePage.mock.calls.length; + copy.resolve({ + toJPEG: () => Buffer.from("unused"), + getSize: () => ({ width: 800, height: 600 }), + }); + yield* TestClock.adjust(100); + expect(capturePage).toHaveBeenCalledTimes(copies); + }).pipe(Effect.provide(managerLayer()), Effect.scoped), + ); + + effectIt.effect( + "expires queued agent screenshots before starting their renderer preparation", + () => + Effect.gen(function* () { + const h = yield* setupAgentScreenshot(); + const release = Promise.withResolvers(); + h.hostWebContents.executeJavaScript + .mockImplementationOnce(async () => { + h.preparing.resolve(); + return h.ready.promise; + }) + .mockImplementation(async () => release.promise); + yield* h.send(1); + yield* Effect.promise(() => h.preparing.promise); + yield* h.send(2); + yield* TestClock.adjust("4 seconds"); + const replies = [ + JSON.parse(yield* Queue.take(h.replies)), + JSON.parse(yield* Queue.take(h.replies)), + ]; + expect(replies.map((reply) => reply.id).sort()).toEqual([1, 2]); + expect(replies.every((reply) => reply.error)).toBe(true); + expect(h.hostWebContents.executeJavaScript).toHaveBeenCalledTimes(2); + expect(h.wc.debugger.sendCommand).not.toHaveBeenCalledWith( + "Page.captureScreenshot", + expect.anything(), + ); + expect(h.capturePage).not.toHaveBeenCalled(); + h.ready.resolve(true); + release.resolve(true); + yield* TestClock.adjust(0); + expect(h.hostWebContents.executeJavaScript).toHaveBeenCalledTimes(2); + expect(h.setBackgroundThrottling).toHaveBeenLastCalledWith(true); + }).pipe(Effect.provide(managerLayer()), Effect.scoped), + ); + + effectIt.effect("preserves native screenshot errors through the Manager and CDP relay", () => + Effect.gen(function* () { + const h = yield* setupAgentScreenshot(); + h.ready.resolve(true); + yield* h.send(1); + yield* Effect.promise(() => h.copying.promise); + h.capture.reject(new Error("compositor unavailable")); + expect(JSON.parse(yield* Queue.take(h.replies))).toMatchObject({ + id: 1, + error: { code: -32000, message: "compositor unavailable" }, + }); + expect(h.leases.size).toBe(0); + expect(h.wc.setBackgroundThrottling).not.toHaveBeenCalled(); + expect(h.setBackgroundThrottling).toHaveBeenLastCalledWith(true); + h.copy.resolve({ + toJPEG: () => Buffer.from("unused"), + getSize: () => ({ width: 800, height: 600 }), + }); + yield* TestClock.adjust(100); + expect(h.capturePage).toHaveBeenCalledTimes(1); + }).pipe(Effect.provide(managerLayer()), Effect.scoped), + ); + + effectIt.effect.each(["preparing", "capturing"] as const)( + "cancels a replaced guest while %s and ignores its late completion", + (phase) => + Effect.gen(function* () { + const h = yield* setupAgentScreenshot(); + yield* h.send(1); + yield* Effect.promise(() => h.preparing.promise); + if (phase === "capturing") { + h.ready.resolve(true); + yield* Effect.promise(() => h.copying.promise); + } + const replacement = Object.assign( + makeTestPreviewWebContents(h.capturePage, 43, h.hostWebContents), + { + isDevToolsOpened: () => false, + getUserAgent: () => "Electron", + }, + ); + vi.mocked(replacement.debugger.sendCommand).mockResolvedValue({ + data: "replacement-frame", + }); + fromId.mockImplementation((id) => (id === replacement.id ? replacement : h.wc)); + yield* h.manager.registerWebview("tab_capture", replacement.id); + yield* Queue.take(h.attached); + expect(h.leases.size).toBe(0); + expect(replacement.setBackgroundThrottling).not.toHaveBeenCalled(); + h.ready.resolve(true); + h.capture.resolve({ data: "late-old-frame" }); + h.copy.resolve({ + toJPEG: () => Buffer.from("unused"), + getSize: () => ({ width: 800, height: 600 }), + }); + yield* TestClock.adjust(100); + expect(yield* Queue.poll(h.replies)).toEqual(Option.none()); + expect(h.capturePage).toHaveBeenCalledTimes(phase === "capturing" ? 1 : 0); + yield* h.send(2); + expect(JSON.parse(yield* Queue.take(h.replies))).toMatchObject({ + id: 2, + result: { data: "replacement-frame" }, + }); + expect(h.leases.size).toBe(0); + expect(h.wc.setBackgroundThrottling).not.toHaveBeenCalled(); + expect(h.setBackgroundThrottling).toHaveBeenLastCalledWith(true); + }).pipe(Effect.provide(managerLayer()), Effect.scoped), + ); + effectIt.effect("tells the server when a tab it renders natively closes", () => Effect.gen(function* () { const host = yield* DesktopBrowserHost.DesktopBrowserHost; diff --git a/apps/desktop/src/preview/Manager.ts b/apps/desktop/src/preview/Manager.ts index da2fc26c642c4..699213301125c 100644 --- a/apps/desktop/src/preview/Manager.ts +++ b/apps/desktop/src/preview/Manager.ts @@ -739,7 +739,11 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function ) { yield* SynchronizedRef.modifyEffect(frameCaptureSessionsRef, (sessions) => { const current = sessions.get(tabId); - if (!current || current.unthrottledWebContentsIds.has(wc.id)) { + if ( + !current || + ![...current.consumers].some((consumer) => consumer !== "screenshot") || + current.unthrottledWebContentsIds.has(wc.id) + ) { return Effect.succeed([undefined, sessions] as const); } return setFrameCaptureWebContentsBackgroundThrottling(wc, false).pipe( @@ -2026,6 +2030,7 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function audible: current?.audible ?? false, controller: current?.controller ?? "none", ...(current?.favicon ? { favicon: current.favicon } : {}), + ...(current?.serverTab ? { serverTab: current.serverTab } : {}), updatedAt, }; return [ @@ -2396,30 +2401,34 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function runCapture( Effect.acquireUseRelease( Effect.forkIn( - control.semaphore.withPermit( - Effect.scoped( - Effect.gen(function* () { - yield* Effect.acquireRelease(startFrameCapture(tabId, "screenshot"), () => - stopFrameCapture(tabId, "screenshot").pipe(Effect.ignore), - ); - const token = yield* crypto.randomUUIDv4(); - return yield* captureFreshScreenshot( - wc, - control.debugger, - params, - tabId, - token, - ); - }), - ), - ), + control.semaphore + .withPermit( + Effect.scoped( + Effect.gen(function* () { + yield* Effect.acquireRelease(startFrameCapture(tabId, "screenshot"), () => + stopFrameCapture(tabId, "screenshot").pipe(Effect.ignore), + ); + const token = yield* crypto.randomUUIDv4(); + return yield* captureFreshScreenshot( + wc, + control.debugger, + params, + tabId, + token, + ); + }), + ), + ) + .pipe(Effect.timeout("3 seconds")), control.scope, ), Fiber.join, Fiber.interrupt, ), { signal }, - ), + ).catch((error: unknown) => { + throw Cause.isUnknownError(error) ? error.cause : error; + }), }); } if (afterAttach.colorScheme !== "system") { @@ -2743,7 +2752,7 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function if (current.consumers.has(consumer)) { return [false, sessions] as const; } - if (!current.unthrottledWebContentsIds.has(wc.id)) { + if (consumer !== "screenshot" && !current.unthrottledWebContentsIds.has(wc.id)) { yield* setFrameCaptureWebContentsBackgroundThrottling(wc, false); } let scope = current.scope; @@ -2758,7 +2767,10 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function ...current, scope, consumers: new Set([...current.consumers, consumer]), - unthrottledWebContentsIds: new Set([...current.unthrottledWebContentsIds, wc.id]), + unthrottledWebContentsIds: new Set([ + ...current.unthrottledWebContentsIds, + ...(consumer !== "screenshot" ? [wc.id] : []), + ]), }); }), ] as const; @@ -2766,13 +2778,16 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function if (sessions.size === 0) { yield* setFrameCaptureBackgroundThrottling(false); } - yield* setFrameCaptureWebContentsBackgroundThrottling(wc, false).pipe( - Effect.onError(() => - sessions.size === 0 - ? setFrameCaptureBackgroundThrottling(true).pipe(Effect.ignore) - : Effect.void, - ), - ); + // Still captures need host presentation without a guest visibility override. + if (consumer !== "screenshot") { + yield* setFrameCaptureWebContentsBackgroundThrottling(wc, false).pipe( + Effect.onError(() => + sessions.size === 0 + ? setFrameCaptureBackgroundThrottling(true).pipe(Effect.ignore) + : Effect.void, + ), + ); + } const scope = consumer === "picture-in-picture" ? yield* Scope.fork(parentScope, "sequential") : null; if (scope !== null) { @@ -2784,7 +2799,7 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function copy.set(tabId, { scope, consumers: new Set([consumer]), - unthrottledWebContentsIds: new Set([wc.id]), + unthrottledWebContentsIds: new Set(consumer !== "screenshot" ? [wc.id] : []), lastPictureInPictureFingerprint: null, }); }), diff --git a/apps/desktop/src/preview/ScreenshotCapture.test.ts b/apps/desktop/src/preview/ScreenshotCapture.test.ts index ae1aa26d838e4..ee92a3b229a15 100644 --- a/apps/desktop/src/preview/ScreenshotCapture.test.ts +++ b/apps/desktop/src/preview/ScreenshotCapture.test.ts @@ -51,6 +51,7 @@ describe("captureFreshScreenshot", () => { }; const fiber = yield* h.capture(params).pipe(Effect.forkChild); yield* Effect.promise(() => h.copying.promise); + expect(h.capturePage).toHaveBeenCalledWith(undefined, { stayHidden: true }); h.reply.resolve({ data: "fresh-cdp-image" }); expect(yield* Fiber.join(fiber)).toEqual({ data: "fresh-cdp-image" }); h.copy.resolve(h.image); diff --git a/apps/desktop/src/preview/ScreenshotCapture.ts b/apps/desktop/src/preview/ScreenshotCapture.ts index 6143e6089d55d..cf676ddea1f03 100644 --- a/apps/desktop/src/preview/ScreenshotCapture.ts +++ b/apps/desktop/src/preview/ScreenshotCapture.ts @@ -29,7 +29,7 @@ export const captureFreshScreenshot = Effect.fn("Preview.captureFreshScreenshot" // CDP requests a redraw, but an occluded guest needs page copies to present it. // Discard those copies: only the CDP reply carries the requested clip and scale. const copies = Effect.forever( - Effect.tryPromise(() => contents.capturePage()).pipe( + Effect.tryPromise(() => contents.capturePage(undefined, { stayHidden: true })).pipe( // A cold guest can reject its first copy with Chromium's UnknownVizError. Effect.retry({ times: 2, schedule: Schedule.spaced("120 millis") }), Effect.filterOrFail(