diff --git a/apps/desktop/src/preview/Manager.test.ts b/apps/desktop/src/preview/Manager.test.ts index 304efeafefe9..a9ca5c7e310f 100644 --- a/apps/desktop/src/preview/Manager.test.ts +++ b/apps/desktop/src/preview/Manager.test.ts @@ -182,6 +182,7 @@ describe("previewWindowOpenAction", () => { const { browserWindowConstructor, clipboardItemConstructor, + createFromBuffer, createFromPath, fromId, getFocusedWebContents, @@ -193,6 +194,11 @@ const { } = vi.hoisted(() => ({ browserWindowConstructor: vi.fn(), clipboardItemConstructor: vi.fn(), + createFromBuffer: vi.fn((data: Buffer) => ({ + isEmpty: () => data.length === 0, + getSize: () => ({ width: 800, height: 600 }), + toPNG: () => data, + })), createFromPath: vi.fn((): { readonly isEmpty: () => boolean; readonly toPNG: () => Buffer } => ({ isEmpty: () => false, toPNG: () => Buffer.from("png"), @@ -217,6 +223,7 @@ vi.mock("electron", () => ({ write: writeClipboard, }, nativeImage: { + createFromBuffer, createFromPath, }, shell: { @@ -2223,6 +2230,188 @@ describe("PreviewManager", () => { ), ); + effectIt.effect("keeps the main window painting while a screenshot is captured", () => + withManager((manager) => + Effect.gen(function* () { + const setBackgroundThrottling = vi.fn<(enabled: boolean) => void>(); + const image = { + toPNG: () => Buffer.from("png"), + toJPEG: () => Buffer.from("jpeg"), + getSize: () => ({ width: 1, height: 1 }), + }; + const pending = Promise.withResolvers(); + const capturePage = vi.fn(() => pending.promise); + fromId.mockReturnValue(makeTestPreviewWebContents(capturePage, 42)); + yield* manager.createTab("tab_1"); + yield* manager.registerWebview("tab_1", 42); + yield* manager.setMainWindow({ + isDestroyed: () => false, + once: vi.fn(), + webContents: { setBackgroundThrottling }, + } as never); + + const capture = yield* manager + .captureScreenshot("tab_1") + .pipe(Effect.forkChild({ startImmediately: true })); + yield* TestClock.adjust(0); + expect(capturePage).toHaveBeenCalledOnce(); + expect(setBackgroundThrottling.mock.calls).toEqual([[false]]); + + pending.resolve(image); + yield* Fiber.join(capture); + expect(setBackgroundThrottling.mock.calls).toEqual([[false], [true]]); + }), + ), + ); + + effectIt.effect("leaves the main window painting when a recording stops mid-capture", () => + withManager((manager) => + Effect.gen(function* () { + const setBackgroundThrottling = vi.fn<(enabled: boolean) => void>(); + const image = { + toPNG: () => Buffer.from("png"), + toJPEG: () => Buffer.from("jpeg"), + getSize: () => ({ width: 1, height: 1 }), + }; + const pending = Promise.withResolvers(); + const host = makeTestHostWebContents(); + const snapshotWebContents = makeTestPreviewWebContents(() => pending.promise, 41, host); + const recordedWebContents = makeTestPreviewWebContents( + async () => ({ + toJPEG: () => Buffer.from("recording-frame"), + getSize: () => ({ width: 1280, height: 720 }), + }), + 42, + host, + ); + const webContentsById = new Map([ + [41, snapshotWebContents], + [42, recordedWebContents], + ]); + fromId.mockImplementation((id) => + id === undefined ? null : (webContentsById.get(id) ?? null), + ); + yield* manager.createTab("tab_snapshot"); + yield* manager.createTab("tab_recording"); + yield* manager.registerWebview("tab_snapshot", 41); + yield* manager.registerWebview("tab_recording", 42); + yield* manager.setMainWindow({ + isDestroyed: () => false, + once: vi.fn(), + webContents: { setBackgroundThrottling }, + } as never); + + const capture = yield* manager + .captureScreenshot("tab_snapshot") + .pipe(Effect.forkChild({ startImmediately: true })); + yield* TestClock.adjust(0); + yield* manager.startRecording("tab_recording"); + yield* manager.stopRecording("tab_recording"); + expect(setBackgroundThrottling.mock.calls).not.toContainEqual([true]); + + pending.resolve(image); + yield* Fiber.join(capture); + expect(setBackgroundThrottling.mock.calls.at(-1)).toEqual([true]); + }), + ), + ); + + effectIt.effect( + "snapshots a frame redrawn after reading the page, not the guest's last frame", + () => + withManager((manager) => + Effect.gen(function* () { + const setBackgroundThrottling = vi.fn<(enabled: boolean) => void>(); + // capturePage only nudges the window to draw; its stale pixels must not be the snapshot. + const copies: Array> = []; + const capturePage = vi.fn((..._args: ReadonlyArray) => { + const copy = Promise.withResolvers(); + copies.push(copy); + return copy.promise; + }); + const drawnCopy = { + isEmpty: () => false, + toJPEG: () => Buffer.from("stale"), + getSize: () => ({ width: 1, height: 1 }), + }; + const settlePromises = Effect.promise( + () => new Promise((resolve) => setImmediate(resolve)), + ); + const redrawnPng = Buffer.from("png drawn after the page changed"); + const pendingRedraw = Promise.withResolvers<{ data: string }>(); + const screenshotResults = [Promise.resolve({}), pendingRedraw.promise]; + const sendCommand = vi.fn(async (method: string): Promise => { + if (method === "Page.captureScreenshot") return screenshotResults.shift(); + if (method === "Runtime.evaluate") { + return { + result: { + value: { + url: "https://example.com", + title: "Example", + loading: false, + visibleText: "QA new marker", + interactiveElements: [], + }, + }, + }; + } + return method === "Accessibility.getFullAXTree" ? { nodes: [] } : undefined; + }); + const wc = makeTestPreviewWebContents(capturePage); + Object.assign(wc, { isDevToolsOpened: () => false }); + Object.assign(wc.debugger, { sendCommand }); + fromId.mockReturnValue(wc); + yield* manager.createTab("tab_1"); + yield* manager.registerWebview("tab_1", 42); + yield* manager.setMainWindow({ + isDestroyed: () => false, + once: vi.fn(), + webContents: { setBackgroundThrottling }, + } as never); + + const snapshot = yield* manager + .automationSnapshot("tab_1") + .pipe(Effect.forkChild({ startImmediately: true })); + // The first capture returned no image; the retry waits on Chromium's redraw. + yield* TestClock.adjust(200); + const screenshotCalls = sendCommand.mock.calls + .map(([method], index) => ({ + method, + order: sendCommand.mock.invocationCallOrder[index]!, + })) + .filter(({ method }) => method === "Page.captureScreenshot"); + expect(screenshotCalls).toHaveLength(2); + const pageRead = sendCommand.mock.calls.findIndex( + ([method]) => method === "Runtime.evaluate", + ); + expect(screenshotCalls[0]!.order).toBeGreaterThan( + sendCommand.mock.invocationCallOrder[pageRead]!, + ); + // Presenting the redrawn frame needs the main window to keep drawing. + expect(setBackgroundThrottling.mock.calls).toEqual([[false]]); + // Whole-view copies that leave the page hidden make the window draw the guest. + expect(capturePage).toHaveBeenLastCalledWith(undefined, { stayHidden: true }); + + // Every drawn copy is followed by another until the redraw arrives. + const copiesBeforeDraw = capturePage.mock.calls.length; + copies.at(-1)!.resolve(drawnCopy); + yield* settlePromises; + expect(capturePage).toHaveBeenCalledTimes(copiesBeforeDraw + 1); + + pendingRedraw.resolve({ data: redrawnPng.toString("base64") }); + const result = yield* Fiber.join(snapshot); + expect(result.visibleText).toBe("QA new marker"); + expect(result.screenshot.data).toBe(redrawnPng.toString("base64")); + expect(setBackgroundThrottling.mock.calls).toEqual([[false], [true]]); + + // Once the screenshot settles, the copy in flight is the last one. + for (const copy of copies) copy.resolve(drawnCopy); + yield* settlePromises; + expect(capturePage).toHaveBeenCalledTimes(copiesBeforeDraw + 1); + }), + ), + ); + effectIt.effect("keeps every recorded guest unthrottled until its frame capture stops", () => withManager((manager) => Effect.gen(function* () { @@ -2608,11 +2797,26 @@ describe("PreviewManager", () => { effectIt.effect("releases snapshot control when every capture attempt stalls", () => withManager((manager) => Effect.gen(function* () { - const capturePage = vi.fn(() => new Promise(() => {})); + const copies: Array> = []; + const capturePage = vi.fn(() => { + const copy = Promise.withResolvers(); + copies.push(copy); + return copy.promise; + }); + const copyImage = (empty: boolean) => ({ + isEmpty: () => empty, + toJPEG: () => Buffer.from("copy"), + getSize: () => ({ width: empty ? 0 : 1, height: empty ? 0 : 1 }), + }); + const settlePromises = Effect.promise( + () => new Promise((resolve) => setImmediate(resolve)), + ); + const captureScreenshot = vi.fn(() => new Promise(() => {})); const wc = makeTestPreviewWebContents(capturePage); Object.assign(wc, { isDevToolsOpened: () => false }); Object.assign(wc.debugger, { sendCommand: vi.fn(async (method: string, params?: Record) => { + if (method === "Page.captureScreenshot") return captureScreenshot(); if (method === "Runtime.evaluate") { return { result: { @@ -2640,21 +2844,37 @@ describe("PreviewManager", () => { Effect.forkChild({ startImmediately: true }), ); yield* TestClock.adjust(100); + expect(captureScreenshot).toHaveBeenCalledOnce(); expect(capturePage).toHaveBeenCalledOnce(); const evaluate = yield* manager .automationEvaluate("tab_1", { expression: "42" }) .pipe(Effect.forkChild({ startImmediately: true })); expect(evaluate.pollUnsafe()).toBeUndefined(); + // The first attempt timed out; its copy landing afterwards must not request another. + yield* TestClock.adjust(950); + copies[0]!.resolve(copyImage(false)); + yield* settlePromises; + expect(capturePage).toHaveBeenCalledOnce(); + + // An empty copy did not make the window draw, so the retry stops requesting copies. + yield* TestClock.adjust(100); + expect(captureScreenshot).toHaveBeenCalledTimes(2); + expect(capturePage).toHaveBeenCalledTimes(2); + copies[1]!.resolve(copyImage(true)); + yield* settlePromises; + expect(capturePage).toHaveBeenCalledTimes(2); + yield* TestClock.adjust(4_000); const exit = yield* Fiber.join(snapshot); expect(Exit.isFailure(exit)).toBe(true); + expect(captureScreenshot).toHaveBeenCalledTimes(3); expect(capturePage).toHaveBeenCalledTimes(3); if (Exit.isSuccess(exit)) return; const error = Option.getOrThrow(Cause.findErrorOption(exit.cause)); expect(error).toMatchObject({ _tag: "PreviewOperationError", - operation: "automationSnapshot.capturePage", + operation: "automationSnapshot.captureScreenshot", tabId: "tab_1", webContentsId: 42, cause: { _tag: "TimeoutError" }, diff --git a/apps/desktop/src/preview/Manager.ts b/apps/desktop/src/preview/Manager.ts index b97af6f05204..1c55059608aa 100644 --- a/apps/desktop/src/preview/Manager.ts +++ b/apps/desktop/src/preview/Manager.ts @@ -343,6 +343,60 @@ const normalizeCaptureRect = (value: unknown): PreviewAnnotationRect | null => { }; }; +/** + * Captures a guest frame drawn after this call. `capturePage` copies whatever frame the guest + * last drew, and a guest hidden behind a minimized or hidden window has not drawn since its DOM + * changed, so its copy shows the previous state. `Page.captureScreenshot` keeps the guest + * painting while hidden, forces a redraw, and copies only after that frame is presented. + * Needs an attached debugger, so it serves agent snapshots. The screenshot button and + * annotations keep `capturePage`: a person triggers them from a visible preview whose guest + * is already drawing what they see. + */ +const captureRedrawnFrame = async ( + wc: Electron.WebContents, + signal: AbortSignal, +): Promise => { + const screenshot = wc.debugger.sendCommand("Page.captureScreenshot", { + format: "png", + // Decoded and re-encoded below, so trade compression for main-thread time. + optimizeForSpeed: true, + }); + let settled = false; + const settle = () => { + settled = true; + }; + screenshot.then(settle, settle); + // The window does not draw a guest it does not show (behind the app, or in a minimized + // window) except to serve a copy request, and Chromium throttles an undrawn guest to one + // frame a second, so the forced redraw is presented seconds late. Each copy makes the window + // draw the guest's latest frame, so keep requesting copies, one at a time, until the + // screenshot settles or this attempt is abandoned. A copy that fails or comes back empty ends + // the requests rather than spinning; the attempt then falls back to its timeout and retry. + // stayHidden leaves the page's visibility state alone, and Chromium only allows it for a + // whole-view copy (an empty capture size). + const requestDraw = (): void => { + if (settled || signal.aborted || wc.isDestroyed()) return; + wc.capturePage(undefined, { stayHidden: true }).then( + (copy) => { + if (!copy.isEmpty()) requestDraw(); + }, + () => undefined, + ); + }; + requestDraw(); + const result: unknown = await screenshot; + const data = + typeof result === "object" && result !== null + ? (result as Record)["data"] + : undefined; + const image = + typeof data === "string" ? nativeImage.createFromBuffer(Buffer.from(data, "base64")) : null; + if (!image || image.isEmpty()) { + throw new Error("Page.captureScreenshot returned no image."); + } + return image; +}; + /** `capturePage` never settles when the guest's compositor is wedged. */ const ANNOTATION_SCREENSHOT_TIMEOUT = "5 seconds"; @@ -674,6 +728,8 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function const frameCaptureSessionsRef = yield* SynchronizedRef.make< ReadonlyMap >(new Map()); + // capturePage calls in flight. Like frame capture sessions, each keeps the main window unthrottled. + const paintingCapturesRef = yield* Ref.make(0); const pictureInPictureSessionsRef = yield* SynchronizedRef.make< ReadonlyMap >(new Map()); @@ -707,10 +763,11 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function try: evaluate, catch: (cause) => new PreviewOperationError({ ...errorContext, cause }), }); - const capturePageWithRetry = Effect.fn("PreviewManager.capturePageWithRetry")(function* ( + const captureGuestWithRetry = Effect.fn("PreviewManager.captureGuestWithRetry")(function* ( errorContext: PreviewOperationContext, tabId: string, wc: Electron.WebContents, + captureImage: (wc: Electron.WebContents, signal: AbortSignal) => Promise, ) { const requireCurrentGuest = Effect.gen(function* () { const tabs = yield* SynchronizedRef.get(tabsRef); @@ -723,7 +780,7 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function yield* requireCurrentGuest; const image = yield* Effect.tryPromise({ // An abort-signal parameter makes a stalled promise interruptible. - try: (_signal) => wc.capturePage(), + try: (signal) => captureImage(wc, signal), catch: (cause) => new PreviewOperationError({ ...errorContext, cause }), }).pipe( Effect.timeout(CAPTURE_PAGE_ATTEMPT_TIMEOUT_MS), @@ -735,12 +792,19 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function yield* requireCurrentGuest; return image; }); - return yield* capture.pipe( - Effect.retry({ - times: CAPTURE_PAGE_RETRY_ATTEMPTS - 1, - schedule: Schedule.spaced(CAPTURE_PAGE_RETRY_DELAY_MS), - while: isPreviewOperationError, - }), + // Both captures wait for a window frame that contains the guest. A throttled main window + // that is covered, minimized, or on another Space draws none, so the capture never settles. + return yield* Effect.acquireUseRelease( + holdMainWindowPainting, + () => + capture.pipe( + Effect.retry({ + times: CAPTURE_PAGE_RETRY_ATTEMPTS - 1, + schedule: Schedule.spaced(CAPTURE_PAGE_RETRY_DELAY_MS), + while: isPreviewOperationError, + }), + ), + () => releaseMainWindowPainting, ); }); const currentIso = DateTime.now.pipe(Effect.map(DateTime.formatIso)); @@ -795,6 +859,41 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function if (Option.isNone(mainWindow)) return; yield* setWindowBackgroundThrottling(mainWindow.value, enabled); }); + /** Re-throttles the main window once no frame capture session or capturePage call needs frames. */ + const throttleMainWindowWhenIdle = Effect.fnUntraced(function* (remainingSessions: number) { + if (remainingSessions > 0 || (yield* Ref.get(paintingCapturesRef)) > 0) return; + yield* setFrameCaptureBackgroundThrottling(true).pipe( + Effect.retry({ times: 2 }), + Effect.catch((error) => + Effect.logWarning("Failed to restore preview frame capture throttling.", { error }), + ), + ); + }); + // Both run under the frame capture lock so a session that starts or stops mid-capture sees the count. + const holdMainWindowPainting = SynchronizedRef.modifyEffect(frameCaptureSessionsRef, (sessions) => + Effect.gen(function* () { + const held = yield* Ref.getAndUpdate(paintingCapturesRef, (count) => count + 1); + if (held === 0 && sessions.size === 0) { + // Best effort: a visible window still paints while throttled. + yield* setFrameCaptureBackgroundThrottling(false).pipe( + Effect.catch((error) => + Effect.logWarning("Failed to unthrottle the main window for a preview capture.", { + error, + }), + ), + ); + } + return [undefined, sessions] as const; + }), + ); + const releaseMainWindowPainting = SynchronizedRef.modifyEffect( + frameCaptureSessionsRef, + (sessions) => + Ref.update(paintingCapturesRef, (count) => count - 1).pipe( + Effect.andThen(throttleMainWindowWhenIdle(sessions.size)), + Effect.as([undefined, sessions] as const), + ), + ); const setFrameCaptureWebContentsBackgroundThrottling = Effect.fnUntraced(function* ( wc: Electron.WebContents, enabled: boolean, @@ -909,14 +1008,7 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function yield* restoreFrameCaptureWebContentsBackgroundThrottling( current.unthrottledWebContentsIds, ); - if (remainingSessions.size === 0) { - yield* setFrameCaptureBackgroundThrottling(true).pipe( - Effect.retry({ times: 2 }), - Effect.catch((error) => - Effect.logWarning("Failed to restore preview frame capture throttling.", { error }), - ), - ); - } + yield* throttleMainWindowWhenIdle(remainingSessions.size); return [current.scope, remainingSessions] as const; }), ).pipe( @@ -2079,7 +2171,7 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function } yield* SynchronizedRef.modifyEffect(frameCaptureSessionsRef, (sessions) => Effect.gen(function* () { - if (sessions.size > 0) { + if (sessions.size > 0 || (yield* Ref.get(paintingCapturesRef)) > 0) { yield* setWindowBackgroundThrottling(window, false); } yield* Ref.set(mainWindowRef, Option.some(window)); @@ -2846,7 +2938,7 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function const [createdAt, millis, image] = yield* Effect.all([ currentIso, currentMillis, - capturePageWithRetry( + captureGuestWithRetry( { operation: "captureScreenshot.capturePage", tabId, @@ -2854,6 +2946,7 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function }, tabId, wc, + (guest) => guest.capturePage(), ), ]); const id = `browser-screenshot-${artifactSiteSlug(wc.getURL())}-${millis.toString(36)}`; @@ -3090,11 +3183,7 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function yield* setFrameCaptureBackgroundThrottling(false); } yield* setFrameCaptureWebContentsBackgroundThrottling(wc, false).pipe( - Effect.onError(() => - sessions.size === 0 - ? setFrameCaptureBackgroundThrottling(true).pipe(Effect.ignore) - : Effect.void, - ), + Effect.onError(() => throttleMainWindowWhenIdle(sessions.size)), ); const scope = consumer === "picture-in-picture" ? yield* Scope.fork(parentScope, "sequential") : null; @@ -3725,14 +3814,15 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function ); const [accessibility, sourceImage, diagnostics, timelines] = yield* Effect.all([ send("Accessibility.getFullAXTree"), - capturePageWithRetry( + captureGuestWithRetry( { - operation: "automationSnapshot.capturePage", + operation: "automationSnapshot.captureScreenshot", tabId, webContentsId: wc.id, }, tabId, wc, + captureRedrawnFrame, ), Ref.get(diagnosticsRef), Ref.get(actionTimelineRef),