mirror of
https://github.com/stablyai/orca.git
synced 2026-09-22 00:02:31 +00:00
Avoid synchronous webview URL IPC during browser pane render (#13646)
* perf(browser): avoid synchronous URL IPC during render * fix(browser): sync live URL after CDP navigation
This commit is contained in:
@@ -0,0 +1,19 @@
|
||||
import { readFileSync } from 'node:fs'
|
||||
import { fileURLToPath } from 'node:url'
|
||||
import { describe, expect, it } from 'vitest'
|
||||
|
||||
describe('BrowserPagePane render IPC boundary', () => {
|
||||
it('derives toolbar URLs without querying the webview', () => {
|
||||
const source = readFileSync(
|
||||
fileURLToPath(new URL('./BrowserPane.tsx', import.meta.url)),
|
||||
'utf8'
|
||||
)
|
||||
const start = source.indexOf('const isBlankTab =', source.indexOf('function BrowserPagePane'))
|
||||
const end = source.indexOf('useEffect(() => {', start)
|
||||
const renderUrlDerivation = source.slice(start, end)
|
||||
|
||||
expect(renderUrlDerivation).toContain('getLiveBrowserUrl(browserTab.id)')
|
||||
expect(renderUrlDerivation).not.toContain('webviewRef')
|
||||
expect(renderUrlDerivation).not.toContain('.getURL(')
|
||||
})
|
||||
})
|
||||
@@ -82,7 +82,7 @@ import {
|
||||
browserViewportPresetToOverride,
|
||||
getBrowserViewportPreset
|
||||
} from '../../../../shared/browser-viewport-presets'
|
||||
import { rememberLiveBrowserUrl } from './browser-runtime'
|
||||
import { getLiveBrowserUrl, rememberLiveBrowserUrl, seedLiveBrowserUrl } from './browser-runtime'
|
||||
import { ensureBrowserPageWebview } from './browser-page-webview'
|
||||
import { RemoteBrowserStreamLifecycle } from './remote-browser-stream-lifecycle'
|
||||
import { isRemoteBrowserPageMissingError } from './remote-browser-stream-errors'
|
||||
@@ -688,35 +688,10 @@ function getRemoteBrowserDeviceScaleFactor(): number {
|
||||
return Math.min(2, Math.max(1, Number(scale.toFixed(2))))
|
||||
}
|
||||
|
||||
function getOpenableExternalUrl(
|
||||
webview: Electron.WebviewTag | null,
|
||||
fallbackUrl: string
|
||||
): string | null {
|
||||
let currentUrl = fallbackUrl
|
||||
if (webview) {
|
||||
try {
|
||||
currentUrl = webview.getURL() || fallbackUrl
|
||||
} catch {
|
||||
// Why: querying nav state before dom-ready throws and blanks the whole IDE on launch; fall back to the persisted URL.
|
||||
currentUrl = fallbackUrl
|
||||
}
|
||||
}
|
||||
function getOpenableExternalUrl(currentUrl: string): string | null {
|
||||
return normalizeExternalBrowserUrl(redactKagiSessionToken(currentUrl))
|
||||
}
|
||||
|
||||
function getCurrentBrowserUrl(webview: Electron.WebviewTag | null, fallbackUrl: string): string {
|
||||
let currentUrl = fallbackUrl
|
||||
if (webview) {
|
||||
try {
|
||||
currentUrl = webview.getURL() || fallbackUrl
|
||||
} catch {
|
||||
// Why: toolbar actions need a stable URL during early guest attach/restore; fall back to the persisted URL instead of throwing.
|
||||
currentUrl = fallbackUrl
|
||||
}
|
||||
}
|
||||
return toDisplayUrl(currentUrl)
|
||||
}
|
||||
|
||||
function retryBrowserTabLoad(
|
||||
webview: Electron.WebviewTag | null,
|
||||
browserTab: BrowserPageState,
|
||||
@@ -3430,6 +3405,7 @@ function BrowserPagePane({
|
||||
container = ensuredWebview.container
|
||||
const webview = ensuredWebview.webview
|
||||
const needsInitialNavigation = ensuredWebview.created
|
||||
seedLiveBrowserUrl(browserTab.id, redactKagiSessionToken(browserTabUrlRef.current))
|
||||
|
||||
if (!ensuredWebview.created) {
|
||||
// pointerEvents already applied inside ensureBrowserPageWebview for the reused-webview path.
|
||||
@@ -4556,8 +4532,10 @@ function BrowserPagePane({
|
||||
|
||||
// Why: a blank tab reads as 'about:blank' or the resolved data: URL, so match both to keep the "New Browser Tab" overlay visible.
|
||||
const isBlankTab = browserTab.url === 'about:blank' || browserTab.url === ORCA_BROWSER_BLANK_URL
|
||||
const externalUrl = getOpenableExternalUrl(webviewRef.current, browserTab.url)
|
||||
const currentBrowserUrl = getCurrentBrowserUrl(webviewRef.current, browserTab.url)
|
||||
// Why: synchronous webview URL access blocks render; navigation handlers update this cache before their store writes can re-render the pane.
|
||||
const liveBrowserUrl = getLiveBrowserUrl(browserTab.id) ?? browserTab.url
|
||||
const externalUrl = getOpenableExternalUrl(liveBrowserUrl)
|
||||
const currentBrowserUrl = toDisplayUrl(liveBrowserUrl)
|
||||
const shareableArtifactFile =
|
||||
workspaceConnectionId === null ? getShareableBrowserArtifactFile(currentBrowserUrl) : null
|
||||
const failedNavigationUrl = browserTab.loadError?.validatedUrl ?? currentBrowserUrl
|
||||
|
||||
@@ -1,9 +1,15 @@
|
||||
import { beforeEach, describe, expect, it } from 'vitest'
|
||||
import { clearLiveBrowserUrl, getLiveBrowserUrl, rememberLiveBrowserUrl } from './browser-runtime'
|
||||
import { afterEach, describe, expect, it } from 'vitest'
|
||||
import {
|
||||
clearLiveBrowserUrl,
|
||||
getLiveBrowserUrl,
|
||||
rememberLiveBrowserUrl,
|
||||
seedLiveBrowserUrl
|
||||
} from './browser-runtime'
|
||||
|
||||
describe('browser runtime live URL cache', () => {
|
||||
beforeEach(() => {
|
||||
afterEach(() => {
|
||||
clearLiveBrowserUrl('page-1')
|
||||
clearLiveBrowserUrl('popup-1')
|
||||
})
|
||||
|
||||
it('remembers and clears the last live URL for a browser page', () => {
|
||||
@@ -15,4 +21,30 @@ describe('browser runtime live URL cache', () => {
|
||||
|
||||
expect(getLiveBrowserUrl('page-1')).toBeNull()
|
||||
})
|
||||
|
||||
it('seeds an initial URL without replacing a committed navigation', () => {
|
||||
seedLiveBrowserUrl('page-1', 'https://initial.example/')
|
||||
rememberLiveBrowserUrl('page-1', 'https://committed.example/')
|
||||
seedLiveBrowserUrl('page-1', 'https://stale-persisted.example/')
|
||||
|
||||
expect(getLiveBrowserUrl('page-1')).toBe('https://committed.example/')
|
||||
})
|
||||
|
||||
it('retains the last committed URL when a later load fails', () => {
|
||||
seedLiveBrowserUrl('page-1', 'https://initial.example/')
|
||||
rememberLiveBrowserUrl('page-1', 'https://committed.example/')
|
||||
// A failure event records its validated URL in loadError, not the live URL cache.
|
||||
|
||||
expect(getLiveBrowserUrl('page-1')).toBe('https://committed.example/')
|
||||
})
|
||||
|
||||
it('keeps popup page URLs independent and clears only the destroyed page', () => {
|
||||
seedLiveBrowserUrl('page-1', 'https://opener.example/')
|
||||
seedLiveBrowserUrl('popup-1', 'https://popup.example/')
|
||||
|
||||
clearLiveBrowserUrl('popup-1')
|
||||
|
||||
expect(getLiveBrowserUrl('page-1')).toBe('https://opener.example/')
|
||||
expect(getLiveBrowserUrl('popup-1')).toBeNull()
|
||||
})
|
||||
})
|
||||
|
||||
@@ -1,5 +1,11 @@
|
||||
const liveBrowserUrlByTabId = new Map<string, string>()
|
||||
|
||||
export function seedLiveBrowserUrl(browserTabId: string, initialUrl: string): void {
|
||||
if (!liveBrowserUrlByTabId.has(browserTabId)) {
|
||||
liveBrowserUrlByTabId.set(browserTabId, initialUrl)
|
||||
}
|
||||
}
|
||||
|
||||
export function rememberLiveBrowserUrl(browserTabId: string, url: string): void {
|
||||
liveBrowserUrlByTabId.set(browserTabId, url)
|
||||
}
|
||||
|
||||
@@ -125,6 +125,7 @@ export type IpcEventsHarness = {
|
||||
/** Fire a main-process digit chord (zero-based index). */
|
||||
jumpToWorktreeIndex: (index: number) => void
|
||||
jumpToTabIndex: (index: number) => void
|
||||
navigationUpdate: (event: { browserPageId: string; url: string; title: string }) => void
|
||||
/** Standard (non-palette) target of a workspace digit chord. */
|
||||
activateAndRevealWorkspace: ReturnType<typeof vi.fn>
|
||||
}
|
||||
@@ -146,6 +147,9 @@ export async function loadIpcEventsHarness(
|
||||
const activateAndRevealWorkspace = vi.fn()
|
||||
let createTerminalListener: ((request: CreateTerminalRequest) => void) | null = null
|
||||
let requestTerminalCreateListener: ((request: RequestTerminalCreateRequest) => void) | null = null
|
||||
let navigationUpdateListener:
|
||||
| ((event: { browserPageId: string; url: string; title: string }) => void)
|
||||
| null = null
|
||||
const indexJumpListeners = new Map<string, (index: number) => void>()
|
||||
|
||||
vi.resetModules()
|
||||
@@ -243,6 +247,14 @@ export async function loadIpcEventsHarness(
|
||||
onStatus: () => () => {},
|
||||
onClearDismissal: () => () => {}
|
||||
},
|
||||
browser: createApiNamespaceStub({
|
||||
onNavigationUpdate: (
|
||||
listener: (event: { browserPageId: string; url: string; title: string }) => void
|
||||
) => {
|
||||
navigationUpdateListener = listener
|
||||
return () => {}
|
||||
}
|
||||
}),
|
||||
mobile: createApiNamespaceStub({
|
||||
consumePendingUnpairedDeviceAuthFailure: () => Promise.resolve(false)
|
||||
}),
|
||||
@@ -270,6 +282,12 @@ export async function loadIpcEventsHarness(
|
||||
replyTerminalCreate,
|
||||
jumpToWorktreeIndex: (index) => fireIndexJump(indexJumpListeners, 'worktree', index),
|
||||
jumpToTabIndex: (index) => fireIndexJump(indexJumpListeners, 'tab', index),
|
||||
navigationUpdate: (event) => {
|
||||
if (typeof navigationUpdateListener !== 'function') {
|
||||
throw new Error('Expected the browser navigation listener to be registered')
|
||||
}
|
||||
navigationUpdateListener(event)
|
||||
},
|
||||
activateAndRevealWorkspace
|
||||
}
|
||||
}
|
||||
|
||||
@@ -108,6 +108,45 @@ describe('getRuntimeProjectRefreshEnvironmentIds', () => {
|
||||
})
|
||||
})
|
||||
|
||||
describe('browser navigation updates', () => {
|
||||
it('commits CDP navigation URLs to the render-time cache before updating the store', async () => {
|
||||
let liveUrlDuringStoreWrite: string | null = null
|
||||
let readLiveUrl = (_browserPageId: string): string | null => null
|
||||
const setBrowserPageUrl = vi.fn((browserPageId: string) => {
|
||||
liveUrlDuringStoreWrite = readLiveUrl(browserPageId)
|
||||
})
|
||||
const updateBrowserPageState = vi.fn()
|
||||
const storeState = createHarnessStoreState({
|
||||
tabsByWorktree: {},
|
||||
setBrowserPageUrl,
|
||||
updateBrowserPageState
|
||||
})
|
||||
const harness = await loadIpcEventsHarness(storeState)
|
||||
const { clearLiveBrowserUrl, getLiveBrowserUrl } =
|
||||
await import('@/components/browser-pane/browser-runtime')
|
||||
readLiveUrl = getLiveBrowserUrl
|
||||
harness.useIpcEvents()
|
||||
|
||||
harness.navigationUpdate({
|
||||
browserPageId: 'page-1',
|
||||
url: 'https://kagi.com/search?token=secret&q=next',
|
||||
title: 'Next'
|
||||
})
|
||||
|
||||
expect(liveUrlDuringStoreWrite).toBe('https://kagi.com/search?q=next')
|
||||
expect(getLiveBrowserUrl('page-1')).toBe('https://kagi.com/search?q=next')
|
||||
expect(setBrowserPageUrl).toHaveBeenCalledWith(
|
||||
'page-1',
|
||||
'https://kagi.com/search?token=secret&q=next'
|
||||
)
|
||||
expect(updateBrowserPageState).toHaveBeenCalledWith('page-1', {
|
||||
title: 'Next',
|
||||
loading: false
|
||||
})
|
||||
clearLiveBrowserUrl('page-1')
|
||||
})
|
||||
})
|
||||
|
||||
function expectWorktreeRouting(worktreeId: string): unknown {
|
||||
return expect.objectContaining({ worktreeId })
|
||||
}
|
||||
|
||||
@@ -86,6 +86,7 @@ import {
|
||||
setDriverForBrowserPage
|
||||
} from '@/lib/pane-manager/browser-mobile-driver-state'
|
||||
import { destroyPersistentWebview } from '@/components/browser-pane/webview-registry'
|
||||
import { rememberLiveBrowserUrl } from '@/components/browser-pane/browser-runtime'
|
||||
import {
|
||||
acquireBrowserAutomationVisibility,
|
||||
releaseBrowserAutomationVisibility
|
||||
@@ -156,6 +157,7 @@ import { getRuntimeEnvironmentIdForWorktree } from '@/lib/worktree-runtime-owner
|
||||
import { resolveTerminalWorktreeRoute } from '@/lib/terminal-worktree-route'
|
||||
import { resolveAgentPaneAuthorityKey } from '@/store/slices/agent-pane-authority'
|
||||
import { translate } from '@/i18n/i18n'
|
||||
import { redactKagiSessionToken } from '../../../shared/browser-url'
|
||||
import { closeTerminalTab } from '@/components/terminal/terminal-tab-actions'
|
||||
import { initialAgentTabViewModeProps } from '@/lib/native-chat-initial-view-mode'
|
||||
import { getConnectionIdFromState } from '@/lib/connection-context'
|
||||
@@ -2086,6 +2088,7 @@ export function useIpcEvents(): void {
|
||||
return
|
||||
}
|
||||
const store = useAppStore.getState()
|
||||
rememberLiveBrowserUrl(browserPageId, redactKagiSessionToken(url))
|
||||
store.setBrowserPageUrl(browserPageId, url)
|
||||
store.updateBrowserPageState(browserPageId, { title, loading: false })
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user