From cda1ba43cc47edb594c9e762e1cdba3309c2c833 Mon Sep 17 00:00:00 2001 From: Mohamed Boudra Date: Fri, 3 Jul 2026 07:18:45 +0200 Subject: [PATCH] Fix fresh browser screenshot prep race --- docs/browser-capture-harness.md | 9 +- .../browser-webview-resident.browser.test.ts | 28 ++++ .../components/browser-webview-resident.ts | 6 +- packages/desktop/capture-harness/index.html | 14 +- packages/desktop/capture-harness/main.js | 75 +++++++++- .../features/browser-automation/ipc.test.ts | 119 ++++++++++++++- .../src/features/browser-automation/ipc.ts | 80 +++++++++- .../browser-automation/service.test.ts | 139 +++++++++++++++++- .../features/browser-automation/service.ts | 71 ++++++--- 9 files changed, 497 insertions(+), 44 deletions(-) diff --git a/docs/browser-capture-harness.md b/docs/browser-capture-harness.md index 05a2c9dad..9c8009dfb 100644 --- a/docs/browser-capture-harness.md +++ b/docs/browser-capture-harness.md @@ -8,6 +8,8 @@ It validates the compositor behavior that unit tests cannot see: - the resident webview guest is sized to 1280x800 logical pixels; - multiple resident webviews are parked as an overlapping stack, and the capture target is raised above its sibling webviews before capture; +- a newly attached resident webview can be captured immediately after load, without + an extra settle delay; - the app-style prep sequence moves the host to a paintable 1x1 clipped state, waits two animation frames and a layout read, then both viewport `capturePage` and full-page CDP screenshots return real pixels; - restore returns the host to offscreen parking after every capture. @@ -25,9 +27,10 @@ packages/desktop/capture-harness/out/ A passing run prints `PASS` lines for both guest sizes, the expected parked-capture failure, the legacy stacked-below-the-clip failure for the second webview, five viewport -prep captures and five full-page prep captures for each of the first two webviews, and -final completion. The PNG sizes may be device-pixel scaled; on a Retina display the -1280x800 logical viewport is usually saved as 2560x1600. +prep captures and five full-page prep captures for each of the first two webviews, a +fresh-immediate viewport and full-page capture for a newly attached webview, and final +completion. The PNG sizes may be device-pixel scaled; on a Retina display the 1280x800 +logical viewport is usually saved as 2560x1600. ## Mechanism diff --git a/packages/app/src/components/browser-webview-resident.browser.test.ts b/packages/app/src/components/browser-webview-resident.browser.test.ts index aafdca080..fc1510803 100644 --- a/packages/app/src/components/browser-webview-resident.browser.test.ts +++ b/packages/app/src/components/browser-webview-resident.browser.test.ts @@ -144,6 +144,34 @@ describe("resident browser webviews", () => { expect(secondWebview.style.zIndex).toBe("0"); }); + it("clears capture preparation when a resident webview is taken visible", async () => { + const webview = ensureResidentBrowserWebview({ + browserId: "browser-visible", + url: "https://example.com", + }); + if (!webview) { + throw new Error("Expected resident browser webview"); + } + + const preparation = await prepareResidentBrowserWebviewForPixelCapture({ + browserId: "browser-visible", + }); + const visibleWebview = takeResidentBrowserWebview("browser-visible"); + + expect(visibleWebview).toBe(webview); + expect(webview.style.position).toBe(""); + expect(webview.style.left).toBe(""); + expect(webview.style.top).toBe(""); + expect(webview.style.zIndex).toBe(""); + + await restoreResidentBrowserWebviewAfterPixelCapture(preparation); + + expect(webview.style.position).toBe(""); + expect(webview.style.left).toBe(""); + expect(webview.style.top).toBe(""); + expect(webview.style.zIndex).toBe(""); + }); + it("keeps the resident host paintable until every capture token is restored", async () => { ensureResidentBrowserWebview({ browserId: "browser-overlap", diff --git a/packages/app/src/components/browser-webview-resident.ts b/packages/app/src/components/browser-webview-resident.ts index 1e6da6a97..a6e37aed2 100644 --- a/packages/app/src/components/browser-webview-resident.ts +++ b/packages/app/src/components/browser-webview-resident.ts @@ -202,7 +202,10 @@ function releaseCapturePreparationToken(token: string): void { } activeCapturePreparations.delete(token); if (preparation.preparesResidentHost) { - applyResidentWebviewStyle(preparation.webview); + const host = readDocument()?.getElementById(RESIDENT_BROWSER_HOST_ID); + if (host instanceof HTMLElement && preparation.webview.parentElement === host) { + applyResidentWebviewStyle(preparation.webview); + } syncResidentHostCaptureState(); } } @@ -276,6 +279,7 @@ export function takeResidentBrowserWebview(browserId: string): HTMLElement | nul } residentWebviewsByBrowserId.delete(normalizedBrowserId); + releaseCapturePreparationsForBrowser(normalizedBrowserId); clearResidentWebviewParkingStyle(webview); return webview; } diff --git a/packages/desktop/capture-harness/index.html b/packages/desktop/capture-harness/index.html index e631d3497..83ca52968 100644 --- a/packages/desktop/capture-harness/index.html +++ b/packages/desktop/capture-harness/index.html @@ -48,7 +48,8 @@ const activeTokens = new Set(); let nextTokenId = 0; - for (let index = 0; index < webviewCount; index += 1) { + function appendHarnessWebview(sourceUrl) { + const index = webviews.length; const webview = document.createElement("webview"); webview.id = `target-webview-${index + 1}`; webview.className = "capture-harness-webview"; @@ -57,9 +58,15 @@ webview.setAttribute("allowpopups", "true"); webview.setAttribute("spellcheck", "false"); webview.setAttribute("autosize", "on"); - webview.src = params.get("targetUrl") || "bright.html"; + webview.src = sourceUrl; + applyStackedWebviewStyle(webview); host.appendChild(webview); webviews.push(webview); + return index; + } + + for (let index = 0; index < webviewCount; index += 1) { + appendHarnessWebview(params.get("targetUrl") || "bright.html"); } function applyHostParking() { @@ -203,6 +210,9 @@ typeof webview.getWebContentsId === "function" ? webview.getWebContentsId() : null, ); }, + addWebview(sourceUrl) { + return appendHarnessWebview(sourceUrl || params.get("targetUrl") || "bright.html"); + }, async prepareForPixelCapture(targetIndex = 0) { const token = `capture-${++nextTokenId}`; activeTokens.add(token); diff --git a/packages/desktop/capture-harness/main.js b/packages/desktop/capture-harness/main.js index 4e7459b2b..8523bab22 100644 --- a/packages/desktop/capture-harness/main.js +++ b/packages/desktop/capture-harness/main.js @@ -143,7 +143,7 @@ async function saveImage(image, outputPath) { await fsp.writeFile(outputPath, image.toPNG()); } -async function waitForGuestLoad(contents) { +async function waitForGuestLoad(contents, input = {}) { await new Promise((resolve) => { if (!contents.isLoading()) { resolve(); @@ -152,7 +152,10 @@ async function waitForGuestLoad(contents) { contents.once("did-finish-load", resolve); contents.once("did-fail-load", resolve); }); - await new Promise((resolve) => setTimeout(resolve, 500)); + const settleMs = input.settleMs ?? 500; + if (settleMs > 0) { + await new Promise((resolve) => setTimeout(resolve, settleMs)); + } } async function renderer(win, expression) { @@ -228,14 +231,23 @@ async function captureFullPage(contents) { } } -async function captureWithPrep({ win, contents, mode, repeatIndex, targetIndex, guestMetrics }) { +async function captureWithPrep({ + win, + contents, + mode, + repeatIndex, + targetIndex, + guestMetrics, + repeatTotal = REPEAT_COUNT, + label = "prep", +}) { const preparation = await renderer( win, `window.captureHarness.prepareForPixelCapture(${JSON.stringify(targetIndex)})`, ); const outputPath = path.join( OUT_DIR, - `${mode}-webview-${targetIndex + 1}-prep-${repeatIndex}.png`, + `${mode}-webview-${targetIndex + 1}-${label}-${repeatIndex}.png`, ); try { const image = @@ -251,11 +263,11 @@ async function captureWithPrep({ win, contents, mode, repeatIndex, targetIndex, const bright = analysis.brightRatio.toFixed(4); if (!analysis.pass) { fail( - `${mode} webview ${targetIndex + 1} prep ${repeatIndex}/${REPEAT_COUNT} size=${size} logical=${logicalSize} bright=${bright} text=${analysis.textNonUniform} file=${outputPath}`, + `${mode} webview ${targetIndex + 1} ${label} ${repeatIndex}/${repeatTotal} size=${size} logical=${logicalSize} bright=${bright} text=${analysis.textNonUniform} file=${outputPath}`, ); } pass( - `${mode} webview ${targetIndex + 1} prep ${repeatIndex}/${REPEAT_COUNT} size=${size} logical=${logicalSize} bright=${bright} text=${analysis.textNonUniform} file=${outputPath}`, + `${mode} webview ${targetIndex + 1} ${label} ${repeatIndex}/${repeatTotal} size=${size} logical=${logicalSize} bright=${bright} text=${analysis.textNonUniform} file=${outputPath}`, ); return analysis; } finally { @@ -308,14 +320,58 @@ async function expectLegacySecondWebviewFailure({ win, contents, mode, guestMetr } } +async function captureFreshWebviewImmediately({ win, waitForNextAttachedGuest }) { + const freshGuestPromise = waitForNextAttachedGuest(); + const targetIndex = await renderer( + win, + `window.captureHarness.addWebview(${JSON.stringify(fileUrl(path.join(ROOT, "bright.html")))})`, + ); + const guest = await withTimeout(freshGuestPromise, "fresh did-attach-webview"); + await waitForGuestLoad(guest, { settleMs: 0 }); + const guestMetrics = await readGuestMetrics(guest); + if (guestMetrics.innerWidth !== VIEWPORT_WIDTH || guestMetrics.innerHeight !== VIEWPORT_HEIGHT) { + fail( + `fresh guest viewport sizing webview ${targetIndex + 1} inner=${guestMetrics.innerWidth}x${guestMetrics.innerHeight} expected=${VIEWPORT_WIDTH}x${VIEWPORT_HEIGHT}`, + ); + } + pass( + `fresh guest viewport sizing webview ${targetIndex + 1} inner=${guestMetrics.innerWidth}x${guestMetrics.innerHeight} dpr=${guestMetrics.devicePixelRatio}`, + ); + await captureWithPrep({ + win, + contents: guest, + mode: "viewport", + repeatIndex: 1, + targetIndex, + guestMetrics, + repeatTotal: 1, + label: "fresh-immediate", + }); + await captureWithPrep({ + win, + contents: guest, + mode: "full-page", + repeatIndex: 1, + targetIndex, + guestMetrics, + repeatTotal: 1, + label: "fresh-immediate", + }); +} + async function main() { ensureDirSync(OUT_DIR); const attachedGuests = []; + const freshGuestWaiters = []; let resolveGuests; const guestsPromise = new Promise((resolve) => { resolveGuests = resolve; }); + const waitForNextAttachedGuest = () => + new Promise((resolve) => { + freshGuestWaiters.push(resolve); + }); const win = new BrowserWindow({ width: 1000, height: 700, @@ -335,6 +391,10 @@ async function main() { }); win.webContents.on("did-attach-webview", (_event, contents) => { attachedGuests.push(contents); + const waiter = freshGuestWaiters.shift(); + if (waiter) { + waiter(contents); + } if (attachedGuests.length >= 2) { resolveGuests(attachedGuests); } @@ -398,6 +458,9 @@ async function main() { await renderer(win, "window.captureHarness.restoreParking()"); + await captureFreshWebviewImmediately({ win, waitForNextAttachedGuest }); + await renderer(win, "window.captureHarness.restoreParking()"); + const results = []; for (const targetIndex of [0, 1]) { for (let index = 1; index <= REPEAT_COUNT; index += 1) { diff --git a/packages/desktop/src/features/browser-automation/ipc.test.ts b/packages/desktop/src/features/browser-automation/ipc.test.ts index f678c0f0e..ead07c678 100644 --- a/packages/desktop/src/features/browser-automation/ipc.test.ts +++ b/packages/desktop/src/features/browser-automation/ipc.test.ts @@ -299,6 +299,101 @@ describe("browser automation IPC adapter", () => { expect(ipc.listenerCount("paseo:browser:capture-prepared")).toBe(0); }); + test("prepareForPixelCapture waits for the guest host renderer before asking for prep", async () => { + vi.useFakeTimers(); + try { + const host = new FakeHostWebContents(10); + const contents = new FakeWebContents(20, null); + const ipc = new FakeIpcBridge(); + const tab = adaptWebContents(contents, "browser-a", { + ipc, + createRequestId: () => "prepare-1", + timeoutMs: 250, + }); + + const preparation = tab.prepareForPixelCapture(); + await vi.advanceTimersByTimeAsync(49); + + expect(host.sentMessages).toEqual([]); + + contents.hostWebContents = host; + await vi.advanceTimersByTimeAsync(1); + + expect(host.sentMessages).toEqual([ + { + channel: "paseo:browser:capture-prepare", + payload: { requestId: "prepare-1", browserId: "browser-a" }, + }, + ]); + ipc.emit("paseo:browser:capture-prepared", { + requestId: "prepare-1", + ok: true, + token: "token-a", + }); + + await expect(preparation).resolves.toEqual({ token: "token-a" }); + } finally { + vi.useRealTimers(); + } + }); + + test("prepareForPixelCapture retries when the renderer prep handler is not registered yet", async () => { + vi.useFakeTimers(); + try { + const host = new FakeHostWebContents(10); + const contents = new FakeWebContents(20, host); + const ipc = new FakeIpcBridge(); + const requestIds = ["prepare-1", "prepare-2"]; + const tab = adaptWebContents(contents, "browser-a", { + ipc, + createRequestId: () => { + const requestId = requestIds.shift(); + if (!requestId) { + throw new Error("Missing request id"); + } + return requestId; + }, + timeoutMs: 250, + }); + + const preparation = tab.prepareForPixelCapture(); + + expect(host.sentMessages).toEqual([ + { + channel: "paseo:browser:capture-prepare", + payload: { requestId: "prepare-1", browserId: "browser-a" }, + }, + ]); + ipc.emit("paseo:browser:capture-prepared", { + requestId: "prepare-1", + ok: false, + message: "Browser pixel capture preparation is unavailable.", + }); + + await vi.advanceTimersByTimeAsync(50); + + expect(host.sentMessages).toEqual([ + { + channel: "paseo:browser:capture-prepare", + payload: { requestId: "prepare-1", browserId: "browser-a" }, + }, + { + channel: "paseo:browser:capture-prepare", + payload: { requestId: "prepare-2", browserId: "browser-a" }, + }, + ]); + ipc.emit("paseo:browser:capture-prepared", { + requestId: "prepare-2", + ok: true, + token: "token-a", + }); + + await expect(preparation).resolves.toEqual({ token: "token-a" }); + } finally { + vi.useRealTimers(); + } + }); + test("prepareForPixelCapture ignores matching responses from the wrong sender", async () => { const host = new FakeHostWebContents(10); const contents = new FakeWebContents(20, host); @@ -364,12 +459,28 @@ describe("browser automation IPC adapter", () => { } }); + test("prepareForPixelCapture reports prep_unavailable when the guest host renderer never appears", async () => { + vi.useFakeTimers(); + try { + const contents = new FakeWebContents(20, null); + const tab = adaptWebContents(contents, "browser-a", { timeoutMs: 25 }); + + const preparation = tab.prepareForPixelCapture(); + const rejection = expect(preparation).rejects.toThrow( + "Browser screenshot prep_unavailable: Browser host renderer is not available.", + ); + await vi.advanceTimersByTimeAsync(25); + + await rejection; + } finally { + vi.useRealTimers(); + } + }); + test("prepareForPixelCapture rejects when the guest has no embedder renderer", async () => { const contents = new FakeWebContents(20, null); - const tab = adaptWebContents(contents, "browser-a"); + const tab = adaptWebContents(contents, "browser-a", { timeoutMs: 1 }); - await expect(tab.prepareForPixelCapture()).rejects.toThrow( - "Browser host renderer is not available.", - ); + await expect(tab.prepareForPixelCapture()).rejects.toThrow("prep_unavailable"); }); }); diff --git a/packages/desktop/src/features/browser-automation/ipc.ts b/packages/desktop/src/features/browser-automation/ipc.ts index 478c2eaab..16925ee9e 100644 --- a/packages/desktop/src/features/browser-automation/ipc.ts +++ b/packages/desktop/src/features/browser-automation/ipc.ts @@ -14,6 +14,8 @@ import { const MAX_CONSOLE_MESSAGES_PER_TAB = 200; const PIXEL_CAPTURE_BRIDGE_TIMEOUT_MS = 5_000; +const PIXEL_CAPTURE_PREP_RETRY_INTERVAL_MS = 50; +const PIXEL_CAPTURE_PREP_UNAVAILABLE_PREFIX = "Browser screenshot prep_unavailable:"; const consoleMessagesByContentsId = new Map(); const observedContentsIds = new Set(); let nextPixelCaptureBridgeRequest = 0; @@ -121,12 +123,11 @@ export function adaptWebContents( reload: () => contents.reload(), capturePage: (captureOptions) => contents.capturePage(undefined, captureOptions), prepareForPixelCapture: async () => { - const host = getPixelCaptureHost(contents); - const result = await requestPixelCaptureBridge({ host, browserId, kind: "prepare", options }); + const result = await preparePixelCaptureBridgeWithRetry({ contents, browserId, options }); if (!result.token) { throw new Error("Browser pixel capture preparation did not return a token."); } - preparedPixelCapturesByToken.set(result.token, { browserId, host }); + preparedPixelCapturesByToken.set(result.token, { browserId, host: result.host }); return { token: result.token }; }, restorePixelCapture: async (preparation) => { @@ -159,14 +160,83 @@ export function adaptWebContents( }; } -function getPixelCaptureHost(contents: BrowserAutomationWebContents): HostWebContents { +function getPixelCaptureHost(contents: BrowserAutomationWebContents): HostWebContents | null { const host = contents.hostWebContents; if (!host || host.isDestroyed()) { - throw new Error("Browser host renderer is not available."); + return null; } return host; } +async function preparePixelCaptureBridgeWithRetry(input: { + contents: BrowserAutomationWebContents; + browserId: string; + options: PixelCaptureBridgeOptions | undefined; +}): Promise { + const timeoutMs = input.options?.timeoutMs ?? PIXEL_CAPTURE_BRIDGE_TIMEOUT_MS; + const deadline = Date.now() + timeoutMs; + let lastUnavailableReason = "Browser host renderer is not available."; + + while (Date.now() < deadline) { + const host = getPixelCaptureHost(input.contents); + if (!host) { + await delayUntilRetry(deadline); + continue; + } + + try { + const result = await requestPixelCaptureBridge({ + host, + browserId: input.browserId, + kind: "prepare", + options: withBridgeTimeout(input.options, Math.max(1, deadline - Date.now())), + }); + return { ...result, host }; + } catch (error) { + if (!isRetryablePrepareUnavailableError(error)) { + throw error; + } + lastUnavailableReason = errorMessage(error); + await delayUntilRetry(deadline); + } + } + + throw new Error(`${PIXEL_CAPTURE_PREP_UNAVAILABLE_PREFIX} ${lastUnavailableReason}`); +} + +function withBridgeTimeout( + options: PixelCaptureBridgeOptions | undefined, + timeoutMs: number, +): PixelCaptureBridgeOptions { + return { + ...options, + timeoutMs, + }; +} + +function isRetryablePrepareUnavailableError(error: unknown): boolean { + const message = errorMessage(error); + return ( + message.includes("Browser pixel capture preparation is unavailable.") || + message.includes("Browser host renderer is not available.") || + message.includes("is not mounted.") + ); +} + +function errorMessage(error: unknown): string { + return error instanceof Error ? error.message : String(error); +} + +async function delayUntilRetry(deadline: number): Promise { + const remainingMs = deadline - Date.now(); + if (remainingMs <= 0) { + return; + } + await new Promise((resolve) => { + setTimeout(resolve, Math.min(PIXEL_CAPTURE_PREP_RETRY_INTERVAL_MS, remainingMs)); + }); +} + function requestPixelCaptureBridge(input: { host: HostWebContents; browserId: string; diff --git a/packages/desktop/src/features/browser-automation/service.test.ts b/packages/desktop/src/features/browser-automation/service.test.ts index d1c56ddcd..8006ddd44 100644 --- a/packages/desktop/src/features/browser-automation/service.test.ts +++ b/packages/desktop/src/features/browser-automation/service.test.ts @@ -53,9 +53,12 @@ class FakeTab implements TabContents { public consoleMessages: BrowserAutomationConsoleLogEntry[] = []; public captureNeverPaints = false; public captureThrows = false; + public captureErrorMessage = "capture failed"; public deferCaptures = false; public prepareNeverAcks = false; + public prepareErrorMessage: string | null = null; public fullPageScreenshotThrows = false; + public fullPageScreenshotErrorMessage = "UnknownVizError"; public layoutMetrics = { cssLayoutViewport: { clientWidth: 390, clientHeight: 844 }, cssContentSize: { width: 390, height: 1200 }, @@ -131,7 +134,7 @@ class FakeTab implements TabContents { this.actions.push("capture"); this.resolveCaptureStartWaiters(); if (this.captureThrows) { - throw new Error("capture failed"); + throw new Error(this.captureErrorMessage); } if (this.captureNeverPaints) { return new Promise(() => {}); @@ -146,6 +149,9 @@ class FakeTab implements TabContents { public async prepareForPixelCapture(): Promise { this.actions.push("prepare"); + if (this.prepareErrorMessage) { + throw new Error(this.prepareErrorMessage); + } if (this.prepareNeverAcks) { return new Promise(() => {}); } @@ -186,7 +192,7 @@ class FakeTab implements TabContents { } if (command === "Page.captureScreenshot") { if (this.fullPageScreenshotThrows) { - throw new Error("UnknownVizError"); + throw new Error(this.fullPageScreenshotErrorMessage); } return { data: this.fullPageScreenshotData }; } @@ -963,10 +969,32 @@ describe("executeAutomationCommand", () => { } }); - test("screenshot returns no-frame and restores capture preparation when viewport capture throws", async () => { + test("screenshot restores capture preparation when viewport capture fails with an ordinary error", async () => { const browser = new BrowserAutomationHarness(); browser.tab.captureThrows = true; + await expect( + browser.execute({ + command: "screenshot", + args: { browserId: BROWSER_A }, + }), + ).rejects.toThrow("capture failed"); + + expect(browser.tab.actions).toEqual([ + "prepare", + "background:false", + "invalidate", + "capture", + "restore:capture-1", + "background:true", + ]); + }); + + test("screenshot returns no-frame and restores capture preparation when viewport capture throws UnknownVizError", async () => { + const browser = new BrowserAutomationHarness(); + browser.tab.captureThrows = true; + browser.tab.captureErrorMessage = "UnknownVizError"; + await expect( browser.execute({ command: "screenshot", @@ -1010,7 +1038,7 @@ describe("executeAutomationCommand", () => { error: { code: "screenshot_no_frame", message: - "The browser tab has no painted frame. Focus the tab in the app, then try again.", + "Browser screenshot prep_unavailable: the app renderer did not acknowledge capture preparation before the timeout.", retryable: false, }, }); @@ -1020,6 +1048,28 @@ describe("executeAutomationCommand", () => { } }); + test("screenshot returns prep_unavailable and does not capture when preparation cannot be honored", async () => { + const browser = new BrowserAutomationHarness(); + browser.tab.prepareErrorMessage = + "Browser screenshot prep_unavailable: Browser host renderer is not available."; + + const result = await browser.execute({ + command: "screenshot", + args: { browserId: BROWSER_A }, + }); + + expect(result).toEqual({ + requestId: "req-screenshot", + ok: false, + error: { + code: "screenshot_no_frame", + message: "Browser screenshot prep_unavailable: Browser host renderer is not available.", + retryable: false, + }, + }); + expect(browser.tab.actions).toEqual(["prepare", "background:true"]); + }); + test("overlapping screenshots serialize capture preparation and restore", async () => { const browser = new BrowserAutomationHarness(); browser.tab.deferCaptures = true; @@ -1076,6 +1126,65 @@ describe("executeAutomationCommand", () => { ]); }); + test("overlapping screenshots across browser tabs serialize capture preparation and restore", async () => { + const registry = new FakeRegistry(); + const firstTab = new FakeTab(1, "https://a.test", "A"); + const secondTab = new FakeTab(2, "https://b.test", "B"); + firstTab.deferCaptures = true; + secondTab.deferCaptures = true; + registry.register(BROWSER_A, WORKSPACE_A, firstTab); + registry.register(BROWSER_B, WORKSPACE_A, secondTab); + + const first = executeAutomationCommand( + automationRequest({ + command: "screenshot", + args: { browserId: BROWSER_A }, + }), + registry, + ); + await firstTab.waitForCaptureStart(1); + + const second = executeAutomationCommand( + automationRequest( + { + command: "screenshot", + args: { browserId: BROWSER_B }, + }, + { requestId: "req-screenshot-2" }, + ), + registry, + ); + await Promise.resolve(); + + expect(firstTab.actions).toEqual(["prepare", "background:false", "invalidate", "capture"]); + expect(secondTab.actions).toEqual([]); + + firstTab.finishNextCapture(); + await secondTab.waitForCaptureStart(1); + + expect(firstTab.actions).toEqual([ + "prepare", + "background:false", + "invalidate", + "capture", + "restore:capture-1", + "background:true", + ]); + expect(secondTab.actions).toEqual(["prepare", "background:false", "invalidate", "capture"]); + + secondTab.finishNextCapture(); + await expect(first).resolves.toMatchObject({ requestId: "req-screenshot", ok: true }); + await expect(second).resolves.toMatchObject({ requestId: "req-screenshot-2", ok: true }); + expect(secondTab.actions).toEqual([ + "prepare", + "background:false", + "invalidate", + "capture", + "restore:capture-1", + "background:true", + ]); + }); + test("screenshot with fullPage captures the page content area through CDP", async () => { const browser = new BrowserAutomationHarness(); @@ -1176,6 +1285,28 @@ describe("executeAutomationCommand", () => { ]); }); + test("screenshot with fullPage restores capture preparation when CDP capture fails with an ordinary error", async () => { + const browser = new BrowserAutomationHarness(); + browser.tab.fullPageScreenshotThrows = true; + browser.tab.fullPageScreenshotErrorMessage = "Debugger detached"; + + await expect( + browser.execute({ + command: "screenshot", + args: { browserId: BROWSER_A, fullPage: true }, + }), + ).rejects.toThrow("Debugger detached"); + expect(browser.tab.actions).toEqual([ + "prepare", + "background:false", + "invalidate", + "debug:Page.getLayoutMetrics", + "debug:Page.captureScreenshot", + "restore:capture-1", + "background:true", + ]); + }); + test("upload resolves workspace files before setting them on the file input", async () => { const browser = new BrowserAutomationHarness(); browser.tab.snapshotElements = [ diff --git a/packages/desktop/src/features/browser-automation/service.ts b/packages/desktop/src/features/browser-automation/service.ts index a7c16ea54..06f3fb4f3 100644 --- a/packages/desktop/src/features/browser-automation/service.ts +++ b/packages/desktop/src/features/browser-automation/service.ts @@ -63,8 +63,9 @@ const WAIT_POLL_INTERVAL_MS = 25; const PIXEL_CAPTURE_TIMEOUT_MS = 5_000; const SCREENSHOT_NO_FRAME_MESSAGE = "The browser tab has no painted frame. Focus the tab in the app, then try again."; +const SCREENSHOT_PREP_UNAVAILABLE_PREFIX = "Browser screenshot prep_unavailable:"; const ALLOWED_PAGE_URL_PROTOCOLS = new Set(["http:", "https:"]); -const pixelCaptureQueuesByContentsId = new Map>(); +let pixelCaptureQueue: Promise = Promise.resolve(); function fail( requestId: string, @@ -76,8 +77,8 @@ function fail( } class ScreenshotNoFrameError extends Error { - public constructor() { - super(SCREENSHOT_NO_FRAME_MESSAGE); + public constructor(message = SCREENSHOT_NO_FRAME_MESSAGE) { + super(message); this.name = "ScreenshotNoFrameError"; } } @@ -86,8 +87,11 @@ function isScreenshotNoFrameError(error: unknown): error is ScreenshotNoFrameErr return error instanceof ScreenshotNoFrameError; } -function screenshotNoFrameFailure(requestId: string): FailurePayload { - return fail(requestId, "screenshot_no_frame", SCREENSHOT_NO_FRAME_MESSAGE); +function screenshotNoFrameFailure( + requestId: string, + error: ScreenshotNoFrameError, +): FailurePayload { + return fail(requestId, "screenshot_no_frame", error.message); } async function withPixelCaptureTimeout(capture: Promise): Promise { @@ -107,25 +111,22 @@ async function withPixelCaptureTimeout(capture: Promise): Promise { } } -async function runSerializedPixelCapture( - contents: TabContents, - capture: () => Promise, -): Promise { - const previous = pixelCaptureQueuesByContentsId.get(contents.id) ?? Promise.resolve(); +async function runSerializedPixelCapture(capture: () => Promise): Promise { + const previous = pixelCaptureQueue; let releaseCurrent = () => {}; const current = new Promise((resolve) => { releaseCurrent = resolve; }); const tail = previous.catch(() => {}).then(() => current); - pixelCaptureQueuesByContentsId.set(contents.id, tail); + pixelCaptureQueue = tail; await previous.catch(() => {}); try { return await capture(); } finally { releaseCurrent(); - if (pixelCaptureQueuesByContentsId.get(contents.id) === tail) { - pixelCaptureQueuesByContentsId.delete(contents.id); + if (pixelCaptureQueue === tail) { + pixelCaptureQueue = Promise.resolve(); } } } @@ -133,8 +134,8 @@ async function runSerializedPixelCapture( async function prepareForPixelCapture(contents: TabContents): Promise { try { return await withPixelCaptureTimeout(contents.prepareForPixelCapture()); - } catch { - throw new ScreenshotNoFrameError(); + } catch (error) { + throw screenshotPreparationError(error); } } @@ -156,15 +157,47 @@ async function capturePreparedPixelFrame(capture: () => Promise): Promise< if (isScreenshotNoFrameError(error)) { throw error; } - throw new ScreenshotNoFrameError(); + if (isKnownNoFrameCaptureError(error)) { + throw new ScreenshotNoFrameError(); + } + throw error; } } +function screenshotPreparationError(error: unknown): ScreenshotNoFrameError { + const message = error instanceof Error ? error.message : String(error); + if (isScreenshotNoFrameError(error)) { + return new ScreenshotNoFrameError( + `${SCREENSHOT_PREP_UNAVAILABLE_PREFIX} the app renderer did not acknowledge capture preparation before the timeout.`, + ); + } + if (message.includes(SCREENSHOT_PREP_UNAVAILABLE_PREFIX)) { + return new ScreenshotNoFrameError(message); + } + if ( + message.includes("Browser pixel capture preparation is unavailable.") || + message.includes("Browser host renderer is not available.") || + message.includes("is not mounted.") + ) { + return new ScreenshotNoFrameError(`${SCREENSHOT_PREP_UNAVAILABLE_PREFIX} ${message}`); + } + return new ScreenshotNoFrameError(`${SCREENSHOT_PREP_UNAVAILABLE_PREFIX} ${message}`); +} + +function isKnownNoFrameCaptureError(error: unknown): boolean { + const message = error instanceof Error ? error.message : String(error); + return ( + message.includes("UnknownVizError") || + message.includes("No frame") || + message.includes("no painted frame") + ); +} + async function runPreparedPixelCapture( contents: TabContents, capture: () => Promise, ): Promise { - return runSerializedPixelCapture(contents, async () => { + return runSerializedPixelCapture(async () => { const previousBackgroundThrottling = contents.isBackgroundThrottlingAllowed(); let preparation: TabPixelCapturePreparation | null = null; try { @@ -839,7 +872,7 @@ async function executeScreenshot( image = await capturePaintedViewport(target.contents); } catch (error) { if (isScreenshotNoFrameError(error)) { - return screenshotNoFrameFailure(requestId); + return screenshotNoFrameFailure(requestId, error); } throw error; } @@ -931,7 +964,7 @@ async function executeFullPageScreenshot( }); } catch (error) { if (isScreenshotNoFrameError(error)) { - return screenshotNoFrameFailure(requestId); + return screenshotNoFrameFailure(requestId, error); } throw error; }