diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 3452f38ec2..6c3f76d497 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -211,6 +211,12 @@ jobs: if: ${{ runner.os == 'Linux' }} run: npm run test:e2e:renderer --workspace=@getpaseo/desktop + - name: Verify hidden browser throttling and screenshots + if: ${{ runner.os == 'Linux' }} + run: xvfb-run -a npm run test:e2e:browser-throttling --workspace=@getpaseo/desktop + env: + PASEO_BROWSER_THROTTLING_ARTIFACT_DIR: ${{ runner.temp }}/desktop-browser-e2e/throttling + - name: Run desktop browser E2E if: ${{ runner.os == 'Linux' }} run: npm run test:e2e:browser-tabs --workspace=@getpaseo/desktop diff --git a/docs/browser-capture-harness.md b/docs/browser-capture-harness.md index 734dda3c45..0a53e9a775 100644 --- a/docs/browser-capture-harness.md +++ b/docs/browser-capture-harness.md @@ -12,8 +12,7 @@ It validates the compositor behavior that unit tests cannot see: by retrying until the frame appears; - both viewport `capturePage` and full-page CDP screenshots return real pixels from the permanent production parking state; -- guest background throttling can be disabled once at attach without per-capture - renderer coordination; +- parked guests remain capturable with Chromium background throttling enabled; - the real-Electron host-composer sentinel proves guest Enter cannot submit a focused host composer; - the automation group loads the compiled production keyboard boundary and guest @@ -73,11 +72,19 @@ The harness writes PNG evidence and `results.json` to: packages/desktop/capture-harness/out/ ``` -A passing run prints `PASS` lines for the production P1 attach-off parking state, +A passing run prints `PASS` lines for the production P1 default-throttling parking state, including fresh, settled, 75-second soak, multi-tab, viewport, and full-page checks. The PNG sizes may be device-pixel scaled; on a Retina display the 1280x800 logical viewport is usually saved as 2560x1600. +Run `npm run test:e2e:browser-throttling --workspace=@getpaseo/desktop` to verify +that hiding the window stops guest animation, viewport screenshots contain fresh +pixels, and restoring or closing the browser still works. Pass `-- --full-page` to +also run the below-viewport pixel assertion: this reproduces the existing +[full-page repetition bug](https://github.com/getpaseo/paseo/issues/3196), which the +capture harness's frame-availability checks do not detect. +On Linux, run it under `xvfb-run -a` when no display is available. + ## Mechanism Electron captures copy from the guest web contents' compositor surface. A resident @@ -96,8 +103,8 @@ plane stays below the overlay plane regardless of body insertion order; menus ke layering inside `overlay-root`. Activating a presented browser also focuses its registered guest `WebContents` in main so macOS assigns keyboard first-responder ownership to the page. -There is no renderer prep/restore handshake. Main disables guest background throttling -once when the webview attaches, then screenshot capture uses the shared serialized queue, -invalidates before each attempt, and retries known first-frame failures within the -5-second capture budget. Viewport screenshots use `capturePage({ stayHidden:false })`; +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. diff --git a/packages/desktop/capture-harness/main.js b/packages/desktop/capture-harness/main.js index b51735275b..4beed26643 100644 --- a/packages/desktop/capture-harness/main.js +++ b/packages/desktop/capture-harness/main.js @@ -4,6 +4,7 @@ const http = require("node:http"); const path = require("node:path"); const { isDeepStrictEqual } = require("node:util"); const { app, BrowserWindow, ipcMain, Menu, nativeImage, screen, session } = require("electron"); +const { adaptWebContents } = require("../dist/features/browser-automation/ipc.js"); const ROOT = __dirname; const OUT_DIR = process.env.PASEO_CAPTURE_HARNESS_OUT_DIR || path.join(ROOT, "out"); @@ -48,7 +49,7 @@ const PERMANENT_STATE_FILTER = new Set( .filter(Boolean), ); const PERMANENT_VARIANT_FILTER = new Set( - (process.env.PASEO_CAPTURE_HARNESS_VARIANTS || "attach-off") + (process.env.PASEO_CAPTURE_HARNESS_VARIANTS || "default") .split(",") .map((variant) => variant.trim()) .filter(Boolean), @@ -56,9 +57,9 @@ const PERMANENT_VARIANT_FILTER = new Set( const PERMANENT_CAPTURE_MODES = ["viewport", "full-page"]; const PERMANENT_THROTTLING_VARIANTS = [ { - id: "capture-only-throttling", - code: "capture-only", - label: "backgroundThrottling disabled only during each capture", + id: "default-throttling", + code: "default", + label: "Chromium background throttling enabled", disableGuestBackgroundThrottlingAtAttach: false, }, { @@ -442,8 +443,10 @@ async function readGuestMetrics(contents) { } async function capturePageSequence(contents) { - contents.invalidate(); - return await withTimeout(contents.capturePage(undefined, { stayHidden: false }), "capturePage"); + return adaptWebContents(contents).withFrameProduction(async () => { + contents.invalidate(); + return await withTimeout(contents.capturePage(undefined, { stayHidden: false }), "capturePage"); + }); } async function captureFullPage(contents) { @@ -485,8 +488,10 @@ async function captureFullPage(contents) { } async function captureFullPageSequence(contents) { - contents.invalidate(); - return await captureFullPage(contents); + return adaptWebContents(contents).withFrameProduction(async () => { + contents.invalidate(); + return await captureFullPage(contents); + }); } function installHarnessWebviewGuards(win, options = {}) { @@ -2115,7 +2120,7 @@ async function runAutomationGroup() { installHarnessWebviewGuards(win, { preloadPath: PRODUCTION_BROWSER_GUEST_PRELOAD_PATH, }); - const tracker = trackAttachedGuests(win, { disableGuestBackgroundThrottlingAtAttach: true }); + const tracker = trackAttachedGuests(win); try { await withTimeout( win.loadFile(path.join(ROOT, "index.html"), { diff --git a/packages/desktop/e2e/browser-throttling.electron.cjs b/packages/desktop/e2e/browser-throttling.electron.cjs new file mode 100644 index 0000000000..e9996cbaee --- /dev/null +++ b/packages/desktop/e2e/browser-throttling.electron.cjs @@ -0,0 +1,161 @@ +// Run with the repository Electron after npm run build:main. +const assert = require("node:assert/strict"); +const fs = require("node:fs"); +const os = require("node:os"); +const path = require("node:path"); +const { setTimeout: delay } = require("node:timers/promises"); +const { app, BrowserWindow, nativeImage } = require("electron"); +const { preparePaseoBrowserWebContents } = require("../dist/features/browser-webviews/index.js"); +const { adaptWebContents } = require("../dist/features/browser-automation/ipc.js"); +const { executeAutomationCommand } = require("../dist/features/browser-automation/service.js"); + +const output = + process.env.PASEO_BROWSER_THROTTLING_ARTIFACT_DIR || + fs.mkdtempSync(path.join(os.tmpdir(), "paseo-browser-throttling-")); +fs.mkdirSync(output, { recursive: true }); +const profile = fs.mkdtempSync(path.join(os.tmpdir(), "paseo-browser-throttling-profile-")); +app.setPath("userData", profile); +if (process.platform === "darwin") app.setActivationPolicy("accessory"); +const measurements = []; +// The existing full-page repetition bug (#3196) is independently reproducible. +// Keep its strict pixel assertion available without conflating it with throttling. +const captureModes = process.argv.includes("--full-page") ? [false, true] : [false]; + +function pageUrl(html) { + return `data:text/html,${encodeURIComponent(html)}`; +} + +async function openParkedBrowser() { + const win = new BrowserWindow({ + width: 900, + height: 600, + show: false, + skipTaskbar: true, + webPreferences: { webviewTag: true }, + }); + const attached = new Promise((resolve) => + win.webContents.once("did-attach-webview", (_event, guest) => { + preparePaseoBrowserWebContents(guest); + resolve(guest); + }), + ); + await win.loadURL( + pageUrl( + '
', + ), + ); + const url = pageUrl( + `
TOP
BOTTOM
`, + ); + await win.webContents.executeJavaScript( + `(() => { const view = document.createElement('webview'); view.style.cssText='display:inline-flex;width:640px;height:480px'; view.src=${JSON.stringify(url)}; document.getElementById('parking').appendChild(view); })()`, + ); + const guest = await attached; + if (guest.isLoading()) await new Promise((resolve) => guest.once("did-finish-load", resolve)); + win.showInactive(); + return { win, guest }; +} + +async function expectBrowserToAnimate(guest) { + const start = await guest.executeJavaScript("window.frameCount"); + const deadline = Date.now() + 5000; + while (Date.now() < deadline) { + if ((await guest.executeJavaScript("window.frameCount")) > start + 2) return; + await delay(50); + } + assert.fail("Browser did not resume animation after restoring the window"); +} + +async function expectHiddenBrowserToIdle(win, guest, label) { + win.hide(); + await delay(500); // Allow the compositor to consume the visibility change before sampling. + const start = await guest.executeJavaScript("window.frameCount"); + await delay(1000); // Measure a fixed interval; this is the behavior under test. + const frames = (await guest.executeJavaScript("window.frameCount")) - start; + measurements.push({ label, frames, backgroundThrottling: guest.getBackgroundThrottling() }); + fs.writeFileSync(path.join(output, "measurements.json"), JSON.stringify(measurements, null, 2)); + assert.equal(frames, 0, `${label}: hidden browser continues animating`); +} + +function browserRegistry(guest) { + const contents = adaptWebContents(guest); + return { + getTabContents: () => contents, + getBrowserWorkspaceId: () => "fixture", + listRegisteredBrowserIds: () => ["preview"], + listRegisteredBrowserIdsForWorkspace: () => ["preview"], + getWorkspaceActiveBrowserId: () => "preview", + }; +} + +function expectPixel(image, x, y, rgb) { + const { width } = image.getSize(); + const offset = (y * width + x) * 4; + const bitmap = image.toBitmap(); + assert.deepEqual( + [bitmap[offset + 2], bitmap[offset + 1], bitmap[offset]], + rgb, + `Incorrect page content at ${x},${y}`, + ); +} + +async function expectFreshScreenshots(guest, label) { + // Mutate while backgrounded: successful capture must show fresh pixels, not a cached frame. + await guest.executeJavaScript("document.getElementById('top').style.background='rgb(0,255,0)'"); + for (const fullPage of captureModes) { + const response = await executeAutomationCommand( + { + requestId: label, + workspaceId: "fixture", + command: { command: "screenshot", args: { browserId: "preview", fullPage } }, + }, + browserRegistry(guest), + ); + assert.equal(response.ok, true, JSON.stringify(response)); + const image = nativeImage.createFromBuffer(Buffer.from(response.result.dataBase64, "base64")); + fs.writeFileSync( + path.join(output, `${label}-${fullPage ? "full-page" : "viewport"}.png`), + image.toPNG(), + ); + assert.deepEqual(image.getSize(), { width: 640, height: fullPage ? 960 : 480 }); + expectPixel(image, 320, 240, [0, 255, 0]); + if (fullPage) expectPixel(image, 320, 720, [0, 0, 255]); + } + await guest.executeJavaScript("document.getElementById('top').style.background='rgb(255,0,0)'"); +} + +async function verifyBrowserLifecycle() { + const { win, guest } = await openParkedBrowser(); + try { + await expectBrowserToAnimate(guest); + await expectHiddenBrowserToIdle(win, guest, "before-capture"); + await expectFreshScreenshots(guest, "hidden"); + await expectHiddenBrowserToIdle(win, guest, "after-capture"); + win.showInactive(); + await expectBrowserToAnimate(guest); + await expectFreshScreenshots(guest, "parked"); + const destroyed = new Promise((resolve) => guest.once("destroyed", resolve)); + await win.webContents.executeJavaScript("document.querySelector('webview').remove()"); + await destroyed; + console.log( + `PASS hidden browser idles; fresh screenshot pixels; restore and close. Artifacts: ${output}`, + ); + } finally { + win.destroy(); + } +} + +const deadline = setTimeout(() => { + console.error("Browser throttling regression timed out"); + app.exit(1); +}, 30000); +app + .whenReady() + .then(verifyBrowserLifecycle) + .then(() => app.quit()) + .catch((error) => { + console.error(error); + app.exit(1); + }) + .finally(() => clearTimeout(deadline)); +app.on("will-quit", () => fs.rmSync(profile, { recursive: true, force: true })); diff --git a/packages/desktop/package.json b/packages/desktop/package.json index 84d71dfafa..1174c74457 100644 --- a/packages/desktop/package.json +++ b/packages/desktop/package.json @@ -17,11 +17,12 @@ "scripts": { "build": "npm --prefix ../.. run build:server:clean && npm run build:main && electron-builder --config electron-builder.yml", "build:main": "tsc -p tsconfig.json", - "capture-harness": "./capture-harness/run.sh", + "capture-harness": "npm run build:main && ./capture-harness/run.sh", "dev": "./scripts/dev.sh", "dev:win": "powershell ./scripts/dev.ps1", "test:e2e:renderer": "cross-env E2E_DESKTOP_RUNTIME=1 playwright test --config=playwright.config.ts --project=desktop", "test:e2e:browser-tabs": "npm run build:main && node ./e2e/browser-tabs.e2e.mjs", + "test:e2e:browser-throttling": "npm run build:main && electron --no-sandbox e2e/browser-throttling.electron.cjs", "verify:electron-cdp": "node ./scripts/verify-electron-cdp.mjs", "test": "vitest run --exclude \"e2e/**\"", "typecheck": "tsgo --noEmit -p tsconfig.json" diff --git a/packages/desktop/src/features/browser-automation/ipc.test.ts b/packages/desktop/src/features/browser-automation/ipc.test.ts index dba7581964..a36b7dea0d 100644 --- a/packages/desktop/src/features/browser-automation/ipc.test.ts +++ b/packages/desktop/src/features/browser-automation/ipc.test.ts @@ -108,6 +108,14 @@ type ConsoleMessageListener = ( ) => void; class FakeWebContents { + public backgroundThrottling = true; + public getBackgroundThrottling(): boolean { + return this.backgroundThrottling; + } + public setBackgroundThrottling(allowed: boolean): void { + if (this.destroyed) throw new Error("Object has been destroyed"); + this.backgroundThrottling = allowed; + } public readonly debugger = new FakeDebugger(); public readonly inputEvents: IsolatedKeyboardInputEvent[] = []; public readonly loadedUrls: string[] = []; @@ -672,3 +680,38 @@ function deferred() { }); return { promise, resolve }; } + +describe("pixel capture frame production", () => { + test.each([true, false])( + "restores the prior throttling policy (%s) after capture or failure", + async (previous) => { + const contents = new FakeWebContents(8001); + contents.backgroundThrottling = previous; + const tab = adaptWebContents(contents); + expect( + await tab.withFrameProduction(async () => { + expect(contents.backgroundThrottling).toBe(false); + return "pixels"; + }), + ).toBe("pixels"); + expect(contents.backgroundThrottling).toBe(previous); + await expect( + tab.withFrameProduction(async () => { + throw new Error("capture failed"); + }), + ).rejects.toThrow("capture failed"); + expect(contents.backgroundThrottling).toBe(previous); + }, + ); + + test("does not touch a guest destroyed during capture", async () => { + const contents = new FakeWebContents(8002); + const tab = adaptWebContents(contents); + await expect( + tab.withFrameProduction(async () => { + contents.destroyed = true; + throw new Error("capture closed"); + }), + ).rejects.toThrow("capture closed"); + }); +}); diff --git a/packages/desktop/src/features/browser-automation/ipc.ts b/packages/desktop/src/features/browser-automation/ipc.ts index ea1941c318..d21c51b84f 100644 --- a/packages/desktop/src/features/browser-automation/ipc.ts +++ b/packages/desktop/src/features/browser-automation/ipc.ts @@ -107,6 +107,8 @@ interface BrowserAutomationWebContents extends ConsoleMessageEmitter { reload(): void; capturePage(rect?: Rectangle, options?: { stayHidden?: boolean }): Promise; invalidate(): void; + getBackgroundThrottling(): boolean; + setBackgroundThrottling(allowed: boolean): void; sendInputEvent(event: IsolatedKeyboardInputEvent): void; } @@ -130,6 +132,15 @@ export function adaptWebContents(contents: BrowserAutomationWebContents): TabCon reload: () => contents.reload(), capturePage: (captureOptions) => contents.capturePage(undefined, captureOptions), invalidate: () => contents.invalidate(), + withFrameProduction: async (capture) => { + const previous = contents.getBackgroundThrottling(); + contents.setBackgroundThrottling(false); + try { + return await capture(); + } finally { + if (!contents.isDestroyed()) contents.setBackgroundThrottling(previous); + } + }, sendInputEvent: (event) => contents.sendInputEvent(event), getConsoleMessages: () => consoleMessagesByContentsId.get(contentsId) ?? [], captureDialogs: (task) => dialogMonitor.capture(task), diff --git a/packages/desktop/src/features/browser-automation/service.test.ts b/packages/desktop/src/features/browser-automation/service.test.ts index c9246324b2..8b33b3d89d 100644 --- a/packages/desktop/src/features/browser-automation/service.test.ts +++ b/packages/desktop/src/features/browser-automation/service.test.ts @@ -33,6 +33,9 @@ class FakeImage implements TabImage { } class FakeTab implements TabContents { + public withFrameProduction(capture: () => Promise): Promise { + return capture(); + } public readonly loadedUrls: string[] = []; public readonly scripts: string[] = []; public readonly actions: string[] = []; diff --git a/packages/desktop/src/features/browser-automation/service.ts b/packages/desktop/src/features/browser-automation/service.ts index 7ab9099db2..a146708e17 100644 --- a/packages/desktop/src/features/browser-automation/service.ts +++ b/packages/desktop/src/features/browser-automation/service.ts @@ -37,6 +37,7 @@ export interface TabContents { reload(): void; capturePage(options?: TabCapturePageOptions): Promise; invalidate(): void; + withFrameProduction(capture: () => Promise): Promise; sendInputEvent(event: IsolatedKeyboardInputEvent): void; getConsoleMessages?(): BrowserAutomationConsoleLogEntry[]; captureDialogs?( @@ -189,11 +190,26 @@ function isKnownNoFrameCaptureError(error: unknown): boolean { ); } +async function waitForPaint(contents: TabContents): Promise { + // A hidden page may have unpainted DOM updates. The first animation callback + // precedes paint; the next frame ensures capture cannot reuse the old surface. + await withPixelCaptureTimeout( + contents.executeJavaScript( + "new Promise(resolve => requestAnimationFrame(() => requestAnimationFrame(() => resolve())))", + ), + ); +} + async function runPaintedPixelCapture( contents: TabContents, capture: () => Promise, ): Promise { - return runSerializedPixelCapture(() => capturePixelFrameWithRetry(contents, capture)); + return runSerializedPixelCapture(() => + contents.withFrameProduction(async () => { + await waitForPaint(contents); + return capturePixelFrameWithRetry(contents, capture); + }), + ); } async function capturePaintedViewport(contents: TabContents): Promise { diff --git a/packages/desktop/src/features/browser-webviews/index.test.ts b/packages/desktop/src/features/browser-webviews/index.test.ts index bbeffbe8c3..cac803134a 100644 --- a/packages/desktop/src/features/browser-webviews/index.test.ts +++ b/packages/desktop/src/features/browser-webviews/index.test.ts @@ -19,7 +19,6 @@ class FakeRenderer { } class FakeBrowserGuest { - public readonly backgroundThrottlingCalls: boolean[] = []; private destroyedListener: (() => void) | null = null; private destroyed = false; @@ -33,10 +32,6 @@ class FakeBrowserGuest { return this.destroyed; } - public setBackgroundThrottling(allowed: boolean): void { - this.backgroundThrottlingCalls.push(allowed); - } - public once(event: "destroyed", listener: () => void): void { expect(event).toBe("destroyed"); this.destroyedListener = listener; @@ -187,7 +182,7 @@ describe("browser webview attachment", () => { unregisterPaseoBrowser("browser-shared-hosts"); }); - test("prepares throttling once and removes registration when the guest is destroyed", () => { + test("removes registration when the guest is destroyed", () => { const profileSession = {}; const renderer = new FakeRenderer(31); const guest = new FakeBrowserGuest(601, renderer, profileSession); @@ -201,12 +196,10 @@ describe("browser webview attachment", () => { findWebContents: () => guest, }); - expect(guest.backgroundThrottlingCalls).toEqual([false]); expect(getPaseoBrowserIdForWebContents(guest)).toBe("browser-cleanup"); guest.destroy(); expect(getPaseoBrowserIdForWebContents(guest)).toBeNull(); - expect(guest.backgroundThrottlingCalls).toEqual([false]); }); }); diff --git a/packages/desktop/src/features/browser-webviews/index.ts b/packages/desktop/src/features/browser-webviews/index.ts index b415f43397..319d4922cb 100644 --- a/packages/desktop/src/features/browser-webviews/index.ts +++ b/packages/desktop/src/features/browser-webviews/index.ts @@ -24,7 +24,6 @@ interface BrowserWebContentsIdentity { interface RegisteredBrowserWebContents extends BrowserWebContentsIdentity { readonly hostWebContents: BrowserWebContentsIdentity | null; readonly session: object; - setBackgroundThrottling(allowed: boolean): void; once(event: "destroyed", listener: () => void): void; } @@ -56,7 +55,8 @@ export function getPaseoBrowserWebviewRegistry(): PaseoBrowserWebviewRegistry { export function preparePaseoBrowserWebContents(contents: RegisteredBrowserWebContents): void { const webContentsId = contents.id; - contents.setBackgroundThrottling(false); + // Preserve Chromium throttling when the host window is hidden. Browser + // residency and screenshot capture do not require a lifetime override. contents.once("destroyed", () => { browserRegistry.unregisterWebContents(webContentsId); });