diff --git a/packages/desktop/src/features/browser-automation/ipc.ts b/packages/desktop/src/features/browser-automation/ipc.ts index 54295f96e..3e5994561 100644 --- a/packages/desktop/src/features/browser-automation/ipc.ts +++ b/packages/desktop/src/features/browser-automation/ipc.ts @@ -23,7 +23,6 @@ import { const MAX_CONSOLE_MESSAGES_PER_TAB = 200; const consoleMessagesByContentsId = new Map(); const observedContentsIds = new Set(); -const backgroundThrottlingByContentsId = new Map(); interface IpcHandlerRegistry { handle(channel: string, listener: (event: unknown, ...args: unknown[]) => unknown): void; @@ -46,8 +45,8 @@ function adaptWebContents(contents: WebContents): TabContents { reload: () => contents.reload(), capturePage: (options) => contents.capturePage(undefined, options), invalidate: () => contents.invalidate(), - isBackgroundThrottlingAllowed: () => readBackgroundThrottling(contents), - setBackgroundThrottling: (allowed) => setBackgroundThrottling(contents, allowed), + isBackgroundThrottlingAllowed: () => contents.getBackgroundThrottling(), + setBackgroundThrottling: (allowed) => contents.setBackgroundThrottling(allowed), getConsoleMessages: () => consoleMessagesByContentsId.get(contents.id) ?? [], getCookies: async (url: string) => (await contents.session.cookies.get({ url })).map(normalizeCookie), @@ -107,15 +106,6 @@ function normalizeCookie(cookie: Electron.Cookie): BrowserAutomationCookieEntry }; } -function readBackgroundThrottling(contents: WebContents): boolean { - return backgroundThrottlingByContentsId.get(contents.id) ?? true; -} - -function setBackgroundThrottling(contents: WebContents, allowed: boolean): void { - backgroundThrottlingByContentsId.set(contents.id, allowed); - contents.setBackgroundThrottling(allowed); -} - function observeConsoleMessages(contents: WebContents): void { if (observedContentsIds.has(contents.id)) { return; @@ -130,7 +120,6 @@ function observeConsoleMessages(contents: WebContents): void { contents.once("destroyed", () => { observedContentsIds.delete(contents.id); consoleMessagesByContentsId.delete(contents.id); - backgroundThrottlingByContentsId.delete(contents.id); }); } diff --git a/packages/desktop/src/features/browser-automation/service.test.ts b/packages/desktop/src/features/browser-automation/service.test.ts index c01a50c17..8a902ba6d 100644 --- a/packages/desktop/src/features/browser-automation/service.test.ts +++ b/packages/desktop/src/features/browser-automation/service.test.ts @@ -2,7 +2,7 @@ import { resolve as resolvePath } from "node:path"; import { describe, expect, it, vi } from "vitest"; import { BrowserSnapshotEngine } from "./snapshot-engine.js"; -import type { TabContents, BrowserRegistry } from "./service.js"; +import type { TabContents, BrowserRegistry, TabImage } from "./service.js"; import { executeAutomationCommand } from "./service.js"; function fakeTab(overrides: Partial & { id: number }): TabContents { @@ -1991,6 +1991,93 @@ describe("executeAutomationCommand", () => { expect(actions).toEqual(["background:false", "invalidate", "capture", "background:true"]); }); + it("serializes overlapping viewport captures before restoring background throttling", async () => { + const actions: string[] = []; + const captures: Array<{ resolve: (image: TabImage) => void }> = []; + let backgroundThrottlingAllowed = true; + const image: TabImage = { + toPNG: () => new Uint8Array([137, 80, 78, 71]), + getSize: () => ({ width: 640, height: 480 }), + }; + const tab = fakeTab({ + id: 13, + capturePage: async () => { + actions.push("capture"); + return new Promise((resolve) => { + captures.push({ resolve }); + }); + }, + invalidate: () => { + actions.push("invalidate"); + }, + isBackgroundThrottlingAllowed: () => backgroundThrottlingAllowed, + setBackgroundThrottling: (allowed) => { + backgroundThrottlingAllowed = allowed; + actions.push(`background:${allowed}`); + }, + }); + const registry = createRegistry({ + getWorkspaceActiveTabContents: (workspaceId) => + workspaceId === "workspace-a" ? tab : null, + getWorkspaceActiveBrowserId: (workspaceId) => (workspaceId === "workspace-a" ? "a" : null), + getBrowserWorkspaceId: (id) => (id === "a" ? "workspace-a" : null), + }); + + const first = executeAutomationCommand( + { + type: "browser.automation.execute.request", + requestId: "r-screenshot-overlap-first", + workspaceId: "workspace-a", + command: { command: "screenshot", args: { workspaceId: "workspace-a" } }, + }, + registry, + ); + const second = executeAutomationCommand( + { + type: "browser.automation.execute.request", + requestId: "r-screenshot-overlap-second", + workspaceId: "workspace-a", + command: { command: "screenshot", args: { workspaceId: "workspace-a" } }, + }, + registry, + ); + + await Promise.resolve(); + await Promise.resolve(); + expect(captures).toHaveLength(1); + expect(backgroundThrottlingAllowed).toBe(false); + + const firstCapture = captures[0]; + if (!firstCapture) { + throw new Error("expected first capture to start"); + } + firstCapture.resolve(image); + await expect(first).resolves.toMatchObject({ requestId: "r-screenshot-overlap-first" }); + await Promise.resolve(); + await Promise.resolve(); + expect(captures).toHaveLength(2); + expect(backgroundThrottlingAllowed).toBe(false); + + const secondCapture = captures[1]; + if (!secondCapture) { + throw new Error("expected second capture to start"); + } + secondCapture.resolve(image); + await expect(second).resolves.toMatchObject({ requestId: "r-screenshot-overlap-second" }); + + expect(backgroundThrottlingAllowed).toBe(true); + expect(actions).toEqual([ + "background:false", + "invalidate", + "capture", + "background:true", + "background:false", + "invalidate", + "capture", + "background:true", + ]); + }); + it("returns screenshot_no_frame when viewport capture never paints", async () => { vi.useFakeTimers(); try { @@ -2132,30 +2219,4 @@ describe("executeAutomationCommand", () => { } }); }); - - describe("unsupported command", () => { - it("returns browser_unsupported for unknown commands", () => { - const registry = createRegistry(); - - const result = executeAutomationCommand( - { - type: "browser.automation.execute.request", - requestId: "r9", - // Cast to bypass discriminated union — tests forward-compat fallback - command: { command: "future_click", args: {} } as never, - }, - registry, - ); - - expect(result).toEqual({ - requestId: "r9", - ok: false, - error: { - code: "browser_unsupported", - message: "Unsupported command: future_click", - retryable: false, - }, - }); - }); - }); }); diff --git a/packages/desktop/src/features/browser-automation/service.ts b/packages/desktop/src/features/browser-automation/service.ts index b7138251f..b5f660633 100644 --- a/packages/desktop/src/features/browser-automation/service.ts +++ b/packages/desktop/src/features/browser-automation/service.ts @@ -69,6 +69,7 @@ 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 ALLOWED_PAGE_URL_PROTOCOLS = new Set(["http:", "https:"]); +const pixelCaptureQueuesByContentsId = new Map>(); function fail( requestId: string, @@ -111,17 +112,42 @@ async function withPixelCaptureTimeout(capture: Promise): Promise { } } -async function capturePaintedViewport(contents: TabContents): Promise { - const previousBackgroundThrottling = contents.isBackgroundThrottlingAllowed(); - contents.setBackgroundThrottling(false); +async function runSerializedPixelCapture( + contents: TabContents, + capture: () => Promise, +): Promise { + const previous = pixelCaptureQueuesByContentsId.get(contents.id) ?? Promise.resolve(); + let releaseCurrent = () => {}; + const current = new Promise((resolve) => { + releaseCurrent = resolve; + }); + const tail = previous.catch(() => {}).then(() => current); + pixelCaptureQueuesByContentsId.set(contents.id, tail); + + await previous.catch(() => {}); try { - contents.invalidate(); - return await withPixelCaptureTimeout(contents.capturePage({ stayHidden: false })); + return await capture(); } finally { - contents.setBackgroundThrottling(previousBackgroundThrottling); + releaseCurrent(); + if (pixelCaptureQueuesByContentsId.get(contents.id) === tail) { + pixelCaptureQueuesByContentsId.delete(contents.id); + } } } +async function capturePaintedViewport(contents: TabContents): Promise { + return runSerializedPixelCapture(contents, async () => { + const previousBackgroundThrottling = contents.isBackgroundThrottlingAllowed(); + contents.setBackgroundThrottling(false); + try { + contents.invalidate(); + return await withPixelCaptureTimeout(contents.capturePage({ stayHidden: false })); + } finally { + contents.setBackgroundThrottling(previousBackgroundThrottling); + } + }); +} + function tabInfoFromContents( browserId: string, contents: TabContents, @@ -151,14 +177,6 @@ export function executeAutomationCommand( const snapshotEngine = options?.snapshotEngine ?? defaultSnapshotEngine; const handler = commandHandlers[command.command]; - if (!handler) { - return fail( - requestId, - "browser_unsupported", - `Unsupported command: ${(command as { command: string }).command}`, - ); - } - return handler({ request, command, requestId, workspaceId, registry, snapshotEngine }); } @@ -201,9 +219,11 @@ type CommandHandler = ( context: CommandHandlerContext, ) => AutomationCommandPayload | Promise; -const commandHandlers: Partial> = { +const commandHandlers: Record = { list_tabs: ({ requestId, workspaceId, registry }) => executeListTabs(requestId, workspaceId, registry), + new_tab: ({ requestId }) => + fail(requestId, "browser_unsupported", "browser_new_tab is handled by the app runtime."), page_info: ({ request, command, requestId, workspaceId, registry }) => { const pageInfoCommand = command as Extract; return executePageInfo(