perf: clean up destroyed browser guest policies (#4132)

This commit is contained in:
Neil
2026-05-31 05:36:16 -07:00
committed by GitHub
parent 15c6b58db8
commit ea0b52304c
3 changed files with 55 additions and 16 deletions
@@ -817,9 +817,9 @@ describe('browserManager grab operations', () => {
const promise = browserManager.awaitGrabSelection('tab-1', 'op-1', guest)
// Find the destroyed handler and trigger it
const destroyedHandler = guestOnMock.mock.calls.find(
([event]) => event === 'destroyed'
)?.[1] as (() => void) | undefined
const destroyedHandler = guestOnMock.mock.calls
.filter(([event]) => event === 'destroyed')
.at(-1)?.[1] as (() => void) | undefined
expect(destroyedHandler).toBeTypeOf('function')
destroyedHandler?.()
+30
View File
@@ -819,6 +819,36 @@ describe('browserManager', () => {
expect(guestOnMock.mock.calls.filter(([event]) => event === 'will-redirect')).toHaveLength(1)
})
it('cleans attached guest policy state when a guest is destroyed before registration', () => {
const guest = {
id: 304,
isDestroyed: vi.fn(() => false),
getType: vi.fn(() => 'webview'),
setBackgroundThrottling: guestSetBackgroundThrottlingMock,
setWindowOpenHandler: guestSetWindowOpenHandlerMock,
on: guestOnMock,
off: guestOffMock,
openDevTools: guestOpenDevToolsMock
}
webContentsFromIdMock.mockReturnValue(guest)
browserManager.attachGuestPolicies(guest as never)
const destroyedHandler = guestOnMock.mock.calls.find(
([event]) => event === 'destroyed'
)?.[1] as (() => void) | undefined
expect(destroyedHandler).toBeTypeOf('function')
destroyedHandler?.()
browserManager.registerGuest({
browserPageId: 'browser-destroyed-before-register',
webContentsId: guest.id,
rendererWebContentsId
})
expect(browserManager.getGuestWebContentsId('browser-destroyed-before-register')).toBeNull()
})
it('replays a queued main-frame load failure after the guest registers', () => {
const rendererSendMock = vi.fn()
const guest = {
+22 -13
View File
@@ -640,12 +640,23 @@ export class BrowserManager {
guest.on('will-navigate', navigationGuard)
guest.on('will-redirect', navigationGuard)
guest.on('did-fail-load', didFailLoadHandler)
const handleDestroyed = (): void => {
// Why: guests can be destroyed before renderer registration. Without
// this, attach-time policy closures remain retained until app shutdown.
this.cleanupGuestPolicyAttachment(guest.id)
}
guest.on('destroyed', handleDestroyed)
// Why: store cleanup so unregisterGuest can remove these listeners when the
// guest surface is torn down, preventing the callbacks from preventing GC of
// the underlying WebContents wrapper.
this.policyCleanupByGuestId.set(guest.id, () => {
disposeAntiDetection()
try {
guest.off('destroyed', handleDestroyed)
} catch {
// guest may already be destroyed
}
if (!guest.isDestroyed()) {
guest.off('will-navigate', navigationGuard)
guest.off('will-redirect', navigationGuard)
@@ -660,17 +671,20 @@ export class BrowserManager {
// resolving to the live page, or stale download/popup/permission callbacks
// can be delivered to the wrong session after the swap.
this.tabIdByWebContentsId.delete(previousWebContentsId)
this.cleanupGuestPolicyAttachment(previousWebContentsId)
}
const policyCleanup = this.policyCleanupByGuestId.get(previousWebContentsId)
private cleanupGuestPolicyAttachment(guestWebContentsId: number): void {
const policyCleanup = this.policyCleanupByGuestId.get(guestWebContentsId)
if (policyCleanup) {
policyCleanup()
this.policyCleanupByGuestId.delete(previousWebContentsId)
this.policyCleanupByGuestId.delete(guestWebContentsId)
}
this.policyAttachedGuestIds.delete(previousWebContentsId)
this.pendingLoadFailuresByGuestId.delete(previousWebContentsId)
this.pendingPermissionEventsByGuestId.delete(previousWebContentsId)
this.pendingPopupEventsByGuestId.delete(previousWebContentsId)
this.pendingDownloadIdsByGuestId.delete(previousWebContentsId)
this.policyAttachedGuestIds.delete(guestWebContentsId)
this.pendingLoadFailuresByGuestId.delete(guestWebContentsId)
this.pendingPermissionEventsByGuestId.delete(guestWebContentsId)
this.pendingPopupEventsByGuestId.delete(guestWebContentsId)
this.pendingDownloadIdsByGuestId.delete(guestWebContentsId)
}
registerGuest({
@@ -754,12 +768,7 @@ export class BrowserManager {
// the underlying Chromium surface after the guest is destroyed.
const guestWebContentsId = this.webContentsIdByTabId.get(browserTabId)
if (guestWebContentsId !== undefined) {
const policyCleanup = this.policyCleanupByGuestId.get(guestWebContentsId)
if (policyCleanup) {
policyCleanup()
this.policyCleanupByGuestId.delete(guestWebContentsId)
}
this.policyAttachedGuestIds.delete(guestWebContentsId)
this.cleanupGuestPolicyAttachment(guestWebContentsId)
}
const cleanup = this.contextMenuCleanupByTabId.get(browserTabId)