fix(browser): resolve grab mode race conditions and teardown (#11661)
- Serialize grab mode operations to prevent concurrent state changes - Wait for guest registration with timeout when page hasn't registered yet - Tear down armed overlay when disabling before selection starts - Distinguish injection-failed from not-ready errors in UI - Track arm generations to cancel stale operations after page switches - Add localized error messages for each failure reason
This commit is contained in:
parent
6ae19be723
commit
aeda3c6e32
|
|
@ -195,6 +195,14 @@ describe('browserManager grab operations', () => {
|
|||
expect(selection.opId).toBe('op-1')
|
||||
})
|
||||
|
||||
it('tears down an armed overlay when disabling before selection starts', async () => {
|
||||
const result = await browserManager.setGrabMode('tab-1', false, guest)
|
||||
|
||||
expect(result).toBe(true)
|
||||
expect(guestExecuteJavaScriptMock).toHaveBeenCalledTimes(1)
|
||||
expect(guestExecuteJavaScriptMock.mock.calls[0][0]).toContain('grab.cleanup()')
|
||||
})
|
||||
|
||||
it('returns false if injection fails', async () => {
|
||||
guestExecuteJavaScriptMock.mockRejectedValue(new Error('Injection failed'))
|
||||
const result = await browserManager.setGrabMode('tab-1', true, guest)
|
||||
|
|
|
|||
|
|
@ -1610,8 +1610,17 @@ export class BrowserManager {
|
|||
guest: Electron.WebContents
|
||||
): Promise<boolean> {
|
||||
if (!enabled) {
|
||||
const hadActiveGrabOp = this.hasActiveGrabOp(browserTabId)
|
||||
this.cancelGrabOp(browserTabId, 'user')
|
||||
return true
|
||||
if (hadActiveGrabOp) {
|
||||
return true
|
||||
}
|
||||
try {
|
||||
await guest.executeJavaScript(buildGuestOverlayScript('teardown'))
|
||||
return true
|
||||
} catch {
|
||||
return false
|
||||
}
|
||||
}
|
||||
// Why: inject the overlay runtime eagerly on arm so the hover UI appears instantly; re-injection is idempotent/safe.
|
||||
try {
|
||||
|
|
|
|||
|
|
@ -8,6 +8,8 @@ const {
|
|||
getGuestWebContentsIdMock,
|
||||
getWebContentsIdByTabIdMock,
|
||||
getWorktreeIdForTabMock,
|
||||
getAuthorizedGuestMock,
|
||||
setGrabModeMock,
|
||||
openDevToolsMock,
|
||||
setAnnotationViewportBridgeMock,
|
||||
cancelDownloadMock,
|
||||
|
|
@ -22,6 +24,8 @@ const {
|
|||
getGuestWebContentsIdMock: vi.fn(),
|
||||
getWebContentsIdByTabIdMock: vi.fn(() => new Map()),
|
||||
getWorktreeIdForTabMock: vi.fn(),
|
||||
getAuthorizedGuestMock: vi.fn(),
|
||||
setGrabModeMock: vi.fn(),
|
||||
openDevToolsMock: vi.fn().mockResolvedValue(true),
|
||||
setAnnotationViewportBridgeMock: vi.fn().mockResolvedValue(true),
|
||||
cancelDownloadMock: vi.fn(),
|
||||
|
|
@ -53,6 +57,8 @@ vi.mock('../browser/browser-manager', () => ({
|
|||
getGuestWebContentsId: getGuestWebContentsIdMock,
|
||||
getWebContentsIdByTabId: getWebContentsIdByTabIdMock,
|
||||
getWorktreeIdForTab: getWorktreeIdForTabMock,
|
||||
getAuthorizedGuest: getAuthorizedGuestMock,
|
||||
setGrabMode: setGrabModeMock,
|
||||
openDevTools: openDevToolsMock,
|
||||
setAnnotationViewportBridge: setAnnotationViewportBridgeMock,
|
||||
cancelDownload: cancelDownloadMock
|
||||
|
|
@ -79,6 +85,9 @@ describe('registerBrowserHandlers', () => {
|
|||
getWebContentsIdByTabIdMock.mockReset()
|
||||
getWebContentsIdByTabIdMock.mockReturnValue(new Map())
|
||||
getWorktreeIdForTabMock.mockReset()
|
||||
getAuthorizedGuestMock.mockReset()
|
||||
setGrabModeMock.mockReset()
|
||||
setGrabModeMock.mockResolvedValue(true)
|
||||
openDevToolsMock.mockReset()
|
||||
setAnnotationViewportBridgeMock.mockReset()
|
||||
cancelDownloadMock.mockReset()
|
||||
|
|
@ -331,6 +340,255 @@ describe('registerBrowserHandlers', () => {
|
|||
}
|
||||
})
|
||||
|
||||
it('waits for authorized registration even when an old guest is still live', async () => {
|
||||
const guest = { id: 123 } as Electron.WebContents
|
||||
getGuestWebContentsIdMock.mockReturnValue(122)
|
||||
getAuthorizedGuestMock.mockReturnValueOnce(null).mockReturnValue(guest)
|
||||
registerBrowserHandlers()
|
||||
const sender = {
|
||||
id: 91,
|
||||
isDestroyed: () => false,
|
||||
getType: () => 'window',
|
||||
getURL: () => 'file:///renderer/index.html'
|
||||
} as Electron.WebContents
|
||||
const setGrabModeHandler = handleMock.mock.calls.find(
|
||||
([channel]) => channel === 'browser:setGrabMode'
|
||||
)?.[1] as (
|
||||
event: { sender: Electron.WebContents },
|
||||
args: { browserPageId: string; enabled: boolean }
|
||||
) => Promise<unknown>
|
||||
const registerHandler = handleMock.mock.calls.find(
|
||||
([channel]) => channel === 'browser:registerGuest'
|
||||
)?.[1] as (
|
||||
event: { sender: Electron.WebContents },
|
||||
args: {
|
||||
browserPageId: string
|
||||
workspaceId: string
|
||||
worktreeId: string
|
||||
webContentsId: number
|
||||
}
|
||||
) => boolean
|
||||
|
||||
const pendingResult = setGrabModeHandler({ sender }, { browserPageId: 'page-1', enabled: true })
|
||||
await Promise.resolve()
|
||||
expect(setGrabModeMock).not.toHaveBeenCalled()
|
||||
|
||||
expect(
|
||||
registerHandler(
|
||||
{ sender },
|
||||
{
|
||||
browserPageId: 'page-1',
|
||||
workspaceId: 'workspace-1',
|
||||
worktreeId: 'worktree-1',
|
||||
webContentsId: 123
|
||||
}
|
||||
)
|
||||
).toBe(true)
|
||||
|
||||
await expect(pendingResult).resolves.toEqual({ ok: true })
|
||||
expect(setGrabModeMock).toHaveBeenCalledWith('page-1', true, guest)
|
||||
})
|
||||
|
||||
it('returns not-ready when grab registration does not arrive', async () => {
|
||||
vi.useFakeTimers()
|
||||
try {
|
||||
getGuestWebContentsIdMock.mockReturnValue(null)
|
||||
getAuthorizedGuestMock.mockReturnValue(null)
|
||||
registerBrowserHandlers()
|
||||
const sender = {
|
||||
id: 91,
|
||||
isDestroyed: () => false,
|
||||
getType: () => 'window',
|
||||
getURL: () => 'file:///renderer/index.html'
|
||||
} as Electron.WebContents
|
||||
const setGrabModeHandler = handleMock.mock.calls.find(
|
||||
([channel]) => channel === 'browser:setGrabMode'
|
||||
)?.[1] as (
|
||||
event: { sender: Electron.WebContents },
|
||||
args: { browserPageId: string; enabled: boolean }
|
||||
) => Promise<unknown>
|
||||
|
||||
const pendingResult = setGrabModeHandler(
|
||||
{ sender },
|
||||
{ browserPageId: 'page-1', enabled: true }
|
||||
)
|
||||
await vi.advanceTimersByTimeAsync(1_001)
|
||||
|
||||
await expect(pendingResult).resolves.toEqual({ ok: false, reason: 'not-ready' })
|
||||
expect(setGrabModeMock).not.toHaveBeenCalled()
|
||||
} finally {
|
||||
vi.useRealTimers()
|
||||
}
|
||||
})
|
||||
|
||||
it('does not enable grab mode after a pending request is cancelled', async () => {
|
||||
const guest = { id: 123 } as Electron.WebContents
|
||||
let registered = false
|
||||
getAuthorizedGuestMock.mockImplementation(() => (registered ? guest : null))
|
||||
registerBrowserHandlers()
|
||||
const sender = {
|
||||
id: 91,
|
||||
isDestroyed: () => false,
|
||||
getType: () => 'window',
|
||||
getURL: () => 'file:///renderer/index.html'
|
||||
} as Electron.WebContents
|
||||
const setGrabModeHandler = handleMock.mock.calls.find(
|
||||
([channel]) => channel === 'browser:setGrabMode'
|
||||
)?.[1] as (
|
||||
event: { sender: Electron.WebContents },
|
||||
args: { browserPageId: string; enabled: boolean }
|
||||
) => Promise<unknown>
|
||||
const registerHandler = handleMock.mock.calls.find(
|
||||
([channel]) => channel === 'browser:registerGuest'
|
||||
)?.[1] as (
|
||||
event: { sender: Electron.WebContents },
|
||||
args: {
|
||||
browserPageId: string
|
||||
workspaceId: string
|
||||
worktreeId: string
|
||||
webContentsId: number
|
||||
}
|
||||
) => boolean
|
||||
|
||||
const pendingEnable = setGrabModeHandler({ sender }, { browserPageId: 'page-1', enabled: true })
|
||||
await expect(
|
||||
setGrabModeHandler({ sender }, { browserPageId: 'page-1', enabled: false })
|
||||
).resolves.toEqual({ ok: true })
|
||||
|
||||
registered = true
|
||||
expect(
|
||||
registerHandler(
|
||||
{ sender },
|
||||
{
|
||||
browserPageId: 'page-1',
|
||||
workspaceId: 'workspace-1',
|
||||
worktreeId: 'worktree-1',
|
||||
webContentsId: 123
|
||||
}
|
||||
)
|
||||
).toBe(true)
|
||||
|
||||
await expect(pendingEnable).resolves.toEqual({ ok: true })
|
||||
expect(setGrabModeMock).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('coalesces a cancelled pending enable into one later enable', async () => {
|
||||
const guest = { id: 123 } as Electron.WebContents
|
||||
let registered = false
|
||||
getAuthorizedGuestMock.mockImplementation(() => (registered ? guest : null))
|
||||
registerBrowserHandlers()
|
||||
const sender = {
|
||||
id: 91,
|
||||
isDestroyed: () => false,
|
||||
getType: () => 'window',
|
||||
getURL: () => 'file:///renderer/index.html'
|
||||
} as Electron.WebContents
|
||||
const setGrabModeHandler = handleMock.mock.calls.find(
|
||||
([channel]) => channel === 'browser:setGrabMode'
|
||||
)?.[1] as (
|
||||
event: { sender: Electron.WebContents },
|
||||
args: { browserPageId: string; enabled: boolean }
|
||||
) => Promise<unknown>
|
||||
const registerHandler = handleMock.mock.calls.find(
|
||||
([channel]) => channel === 'browser:registerGuest'
|
||||
)?.[1] as (
|
||||
event: { sender: Electron.WebContents },
|
||||
args: {
|
||||
browserPageId: string
|
||||
workspaceId: string
|
||||
worktreeId: string
|
||||
webContentsId: number
|
||||
}
|
||||
) => boolean
|
||||
|
||||
const firstEnable = setGrabModeHandler({ sender }, { browserPageId: 'page-1', enabled: true })
|
||||
await setGrabModeHandler({ sender }, { browserPageId: 'page-1', enabled: false })
|
||||
const latestEnable = setGrabModeHandler({ sender }, { browserPageId: 'page-1', enabled: true })
|
||||
|
||||
registered = true
|
||||
registerHandler(
|
||||
{ sender },
|
||||
{
|
||||
browserPageId: 'page-1',
|
||||
workspaceId: 'workspace-1',
|
||||
worktreeId: 'worktree-1',
|
||||
webContentsId: 123
|
||||
}
|
||||
)
|
||||
|
||||
await expect(Promise.all([firstEnable, latestEnable])).resolves.toEqual([
|
||||
{ ok: true },
|
||||
{ ok: true }
|
||||
])
|
||||
expect(setGrabModeMock).toHaveBeenCalledTimes(1)
|
||||
expect(setGrabModeMock).toHaveBeenCalledWith('page-1', true, guest)
|
||||
})
|
||||
|
||||
it('serializes in-flight mode changes so a stale enable cannot tear down the latest one', async () => {
|
||||
const guest = { id: 123 } as Electron.WebContents
|
||||
let resolveFirstEnable!: (success: boolean) => void
|
||||
const firstEnable = new Promise<boolean>((resolve) => {
|
||||
resolveFirstEnable = resolve
|
||||
})
|
||||
getAuthorizedGuestMock.mockReturnValue(guest)
|
||||
setGrabModeMock.mockReturnValueOnce(firstEnable).mockResolvedValue(true)
|
||||
registerBrowserHandlers()
|
||||
const sender = {
|
||||
id: 91,
|
||||
isDestroyed: () => false,
|
||||
getType: () => 'window',
|
||||
getURL: () => 'file:///renderer/index.html'
|
||||
} as Electron.WebContents
|
||||
const setGrabModeHandler = handleMock.mock.calls.find(
|
||||
([channel]) => channel === 'browser:setGrabMode'
|
||||
)?.[1] as (
|
||||
event: { sender: Electron.WebContents },
|
||||
args: { browserPageId: string; enabled: boolean }
|
||||
) => Promise<unknown>
|
||||
|
||||
const staleEnable = setGrabModeHandler({ sender }, { browserPageId: 'page-1', enabled: true })
|
||||
await vi.waitFor(() => {
|
||||
expect(setGrabModeMock).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
const disable = setGrabModeHandler({ sender }, { browserPageId: 'page-1', enabled: false })
|
||||
const latestEnable = setGrabModeHandler({ sender }, { browserPageId: 'page-1', enabled: true })
|
||||
|
||||
resolveFirstEnable(true)
|
||||
await expect(Promise.all([staleEnable, disable, latestEnable])).resolves.toEqual([
|
||||
{ ok: true },
|
||||
{ ok: true },
|
||||
{ ok: true }
|
||||
])
|
||||
|
||||
expect(setGrabModeMock.mock.calls).toEqual([
|
||||
['page-1', true, guest],
|
||||
['page-1', true, guest]
|
||||
])
|
||||
})
|
||||
|
||||
it('distinguishes picker injection failure from guest readiness', async () => {
|
||||
const guest = { id: 123 } as Electron.WebContents
|
||||
getAuthorizedGuestMock.mockReturnValue(guest)
|
||||
setGrabModeMock.mockResolvedValue(false)
|
||||
registerBrowserHandlers()
|
||||
const sender = {
|
||||
id: 91,
|
||||
isDestroyed: () => false,
|
||||
getType: () => 'window',
|
||||
getURL: () => 'file:///renderer/index.html'
|
||||
} as Electron.WebContents
|
||||
const setGrabModeHandler = handleMock.mock.calls.find(
|
||||
([channel]) => channel === 'browser:setGrabMode'
|
||||
)?.[1] as (
|
||||
event: { sender: Electron.WebContents },
|
||||
args: { browserPageId: string; enabled: boolean }
|
||||
) => Promise<unknown>
|
||||
|
||||
await expect(
|
||||
setGrabModeHandler({ sender }, { browserPageId: 'page-1', enabled: true })
|
||||
).resolves.toEqual({ ok: false, reason: 'injection-failed' })
|
||||
})
|
||||
|
||||
it('resolves worktree and any-tab registration waiters when a guest registers', async () => {
|
||||
vi.useFakeTimers()
|
||||
try {
|
||||
|
|
|
|||
|
|
@ -45,6 +45,9 @@ let agentBrowserBridgeRef: AgentBrowserBridge | null = null
|
|||
const pendingTabRegistrations = new Map<string, Set<() => void>>()
|
||||
const pendingWorktreeTabRegistrations = new Map<string, Set<() => void>>()
|
||||
const pendingAnyTabRegistrations = new Set<() => void>()
|
||||
const grabModeIntentByPageId = new Map<string, { generation: number; enabled: boolean }>()
|
||||
const grabModeOperationByPageId = new Map<string, Promise<void>>()
|
||||
const GRAB_REGISTRATION_WAIT_MS = 1_000
|
||||
|
||||
function waitForRegistrationSet(
|
||||
registrationResolvers: Set<() => void>,
|
||||
|
|
@ -100,6 +103,10 @@ export function waitForTabRegistration(browserPageId: string, timeoutMs = 8_000)
|
|||
if (isLiveBrowserWebContentsId(browserManager.getGuestWebContentsId(browserPageId))) {
|
||||
return Promise.resolve()
|
||||
}
|
||||
return waitForNextTabRegistration(browserPageId, timeoutMs)
|
||||
}
|
||||
|
||||
function waitForNextTabRegistration(browserPageId: string, timeoutMs: number): Promise<void> {
|
||||
let registrationResolvers = pendingTabRegistrations.get(browserPageId)
|
||||
if (!registrationResolvers) {
|
||||
registrationResolvers = new Set()
|
||||
|
|
@ -110,6 +117,24 @@ export function waitForTabRegistration(browserPageId: string, timeoutMs = 8_000)
|
|||
})
|
||||
}
|
||||
|
||||
function queueGrabModeOperation(
|
||||
browserPageId: string,
|
||||
operation: () => Promise<BrowserSetGrabModeResult>
|
||||
): Promise<BrowserSetGrabModeResult> {
|
||||
const previous = grabModeOperationByPageId.get(browserPageId) ?? Promise.resolve()
|
||||
const result = previous.then(operation)
|
||||
const completion = result.then(
|
||||
() => {},
|
||||
() => {}
|
||||
)
|
||||
grabModeOperationByPageId.set(browserPageId, completion)
|
||||
return result.finally(() => {
|
||||
if (grabModeOperationByPageId.get(browserPageId) === completion) {
|
||||
grabModeOperationByPageId.delete(browserPageId)
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
export function waitForWorktreeTabRegistration(
|
||||
worktreeId: string | undefined,
|
||||
timeoutMs = 8_000
|
||||
|
|
@ -168,6 +193,7 @@ function isTrustedBrowserRenderer(sender: Electron.WebContents): boolean {
|
|||
}
|
||||
|
||||
export function registerBrowserHandlers(): void {
|
||||
grabModeIntentByPageId.clear()
|
||||
ipcMain.removeHandler('browser:registerGuest')
|
||||
ipcMain.removeHandler('browser:unregisterGuest')
|
||||
ipcMain.removeHandler('browser:openDevTools')
|
||||
|
|
@ -237,6 +263,7 @@ export function registerBrowserHandlers(): void {
|
|||
agentBrowserBridgeRef.onTabClosed(wcId)
|
||||
}
|
||||
browserManager.unregisterGuest(args.browserPageId)
|
||||
grabModeIntentByPageId.delete(args.browserPageId)
|
||||
return true
|
||||
})
|
||||
|
||||
|
|
@ -370,12 +397,44 @@ export function registerBrowserHandlers(): void {
|
|||
if (!isTrustedBrowserRenderer(event.sender)) {
|
||||
return { ok: false, reason: 'not-authorized' }
|
||||
}
|
||||
const guest = browserManager.getAuthorizedGuest(args.browserPageId, event.sender.id)
|
||||
const intent = {
|
||||
generation: (grabModeIntentByPageId.get(args.browserPageId)?.generation ?? 0) + 1,
|
||||
enabled: args.enabled
|
||||
}
|
||||
grabModeIntentByPageId.set(args.browserPageId, intent)
|
||||
const isCurrentIntent = (): boolean =>
|
||||
grabModeIntentByPageId.get(args.browserPageId) === intent
|
||||
let guest = browserManager.getAuthorizedGuest(args.browserPageId, event.sender.id)
|
||||
if (!guest && args.enabled) {
|
||||
// Why: fast file:// pages can expose the toolbar before did-attach registration reaches main.
|
||||
await waitForNextTabRegistration(args.browserPageId, GRAB_REGISTRATION_WAIT_MS).catch(
|
||||
() => {}
|
||||
)
|
||||
if (!isCurrentIntent()) {
|
||||
return { ok: true }
|
||||
}
|
||||
guest = browserManager.getAuthorizedGuest(args.browserPageId, event.sender.id)
|
||||
}
|
||||
if (!guest) {
|
||||
if (!args.enabled) {
|
||||
return { ok: true }
|
||||
}
|
||||
return { ok: false, reason: 'not-ready' }
|
||||
}
|
||||
const success = await browserManager.setGrabMode(args.browserPageId, args.enabled, guest)
|
||||
return success ? { ok: true } : { ok: false, reason: 'not-ready' }
|
||||
return queueGrabModeOperation(args.browserPageId, async () => {
|
||||
if (!isCurrentIntent()) {
|
||||
return { ok: true }
|
||||
}
|
||||
guest = browserManager.getAuthorizedGuest(args.browserPageId, event.sender.id)
|
||||
if (!guest) {
|
||||
return args.enabled ? { ok: false, reason: 'not-ready' } : { ok: true }
|
||||
}
|
||||
const success = await browserManager.setGrabMode(args.browserPageId, args.enabled, guest)
|
||||
if (!isCurrentIntent()) {
|
||||
return { ok: true }
|
||||
}
|
||||
return success ? { ok: true } : { ok: false, reason: 'injection-failed' }
|
||||
})
|
||||
}
|
||||
)
|
||||
|
||||
|
|
|
|||
|
|
@ -84,4 +84,191 @@ describe('useGrabMode', () => {
|
|||
enabled: true
|
||||
})
|
||||
})
|
||||
|
||||
it('reports a picker injection failure without calling it a readiness error', async () => {
|
||||
const harness = createReactHookHarness()
|
||||
vi.doMock('react', () => harness.react)
|
||||
vi.doMock('@/hooks/useMountedRef', () => ({
|
||||
useMountedRef: () => ({ current: true })
|
||||
}))
|
||||
vi.stubGlobal('window', {
|
||||
addEventListener: vi.fn(),
|
||||
removeEventListener: vi.fn(),
|
||||
api: {
|
||||
browser: {
|
||||
setGrabMode: vi.fn(async () => ({ ok: false, reason: 'injection-failed' })),
|
||||
awaitGrabSelection: vi.fn(),
|
||||
cancelGrab: vi.fn()
|
||||
}
|
||||
}
|
||||
})
|
||||
const { useGrabMode } = await import('./useGrabMode')
|
||||
const render = () => {
|
||||
harness.beginRender()
|
||||
// oxlint-disable-next-line react-hooks/rules-of-hooks -- test harness mocks React's hook dispatcher directly.
|
||||
return useGrabMode('page-1')
|
||||
}
|
||||
|
||||
render().toggle()
|
||||
await Promise.resolve()
|
||||
|
||||
const grab = render()
|
||||
expect(grab.state).toBe('error')
|
||||
expect(grab.error).toBe('Could not start element selection on this page.')
|
||||
})
|
||||
|
||||
it('arms immediately and treats a second toggle as cancellation while enable is pending', async () => {
|
||||
const harness = createReactHookHarness()
|
||||
let resolveEnable!: (result: { ok: true }) => void
|
||||
const pendingEnable = new Promise<{ ok: true }>((resolve) => {
|
||||
resolveEnable = resolve
|
||||
})
|
||||
const setGrabMode = vi.fn(async ({ enabled }: { enabled: boolean }) =>
|
||||
enabled ? pendingEnable : ({ ok: true } as const)
|
||||
)
|
||||
const awaitGrabSelection = vi.fn(() => new Promise(() => {}))
|
||||
const cancelGrab = vi.fn()
|
||||
vi.doMock('react', () => harness.react)
|
||||
vi.doMock('@/hooks/useMountedRef', () => ({
|
||||
useMountedRef: () => ({ current: true })
|
||||
}))
|
||||
vi.stubGlobal('window', {
|
||||
addEventListener: vi.fn(),
|
||||
removeEventListener: vi.fn(),
|
||||
api: {
|
||||
browser: { setGrabMode, awaitGrabSelection, cancelGrab }
|
||||
}
|
||||
})
|
||||
const { useGrabMode } = await import('./useGrabMode')
|
||||
const render = () => {
|
||||
harness.beginRender()
|
||||
// oxlint-disable-next-line react-hooks/rules-of-hooks -- test harness mocks React's hook dispatcher directly.
|
||||
return useGrabMode('page-1')
|
||||
}
|
||||
|
||||
const grab = render()
|
||||
grab.toggle()
|
||||
expect(render().state).toBe('armed')
|
||||
|
||||
grab.toggle()
|
||||
expect(setGrabMode.mock.calls.filter(([args]) => args.enabled)).toHaveLength(1)
|
||||
expect(render().state).toBe('idle')
|
||||
|
||||
resolveEnable({ ok: true })
|
||||
await pendingEnable
|
||||
await Promise.resolve()
|
||||
|
||||
expect(awaitGrabSelection).not.toHaveBeenCalled()
|
||||
await vi.waitFor(() => {
|
||||
expect(cancelGrab).toHaveBeenCalledWith({ browserPageId: 'page-1' })
|
||||
})
|
||||
})
|
||||
|
||||
it('cancels the tab owning the grab when the page changes before effects run', async () => {
|
||||
const harness = createReactHookHarness()
|
||||
const setGrabMode = vi.fn(async () => ({ ok: true }))
|
||||
const awaitGrabSelection = vi.fn(() => new Promise(() => {}))
|
||||
const cancelGrab = vi.fn()
|
||||
vi.doMock('react', () => harness.react)
|
||||
vi.doMock('@/hooks/useMountedRef', () => ({
|
||||
useMountedRef: () => ({ current: true })
|
||||
}))
|
||||
vi.stubGlobal('window', {
|
||||
addEventListener: vi.fn(),
|
||||
removeEventListener: vi.fn(),
|
||||
api: {
|
||||
browser: { setGrabMode, awaitGrabSelection, cancelGrab }
|
||||
}
|
||||
})
|
||||
const { useGrabMode } = await import('./useGrabMode')
|
||||
const render = (browserPageId: string) => {
|
||||
harness.beginRender()
|
||||
// oxlint-disable-next-line react-hooks/rules-of-hooks -- test harness mocks React's hook dispatcher directly.
|
||||
return useGrabMode(browserPageId)
|
||||
}
|
||||
|
||||
render('page-1').toggle()
|
||||
await vi.waitFor(() => {
|
||||
expect(awaitGrabSelection).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
render('page-2').cancel()
|
||||
|
||||
expect(setGrabMode).toHaveBeenLastCalledWith({
|
||||
browserPageId: 'page-1',
|
||||
enabled: false
|
||||
})
|
||||
expect(cancelGrab).toHaveBeenCalledWith({ browserPageId: 'page-1' })
|
||||
})
|
||||
|
||||
it('ignores a stale screenshot after restarting on the same page', async () => {
|
||||
const harness = createReactHookHarness()
|
||||
let resolveScreenshot!: (result: {
|
||||
ok: true
|
||||
screenshot: { dataUrl: string; width: number; height: number }
|
||||
}) => void
|
||||
const pendingScreenshot = new Promise<{
|
||||
ok: true
|
||||
screenshot: { dataUrl: string; width: number; height: number }
|
||||
}>((resolve) => {
|
||||
resolveScreenshot = resolve
|
||||
})
|
||||
const awaitGrabSelection = vi
|
||||
.fn()
|
||||
.mockResolvedValueOnce({
|
||||
kind: 'selected',
|
||||
payload: {
|
||||
target: {
|
||||
rectViewport: { x: 0, y: 0, width: 1, height: 1 }
|
||||
}
|
||||
}
|
||||
})
|
||||
.mockImplementation(() => new Promise(() => {}))
|
||||
const captureSelectionScreenshot = vi.fn(() => pendingScreenshot)
|
||||
vi.doMock('react', () => harness.react)
|
||||
vi.doMock('@/hooks/useMountedRef', () => ({
|
||||
useMountedRef: () => ({ current: true })
|
||||
}))
|
||||
vi.stubGlobal('window', {
|
||||
addEventListener: vi.fn(),
|
||||
removeEventListener: vi.fn(),
|
||||
api: {
|
||||
browser: {
|
||||
setGrabMode: vi.fn(async () => ({ ok: true })),
|
||||
awaitGrabSelection,
|
||||
captureSelectionScreenshot,
|
||||
cancelGrab: vi.fn()
|
||||
}
|
||||
}
|
||||
})
|
||||
const { useGrabMode } = await import('./useGrabMode')
|
||||
const render = () => {
|
||||
harness.beginRender()
|
||||
// oxlint-disable-next-line react-hooks/rules-of-hooks -- test harness mocks React's hook dispatcher directly.
|
||||
return useGrabMode('page-1')
|
||||
}
|
||||
|
||||
render().toggle()
|
||||
await vi.waitFor(() => {
|
||||
expect(captureSelectionScreenshot).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
render().cancel()
|
||||
render().toggle()
|
||||
await vi.waitFor(() => {
|
||||
expect(awaitGrabSelection).toHaveBeenCalledTimes(2)
|
||||
})
|
||||
|
||||
resolveScreenshot({
|
||||
ok: true,
|
||||
screenshot: { dataUrl: 'data:image/png;base64,AA==', width: 1, height: 1 }
|
||||
})
|
||||
await pendingScreenshot
|
||||
|
||||
await vi.waitFor(() => {
|
||||
const grab = render()
|
||||
expect(grab.state).toBe('awaiting')
|
||||
expect(grab.payload).toBeNull()
|
||||
})
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -1,9 +1,11 @@
|
|||
import { useCallback, useEffect, useRef, useState } from 'react'
|
||||
import type {
|
||||
BrowserGrabPayload,
|
||||
BrowserGrabRejectReason,
|
||||
BrowserGrabScreenshot
|
||||
} from '../../../../shared/browser-grab-types'
|
||||
import { useMountedRef } from '@/hooks/useMountedRef'
|
||||
import { translate } from '@/i18n/i18n'
|
||||
import { isEditableKeyboardTarget } from './browser-keyboard'
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
|
|
@ -32,6 +34,31 @@ function nextOpId(): string {
|
|||
return `grab-${++opIdCounter}-${Date.now()}`
|
||||
}
|
||||
|
||||
function getGrabEnableError(reason: BrowserGrabRejectReason): string {
|
||||
switch (reason) {
|
||||
case 'not-ready':
|
||||
return translate(
|
||||
'auto.components.browser-pane.grab.errorNotReady',
|
||||
"This page isn't ready for element selection yet."
|
||||
)
|
||||
case 'not-authorized':
|
||||
return translate(
|
||||
'auto.components.browser-pane.grab.errorNotAuthorized',
|
||||
'Browser access was denied.'
|
||||
)
|
||||
case 'already-active':
|
||||
return translate(
|
||||
'auto.components.browser-pane.grab.errorAlreadyActive',
|
||||
'Element selection is already active.'
|
||||
)
|
||||
case 'injection-failed':
|
||||
return translate(
|
||||
'auto.components.browser-pane.grab.errorInjectionFailed',
|
||||
'Could not start element selection on this page.'
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Hook that drives the browser grab lifecycle for a single browser page.
|
||||
*
|
||||
|
|
@ -45,6 +72,7 @@ export function useGrabMode(browserPageId: string): GrabModeHook {
|
|||
const [contextMenu, setContextMenu] = useState(false)
|
||||
const activeOpIdRef = useRef<string | null>(null)
|
||||
const grabTabIdRef = useRef<string | null>(null)
|
||||
const armGenerationRef = useRef(0)
|
||||
const browserTabIdRef = useRef(browserPageId)
|
||||
// Why: toolbar/key handlers from the latest render can fire before passive
|
||||
// effects run after a page switch, so keep the target page current in render.
|
||||
|
|
@ -58,6 +86,7 @@ export function useGrabMode(browserPageId: string): GrabModeHook {
|
|||
return () => {
|
||||
const grabTabId = grabTabIdRef.current
|
||||
if (grabTabId) {
|
||||
armGenerationRef.current += 1
|
||||
void window.api.browser.setGrabMode({ browserPageId: grabTabId, enabled: false })
|
||||
void window.api.browser.cancelGrab({ browserPageId: grabTabId })
|
||||
grabTabIdRef.current = null
|
||||
|
|
@ -68,7 +97,9 @@ export function useGrabMode(browserPageId: string): GrabModeHook {
|
|||
|
||||
const armAndAwait = useCallback(async () => {
|
||||
const tabId = browserTabIdRef.current
|
||||
const armGeneration = (armGenerationRef.current += 1)
|
||||
grabTabIdRef.current = tabId
|
||||
setState('armed')
|
||||
|
||||
// Enable grab mode — injects the overlay
|
||||
const setResult = await window.api.browser.setGrabMode({
|
||||
|
|
@ -77,25 +108,28 @@ export function useGrabMode(browserPageId: string): GrabModeHook {
|
|||
})
|
||||
if (
|
||||
!mountedRef.current ||
|
||||
armGenerationRef.current !== armGeneration ||
|
||||
browserTabIdRef.current !== tabId ||
|
||||
grabTabIdRef.current !== tabId
|
||||
) {
|
||||
void window.api.browser.setGrabMode({ browserPageId: tabId, enabled: false })
|
||||
void window.api.browser.cancelGrab({ browserPageId: tabId })
|
||||
if (grabTabIdRef.current === tabId) {
|
||||
grabTabIdRef.current = null
|
||||
const supersededBySameTab =
|
||||
armGenerationRef.current !== armGeneration && grabTabIdRef.current === tabId
|
||||
if (!supersededBySameTab) {
|
||||
void window.api.browser.setGrabMode({ browserPageId: tabId, enabled: false })
|
||||
void window.api.browser.cancelGrab({ browserPageId: tabId })
|
||||
if (grabTabIdRef.current === tabId) {
|
||||
grabTabIdRef.current = null
|
||||
}
|
||||
}
|
||||
return
|
||||
}
|
||||
if (!setResult.ok) {
|
||||
grabTabIdRef.current = null
|
||||
setState('error')
|
||||
setError(`Cannot enable grab mode: ${setResult.reason}`)
|
||||
setError(getGrabEnableError(setResult.reason))
|
||||
return
|
||||
}
|
||||
|
||||
setState('armed')
|
||||
|
||||
// Generate opId and await selection
|
||||
const opId = nextOpId()
|
||||
activeOpIdRef.current = opId
|
||||
|
|
@ -127,7 +161,11 @@ export function useGrabMode(browserPageId: string): GrabModeHook {
|
|||
} catch {
|
||||
// Screenshot failure is non-fatal
|
||||
}
|
||||
if (!mountedRef.current || grabTabIdRef.current !== tabId) {
|
||||
if (
|
||||
!mountedRef.current ||
|
||||
armGenerationRef.current !== armGeneration ||
|
||||
grabTabIdRef.current !== tabId
|
||||
) {
|
||||
return
|
||||
}
|
||||
|
||||
|
|
@ -146,20 +184,22 @@ export function useGrabMode(browserPageId: string): GrabModeHook {
|
|||
}, [mountedRef])
|
||||
|
||||
const toggle = useCallback(() => {
|
||||
if (state === 'idle' || state === 'error') {
|
||||
if ((state === 'idle' || state === 'error') && grabTabIdRef.current === null) {
|
||||
setError(null)
|
||||
setPayload(null)
|
||||
setContextMenu(false)
|
||||
void armAndAwait()
|
||||
} else {
|
||||
// Disable grab mode
|
||||
const targetTabId = grabTabIdRef.current ?? browserTabIdRef.current
|
||||
armGenerationRef.current += 1
|
||||
void window.api.browser.setGrabMode({
|
||||
browserPageId: browserTabIdRef.current,
|
||||
browserPageId: targetTabId,
|
||||
enabled: false
|
||||
})
|
||||
if (activeOpIdRef.current) {
|
||||
void window.api.browser.cancelGrab({
|
||||
browserPageId: browserTabIdRef.current
|
||||
browserPageId: targetTabId
|
||||
})
|
||||
activeOpIdRef.current = null
|
||||
}
|
||||
|
|
@ -172,13 +212,15 @@ export function useGrabMode(browserPageId: string): GrabModeHook {
|
|||
}, [state, armAndAwait])
|
||||
|
||||
const cancel = useCallback(() => {
|
||||
const targetTabId = grabTabIdRef.current ?? browserTabIdRef.current
|
||||
armGenerationRef.current += 1
|
||||
void window.api.browser.setGrabMode({
|
||||
browserPageId: browserTabIdRef.current,
|
||||
browserPageId: targetTabId,
|
||||
enabled: false
|
||||
})
|
||||
if (activeOpIdRef.current) {
|
||||
void window.api.browser.cancelGrab({
|
||||
browserPageId: browserTabIdRef.current
|
||||
browserPageId: targetTabId
|
||||
})
|
||||
activeOpIdRef.current = null
|
||||
}
|
||||
|
|
@ -204,8 +246,10 @@ export function useGrabMode(browserPageId: string): GrabModeHook {
|
|||
}, [armAndAwait])
|
||||
|
||||
const exit = useCallback(() => {
|
||||
const targetTabId = grabTabIdRef.current ?? browserTabIdRef.current
|
||||
armGenerationRef.current += 1
|
||||
void window.api.browser.setGrabMode({
|
||||
browserPageId: browserTabIdRef.current,
|
||||
browserPageId: targetTabId,
|
||||
enabled: false
|
||||
})
|
||||
// Why: clear the active opId so that any in-flight result from the
|
||||
|
|
|
|||
|
|
@ -14327,6 +14327,12 @@
|
|||
},
|
||||
"undo": "Undo",
|
||||
"widthOption": "{{value0}} px"
|
||||
},
|
||||
"grab": {
|
||||
"errorNotReady": "This page isn't ready for element selection yet.",
|
||||
"errorNotAuthorized": "Browser access was denied.",
|
||||
"errorAlreadyActive": "Element selection is already active.",
|
||||
"errorInjectionFailed": "Could not start element selection on this page."
|
||||
}
|
||||
},
|
||||
"pluginCatalog": {
|
||||
|
|
|
|||
|
|
@ -14267,6 +14267,12 @@
|
|||
},
|
||||
"undo": "Deshacer",
|
||||
"widthOption": "{{value0}} px"
|
||||
},
|
||||
"grab": {
|
||||
"errorNotReady": "Esta página aún no está lista para seleccionar elementos.",
|
||||
"errorNotAuthorized": "Se denegó el acceso al navegador.",
|
||||
"errorAlreadyActive": "La selección de elementos ya está activa.",
|
||||
"errorInjectionFailed": "No se pudo iniciar la selección de elementos en esta página."
|
||||
}
|
||||
},
|
||||
"pluginCatalog": {
|
||||
|
|
|
|||
|
|
@ -14267,6 +14267,12 @@
|
|||
},
|
||||
"undo": "元に戻す",
|
||||
"widthOption": "{{value0}} px"
|
||||
},
|
||||
"grab": {
|
||||
"errorNotReady": "このページではまだ要素を選択できません。",
|
||||
"errorNotAuthorized": "ブラウザーへのアクセスが拒否されました。",
|
||||
"errorAlreadyActive": "要素の選択はすでに有効です。",
|
||||
"errorInjectionFailed": "このページで要素の選択を開始できませんでした。"
|
||||
}
|
||||
},
|
||||
"pluginCatalog": {
|
||||
|
|
|
|||
|
|
@ -14267,6 +14267,12 @@
|
|||
},
|
||||
"undo": "실행 취소",
|
||||
"widthOption": "{{value0}} px"
|
||||
},
|
||||
"grab": {
|
||||
"errorNotReady": "이 페이지에서는 아직 요소를 선택할 수 없습니다.",
|
||||
"errorNotAuthorized": "브라우저 접근이 거부되었습니다.",
|
||||
"errorAlreadyActive": "요소 선택이 이미 활성화되어 있습니다.",
|
||||
"errorInjectionFailed": "이 페이지에서 요소 선택을 시작할 수 없습니다."
|
||||
}
|
||||
},
|
||||
"pluginCatalog": {
|
||||
|
|
|
|||
|
|
@ -14267,6 +14267,12 @@
|
|||
},
|
||||
"undo": "撤销",
|
||||
"widthOption": "{{value0}} px"
|
||||
},
|
||||
"grab": {
|
||||
"errorNotReady": "此页面尚未准备好进行元素选择。",
|
||||
"errorNotAuthorized": "浏览器访问被拒绝。",
|
||||
"errorAlreadyActive": "元素选择已处于活动状态。",
|
||||
"errorInjectionFailed": "无法在此页面上启动元素选择。"
|
||||
}
|
||||
},
|
||||
"pluginCatalog": {
|
||||
|
|
|
|||
|
|
@ -121,7 +121,11 @@ export type BrowserSetGrabModeArgs = {
|
|||
}
|
||||
|
||||
/** Why a grab IPC call was rejected before the operation could start. */
|
||||
export type BrowserGrabRejectReason = 'not-ready' | 'not-authorized' | 'already-active'
|
||||
export type BrowserGrabRejectReason =
|
||||
| 'not-ready'
|
||||
| 'not-authorized'
|
||||
| 'already-active'
|
||||
| 'injection-failed'
|
||||
|
||||
export type BrowserSetGrabModeResult = { ok: true } | { ok: false; reason: BrowserGrabRejectReason }
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue