mirror of
https://github.com/stablyai/orca.git
synced 2026-10-31 00:57:45 +00:00
perf(browser): throttle idle guests between active operations (#34776)
* perf(browser): throttle idle guests between active operations * perf(browser): lease only the guest for non-capture agent commands Keep the renderer paint hold, host unthrottle, and host re-hide readback on capture paths only, and never let the one-way paint-hint send fail the capture. * style: format bridge paint-hold test --------- Co-authored-by: OrcaWin <293788423+OrcaWin@users.noreply.github.com>
This commit is contained in:
@@ -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<typeof vi.fn>[] = []
|
||||
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<string, unknown>) {
|
||||
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 () => {
|
||||
|
||||
@@ -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. */
|
||||
|
||||
@@ -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 () => {
|
||||
|
||||
@@ -35,6 +35,8 @@ export function mockBrowserManager(
|
||||
getBrowserPageLoadError: vi.fn(() => null),
|
||||
getBrowserPageCertificateFailure: vi.fn(() => null),
|
||||
unregisterGuest: vi.fn(),
|
||||
holdPaintForCapture: () => () => {},
|
||||
holdGuestActive: () => () => {},
|
||||
beginAgentCommand: () => ({
|
||||
run: (execute: () => Promise<unknown>) => execute(),
|
||||
end: () => {}
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -87,7 +87,12 @@ export abstract class BrowserManagerGrab extends BrowserManagerViewport {
|
||||
rect: BrowserGrabRect,
|
||||
guest: Electron.WebContents
|
||||
): Promise<BrowserGrabScreenshot | null> {
|
||||
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. */
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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 = (
|
||||
|
||||
@@ -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<typeof makeGuest>, 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]])
|
||||
})
|
||||
})
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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',
|
||||
|
||||
@@ -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<string, number>()
|
||||
private readonly capturePaintHolds = new Map<Electron.WebContents, Map<string, number>>()
|
||||
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 {}
|
||||
}
|
||||
|
||||
@@ -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<number>(), hostPatterns, activeTabGrants: new Set<number>() }
|
||||
return {
|
||||
tabs,
|
||||
windows: new Set<number>(),
|
||||
hostPatterns,
|
||||
activeTabGrants: new Set<number>(),
|
||||
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 = ['<all_urls>']
|
||||
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)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -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'
|
||||
|
||||
@@ -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,
|
||||
|
||||
Reference in New Issue
Block a user