perf: index closed browser page IDs during focus cleanup (#19449)

* perf: index closed browser page IDs during focus cleanup

* perf(browser): omit closed page focus keys instead of rebuilding the records

---------

Co-authored-by: m4air <m4air@m4airs-MacBook-Air.local>
Co-authored-by: Neil <4138956+nwparker@users.noreply.github.com>
This commit is contained in:
OrcaWin
2026-09-08 19:42:32 -07:00
committed by GitHub
co-authored by m4air Neil
parent f242c99af4
commit 60456107e6
2 changed files with 82 additions and 9 deletions
@@ -55,6 +55,24 @@ function storeWithOnlyBrowserTab(): {
return { store, workspaceId: workspace.id, pageId }
}
/** Three browser tabs in a known tab-bar order, so neighbor selection is deterministic. */
function storeWithThreeBrowserTabs(): {
store: ReturnType<typeof createTestStore>
ids: string[]
} {
const store = createTestStore()
seedStore(store, {
worktreesByRepo: { repo1: [makeWorktree({ id: WT, repoId: 'repo1', path: '/path/wt1' })] },
activeWorktreeId: WT,
activeTabType: 'terminal'
})
const ids = [0, 1, 2].map(
(index) =>
store.getState().createBrowserTab(WT, `https://example.test/${index}`, { activate: true }).id
)
return { store, ids }
}
describe('closeBrowserTab with reason cleanup', () => {
beforeEach(() => {
vi.clearAllMocks()
@@ -151,4 +169,58 @@ describe('closeBrowserTab with reason cleanup', () => {
expect(recordFeatureInteraction).toHaveBeenCalledWith('terminal-tabs')
})
it('clears focus for closed pages without rescanning pages for unrelated focus entries', () => {
const { store, workspaceId } = storeWithOnlyBrowserTab()
const original = store.getState().browserPagesByWorkspace[workspaceId][0]
let idReads = 0
const pages = Array.from({ length: 1000 }, (_, index) => ({
...original,
get id() {
idReads++
return `closed-${index}`
}
}))
const unrelated = Object.fromEntries(
Array.from({ length: 1000 }, (_, i) => [`other-${i}`, true as const])
)
store.setState({
browserPagesByWorkspace: { [workspaceId]: pages },
pendingAddressBarFocusByPageId: { ...unrelated, 'closed-999': true, [workspaceId]: true },
pendingAddressBarFocusByTabId: { ...unrelated, 'closed-999': true, [workspaceId]: true }
})
idReads = 0
store.getState().closeBrowserTab(workspaceId, { reason: 'cleanup' })
expect(store.getState().pendingAddressBarFocusByPageId).toEqual({
...unrelated,
[workspaceId]: true
})
expect(store.getState().pendingAddressBarFocusByTabId).toEqual(unrelated)
expect(idReads).toBeLessThan(10_000)
})
// Why: the omit path must not hand every pending-focus selector a fresh record on each close.
it('keeps the pending focus records identical when the closed tab had no pending entry', () => {
const { store, workspaceId } = storeWithOnlyBrowserTab()
const pending = { 'other-tab': true as const }
store.setState({
pendingAddressBarFocusByPageId: pending,
pendingAddressBarFocusByTabId: pending
})
store.getState().closeBrowserTab(workspaceId, { reason: 'cleanup' })
expect(store.getState().pendingAddressBarFocusByPageId).toBe(pending)
expect(store.getState().pendingAddressBarFocusByTabId).toBe(pending)
})
// Why: closing a tab the user is not looking at must never move focus, whatever the
// pending-address-bar bookkeeping does around it.
it('leaves the active browser tab alone when a background tab closes', () => {
const { store, ids } = storeWithThreeBrowserTabs()
store.getState().setActiveBrowserTab(ids[0]!)
store.getState().closeBrowserTab(ids[2]!)
expect(store.getState().activeBrowserTabIdByWorktree[WT]).toBe(ids[0]!)
})
})
@@ -138,16 +138,17 @@ export function createBrowserCloseActions(
}
delete nextRecentlyClosedBrowserPagesByWorkspace[tabId]
const nextPendingAddressBarFocusByPageId = Object.fromEntries(
Object.entries(s.pendingAddressBarFocusByPageId).filter(
([pageId]) => !closedPages.some((page) => page.id === pageId)
)
)
const nextPendingAddressBarFocusByTabId = Object.fromEntries(
Object.entries(s.pendingAddressBarFocusByTabId).filter(
([focusId]) => focusId !== tabId && !closedPages.some((page) => page.id === focusId)
)
// Why omit rather than rebuild: only the closed ids can match, so this touches the
// closed pages instead of every pending entry and keeps the record identity when none did.
const closedPageIds = closedPages.map((page) => page.id)
const nextPendingAddressBarFocusByPageId = omitRecordKeys(
s.pendingAddressBarFocusByPageId,
closedPageIds
)
const nextPendingAddressBarFocusByTabId = omitRecordKeys(s.pendingAddressBarFocusByTabId, [
...closedPageIds,
tabId
])
return {
browserTabsByWorktree: nextBrowserTabsByWorktree,