perf(browser): release popup origin bar contents (#17234)

src/main/browser/popup-origin-bar-window.test.ts

@@ -285,6 +285,7 @@ describe('openPopupWithOriginBar', () => {
   it('closes the window when the popup content is destroyed, without re-closing the contents', () => {
     const adopted = createFakeWebContents()
     openPopupWithOriginBar({ webContents: adopted as never }, 'https://example.com/')
+    const { bar } = lastViews()
 
     adopted.emit('destroyed')
 
@@ -292,6 +293,7 @@ describe('openPopupWithOriginBar', () => {
     // The window's closed handler must not call close() on already-destroyed
     // contents — that throws in real Electron.
     expect(adopted.close).not.toHaveBeenCalled()
+    expect(bar.webContents.close).toHaveBeenCalledTimes(1)
   })
 
   it('re-asserts the origin when the popup finishes loading', () => {
@@ -326,11 +328,54 @@ describe('openPopupWithOriginBar', () => {
     const adopted = createFakeWebContents()
     const onClosed = vi.fn()
     const popup = openPopupWithOriginBar({ webContents: adopted as never }, 'https://example.com/')
+    const { bar } = lastViews()
     popup.onClosed(onClosed)
 
     popup.close()
 
     expect(adopted.close).toHaveBeenCalledTimes(1)
+    expect(bar.webContents.close).toHaveBeenCalledTimes(1)
     expect(onClosed).toHaveBeenCalledTimes(1)
   })
+
+  it('releases every origin bar across repeated popup lifecycles', () => {
+    const cycleCount = 25
+    const originBars: FakeWebContents[] = []
+
+    for (let cycle = 0; cycle < cycleCount; cycle += 1) {
+      const popup = openPopupWithOriginBar({}, `https://example.com/${cycle}`)
+      originBars.push(lastViews().bar.webContents)
+      popup.close()
+    }
+
+    expect(originBars.filter((contents) => contents.close.mock.calls.length === 1)).toHaveLength(
+      cycleCount
+    )
+  })
+
+  it('tolerates an origin bar destroyed before its window closes', () => {
+    const adopted = createFakeWebContents()
+    const onClosed = vi.fn()
+    const popup = openPopupWithOriginBar({ webContents: adopted as never }, 'https://example.com/')
+    popup.onClosed(onClosed)
+    const { bar } = lastViews()
+    const barWebContents = bar.webContents
+    barWebContents.emit('destroyed')
+    Object.defineProperty(bar, 'webContents', { get: () => undefined })
+
+    expect(() => popup.close()).not.toThrow()
+    expect(adopted.close).toHaveBeenCalledTimes(1)
+    expect(barWebContents.close).not.toHaveBeenCalled()
+    expect(onClosed).toHaveBeenCalledTimes(1)
+  })
+
+  it('closes both child contents when popup preparation fails', () => {
+    const adopted = createFakeWebContents()
+
+    openPopupWithOriginBar({ webContents: adopted as never }, 'https://example.com/', () => false)
+
+    expect(adopted.close).toHaveBeenCalledTimes(1)
+    expect(lastViews().bar.webContents.close).toHaveBeenCalledTimes(1)
+    expect(lastWindow().close).toHaveBeenCalledTimes(1)
+  })
 })

src/main/browser/popup-origin-bar-window.ts

@@ -92,7 +92,8 @@ function clampPopupContentSize(options: PopupChildWindowOptions): {
 // caller's popup bookkeeping cannot leak an entry for a window that no longer exists.
 function closeUnpreparedPopup(
   window: BaseWindow,
-  contentWebContents: Electron.WebContents
+  contentWebContents: Electron.WebContents,
+  originBarWebContents: Electron.WebContents
 ): PopupOriginBarWindow {
   const closedListeners: (() => void)[] = []
   let closed = false
@@ -105,6 +106,9 @@ function closeUnpreparedPopup(
   if (!contentWebContents.isDestroyed()) {
     contentWebContents.close()
   }
+  if (!originBarWebContents.isDestroyed()) {
+    originBarWebContents.close()
+  }
   if (!window.isDestroyed()) {
     window.close()
   }
@@ -159,6 +163,7 @@ export function openPopupWithOriginBar(
   const originBarView = new WebContentsView({
     webPreferences: { contextIsolation: true, nodeIntegration: false, sandbox: true }
   })
+  const originBarWebContents = originBarView.webContents
   const contentView = new WebContentsView({
     // Why: Electron rejects an explicitly undefined webContents; omitting it
     // lets WebContentsView create contents for Cmd/Ctrl-click popups.
@@ -188,7 +193,7 @@ export function openPopupWithOriginBar(
 
   const contentWebContents = contentView.webContents
   if (prepareContent && !prepareContent(contentWebContents)) {
-    return closeUnpreparedPopup(window, contentWebContents)
+    return closeUnpreparedPopup(window, contentWebContents, originBarWebContents)
   }
   let currentUrl = initialUrl
   const renderOrigin = (): void => {
@@ -202,15 +207,15 @@ export function openPopupWithOriginBar(
     }
     // Why: textContent + JSON encoding — the URL is attacker-controlled and
     // must never be interpolated into the bar's markup.
-    void originBarView.webContents
+    void originBarWebContents
       .executeJavaScript(
         `document.body.classList.toggle('insecure', ${insecure ? 'true' : 'false'});` +
           `document.getElementById('origin').textContent = ${JSON.stringify(label)};`
       )
       .catch(() => {})
   }
-  originBarView.webContents.once('did-finish-load', renderOrigin)
-  void originBarView.webContents.loadURL(
+  originBarWebContents.once('did-finish-load', renderOrigin)
+  void originBarWebContents.loadURL(
     `data:text/html;charset=utf-8,${encodeURIComponent(ORIGIN_BAR_HTML)}`
   )
 
@@ -253,6 +258,10 @@ export function openPopupWithOriginBar(
       // pages often notify the opener from unload.
       contentWebContents.close()
     }
+    // Why: BaseWindow does not destroy child WebContentsView contents when it closes.
+    if (!originBarWebContents.isDestroyed()) {
+      originBarWebContents.close()
+    }
     for (const listener of closedListeners) {
       listener()
     }