From 1c8d281e084ed538494c3c23ecce3bbe22e91e8c Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 3 Oct 2026 00:16:56 -0700 Subject: [PATCH] Stop copying every retained browser page before attachment (#24553) Find the first page/generation match directly in the existing insertion-ordered Map instead of copying all page references into an array. --- ...client-page-attachment-scan-budget.test.ts | 116 ++++++++++++++++++ .../browser-client-page-retained-registry.ts | 7 +- .../browser-client-page-visible-attachment.ts | 16 +++ 3 files changed, 134 insertions(+), 5 deletions(-) create mode 100644 src/renderer/src/components/browser-pane/browser-client-page-attachment-scan-budget.test.ts diff --git a/src/renderer/src/components/browser-pane/browser-client-page-attachment-scan-budget.test.ts b/src/renderer/src/components/browser-pane/browser-client-page-attachment-scan-budget.test.ts new file mode 100644 index 00000000000..3b7276c9953 --- /dev/null +++ b/src/renderer/src/components/browser-pane/browser-client-page-attachment-scan-budget.test.ts @@ -0,0 +1,116 @@ +// @vitest-environment happy-dom +import { afterEach, describe, expect, it, vi } from 'vitest' +import { + createRetainedHostFixture, + disposeRetainedHostFixtures, + RETAINED_FIXTURE_PAGE +} from './browser-client-page-retained-host-fixture' + +afterEach(() => { + vi.restoreAllMocks() + disposeRetainedHostFixtures() + document.body.innerHTML = '' +}) + +function observePageIterations(pages: Map): { + count: () => number + reset: () => void +} { + let steps = 0 + const values = Map.prototype.values + vi.spyOn(Map.prototype, 'values').mockImplementation(function (this: Map) { + const iterator = values.call(this) + if (this !== pages) { + return iterator + } + const next = iterator.next.bind(iterator) + vi.spyOn(iterator, 'next').mockImplementation(() => { + steps += 1 + return next() + }) + return iterator + }) + return { + count: () => steps, + reset: () => { + steps = 0 + } + } +} + +describe('retained browser page attachment lookup', () => { + it('stops at early, middle and last matches without copying the complete page catalog', async () => { + const fixture = createRetainedHostFixture() + const identities = Array.from({ length: 256 }, (_, index) => ({ + ...RETAINED_FIXTURE_PAGE, + partition: `persist:route-${Math.floor(index / 64)}`, + browserPageId: `page-${index}` + })) + for (const identity of identities) { + await fixture.mount(identity) + } + const registry: unknown = fixture.registry + if ( + typeof registry !== 'object' || + registry === null || + !('pages' in registry) || + !(registry.pages instanceof Map) + ) { + throw new Error('Retained page catalog is not a Map') + } + const iterations = observePageIterations(registry.pages) + const counts: number[] = [] + for (const index of [0, 127, 255]) { + iterations.reset() + const attachment = fixture.attach(identities[index]) + expect(attachment.webview.getWebContentsId()).toBe(index + 41) + expect(attachment.nextMetadataRevision()).toBe(1) + expect([...new Map([['unrelated', index]]).values()]).toEqual([index]) + counts.push(iterations.count()) + attachment.detach() + } + iterations.reset() + expect(() => fixture.attach(RETAINED_FIXTURE_PAGE)).toThrow( + 'browser_client_page_renderer_visible_page_unavailable' + ) + counts.push(iterations.count()) + expect(counts).toEqual([1, 128, 256, 257]) + }) + + it('keeps the first matching partition and follows exact generations after rekey and destruction', async () => { + const fixture = createRetainedHostFixture() + const first = RETAINED_FIXTURE_PAGE + const second = { ...first, partition: 'persist:second' } + await fixture.mount(first) + await fixture.mount(second) + const visible = fixture.attach(first) + expect(visible.webview.getWebContentsId()).toBe(41) + expect(() => fixture.attach(second)).toThrow( + 'browser_client_page_renderer_visible_page_claimed' + ) + visible.detach() + const rekeyed = { ...first, pageHostGeneration: first.pageHostGeneration + 1 } + fixture.registry.rekeyPage(first, rekeyed) + + const oldGeneration = fixture.attach(first) + expect(oldGeneration.webview.getWebContentsId()).toBe(42) + oldGeneration.detach() + const newGeneration = fixture.attach(rekeyed) + expect(newGeneration.webview).toBe(visible.webview) + fixture.registry.retirePage(rekeyed) + newGeneration.detach() + expect(() => fixture.attach(rekeyed)).toThrow( + 'browser_client_page_renderer_visible_page_unavailable' + ) + visible.webview.dispatchEvent(new Event('destroyed')) + await fixture.mount(rekeyed) + const replacement = fixture.attach(rekeyed) + expect(replacement.webview.getWebContentsId()).toBe(43) + expect(replacement.webview).not.toBe(visible.webview) + replacement.detach() + fixture.registry.dispose() + expect(() => fixture.attach(rekeyed)).toThrow( + 'browser_client_page_renderer_visible_page_unavailable' + ) + }) +}) diff --git a/src/renderer/src/components/browser-pane/browser-client-page-retained-registry.ts b/src/renderer/src/components/browser-pane/browser-client-page-retained-registry.ts index 34564964c08..3a4114605f0 100644 --- a/src/renderer/src/components/browser-pane/browser-client-page-retained-registry.ts +++ b/src/renderer/src/components/browser-pane/browser-client-page-retained-registry.ts @@ -15,6 +15,7 @@ import type { BrowserClientRetainedRendererPage as RetainedPage } from './browse import { attachBrowserClientRetainedPage, enrolRetainedHostDragPassthrough, + findBrowserClientRetainedPageForAttachment, type BrowserClientPageVisibleAttachment } from './browser-client-page-visible-attachment' import { @@ -101,11 +102,7 @@ export class BrowserClientPageRetainedRegistry { identity: Pick, container: HTMLElement ): BrowserClientPageVisibleAttachment { - const page = [...this.pages.values()].find( - (candidate) => - candidate.identity.browserPageId === identity.browserPageId && - candidate.identity.pageHostGeneration === identity.pageHostGeneration - ) + const page = findBrowserClientRetainedPageForAttachment(this.pages, identity) return attachBrowserClientRetainedPage(page, this.pages, container) } diff --git a/src/renderer/src/components/browser-pane/browser-client-page-visible-attachment.ts b/src/renderer/src/components/browser-pane/browser-client-page-visible-attachment.ts index 8297f865ed9..e01645406a1 100644 --- a/src/renderer/src/components/browser-pane/browser-client-page-visible-attachment.ts +++ b/src/renderer/src/components/browser-pane/browser-client-page-visible-attachment.ts @@ -3,6 +3,7 @@ import { registerWebviewDragPassthroughSurface } from './host-guest/webview-drag-passthrough' import { registerBrowserClientPagePositionSync } from './browser-client-page-position-driver' +import type { BrowserClientPageRendererIdentity as RendererPageIdentity } from '../../../../shared/browser-client-page-renderer-protocol' import type { BrowserClientRetainedRendererPage as RetainedPage } from './browser-client-page-retained-state' export type BrowserClientPageVisibleAttachment = { @@ -11,6 +12,21 @@ export type BrowserClientPageVisibleAttachment = { detach(): void } +export function findBrowserClientRetainedPageForAttachment( + pages: Map, + identity: Pick +): RetainedPage | undefined { + for (const page of pages.values()) { + if ( + page.identity.browserPageId === identity.browserPageId && + page.identity.pageHostGeneration === identity.pageHostGeneration + ) { + return page + } + } + return undefined +} + export function attachBrowserClientRetainedPage( page: RetainedPage | undefined, pages: Map,