fix(browser): keep native popup admission within opener session (#35397)

Co-authored-by: OrcaWin <293788423+OrcaWin@users.noreply.github.com>

src/main/browser/browser-manager-popup-openerless-tab.test.ts

@@ -2,7 +2,7 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'
 
 const browserMocks = vi.hoisted(() => ({
   appGetPathMock: vi.fn(() => '/downloads'),
-  appPrependOnceListenerMock: vi.fn(),
+  appPrependListenerMock: vi.fn(),
   shellOpenExternalMock: vi.fn(),
   browserWindowFromWebContentsMock: vi.fn(),
   menuBuildFromTemplateMock: vi.fn(),
@@ -18,7 +18,7 @@ const browserMocks = vi.hoisted(() => ({
 
 vi.mock('electron', () => ({
   app: {
-    prependOnceListener: browserMocks.appPrependOnceListenerMock,
+    prependListener: browserMocks.appPrependListenerMock,
     off: vi.fn(),
     getPath: browserMocks.appGetPathMock
   },
@@ -42,7 +42,7 @@ import {
 } from './browser-manager-test-harness'
 
 const {
-  appPrependOnceListenerMock,
+  appPrependListenerMock,
   guestOnMock,
   guestSetWindowOpenHandlerMock,
   webContentsFromIdMock,
@@ -123,7 +123,7 @@ describe('tab-shaped OAuth opens without an opener', () => {
   beforeEach(() => {
     resetBrowserManagerMocks(browserMocks)
     resetBrowserManagerState()
-    appPrependOnceListenerMock.mockReset()
+    appPrependListenerMock.mockReset()
     rendererSendMock.mockReset()
     openPopupWithOriginBarMock.mockReturnValue({
       contentWebContents: popupContents(null),
@@ -198,7 +198,7 @@ describe('tab-shaped OAuth opens without an opener', () => {
     })
     expect(response).toMatchObject({ action: 'allow' })
     const onWindowCreated: (event: unknown, window: unknown) => void =
-      appPrependOnceListenerMock.mock.calls.at(-1)?.[1]
+      appPrependListenerMock.mock.calls.at(-1)?.[1]
     const nativeWindow = {
       webContents: popupContents(null),
       isDestroyed: vi.fn(() => false),

src/main/browser/browser-manager-popup-routing.test.ts

@@ -17,7 +17,7 @@ const browserMocks = vi.hoisted(() => ({
 
 vi.mock('electron', () => ({
   app: {
-    prependOnceListener: vi.fn(),
+    prependListener: vi.fn(),
     off: vi.fn(),
     getPath: browserMocks.appGetPathMock
   },

src/main/browser/browser-popup-native-window.test.ts

@@ -1,5 +1,6 @@
 import { EventEmitter } from 'node:events'
 import { afterEach, describe, expect, it, vi } from 'vitest'
+import { isNativeBrowserPopupWindow } from './native-browser-popup-windows'
 
 const { events } = await vi.hoisted(async () => {
   const { EventEmitter } = await import('node:events')
@@ -212,27 +213,50 @@ describe('native popup admission', () => {
     expect(onFirst).not.toHaveBeenCalled()
   })
 
-  it.fails.each([true, false])(
-    'KNOWN BLOCKER: an unrelated app window receives neither policy nor disposal (same session: %s)',
-    async (sameSession) => {
-      const opener = nativeWindow()
-      const unrelated = nativeWindow()
-      if (sameSession) {
-        unrelated.contents.session = opener.contents.session
-      }
-      const onCreated = vi.fn(() => unrelated.browserWindow.webContents)
-      allowNativeBrowserPopup({
-        opener: opener.browserWindow.webContents,
-        options: { show: false },
-        onCreated
-      })
-      events.emit('browser-window-created', {}, unrelated.browserWindow)
-      await Promise.resolve()
-      expect(onCreated).not.toHaveBeenCalled()
-      expect(unrelated.contents.close).not.toHaveBeenCalled()
-      expect(unrelated.window.destroy).not.toHaveBeenCalled()
-    }
-  )
+  it('ignores a different-session app window without consuming the legitimate child admission', async () => {
+    const opener = nativeWindow()
+    const unrelated = nativeWindow()
+    const child = nativeWindow()
+    child.contents.session = opener.contents.session
+    const onCreated = vi.fn(() => child.browserWindow.webContents)
+    allowNativeBrowserPopup({
+      opener: opener.browserWindow.webContents,
+      options: { show: false },
+      onCreated
+    })
+    events.emit('browser-window-created', {}, unrelated.browserWindow)
+    expect(onCreated).not.toHaveBeenCalled()
+    expect(isNativeBrowserPopupWindow(unrelated.browserWindow)).toBe(false)
+    events.emit('browser-window-created', {}, child.browserWindow)
+    await Promise.resolve()
+    opener.contents.emit('did-create-window', child.browserWindow)
+    expect(onCreated).toHaveBeenCalledExactlyOnceWith(
+      expect.objectContaining({ nativeWindow: child.browserWindow })
+    )
+    expect(isNativeBrowserPopupWindow(child.browserWindow)).toBe(true)
+    expect(isNativeBrowserPopupWindow(unrelated.browserWindow)).toBe(false)
+    expect(unrelated.contents.close).not.toHaveBeenCalled()
+    expect(unrelated.window.destroy).not.toHaveBeenCalled()
+    expect(child.contents.close).not.toHaveBeenCalled()
+    expect(child.window.destroy).not.toHaveBeenCalled()
+  })
+
+  it.fails('KNOWN BLOCKER: an unrelated same-session app window receives neither policy nor disposal', async () => {
+    const opener = nativeWindow()
+    const unrelated = nativeWindow()
+    unrelated.contents.session = opener.contents.session
+    const onCreated = vi.fn(() => unrelated.browserWindow.webContents)
+    allowNativeBrowserPopup({
+      opener: opener.browserWindow.webContents,
+      options: { show: false },
+      onCreated
+    })
+    events.emit('browser-window-created', {}, unrelated.browserWindow)
+    await Promise.resolve()
+    expect(onCreated).not.toHaveBeenCalled()
+    expect(unrelated.contents.close).not.toHaveBeenCalled()
+    expect(unrelated.window.destroy).not.toHaveBeenCalled()
+  })
 
   it('handles duplicate did-create-window delivery idempotently for an already marked child', async () => {
     const opener = nativeWindow()

src/main/browser/browser-popup-native-window.ts

@@ -36,11 +36,12 @@ export function allowNativeBrowserPopup(input: {
   canPresent?: () => boolean
   onCreated: (options: PopupChildWindowOptions) => WebContents
 }): Electron.WindowOpenHandlerResponse {
+  const openerSession = input.opener.session
   const mayPresent = !isBackgroundLaunch() && input.options.show !== false
   const admit = (window: BrowserWindow): void => {
     markNativeBrowserPopupWindow(window)
     try {
-      if (input.opener.isDestroyed() || window.webContents.session !== input.opener.session) {
+      if (input.opener.isDestroyed() || window.webContents.session !== openerSession) {
         rejectNativeBrowserPopup(window)
         return
       }
@@ -55,7 +56,7 @@ export function allowNativeBrowserPopup(input: {
               window.isDestroyed() ||
               window.webContents.isDestroyed() ||
               input.opener.isDestroyed() ||
-              window.webContents.session !== input.opener.session ||
+              window.webContents.session !== openerSession ||
               (input.canPresent && !input.canPresent())
             ) {
               return
@@ -70,11 +71,19 @@ export function allowNativeBrowserPopup(input: {
   }
   let admitted = false
   const onCreated = (_event: Electron.Event, window: BrowserWindow): void => {
+    if (
+      window.isDestroyed() ||
+      window.webContents.isDestroyed() ||
+      window.webContents.session !== openerSession
+    ) {
+      return
+    }
+    app.off('browser-window-created', onCreated)
     admitted = true
     admit(window)
   }
   // Prepend prevents another browser-window-created listener from navigating the child first.
-  app.prependOnceListener('browser-window-created', onCreated)
+  app.prependListener('browser-window-created', onCreated)
   // A canceled native creation must not grant the next unrelated window this admission.
   queueMicrotask(() => {
     app.off('browser-window-created', onCreated)
@@ -93,7 +102,7 @@ export function allowNativeBrowserPopup(input: {
       useContentSize: true,
       minWidth: MIN_POPUP_CONTENT_WIDTH,
       minHeight: MIN_POPUP_CONTENT_HEIGHT,
-      webPreferences: { ...input.options.webPreferences, session: input.opener.session }
+      webPreferences: { ...input.options.webPreferences, session: openerSession }
     }
   }
 }