diff --git a/docs/browser-capture-harness.md b/docs/browser-capture-harness.md index 21f80eba39..594aacf5f6 100644 --- a/docs/browser-capture-harness.md +++ b/docs/browser-capture-harness.md @@ -103,6 +103,9 @@ layering inside `overlay-root`. Activating a presented browser also focuses its There is no renderer prep/restore handshake or lifetime background-throttling override. Screenshot capture temporarily enables frame production inside the shared serialized queue, -restores the previous throttling policy on success or failure, invalidates before each attempt, -and retries known first-frame failures within the 5-second capture budget. Viewport screenshots use `capturePage({ stayHidden:false })`; -full-page screenshots use the existing CDP path with layout metrics and screenshot clip. +restores the previous throttling policy on success or failure, and retains a 5-second capture budget. +The browser tool takes one viewport frame through Electron's frame subscription and releases the +subscription on completion or cancellation. A resized resident guest can produce fresh pixels +while `capturePage()` leaves its surface-copy request pending; waiting for animation frames or +retrying the copy does not repair that state. Full-page screenshots retain the CDP path with layout +metrics and screenshot clip. diff --git a/packages/desktop/e2e/browser-tabs.e2e.mjs b/packages/desktop/e2e/browser-tabs.e2e.mjs index 208661e033..40de9de6e2 100644 --- a/packages/desktop/e2e/browser-tabs.e2e.mjs +++ b/packages/desktop/e2e/browser-tabs.e2e.mjs @@ -240,20 +240,6 @@ async function callBrowserTool(client, name, args = {}) { return mcpPayload(await client.callTool({ name, args }), name); } -async function callBrowserToolUntilReady(client, name, args = {}) { - const deadline = Date.now() + timeoutMs; - while (Date.now() < deadline) { - const result = await client.callTool({ name, args }); - const payload = result.structuredContent; - if (payload?.ok === true) return payload.result; - if (payload?.ok !== false || payload.error?.retryable !== true) { - return mcpPayload(result, name); - } - await delay(100); - } - throw new Error(`${name} remained unavailable for ${timeoutMs}ms`); -} - async function waitForGuestSelector(client, browserId) { const deadline = Date.now() + 5_000; while (Date.now() < deadline) { @@ -373,7 +359,8 @@ async function readPresentation(page, browserId) { async function readViewport(client, browserId) { const evaluated = await callBrowserTool(client, "browser_evaluate", { browserId, - function: "() => ({ width: window.innerWidth, height: window.innerHeight })", + function: + "() => ({ width: window.innerWidth, height: window.innerHeight, scale: window.devicePixelRatio })", }); return JSON.parse(evaluated.resultJson); } @@ -639,10 +626,11 @@ async function runRegression({ const deviceSizeMenuPainted = await selectDeviceSize(page, "iPhone SE · 375×667"); assert(deviceSizeMenuPainted, "Device size menu did not paint above the browser surface"); + const deviceViewport = await readViewport(client, browserId); recordViewportMismatch( failures, "device size menu paints and receives input above the browser surface", - await readViewport(client, browserId), + deviceViewport, { width: 375, height: 667 }, ); @@ -651,6 +639,12 @@ async function runRegression({ text: "Bridge target", timeoutMs: 5_000, }); + const resizedScreenshot = await callBrowserTool(client, "browser_screenshot", { browserId }); + assert( + resizedScreenshot.width === Math.round(375 * deviceViewport.scale) && + resizedScreenshot.height === Math.round(667 * deviceViewport.scale), + `Screenshot after resize returned ${resizedScreenshot.width}×${resizedScreenshot.height}`, + ); const requestedViewport = { width: 640, height: 480 }; await callBrowserTool(client, "browser_resize", { browserId, ...requestedViewport }); recordViewportMismatch( @@ -726,7 +720,7 @@ async function runRegression({ { timeout: timeoutMs }, ); try { - await callBrowserToolUntilReady(client, "browser_screenshot", { browserId }); + await callBrowserTool(client, "browser_screenshot", { browserId }); } catch (error) { failures.push(`inactive browser remains captureable: ${String(error)}`); } diff --git a/packages/desktop/src/features/browser-automation/ipc.test.ts b/packages/desktop/src/features/browser-automation/ipc.test.ts index a36b7dea0d..db78fe0205 100644 --- a/packages/desktop/src/features/browser-automation/ipc.test.ts +++ b/packages/desktop/src/features/browser-automation/ipc.test.ts @@ -1,4 +1,3 @@ -import type { Rectangle } from "electron"; import { describe, expect, test, vi } from "vitest"; import type { TabImage } from "./service.js"; import { adaptWebContents, HostSnapshotEngineRegistry } from "./ipc.js"; @@ -119,10 +118,8 @@ class FakeWebContents { public readonly debugger = new FakeDebugger(); public readonly inputEvents: IsolatedKeyboardInputEvent[] = []; public readonly loadedUrls: string[] = []; - public readonly captures: Array<{ - rect: Rectangle | undefined; - options: { stayHidden?: boolean } | undefined; - }> = []; + public frameListener: ((image: TabImage) => void) | null = null; + public endedFrameSubscriptions = 0; public readonly invalidations: string[] = []; private consoleMessageListener: ConsoleMessageListener | null = null; private destroyedListener: (() => void) | null = null; @@ -175,12 +172,14 @@ class FakeWebContents { public reload(): void {} - public async capturePage( - rect?: Rectangle, - options?: { stayHidden?: boolean }, - ): Promise { - this.captures.push({ rect, options }); - return new FakeImage(); + public beginFrameSubscription(onlyDirty: boolean, callback: (image: TabImage) => void): void { + expect(onlyDirty).toBe(false); + this.frameListener = callback; + } + + public endFrameSubscription(): void { + this.frameListener = null; + this.endedFrameSubscriptions += 1; } public invalidate(): void { @@ -245,16 +244,30 @@ describe("browser automation IPC adapter", () => { ]); }); - test("delegates viewport capture to the guest without a renderer prep bridge", async () => { + test("captures one rendered frame and releases the subscription", async () => { const contents = new FakeWebContents(20); const tab = adaptWebContents(contents); + const controller = new AbortController(); + const capture = tab.captureFrame(controller.signal); + contents.frameListener?.(new FakeImage()); - const image = await tab.capturePage({ stayHidden: false }); - tab.invalidate(); + expect((await capture).getSize()).toEqual({ width: 640, height: 480 }); + expect(contents.frameListener).toBeNull(); + controller.abort(); + expect(contents.endedFrameSubscriptions).toBe(1); + }); - expect(image.getSize()).toEqual({ width: 640, height: 480 }); - expect(contents.captures).toEqual([{ rect: undefined, options: { stayHidden: false } }]); - expect(contents.invalidations).toEqual(["invalidate"]); + test("cancels a pending frame subscription when the capture budget expires", async () => { + const contents = new FakeWebContents(2001); + const tab = adaptWebContents(contents); + const controller = new AbortController(); + const capture = tab.captureFrame(controller.signal); + const failure = expect(capture).rejects.toThrow("capture deadline"); + controller.abort(new Error("capture deadline")); + + await failure; + expect(contents.frameListener).toBeNull(); + expect(contents.endedFrameSubscriptions).toBe(1); }); test("collects console messages until the guest is destroyed", () => { diff --git a/packages/desktop/src/features/browser-automation/ipc.ts b/packages/desktop/src/features/browser-automation/ipc.ts index d21c51b84f..0d28d46004 100644 --- a/packages/desktop/src/features/browser-automation/ipc.ts +++ b/packages/desktop/src/features/browser-automation/ipc.ts @@ -1,4 +1,3 @@ -import type { Rectangle } from "electron"; import { ipcMain } from "electron"; import { BrowserAutomationExecuteRequestSchema } from "@getpaseo/protocol/browser-automation/rpc-schemas"; import type { @@ -105,7 +104,8 @@ interface BrowserAutomationWebContents extends ConsoleMessageEmitter { goBack(): void; goForward(): void; reload(): void; - capturePage(rect?: Rectangle, options?: { stayHidden?: boolean }): Promise; + beginFrameSubscription(onlyDirty: boolean, callback: (image: TabImage) => void): void; + endFrameSubscription(): void; invalidate(): void; getBackgroundThrottling(): boolean; setBackgroundThrottling(allowed: boolean): void; @@ -130,7 +130,7 @@ export function adaptWebContents(contents: BrowserAutomationWebContents): TabCon goBack: () => contents.goBack(), goForward: () => contents.goForward(), reload: () => contents.reload(), - capturePage: (captureOptions) => contents.capturePage(undefined, captureOptions), + captureFrame: (signal) => captureViewportFrame(contents, signal), invalidate: () => contents.invalidate(), withFrameProduction: async (capture) => { const previous = contents.getBackgroundThrottling(); @@ -154,6 +154,35 @@ export function adaptWebContents(contents: BrowserAutomationWebContents): TabCon }; } +function captureViewportFrame( + contents: BrowserAutomationWebContents, + signal: AbortSignal, +): Promise { + signal.throwIfAborted(); + return new Promise((resolve, reject) => { + const stop = () => { + signal.removeEventListener("abort", abort); + if (!contents.isDestroyed()) contents.endFrameSubscription(); + }; + const abort = () => { + stop(); + reject(signal.reason); + }; + signal.addEventListener("abort", abort, { once: true }); + try { + // A resized resident guest can paint while capturePage's surface-copy + // request remains pending. Subscribe to its rendered frames instead. + contents.beginFrameSubscription(false, (image) => { + stop(); + resolve(image); + }); + } catch (error) { + stop(); + reject(error); + } + }); +} + function getCdpQueue(contentsId: number): CdpSessionQueue { const existing = cdpQueuesByContentsId.get(contentsId); if (existing) { diff --git a/packages/desktop/src/features/browser-automation/service.test.ts b/packages/desktop/src/features/browser-automation/service.test.ts index cbce7ae3bc..dee624a3f7 100644 --- a/packages/desktop/src/features/browser-automation/service.test.ts +++ b/packages/desktop/src/features/browser-automation/service.test.ts @@ -39,7 +39,7 @@ class FakeTab implements TabContents { public readonly loadedUrls: string[] = []; public readonly scripts: string[] = []; public readonly actions: string[] = []; - public readonly capturedViewports: Array<{ stayHidden?: boolean }> = []; + public readonly capturedViewports: AbortSignal[] = []; public readonly debugCommands: Array<{ command: string; params?: Record }> = []; public readonly inputEvents: IsolatedKeyboardInputEvent[] = []; private readonly captureStartWaiters: Array<() => void> = []; @@ -71,7 +71,6 @@ class FakeTab implements TabContents { public captureNeverPaints = false; public captureThrows = false; public captureErrorMessage = "capture failed"; - public viewportCaptureFailuresBeforeSuccess = 0; public deferCaptures = false; public fullPageScreenshotThrows = false; public fullPageScreenshotErrorMessage = "UnknownVizError"; @@ -168,14 +167,10 @@ class FakeTab implements TabContents { this.actions.push("reload"); } - public async capturePage(options?: { stayHidden?: boolean }): Promise { - this.capturedViewports.push(options ?? {}); + public async captureFrame(signal: AbortSignal): Promise { + this.capturedViewports.push(signal); this.actions.push("capture"); this.resolveCaptureStartWaiters(); - if (this.viewportCaptureFailuresBeforeSuccess > 0) { - this.viewportCaptureFailuresBeforeSuccess -= 1; - throw new Error(this.captureErrorMessage); - } if (this.captureThrows) { throw new Error(this.captureErrorMessage); } @@ -1564,7 +1559,8 @@ describe("executeAutomationCommand", () => { height: 480, }, }); - expect(browser.tab.capturedViewports).toEqual([{ stayHidden: false }]); + expect(browser.tab.capturedViewports).toHaveLength(1); + expect(browser.tab.capturedViewports[0].aborted).toBe(true); expect(browser.tab.actions).toEqual(["invalidate", "capture"]); }); @@ -1590,6 +1586,7 @@ describe("executeAutomationCommand", () => { }, }); expect(browser.tab.actions).toEqual(["invalidate", "capture"]); + expect(browser.tab.capturedViewports[0].aborted).toBe(true); } finally { vi.useRealTimers(); } @@ -1634,76 +1631,6 @@ describe("executeAutomationCommand", () => { expect(browser.tab.actions).toEqual(["invalidate", "capture"]); }); - test("screenshot retries UnknownVizError until the first viewport frame appears", async () => { - vi.useFakeTimers(); - try { - const browser = new BrowserAutomationHarness(); - browser.tab.captureErrorMessage = "UnknownVizError"; - browser.tab.viewportCaptureFailuresBeforeSuccess = 2; - - const resultPromise = browser.execute({ - command: "screenshot", - args: { browserId: BROWSER_A }, - }); - await vi.advanceTimersByTimeAsync(400); - - await expect(resultPromise).resolves.toEqual({ - requestId: "req-screenshot", - ok: true, - result: { - command: "screenshot", - browserId: BROWSER_A, - mimeType: "image/png", - dataBase64: "iVBORwECAw==", - width: 640, - height: 480, - }, - }); - expect(browser.tab.capturedViewports).toHaveLength(3); - expect(browser.tab.actions).toEqual([ - "invalidate", - "capture", - "invalidate", - "capture", - "invalidate", - "capture", - ]); - } finally { - vi.useRealTimers(); - } - }); - - test("screenshot returns no-frame after UnknownVizError spends the retry budget", async () => { - vi.useFakeTimers(); - try { - const browser = new BrowserAutomationHarness(); - browser.tab.captureThrows = true; - browser.tab.captureErrorMessage = "UnknownVizError"; - - const resultPromise = browser.execute({ - command: "screenshot", - args: { browserId: BROWSER_A }, - }); - await vi.advanceTimersByTimeAsync(5_000); - - await expect(resultPromise).resolves.toEqual({ - requestId: "req-screenshot", - ok: false, - error: { - code: "screenshot_no_frame", - message: "The tab has not painted yet. Retry the screenshot.", - retryable: true, - }, - }); - expect(browser.tab.capturedViewports.length).toBeGreaterThan(1); - expect(browser.tab.actions.filter((action) => action === "invalidate")).toHaveLength( - browser.tab.capturedViewports.length, - ); - } finally { - vi.useRealTimers(); - } - }); - test("overlapping screenshots serialize through the shared capture queue", async () => { const browser = new BrowserAutomationHarness(); browser.tab.deferCaptures = true; diff --git a/packages/desktop/src/features/browser-automation/service.ts b/packages/desktop/src/features/browser-automation/service.ts index a5e89378c9..1bcf96eec4 100644 --- a/packages/desktop/src/features/browser-automation/service.ts +++ b/packages/desktop/src/features/browser-automation/service.ts @@ -35,7 +35,7 @@ export interface TabContents { goBack(): void; goForward(): void; reload(): void; - capturePage(options?: TabCapturePageOptions): Promise; + captureFrame(signal: AbortSignal): Promise; invalidate(): void; withFrameProduction(capture: () => Promise): Promise; sendInputEvent(event: IsolatedKeyboardInputEvent): void; @@ -51,10 +51,6 @@ export interface TabImage { getSize(): { width: number; height: number }; } -export interface TabCapturePageOptions { - stayHidden?: boolean; -} - export interface BrowserRegistry { listRegisteredBrowserIds(): string[]; listRegisteredBrowserIdsForWorkspace(workspaceId: string): string[]; @@ -215,7 +211,22 @@ async function runPaintedPixelCapture( } async function capturePaintedViewport(contents: TabContents): Promise { - return runPaintedPixelCapture(contents, () => contents.capturePage({ stayHidden: false })); + return runSerializedPixelCapture(() => + contents.withFrameProduction(async () => { + const deadline = Date.now() + PIXEL_CAPTURE_TIMEOUT_MS; + const controller = new AbortController(); + try { + await waitForPaint(contents, deadline); + contents.invalidate(); + return await withPixelCaptureTimeout( + contents.captureFrame(controller.signal), + deadline - Date.now(), + ); + } finally { + controller.abort(); + } + }), + ); } function tabInfoFromContents(