diff --git a/src/main/browser/agent-browser-bridge-capture-paint-hold.test.ts b/src/main/browser/agent-browser-bridge-capture-paint-hold.test.ts index 4fd504a1418..c26f5c1b4cf 100644 --- a/src/main/browser/agent-browser-bridge-capture-paint-hold.test.ts +++ b/src/main/browser/agent-browser-bridge-capture-paint-hold.test.ts @@ -81,11 +81,17 @@ describe('AgentBrowserBridge', () => { bridge.setActiveTab(100) }) - it('never holds paint for commands that do not capture pixels', async () => { + it('holds only the guest, never paint, for commands that do not capture pixels', async () => { + const releases: ReturnType[] = [] + const holdGuestActive = vi.fn(() => { + const release = vi.fn() + releases.push(release) + return release + }) const holdPaintForCapture = vi.fn(() => () => {}) const printToPDF = vi.fn(async () => Buffer.from('pdf')) const b = new AgentBrowserBridge( - mockBrowserManager(undefined, undefined, { holdPaintForCapture }) + mockBrowserManager(undefined, undefined, { holdGuestActive, holdPaintForCapture }) ) b.setActiveTab(100) webContentsFromIdMock.mockReturnValue({ ...mockWebContents(100), printToPDF }) @@ -98,6 +104,87 @@ describe('AgentBrowserBridge', () => { await expect(b.pdf()).resolves.toEqual({ data: Buffer.from('pdf').toString('base64') }) expect(holdPaintForCapture).not.toHaveBeenCalled() + expect(holdGuestActive.mock.calls).toEqual([[100], [100], [100], [100], [100]]) + expect(releases.map((release) => release.mock.calls.length)).toEqual([1, 1, 1, 1, 1]) + }) + + it('holds only executing commands and resolves a queued command against the replacement guest', async () => { + const tabs = new Map([['tab-1', 100]]) + const holds: number[] = [] + const released: number[] = [] + const b = new AgentBrowserBridge( + mockBrowserManager(tabs, undefined, { + holdGuestActive: (id) => { + holds.push(id) + return () => released.push(id) + } + }) + ) + const oldGuest = mockWebContents(100) + const replacement = mockWebContents(200) + let finish = (_value: unknown): void => {} + oldGuest.debugger.sendCommand.mockImplementation( + () => new Promise((resolve) => (finish = resolve)) + ) + replacement.debugger.sendCommand.mockResolvedValue({ result: { value: 'replacement' } }) + webContentsFromIdMock.mockImplementation((id: number) => (id === 100 ? oldGuest : replacement)) + b.setActiveTab(100) + + const first = b.evaluate('first', undefined, 'tab-1') + const firstResult = expect(first).rejects.toMatchObject({ code: 'browser_tab_changed' }) + const second = b.evaluate('second', undefined, 'tab-1') + const holdsWhileQueued = [...holds] + expect(released).toEqual([]) + tabs.set('tab-1', 200) + finish({ result: { value: 'old' } }) + + await firstResult + await expect(second).resolves.toMatchObject({ result: 'replacement' }) + expect(holdsWhileQueued).toEqual([100]) + expect(holds).toEqual([100, 200]) + expect(released).toEqual([100, 200]) + }) + + it('drops queued commands without acquiring the guest and holds running work until it settles', async () => { + const release = vi.fn() + const holdGuestActive = vi.fn(() => release) + const b = new AgentBrowserBridge(mockBrowserManager(undefined, undefined, { holdGuestActive })) + b.setActiveTab(100) + const guest = mockWebContents(100) + let finish = (_value: unknown): void => {} + guest.debugger.sendCommand.mockImplementation( + () => new Promise((resolve) => (finish = resolve)) + ) + webContentsFromIdMock.mockReturnValue(guest) + + const running = b.evaluate('first') + const queued = b.evaluate('second') + const queuedResult = expect(queued).rejects.toMatchObject({ code: 'browser_tab_closed' }) + await b.onPageClosed('tab-1') + await queuedResult + expect(holdGuestActive.mock.calls).toEqual([[100]]) + expect(release).not.toHaveBeenCalled() + + finish({ result: { value: 'finished' } }) + await running + expect(release).toHaveBeenCalledTimes(1) + }) + + it('releases the background hold when session setup fails', async () => { + const release = vi.fn() + const holdGuestActive = vi.fn(() => release) + const b = new AgentBrowserBridge(mockBrowserManager(undefined, undefined, { holdGuestActive })) + b.setActiveTab(100) + CdpWsProxyMock.mockImplementationOnce(function (this: Record) { + this.start = vi.fn(async () => { + throw new Error('cannot start proxy') + }) + }) + succeedWith(null) + + await expect(b.snapshot()).rejects.toThrow('cannot start proxy') + expect(holdGuestActive).toHaveBeenCalledWith(100) + expect(release).toHaveBeenCalledTimes(1) }) it('captures full-page screenshots directly through CDP using CSS layout bounds', async () => { diff --git a/src/main/browser/agent-browser-bridge-queue.ts b/src/main/browser/agent-browser-bridge-queue.ts index a4cfac92e92..1a550067ebd 100644 --- a/src/main/browser/agent-browser-bridge-queue.ts +++ b/src/main/browser/agent-browser-bridge-queue.ts @@ -120,10 +120,15 @@ export abstract class AgentBrowserBridgeQueue extends AgentBrowserBridgeShutdown const sessionName = `${ORCA_TAB_SESSION_PREFIX}${browserPageId}` // A queued page may acquire a replacement guest before execution. const target = this.resolveCommandTarget(worktreeId, browserPageId) - if (options.ensureSession !== false) { - await this.ensureSession(sessionName, browserPageId, target.webContentsId) + const releaseGuest = this.browserManager.holdGuestActive(target.webContentsId) + try { + if (options.ensureSession !== false) { + await this.ensureSession(sessionName, browserPageId, target.webContentsId) + } + return await execute(sessionName, target) + } finally { + releaseGuest() } - return execute(sessionName, target) } /** Revocation must stop every later step of a host command. */ diff --git a/src/main/browser/agent-browser-bridge-session-lifecycle.test.ts b/src/main/browser/agent-browser-bridge-session-lifecycle.test.ts index 88bb6f4b8c7..1eee2a3fc01 100644 --- a/src/main/browser/agent-browser-bridge-session-lifecycle.test.ts +++ b/src/main/browser/agent-browser-bridge-session-lifecycle.test.ts @@ -254,8 +254,14 @@ describe('AgentBrowserBridge', () => { }) it('cancels the command already running when a session is destroyed', async () => { + const releasePaint = vi.fn() + bridge = new AgentBrowserBridge( + mockBrowserManager(undefined, undefined, { holdGuestActive: () => releasePaint }) + ) + bridge.setActiveTab(100) succeedWith({ snapshot: 'initial' }) await bridge.snapshot() + releasePaint.mockClear() execFileMock.mockClear() @@ -290,6 +296,7 @@ describe('AgentBrowserBridge', () => { message: 'Tab was closed while command was running' }) await destroyPromise + expect(releasePaint).toHaveBeenCalledTimes(1) }) it('destroys session on process swap and re-inits with --cdp', async () => { diff --git a/src/main/browser/agent-browser-bridge-test-harness.ts b/src/main/browser/agent-browser-bridge-test-harness.ts index 0f044d1dfd9..b1f91098095 100644 --- a/src/main/browser/agent-browser-bridge-test-harness.ts +++ b/src/main/browser/agent-browser-bridge-test-harness.ts @@ -35,6 +35,8 @@ export function mockBrowserManager( getBrowserPageLoadError: vi.fn(() => null), getBrowserPageCertificateFailure: vi.fn(() => null), unregisterGuest: vi.fn(), + holdPaintForCapture: () => () => {}, + holdGuestActive: () => () => {}, beginAgentCommand: () => ({ run: (execute: () => Promise) => execute(), end: () => {} diff --git a/src/main/browser/browser-manager-grab-test-harness.ts b/src/main/browser/browser-manager-grab-test-harness.ts index 008fff89a71..60c1817c822 100644 --- a/src/main/browser/browser-manager-grab-test-harness.ts +++ b/src/main/browser/browser-manager-grab-test-harness.ts @@ -53,7 +53,9 @@ export function routeWebContentsIds( if (id === GRAB_RENDERER_WEB_CONTENTS_ID) { return { isDestroyed: mocks.rendererIsDestroyedMock, - send: mocks.rendererSendMock + send: mocks.rendererSendMock, + setBackgroundThrottling: vi.fn(), + capturePage: vi.fn(async () => null) } } return null diff --git a/src/main/browser/browser-manager-grab.ts b/src/main/browser/browser-manager-grab.ts index 2a44dcd2a82..3ee108bb531 100644 --- a/src/main/browser/browser-manager-grab.ts +++ b/src/main/browser/browser-manager-grab.ts @@ -87,7 +87,12 @@ export abstract class BrowserManagerGrab extends BrowserManagerViewport { rect: BrowserGrabRect, guest: Electron.WebContents ): Promise { - return captureGrabSelectionScreenshot(rect, guest) + const releasePaint = this.holdPaintForCapture(guest.id) + try { + return await captureGrabSelectionScreenshot(rect, guest) + } finally { + releasePaint() + } } /** Preserve the active selection overlay and awaitClick listener. */ diff --git a/src/main/browser/browser-manager-guest-lifecycle.test.ts b/src/main/browser/browser-manager-guest-lifecycle.test.ts index bc0c3a30a25..ccdc93c4e92 100644 --- a/src/main/browser/browser-manager-guest-lifecycle.test.ts +++ b/src/main/browser/browser-manager-guest-lifecycle.test.ts @@ -431,7 +431,7 @@ describe('browserManager', () => { browserManager.attachGuestPolicies(guest as never) browserManager.attachGuestPolicies(guest as never) - expect(guestSetBackgroundThrottlingMock).toHaveBeenCalledTimes(1) + expect(guestSetBackgroundThrottlingMock).not.toHaveBeenCalled() expect(guestSetWindowOpenHandlerMock).toHaveBeenCalledTimes(1) expect(guestOnMock.mock.calls.filter(([event]) => event === 'will-navigate')).toHaveLength(1) expect(guestOnMock.mock.calls.filter(([event]) => event === 'will-redirect')).toHaveLength(1) diff --git a/src/main/browser/browser-manager-guest-policy.ts b/src/main/browser/browser-manager-guest-policy.ts index bf9aabd8191..e62752cfff7 100644 --- a/src/main/browser/browser-manager-guest-policy.ts +++ b/src/main/browser/browser-manager-guest-policy.ts @@ -27,8 +27,6 @@ export abstract class BrowserManagerGuestPolicy extends BrowserManagerGuestClean this.popupOwnerContextByGuestId.set(guest.id, inheritedOwnerContext) } const disposeDetachTracking = this.trackDebuggerDetachForCdpOverrides(guest) - // Keep background compositor frames available for screenshots. - guest.setBackgroundThrottling(false) const disposePopupPolicy = this.installGuestPopupPolicy(guest) const disposeNavigationPolicy = this.installGuestNavigationPolicy(guest) const forwardAudioState = ( diff --git a/src/main/browser/browser-manager-guest-visibility.test.ts b/src/main/browser/browser-manager-guest-visibility.test.ts index 709aabce768..f5225a9b97c 100644 --- a/src/main/browser/browser-manager-guest-visibility.test.ts +++ b/src/main/browser/browser-manager-guest-visibility.test.ts @@ -40,6 +40,7 @@ vi.mock('./popup-origin-bar-window', () => ({ })) import { browserManager } from './browser-manager' +import { rendererPublicationThrottle } from '../window/renderer-publication-throttle' import { rendererWebContentsId, resetBrowserManagerMocks, @@ -65,7 +66,7 @@ describe('browserManager', () => { vi.useRealTimers() }) - it('holds capture paint one-way and keeps the renderer unthrottled until the last release', () => { + it('keeps idle guests throttled and holds both contents until the last capture release', () => { vi.useFakeTimers() const guest = { id: 1707, @@ -73,6 +74,7 @@ describe('browserManager', () => { isCurrentlyAudible: vi.fn(() => false), getType: vi.fn(() => 'webview'), setBackgroundThrottling: guestSetBackgroundThrottlingMock, + capturePage: vi.fn(async () => null), setWindowOpenHandler: guestSetWindowOpenHandlerMock, on: guestOnMock, off: guestOffMock, @@ -105,6 +107,7 @@ describe('browserManager', () => { }) // Registration also reports audio state; this test is about paint holds. renderer.send.mockClear() + expect(guestSetBackgroundThrottlingMock).not.toHaveBeenCalled() // Synchronous: a capture never waits on the desktop renderer. const releaseFirst = browserManager.holdPaintForCapture(guest.id) @@ -115,11 +118,13 @@ describe('browserManager', () => { ['browser:capturePaintHold', { browserPageId: 'page-capture', held: true }] ]) expect(renderer.setBackgroundThrottling.mock.calls).toEqual([[false]]) + expect(guestSetBackgroundThrottlingMock.mock.calls).toEqual([[false]]) releaseFirst() releaseFirst() expect(renderer.send).toHaveBeenCalledTimes(1) expect(renderer.setBackgroundThrottling.mock.calls).toEqual([[false]]) + expect(guestSetBackgroundThrottlingMock.mock.calls).toEqual([[false]]) releaseSecond() expect(renderer.send.mock.calls.at(-1)).toEqual([ @@ -129,5 +134,153 @@ describe('browserManager', () => { expect(renderer.setBackgroundThrottling.mock.calls).toEqual([[false]]) vi.runAllTimers() expect(renderer.setBackgroundThrottling.mock.calls).toEqual([[false], [true]]) + expect(guestSetBackgroundThrottlingMock.mock.calls).toEqual([[false], [true]]) + }) + + it('shares guest leases with a concurrent stream even without a desktop renderer', () => { + vi.useFakeTimers() + const guest = { + id: 1708, + isDestroyed: vi.fn(() => false), + setBackgroundThrottling: vi.fn(), + capturePage: vi.fn(async () => null) + } + webContentsFromIdMock.mockReturnValue(guest) + const releaseStream = rendererPublicationThrottle.acquire(guest) + const releaseCapture = browserManager.holdPaintForCapture(guest.id) + + releaseStream() + vi.runAllTimers() + expect(guest.setBackgroundThrottling.mock.calls).toEqual([[false]]) + + releaseCapture() + releaseCapture() + vi.runAllTimers() + expect(guest.setBackgroundThrottling.mock.calls).toEqual([[false], [true]]) + }) + + it.each(['notify', 'acquire', 'release'] as const)( + 'cleans capture ownership after renderer %s fails', + (failure) => { + vi.useFakeTimers() + const guest = { + id: 1711, + isDestroyed: vi.fn(() => false), + isCurrentlyAudible: vi.fn(() => false), + getType: vi.fn(() => 'webview'), + setBackgroundThrottling: vi.fn(), + capturePage: vi.fn(async () => null), + setWindowOpenHandler: vi.fn(), + on: vi.fn(), + off: vi.fn(), + openDevTools: vi.fn() + } + const renderer = { + id: rendererWebContentsId, + isDestroyed: vi.fn(() => false), + send: vi.fn(), + setBackgroundThrottling: vi.fn(), + capturePage: vi.fn(async () => null) + } + webContentsFromIdMock.mockImplementation((id: number) => (id === guest.id ? guest : renderer)) + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: this guest provides the policy and registration members exercised here. + browserManager.attachGuestPolicies(guest as never) + browserManager.registerGuest({ + browserPageId: 'page-failure', + webContentsId: guest.id, + rendererWebContentsId + }) + const fail = (): never => { + throw new Error('renderer unavailable') + } + if (failure === 'release') { + const release = browserManager.holdPaintForCapture(guest.id) + renderer.send.mockImplementationOnce(fail) + expect(release).not.toThrow() + } else if (failure === 'notify') { + renderer.send.mockImplementationOnce(fail) + const release = browserManager.holdPaintForCapture(guest.id) + expect(release).not.toThrow() + } else { + renderer.setBackgroundThrottling.mockImplementationOnce(fail) + expect(() => browserManager.holdPaintForCapture(guest.id)).toThrow('renderer unavailable') + } + vi.runAllTimers() + expect(guest.setBackgroundThrottling.mock.calls).toEqual([[false], [true]]) + + renderer.send.mockClear() + const releaseNext = browserManager.holdPaintForCapture(guest.id) + expect(renderer.send.mock.calls).toEqual([ + ['browser:capturePaintHold', { browserPageId: 'page-failure', held: true }] + ]) + releaseNext() + vi.runAllTimers() + expect(guest.setBackgroundThrottling.mock.calls).toEqual([[false], [true], [false], [true]]) + } + ) + + it('does not let a destroyed guest and renderer release a replacement page hold', () => { + vi.useFakeTimers() + const makeGuest = (id: number) => ({ + id, + isDestroyed: vi.fn(() => false), + isCurrentlyAudible: vi.fn(() => false), + getType: vi.fn(() => 'webview'), + setBackgroundThrottling: vi.fn(), + capturePage: vi.fn(async () => null), + setWindowOpenHandler: vi.fn(), + on: vi.fn(), + off: vi.fn(), + openDevTools: vi.fn() + }) + const makeRenderer = (id: number) => ({ + id, + isDestroyed: vi.fn(() => false), + send: vi.fn(), + setBackgroundThrottling: vi.fn(), + capturePage: vi.fn(async () => null) + }) + const oldGuest = makeGuest(1709) + const newGuest = makeGuest(1710) + const oldRenderer = makeRenderer(rendererWebContentsId) + const newRenderer = makeRenderer(rendererWebContentsId + 1) + webContentsFromIdMock.mockImplementation((id: number) => + [oldGuest, newGuest, oldRenderer, newRenderer].find((contents) => contents.id === id) + ) + const register = (guest: ReturnType, rendererId: number) => { + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: this guest provides the policy and registration members exercised here. + browserManager.attachGuestPolicies(guest as never) + browserManager.registerGuest({ + browserPageId: 'page-replaced', + webContentsId: guest.id, + rendererWebContentsId: rendererId + }) + } + register(oldGuest, oldRenderer.id) + const releaseOld = browserManager.holdPaintForCapture(oldGuest.id) + oldGuest.isDestroyed.mockReturnValue(true) + oldRenderer.isDestroyed.mockReturnValue(true) + register(newGuest, newRenderer.id) + newRenderer.send.mockClear() + const releaseNew = browserManager.holdPaintForCapture(newGuest.id) + + releaseOld() + vi.runAllTimers() + expect(newRenderer.send.mock.calls).toEqual([ + ['browser:capturePaintHold', { browserPageId: 'page-replaced', held: true }] + ]) + expect(oldGuest.setBackgroundThrottling.mock.calls).toEqual([[false]]) + expect(oldRenderer.setBackgroundThrottling.mock.calls).toEqual([[false]]) + expect(newGuest.setBackgroundThrottling.mock.calls).toEqual([[false]]) + expect(newRenderer.setBackgroundThrottling.mock.calls).toEqual([[false]]) + + releaseNew() + vi.runAllTimers() + expect(newRenderer.send.mock.calls.at(-1)).toEqual([ + 'browser:capturePaintHold', + { browserPageId: 'page-replaced', held: false } + ]) + expect(newGuest.setBackgroundThrottling.mock.calls).toEqual([[false], [true]]) + expect(newRenderer.setBackgroundThrottling.mock.calls).toEqual([[false], [true]]) }) }) diff --git a/src/main/browser/browser-manager-popup-child.test.ts b/src/main/browser/browser-manager-popup-child.test.ts index a31fd7c3266..0856e2a665f 100644 --- a/src/main/browser/browser-manager-popup-child.test.ts +++ b/src/main/browser/browser-manager-popup-child.test.ts @@ -112,7 +112,7 @@ describe('browserManager popup child policies', () => { didCreateWindowHandler?.({ webContents: childGuest }) - expect(childSetBackgroundThrottlingMock).toHaveBeenCalledWith(false) + expect(childSetBackgroundThrottlingMock).not.toHaveBeenCalled() expect(childSetWindowOpenHandlerMock).toHaveBeenCalledTimes(1) expect(childOnMock.mock.calls.filter(([event]) => event === 'did-create-window')).toHaveLength( 1 diff --git a/src/main/browser/browser-manager-popup-routing.test.ts b/src/main/browser/browser-manager-popup-routing.test.ts index d0d1c5eb479..e2df6c0b766 100644 --- a/src/main/browser/browser-manager-popup-routing.test.ts +++ b/src/main/browser/browser-manager-popup-routing.test.ts @@ -652,7 +652,7 @@ describe('browserManager', () => { expect(returned).toBe(popupContents) // Custom-created children skip did-create-window, so attach their guest policies directly. expect(popupContents.setWindowOpenHandler).toHaveBeenCalledTimes(1) - expect(popupContents.setBackgroundThrottling).toHaveBeenCalledWith(false) + expect(popupContents.setBackgroundThrottling).not.toHaveBeenCalled() expect(popupContents.on.mock.calls.some(([event]) => event === 'dom-ready')).toBe(false) expect(rendererSendMock).toHaveBeenCalledWith('browser:popup', { browserPageId: 'browser-1', diff --git a/src/main/browser/browser-manager-visibility.ts b/src/main/browser/browser-manager-visibility.ts index c080f1e9bc4..951592284c2 100644 --- a/src/main/browser/browser-manager-visibility.ts +++ b/src/main/browser/browser-manager-visibility.ts @@ -1,3 +1,4 @@ +import { webContents } from 'electron' import { rendererPublicationThrottle } from '../window/renderer-publication-throttle' import { BrowserManagerState } from './browser-manager-state' import { focusGuardUnavailableError, guardPaneFocusForAgentInput } from './agent-input-focus-guard' @@ -8,7 +9,7 @@ import { } from './browser-agent-activity' export abstract class BrowserManagerVisibility extends BrowserManagerState { - private readonly capturePaintHolds = new Map() + private readonly capturePaintHolds = new Map>() protected readonly agentActivity = new BrowserAgentActivity({ pageIdForGuest: (guestWebContentsId) => this.resolveBrowserTabIdForGuestWebContentsId(guestWebContentsId), @@ -26,37 +27,58 @@ export abstract class BrowserManagerVisibility extends BrowserManagerState { return this.agentActivity.showPointer(guest, input) } + // Non-capture commands only need the guest itself unthrottled; host paint stays parked. + holdGuestActive(guestWebContentsId: number): () => void { + const guest = webContents.fromId(guestWebContentsId) + return guest && !guest.isDestroyed() ? rendererPublicationThrottle.acquire(guest) : () => {} + } + // Do not await the renderer: minimized or throttled windows must not stall frame capture. holdPaintForCapture(guestWebContentsId: number): () => void { + const releaseGuestThrottle = this.holdGuestActive(guestWebContentsId) const browserPageId = this.resolveBrowserTabIdForGuestWebContentsId(guestWebContentsId) const renderer = browserPageId ? this.resolveRendererForBrowserTab(browserPageId) : null if (!browserPageId || !renderer || renderer.isDestroyed()) { - return () => {} + return releaseGuestThrottle } - const holds = this.capturePaintHolds.get(browserPageId) ?? 0 - this.capturePaintHolds.set(browserPageId, holds + 1) + let pageHolds = this.capturePaintHolds.get(renderer) + if (!pageHolds) { + pageHolds = new Map() + this.capturePaintHolds.set(renderer, pageHolds) + } + const holds = pageHolds.get(browserPageId) ?? 0 + pageHolds.set(browserPageId, holds + 1) if (holds === 0) { - renderer.send('browser:capturePaintHold', { browserPageId, held: true }) + sendPaintHold(renderer, browserPageId, true) } - // Lift throttling so capture retries do not repeatedly wait on delayed parking changes. - const releaseThrottle = rendererPublicationThrottle.acquire(renderer) + let releaseThrottle = (): void => {} let released = false - return () => { + const release = (): void => { if (released) { return } released = true - releaseThrottle() - const remaining = (this.capturePaintHolds.get(browserPageId) ?? 1) - 1 + const remaining = (pageHolds.get(browserPageId) ?? 1) - 1 if (remaining > 0) { - this.capturePaintHolds.set(browserPageId, remaining) - return - } - this.capturePaintHolds.delete(browserPageId) - if (!renderer.isDestroyed()) { - renderer.send('browser:capturePaintHold', { browserPageId, held: false }) + pageHolds.set(browserPageId, remaining) + } else { + pageHolds.delete(browserPageId) + if (pageHolds.size === 0) { + this.capturePaintHolds.delete(renderer) + } + sendPaintHold(renderer, browserPageId, false) } + releaseThrottle() + releaseGuestThrottle() } + try { + // Lift throttling so capture retries do not wait on delayed parking changes. + releaseThrottle = rendererPublicationThrottle.acquire(renderer) + } catch (error) { + release() + throw error + } + return release } // Agent input moves Chromium focus into the guest; the pane restores the user's focus. @@ -69,3 +91,13 @@ export abstract class BrowserManagerVisibility extends BrowserManagerState { return guardPaneFocusForAgentInput(renderer, browserPageId) } } + +// A paint hint must not fail the capture or command it accompanies; disposed frames throw. +function sendPaintHold(renderer: Electron.WebContents, browserPageId: string, held: boolean): void { + if (renderer.isDestroyed()) { + return + } + try { + renderer.send('browser:capturePaintHold', { browserPageId, held }) + } catch {} +} diff --git a/src/main/browser/extensions/api/tabs-capture.test.ts b/src/main/browser/extensions/api/tabs-capture.test.ts index d9c0b4fb6d3..af11ef1287a 100644 --- a/src/main/browser/extensions/api/tabs-capture.test.ts +++ b/src/main/browser/extensions/api/tabs-capture.test.ts @@ -4,9 +4,20 @@ import type { BrowserExtensionCaller } from '../browser-extension-namespace-regi const mocks = vi.hoisted(() => { const tabs: { windowId: number; active: boolean; contents: unknown }[] = [] const hostPatterns: string[] = [] - return { tabs, windows: new Set(), hostPatterns, activeTabGrants: new Set() } + return { + tabs, + windows: new Set(), + hostPatterns, + activeTabGrants: new Set(), + releasePaint: vi.fn(), + holdPaint: vi.fn() + } }) +vi.mock('../../browser-manager', () => ({ + browserManager: { holdPaintForCapture: mocks.holdPaint } +})) + vi.mock('../browser-extension-tab-model', () => ({ listBrowserExtensionTabs: () => mocks.tabs, resolveCallerWindowId: () => 1 @@ -61,6 +72,8 @@ beforeEach(() => { mocks.windows = new Set([1, 2]) mocks.hostPatterns = [] mocks.activeTabGrants = new Set() + mocks.holdPaint.mockClear().mockReturnValue(mocks.releasePaint) + mocks.releasePaint.mockClear() }) describe('tabs.captureVisibleTab', () => { @@ -107,4 +120,16 @@ describe('tabs.captureVisibleTab', () => { mocks.tabs = [] await expect(capture()).rejects.toThrow('No active web contents to capture') }) + + it('releases hidden-page paint when capture fails', async () => { + mocks.hostPatterns = [''] + const active = tab(7, 'https://site.test/') + active.contents.capturePage.mockRejectedValueOnce(new Error('capture failed')) + mocks.tabs = [active] + + await expect(capture()).rejects.toThrow('capture failed') + + expect(mocks.holdPaint).toHaveBeenCalledWith(7) + expect(mocks.releasePaint).toHaveBeenCalledTimes(1) + }) }) diff --git a/src/main/browser/extensions/api/tabs-capture.ts b/src/main/browser/extensions/api/tabs-capture.ts index ee9464b948e..6cffaa9bdef 100644 --- a/src/main/browser/extensions/api/tabs-capture.ts +++ b/src/main/browser/extensions/api/tabs-capture.ts @@ -1,4 +1,5 @@ import { z } from 'zod' +import { browserManager } from '../../browser-manager' import { parseChromeExtensionUrl } from '../../../../shared/browser-extension-url' import { isChromeWebStoreUrl } from '../../../../shared/browser-extension-webstore' import { hasBrowserExtensionActiveTab } from '../browser-extension-active-tab' @@ -74,7 +75,13 @@ async function captureVisibleTab( if (denial) { throw new BrowserExtensionApiError(denial) } - const image = await tab.contents.capturePage() + const releasePaint = browserManager.holdPaintForCapture(tab.contents.id) + let image: Electron.NativeImage + try { + image = await tab.contents.capturePage() + } finally { + releasePaint() + } if (image.isEmpty()) { throw new BrowserExtensionApiError( 'Internal error while trying to capture visible region of the current tab' diff --git a/src/main/runtime/browser/browser-screencasts.ts b/src/main/runtime/browser/browser-screencasts.ts index 04e09103c77..d22b1aac176 100644 --- a/src/main/runtime/browser/browser-screencasts.ts +++ b/src/main/runtime/browser/browser-screencasts.ts @@ -92,7 +92,7 @@ export class RuntimeBrowserScreencasts { // Hidden host compositors stop producing guest frames when throttled. const win = this.host.getAvailableAuthoritativeWindow() const releaseThrottle = win ? rendererPublicationThrottle.acquire(win.webContents) : () => {} - // Active-tab covers hide the guest widget too; its attach-time unthrottle may predate them. + // Active-tab covers hide the guest widget independently of the host. const releaseGuestThrottle = rendererPublicationThrottle.acquire(guest) const started = startBrowserScreencast(guest, { format: params.format,