From 03e43ef165b3a430bac337fbc8ae5cca5997c8c5 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Tue, 26 May 2026 20:14:38 -0700 Subject: [PATCH] fix: avoid stacking browser download handlers (#2879) --- .../attach-main-window-services.test.ts | 34 +++++++++++++++++-- .../window/attach-main-window-services.ts | 27 +++++++++++---- 2 files changed, 51 insertions(+), 10 deletions(-) diff --git a/src/main/window/attach-main-window-services.test.ts b/src/main/window/attach-main-window-services.test.ts index e0ba16b9b..3e5f1a9dc 100644 --- a/src/main/window/attach-main-window-services.test.ts +++ b/src/main/window/attach-main-window-services.test.ts @@ -171,7 +171,8 @@ describe('attachMainWindowServices', () => { setPermissionRequestHandler: setPermissionRequestHandlerMock, setPermissionCheckHandler: setPermissionCheckHandlerMock, setDisplayMediaRequestHandler: setDisplayMediaRequestHandlerMock, - on: vi.fn() + on: vi.fn(), + removeListener: vi.fn() }) systemPreferencesAskForMediaAccessMock.mockResolvedValue(true) systemPreferencesGetMediaAccessStatusMock.mockReturnValue('granted') @@ -316,7 +317,8 @@ describe('attachMainWindowServices', () => { setPermissionRequestHandler: setPermissionRequestHandlerMock, setPermissionCheckHandler: setPermissionCheckHandlerMock, setDisplayMediaRequestHandler: setDisplayMediaRequestHandlerMock, - on: browserSessionOnMock + on: browserSessionOnMock, + removeListener: vi.fn() }) const mainWindowOnMock = vi.fn() @@ -378,12 +380,38 @@ describe('attachMainWindowServices', () => { }) }) + it('replaces the persistent browser-session download handler on re-attach', () => { + const browserSessionOnMock = vi.fn() + const browserSessionRemoveListenerMock = vi.fn() + sessionFromPartitionMock.mockReturnValue({ + setPermissionRequestHandler: setPermissionRequestHandlerMock, + setPermissionCheckHandler: setPermissionCheckHandlerMock, + setDisplayMediaRequestHandler: setDisplayMediaRequestHandlerMock, + on: browserSessionOnMock, + removeListener: browserSessionRemoveListenerMock + }) + + attachMainWindowServices(createMainWindow() as never, createStore(), createRuntime() as never) + attachMainWindowServices(createMainWindow() as never, createStore(), createRuntime() as never) + + const downloadOnCalls = browserSessionOnMock.mock.calls.filter( + ([eventName]) => eventName === 'will-download' + ) + const downloadRemoveCalls = browserSessionRemoveListenerMock.mock.calls.filter( + ([eventName]) => eventName === 'will-download' + ) + expect(downloadOnCalls).toHaveLength(2) + expect(downloadRemoveCalls).toHaveLength(2) + expect(downloadRemoveCalls[1][1]).toBe(downloadOnCalls[0][1]) + }) + it('clears browser guest registrations when the main window closes', () => { sessionFromPartitionMock.mockReturnValue({ setPermissionRequestHandler: setPermissionRequestHandlerMock, setPermissionCheckHandler: setPermissionCheckHandlerMock, setDisplayMediaRequestHandler: setDisplayMediaRequestHandlerMock, - on: vi.fn() + on: vi.fn(), + removeListener: vi.fn() }) const mainWindowOnMock = vi.fn() const mainWindow = createMainWindow() diff --git a/src/main/window/attach-main-window-services.ts b/src/main/window/attach-main-window-services.ts index 84a2bb658..c982d3361 100644 --- a/src/main/window/attach-main-window-services.ts +++ b/src/main/window/attach-main-window-services.ts @@ -2,7 +2,7 @@ import { randomUUID } from 'node:crypto' import { app, ipcMain, session } from 'electron' -import type { BrowserWindow } from 'electron' +import type { BrowserWindow, Session } from 'electron' import type { Store } from '../persistence' import type { CreateWorktreeResult, WorktreeStartupLaunch } from '../../shared/types' import { ORCA_BROWSER_PARTITION } from '../../shared/constants' @@ -196,12 +196,7 @@ export function attachMainWindowServices( // signature while still denying the request. callback({ video: undefined, audio: undefined }) }) - browserSession.on('will-download', (_event, item, webContents) => { - // Why: browser-tab downloads need explicit product UX before arbitrary sites - // can write files through Orca. Pause the item and route it through - // BrowserManager so the user must explicitly accept the save path first. - browserManager.handleGuestWillDownload({ guestWebContentsId: webContents.id, item }) - }) + registerBrowserDownloadHandler(browserSession) mainWindow.on('closed', () => { // Why: parked browser webviews can outlive the visible tab body until the @@ -212,6 +207,24 @@ export function attachMainWindowServices( }) } +function handleBrowserWillDownload( + _event: Electron.Event, + item: Electron.DownloadItem, + webContents: Electron.WebContents +): void { + // Why: browser-tab downloads need explicit product UX before arbitrary sites + // can write files through Orca. Pause the item and route it through + // BrowserManager so the user must explicitly accept the save path first. + browserManager.handleGuestWillDownload({ guestWebContentsId: webContents.id, item }) +} + +function registerBrowserDownloadHandler(browserSession: Session): void { + // Why: browser sessions are process-persistent while main windows can be + // recreated; replace the named handler so re-attach does not stack listeners. + browserSession.removeListener('will-download', handleBrowserWillDownload) + browserSession.on('will-download', handleBrowserWillDownload) +} + function registerAppReloadHandler( mainWindow: BrowserWindow, onBeforeRendererReload?: (args: { webContentsId: number; ignoreCache: boolean }) => void