perf: reduce browser page update churn (#2591)

This commit is contained in:
Neil 2026-05-21 21:06:38 -07:00 committed by GitHub
parent a0ca64b0f8
commit 0fa566e883
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 374 additions and 48 deletions

View File

@ -101,6 +101,7 @@ import { useGrabMode } from './useGrabMode'
import { formatGrabPayloadAsText } from './GrabConfirmationSheet'
import { formatBrowserAnnotationsAsMarkdown } from './browser-annotation-output'
import { isEditableKeyboardTarget } from './browser-keyboard'
import { getBrowserPagesForWorkspace } from './browser-pane-page-selection'
import BrowserAddressBar from './BrowserAddressBar'
import { BrowserToolbarMenu } from './BrowserToolbarMenu'
import BrowserFind from './BrowserFind'
@ -147,6 +148,7 @@ import {
onBrowserDriverChange,
type BrowserDriverState
} from '@/lib/pane-manager/browser-mobile-driver-state'
import { shouldPollChromiumErrorPage } from './chromium-error-page-polling'
type BrowserTabPageState = Partial<
Pick<
@ -242,7 +244,6 @@ type PendingRemoteBrowserWheel = {
dy: number
}
const EMPTY_BROWSER_PAGES: BrowserPageState[] = []
const EMPTY_BROWSER_ANNOTATIONS: BrowserPageAnnotation[] = []
const PENDING_ANNOTATION_CARD_HEIGHT = 330
const WHEEL_DELTA_LINE = 1
@ -776,8 +777,9 @@ export default function BrowserPane({
const activeRuntimeEnvironmentId = useAppStore(
(s) => s.settings?.activeRuntimeEnvironmentId ?? null
)
const browserPagesByWorkspace = useAppStore((s) => s.browserPagesByWorkspace)
const browserPages = browserPagesByWorkspace[browserTab.id] ?? EMPTY_BROWSER_PAGES
const browserPages = useAppStore((s) =>
getBrowserPagesForWorkspace(s.browserPagesByWorkspace, browserTab.id)
)
const activeBrowserPage =
browserPages.find((page) => page.id === browserTab.activePageId) ?? browserPages[0] ?? null
const updateBrowserPageState = useAppStore((s) => s.updateBrowserPageState)
@ -3663,7 +3665,7 @@ function BrowserPagePane({
}, [browserTab.url, focusWebviewNow])
useEffect(() => {
if (!browserTab.loading) {
if (!shouldPollChromiumErrorPage({ isActive, loading: browserTab.loading })) {
return
}
@ -3696,12 +3698,13 @@ function BrowserPagePane({
// Why: some Electron builds paint Chromium's internal chrome-error page
// without delivering a timely did-fail-load event to the renderer webview.
// Polling only while the tab is "loading" gives Orca a last-resort path to
// swap the black guest surface for the explicit unreachable-page overlay.
// Polling only while the active tab is "loading" gives Orca a last-resort
// path to swap the black guest surface without waking every retained
// inactive browser pane on a 250ms loop.
detectChromiumErrorPage()
const intervalId = window.setInterval(detectChromiumErrorPage, 250)
return () => window.clearInterval(intervalId)
}, [browserTab.id, browserTab.loading])
}, [browserTab.id, browserTab.loading, isActive])
const startGrabIntent = useCallback(
(nextIntent: GrabIntent): void => {

View File

@ -0,0 +1,40 @@
import { describe, expect, it } from 'vitest'
import { getBrowserPagesForWorkspace } from './browser-pane-page-selection'
import type { BrowserPage } from '../../../../shared/types'
function makeBrowserPage(id: string): BrowserPage {
return {
id,
workspaceId: 'workspace-a',
worktreeId: 'worktree-a',
url: `https://example.com/${id}`,
title: id,
loading: false,
faviconUrl: null,
canGoBack: false,
canGoForward: false,
loadError: null,
createdAt: 1
}
}
describe('getBrowserPagesForWorkspace', () => {
it('returns only the owning workspace page array so unrelated page updates keep the selector stable', () => {
const pages = [makeBrowserPage('page-1')]
const browserPagesByWorkspace = {
workspaceA: pages,
workspaceB: [makeBrowserPage('page-2')]
}
expect(getBrowserPagesForWorkspace(browserPagesByWorkspace, 'workspaceA')).toBe(pages)
expect(
getBrowserPagesForWorkspace(
{ ...browserPagesByWorkspace, workspaceB: [makeBrowserPage('page-3')] },
'workspaceA'
)
).toBe(pages)
expect(getBrowserPagesForWorkspace(browserPagesByWorkspace, 'missing')).toBe(
getBrowserPagesForWorkspace({}, 'missing')
)
})
})

View File

@ -0,0 +1,10 @@
import type { BrowserPage } from '../../../../shared/types'
const EMPTY_BROWSER_PAGES: BrowserPage[] = []
export function getBrowserPagesForWorkspace(
browserPagesByWorkspace: Record<string, BrowserPage[]>,
workspaceId: string
): BrowserPage[] {
return browserPagesByWorkspace[workspaceId] ?? EMPTY_BROWSER_PAGES
}

View File

@ -0,0 +1,11 @@
import { describe, expect, it } from 'vitest'
import { shouldPollChromiumErrorPage } from './chromium-error-page-polling'
describe('shouldPollChromiumErrorPage', () => {
it('runs the fallback chrome-error poll only for the active loading browser pane', () => {
expect(shouldPollChromiumErrorPage({ isActive: true, loading: true })).toBe(true)
expect(shouldPollChromiumErrorPage({ isActive: false, loading: true })).toBe(false)
expect(shouldPollChromiumErrorPage({ isActive: true, loading: false })).toBe(false)
expect(shouldPollChromiumErrorPage({ isActive: false, loading: false })).toBe(false)
})
})

View File

@ -0,0 +1,6 @@
export function shouldPollChromiumErrorPage(args: {
isActive: boolean
loading: boolean
}): boolean {
return args.isActive && args.loading
}

View File

@ -58,6 +58,31 @@ function settingsWithRuntime(id: string): AppState['settings'] {
return { activeRuntimeEnvironmentId: id } as AppState['settings']
}
function seedUnifiedBrowserTab(
store: ReturnType<typeof createTestStore>,
entityId: string,
label: string
): void {
store.setState({
unifiedTabsByWorktree: {
'wt-1': [
{
id: 'unified-browser-tab',
entityId,
groupId: 'group-1',
worktreeId: 'wt-1',
contentType: 'browser',
label,
customLabel: null,
color: null,
sortOrder: 0,
createdAt: 1
}
]
}
})
}
function makeAnnotation(pageId: string, id = 'annotation-1'): BrowserPageAnnotation {
return {
id,
@ -134,6 +159,155 @@ describe('createBrowserSlice annotations', () => {
expect(store.getState().browserAnnotationsByPageId[pageId]).toBeUndefined()
})
it('preserves browser map references when a page-state update is unchanged', () => {
const store = createTestStore()
const tab = store.getState().createBrowserTab('wt-1', 'https://example.com', {
title: 'Example'
})
const pageId = tab.activePageId
if (!pageId) {
throw new Error('Expected a new browser page')
}
const page = store.getState().browserPagesByWorkspace[tab.id]?.[0]
if (!page) {
throw new Error('Expected page state')
}
const browserPagesByWorkspace = store.getState().browserPagesByWorkspace
const browserTabsByWorktree = store.getState().browserTabsByWorktree
store.getState().updateBrowserPageState(pageId, {
title: page.title,
loading: page.loading,
faviconUrl: page.faviconUrl,
canGoBack: page.canGoBack,
canGoForward: page.canGoForward,
loadError: page.loadError
})
expect(store.getState().browserPagesByWorkspace).toBe(browserPagesByWorkspace)
expect(store.getState().browserTabsByWorktree).toBe(browserTabsByWorktree)
})
it('repairs a stale active browser unified-tab label on an otherwise unchanged title update', () => {
const store = createTestStore()
const tab = store.getState().createBrowserTab('wt-1', 'https://example.com', {
title: 'Example'
})
const pageId = tab.activePageId
if (!pageId) {
throw new Error('Expected a new browser page')
}
seedUnifiedBrowserTab(store, tab.id, 'Stale label')
const browserPagesByWorkspace = store.getState().browserPagesByWorkspace
const browserTabsByWorktree = store.getState().browserTabsByWorktree
store.getState().updateBrowserPageState(pageId, { title: 'Example' })
expect(store.getState().unifiedTabsByWorktree['wt-1']?.[0]?.label).toBe('Example')
expect(store.getState().browserPagesByWorkspace).toBe(browserPagesByWorkspace)
expect(store.getState().browserTabsByWorktree).toBe(browserTabsByWorktree)
})
it('repairs stale active browser workspace metadata on an otherwise unchanged page update', () => {
const store = createTestStore()
const tab = store.getState().createBrowserTab('wt-1', 'https://example.com', {
title: 'Example'
})
const pageId = tab.activePageId
if (!pageId) {
throw new Error('Expected a new browser page')
}
store.setState((state) => ({
browserTabsByWorktree: {
...state.browserTabsByWorktree,
'wt-1': (state.browserTabsByWorktree['wt-1'] ?? []).map((workspace) =>
workspace.id === tab.id
? {
...workspace,
title: 'Stale workspace',
url: 'https://stale.example.com',
loading: false,
canGoBack: true,
canGoForward: true
}
: workspace
)
}
}))
const browserPagesByWorkspace = store.getState().browserPagesByWorkspace
store.getState().updateBrowserPageState(pageId, { title: 'Example' })
const repaired = store
.getState()
.browserTabsByWorktree['wt-1']?.find((entry) => entry.id === tab.id)
expect(repaired).toMatchObject({
title: 'Example',
url: 'https://example.com',
loading: true,
canGoBack: false,
canGoForward: false
})
expect(store.getState().browserPagesByWorkspace).toBe(browserPagesByWorkspace)
})
it('updates the active browser unified-tab label without a second tab-label write', () => {
const store = createTestStore()
const tab = store.getState().createBrowserTab('wt-1', 'https://example.com', {
title: 'Example'
})
const pageId = tab.activePageId
if (!pageId) {
throw new Error('Expected a new browser page')
}
seedUnifiedBrowserTab(store, tab.id, 'Example')
store.getState().updateBrowserPageState(pageId, { title: 'Next', loading: false })
expect(store.getState().unifiedTabsByWorktree['wt-1']?.[0]?.label).toBe('Next')
expect(store.getState().setTabLabel).not.toHaveBeenCalled()
})
it('updates inactive browser pages without relabeling or rebuilding the workspace map', () => {
const store = createTestStore()
const tab = store.getState().createBrowserTab('wt-1', 'https://example.com', {
title: 'Example'
})
const activePageId = tab.activePageId
if (!activePageId) {
throw new Error('Expected a new browser page')
}
const inactivePage = store
.getState()
.createBrowserPage(tab.id, 'https://example.com/inactive', {
title: 'Inactive',
activate: false
})
if (!inactivePage) {
throw new Error('Expected inactive browser page')
}
seedUnifiedBrowserTab(store, tab.id, 'Example')
const browserPagesByWorkspace = store.getState().browserPagesByWorkspace
const browserTabsByWorktree = store.getState().browserTabsByWorktree
store.getState().updateBrowserPageState(inactivePage.id, {
title: 'Inactive next',
loading: false
})
expect(store.getState().browserPagesByWorkspace).not.toBe(browserPagesByWorkspace)
expect(store.getState().browserTabsByWorktree).toBe(browserTabsByWorktree)
expect(
store.getState().browserPagesByWorkspace[tab.id]?.find((page) => page.id === inactivePage.id)
).toMatchObject({ title: 'Inactive next', loading: false })
expect(store.getState().browserTabsByWorktree['wt-1']?.[0]).toMatchObject({
activePageId,
title: 'Example'
})
expect(store.getState().unifiedTabsByWorktree['wt-1']?.[0]?.label).toBe('Example')
expect(store.getState().setTabLabel).not.toHaveBeenCalled()
})
it('caps stored browser annotations per page', () => {
const store = createTestStore()
const tab = store.getState().createBrowserTab('wt-1', 'https://example.com')

View File

@ -317,6 +317,26 @@ function mirrorWorkspaceFromActivePage(
}
}
function browserWorkspaceMirrorFieldsEqual(
workspace: BrowserWorkspace,
mirrored: BrowserWorkspace
): boolean {
const workspacePageIds = workspace.pageIds ?? []
const mirroredPageIds = mirrored.pageIds ?? []
return (
workspace.activePageId === mirrored.activePageId &&
workspacePageIds.length === mirroredPageIds.length &&
workspacePageIds.every((pageId, index) => pageId === mirroredPageIds[index]) &&
workspace.url === mirrored.url &&
workspace.title === mirrored.title &&
workspace.loading === mirrored.loading &&
workspace.faviconUrl === mirrored.faviconUrl &&
workspace.canGoBack === mirrored.canGoBack &&
workspace.canGoForward === mirrored.canGoForward &&
workspace.loadError === mirrored.loadError
)
}
function getFallbackTabTypeForWorktree(
worktreeId: string,
openFiles: AppState['openFiles'],
@ -335,26 +355,46 @@ function getFallbackTabTypeForWorktree(
return 'terminal'
}
const browserWorkspaceByIdCache = new WeakMap<
Record<string, BrowserWorkspace[]>,
Map<string, BrowserWorkspace>
>()
const browserPageByIdCache = new WeakMap<Record<string, BrowserPage[]>, Map<string, BrowserPage>>()
function findWorkspace(
browserTabsByWorktree: Record<string, BrowserWorkspace[]>,
workspaceId: string
): BrowserWorkspace | null {
return (
Object.values(browserTabsByWorktree)
.flat()
.find((workspace) => workspace.id === workspaceId) ?? null
)
const cached = browserWorkspaceByIdCache.get(browserTabsByWorktree)
if (cached) {
return cached.get(workspaceId) ?? null
}
const workspaceById = new Map<string, BrowserWorkspace>()
for (const workspaces of Object.values(browserTabsByWorktree)) {
for (const workspace of workspaces) {
workspaceById.set(workspace.id, workspace)
}
}
browserWorkspaceByIdCache.set(browserTabsByWorktree, workspaceById)
return workspaceById.get(workspaceId) ?? null
}
function findPage(
browserPagesByWorkspace: Record<string, BrowserPage[]>,
pageId: string
): BrowserPage | null {
return (
Object.values(browserPagesByWorkspace)
.flat()
.find((page) => page.id === pageId) ?? null
)
const cached = browserPageByIdCache.get(browserPagesByWorkspace)
if (cached) {
return cached.get(pageId) ?? null
}
const pageById = new Map<string, BrowserPage>()
for (const pages of Object.values(browserPagesByWorkspace)) {
for (const page of pages) {
pageById.set(page.id, page)
}
}
browserPageByIdCache.set(browserPagesByWorkspace, pageById)
return pageById.get(pageId) ?? null
}
export const createBrowserSlice: StateCreator<AppState, [], [], BrowserSlice> = (set, get) => ({
@ -1094,48 +1134,90 @@ export const createBrowserSlice: StateCreator<AppState, [], [], BrowserSlice> =
if (!workspace) {
return s
}
const nextPages = (s.browserPagesByWorkspace[workspace.id] ?? []).map((entry) =>
entry.id === pageId
? {
...entry,
title:
updates.title === undefined
? entry.title
: normalizeBrowserTitle(updates.title, entry.url),
loading: updates.loading ?? entry.loading,
faviconUrl: updates.faviconUrl === undefined ? entry.faviconUrl : updates.faviconUrl,
canGoBack: updates.canGoBack ?? entry.canGoBack,
canGoForward: updates.canGoForward ?? entry.canGoForward,
loadError: updates.loadError === undefined ? entry.loadError : updates.loadError
}
: entry
)
const nextPage = {
...page,
title:
updates.title === undefined ? page.title : normalizeBrowserTitle(updates.title, page.url),
loading: updates.loading ?? page.loading,
faviconUrl: updates.faviconUrl === undefined ? page.faviconUrl : updates.faviconUrl,
canGoBack: updates.canGoBack ?? page.canGoBack,
canGoForward: updates.canGoForward ?? page.canGoForward,
loadError: updates.loadError === undefined ? page.loadError : updates.loadError
}
const unifiedTabs = s.unifiedTabsByWorktree[workspace.worktreeId] ?? []
const unifiedIndex =
workspace.activePageId === pageId && updates.title !== undefined
? unifiedTabs.findIndex(
(entry) => entry.contentType === 'browser' && entry.entityId === workspace.id
)
: -1
const unifiedLabelNeedsRepair =
unifiedIndex !== -1 && unifiedTabs[unifiedIndex]?.label !== nextPage.title
const pageStateUnchanged =
nextPage.title === page.title &&
nextPage.loading === page.loading &&
nextPage.faviconUrl === page.faviconUrl &&
nextPage.canGoBack === page.canGoBack &&
nextPage.canGoForward === page.canGoForward &&
nextPage.loadError === page.loadError
const currentPages = s.browserPagesByWorkspace[workspace.id] ?? []
const mirroredWorkspace = pageStateUnchanged
? mirrorWorkspaceFromActivePage(workspace, currentPages)
: null
const workspaceNeedsRepair =
mirroredWorkspace !== null &&
!browserWorkspaceMirrorFieldsEqual(workspace, mirroredWorkspace)
if (pageStateUnchanged && !unifiedLabelNeedsRepair && !workspaceNeedsRepair) {
return s
}
if (pageStateUnchanged) {
const nextState: Partial<AppState> = {}
if (workspaceNeedsRepair && mirroredWorkspace) {
nextState.browserTabsByWorktree = {
...s.browserTabsByWorktree,
[workspace.worktreeId]: (s.browserTabsByWorktree[workspace.worktreeId] ?? []).map(
(tab) => (tab.id === workspace.id ? mirroredWorkspace : tab)
)
}
}
if (unifiedLabelNeedsRepair) {
nextState.unifiedTabsByWorktree = {
...s.unifiedTabsByWorktree,
[workspace.worktreeId]: unifiedTabs.map((entry, index) =>
index === unifiedIndex ? { ...entry, label: nextPage.title } : entry
)
}
}
return nextState
}
const nextPages = currentPages.map((entry) => (entry.id === pageId ? nextPage : entry))
const nextWorkspace = mirrorWorkspaceFromActivePage(workspace, nextPages)
return {
const nextState: Partial<AppState> = {
browserPagesByWorkspace: {
...s.browserPagesByWorkspace,
[workspace.id]: nextPages
},
browserTabsByWorktree: {
}
}
if (!browserWorkspaceMirrorFieldsEqual(workspace, nextWorkspace)) {
nextState.browserTabsByWorktree = {
...s.browserTabsByWorktree,
[workspace.worktreeId]: (s.browserTabsByWorktree[workspace.worktreeId] ?? []).map((tab) =>
tab.id === workspace.id ? nextWorkspace : tab
)
}
}
if (workspace.activePageId === pageId && updates.title !== undefined && unifiedIndex !== -1) {
if (unifiedLabelNeedsRepair || unifiedTabs[unifiedIndex]?.label !== nextWorkspace.title) {
nextState.unifiedTabsByWorktree = {
...s.unifiedTabsByWorktree,
[workspace.worktreeId]: unifiedTabs.map((entry, index) =>
index === unifiedIndex ? { ...entry, label: nextWorkspace.title } : entry
)
}
}
}
return nextState
})
const page = findPage(get().browserPagesByWorkspace, pageId)
if (!page) {
return
}
const workspace = findWorkspace(get().browserTabsByWorktree, page.workspaceId)
const item = Object.values(get().unifiedTabsByWorktree)
.flat()
.find((entry) => entry.contentType === 'browser' && entry.entityId === page.workspaceId)
if (item && workspace && workspace.activePageId === pageId && updates.title) {
get().setTabLabel(item.id, workspace.title)
}
},
setBrowserTabUrl: (pageId, url) => get().setBrowserPageUrl(pageId, url),