perf(browser): scope webview drag listeners (#1834)

src/renderer/src/components/browser-pane/BrowserPane.tsx

@@ -55,6 +55,7 @@ import {
   getHiddenContainer,
   MAX_PARKED_WEBVIEWS,
   parkedAtByTabId,
+  registerPersistentWebview,
   registeredWebContentsIds,
   webviewRegistry
 } from './webview-registry'
@@ -965,7 +966,7 @@ function BrowserPagePane({
       // browsers paint the viewport white by default; sites that specify their
       // own background (including dark ones) still override this.
       webview.style.background = '#ffffff'
-      webviewRegistry.set(browserTab.id, webview)
+      registerPersistentWebview(browserTab.id, webview)
       container.appendChild(webview)
       needsInitialNavigation = true
     }

src/renderer/src/components/browser-pane/webview-registry.test.ts

@@ -0,0 +1,99 @@
+import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
+
+type ListenerRecord = {
+  type: string
+  listener: EventListenerOrEventListenerObject
+  options?: boolean | AddEventListenerOptions
+}
+
+function createWebview(): Electron.WebviewTag {
+  return {
+    style: {},
+    remove: vi.fn(),
+    contains: vi.fn(() => false)
+  } as unknown as Electron.WebviewTag
+}
+
+describe('webview registry drag listeners', () => {
+  let addedListeners: ListenerRecord[]
+  let removedListeners: ListenerRecord[]
+  let unregisterGuestMock: ReturnType<typeof vi.fn>
+
+  beforeEach(() => {
+    vi.resetModules()
+    addedListeners = []
+    removedListeners = []
+    unregisterGuestMock = vi.fn()
+
+    vi.stubGlobal('window', {
+      addEventListener: vi.fn(
+        (
+          type: string,
+          listener: EventListenerOrEventListenerObject,
+          options?: boolean | AddEventListenerOptions
+        ) => {
+          addedListeners.push({ type, listener, options })
+        }
+      ),
+      removeEventListener: vi.fn(
+        (
+          type: string,
+          listener: EventListenerOrEventListenerObject,
+          options?: boolean | AddEventListenerOptions
+        ) => {
+          removedListeners.push({ type, listener, options })
+        }
+      ),
+      focus: vi.fn(),
+      api: {
+        browser: {
+          unregisterGuest: unregisterGuestMock
+        }
+      }
+    })
+    vi.stubGlobal('document', { activeElement: null })
+  })
+
+  afterEach(() => {
+    vi.unstubAllGlobals()
+  })
+
+  it('does not install global drag listeners until a webview is registered', async () => {
+    const { registerPersistentWebview } = await import('./webview-registry')
+
+    expect(addedListeners).toEqual([])
+
+    registerPersistentWebview('page-1', createWebview())
+
+    expect(addedListeners.map((entry) => entry.type)).toEqual(['dragstart', 'dragend', 'drop'])
+  })
+
+  it('removes drag listeners after the last webview is destroyed', async () => {
+    const { destroyPersistentWebview, registerPersistentWebview } =
+      await import('./webview-registry')
+
+    registerPersistentWebview('page-1', createWebview())
+    registerPersistentWebview('page-2', createWebview())
+
+    expect(addedListeners).toHaveLength(3)
+
+    destroyPersistentWebview('page-1')
+
+    expect(removedListeners).toHaveLength(0)
+
+    destroyPersistentWebview('page-2')
+
+    expect(removedListeners.map((entry) => entry.type)).toEqual(['dragstart', 'dragend', 'drop'])
+    expect(unregisterGuestMock).toHaveBeenCalledWith({ browserPageId: 'page-1' })
+    expect(unregisterGuestMock).toHaveBeenCalledWith({ browserPageId: 'page-2' })
+  })
+
+  it('keeps one listener set across repeated registrations', async () => {
+    const { registerPersistentWebview } = await import('./webview-registry')
+
+    registerPersistentWebview('page-1', createWebview())
+    registerPersistentWebview('page-2', createWebview())
+
+    expect(addedListeners).toHaveLength(3)
+  })
+})

src/renderer/src/components/browser-pane/webview-registry.ts

@@ -12,6 +12,57 @@ export const parkedAtByTabId = new Map<string, number>()
 export const MAX_PARKED_WEBVIEWS = 6
 
 let hiddenContainer: HTMLDivElement | null = null
+const DRAG_LISTENER_KEY = '__orcaBrowserPaneDragListeners'
+let dragListenersAttached = false
+
+type DragListenerRegistry = {
+  dragstart: () => void
+  dragend: () => void
+  drop: () => void
+}
+
+function getListenerHost(): (Window & { [DRAG_LISTENER_KEY]?: DragListenerRegistry }) | null {
+  if (typeof window === 'undefined' || typeof window.addEventListener !== 'function') {
+    return null
+  }
+  return window as Window & { [DRAG_LISTENER_KEY]?: DragListenerRegistry }
+}
+
+function removeDragListeners(): void {
+  const listenerHost = getListenerHost()
+  const existingListeners = listenerHost?.[DRAG_LISTENER_KEY]
+  if (!listenerHost || !existingListeners) {
+    return
+  }
+  window.removeEventListener('dragstart', existingListeners.dragstart, true)
+  window.removeEventListener('dragend', existingListeners.dragend, true)
+  window.removeEventListener('drop', existingListeners.drop, true)
+  delete listenerHost[DRAG_LISTENER_KEY]
+  dragListenersAttached = false
+}
+
+function ensureDragListeners(): void {
+  const listenerHost = getListenerHost()
+  if (!listenerHost) {
+    return
+  }
+  if (dragListenersAttached && listenerHost[DRAG_LISTENER_KEY]) {
+    return
+  }
+  removeDragListeners()
+
+  const dragstart = (): void => setWebviewsDragPassthrough(true)
+  const dragend = (): void => setWebviewsDragPassthrough(false)
+  const drop = (): void => setWebviewsDragPassthrough(false)
+
+  window.addEventListener('dragstart', dragstart, true)
+  window.addEventListener('dragend', dragend, true)
+  window.addEventListener('drop', drop, true)
+  // Why: only live webviews need drag passthrough listeners; removing them
+  // when the registry empties keeps browserless sessions free of global hooks.
+  listenerHost[DRAG_LISTENER_KEY] = { dragstart, dragend, drop }
+  dragListenersAttached = true
+}
 
 export function getHiddenContainer(): HTMLDivElement {
   if (!hiddenContainer) {
@@ -34,37 +85,19 @@ export function setWebviewsDragPassthrough(passthrough: boolean): void {
   }
 }
 
-const DRAG_LISTENER_KEY = '__orcaBrowserPaneDragListeners'
+export function registerPersistentWebview(
+  browserTabId: string,
+  webview: Electron.WebviewTag
+): void {
+  webviewRegistry.set(browserTabId, webview)
+  ensureDragListeners()
+}
 
-// Why: vitest 'node' env stubs `window` as a plain object via vi.stubGlobal,
-// so `typeof window !== 'undefined'` is true but addEventListener is missing.
-// Gate on the function we actually call so importing this module from a hook
-// test (which transitively pulls us in) does not throw at module load.
-if (typeof window !== 'undefined' && typeof window.addEventListener === 'function') {
-  type DragListenerRegistry = {
-    dragstart: () => void
-    dragend: () => void
-    drop: () => void
+export function unregisterPersistentWebview(browserTabId: string): void {
+  webviewRegistry.delete(browserTabId)
+  if (webviewRegistry.size === 0) {
+    removeDragListeners()
   }
-  const listenerHost = window as Window & { [DRAG_LISTENER_KEY]?: DragListenerRegistry }
-  const existingListeners = listenerHost[DRAG_LISTENER_KEY]
-  if (existingListeners) {
-    window.removeEventListener('dragstart', existingListeners.dragstart, true)
-    window.removeEventListener('dragend', existingListeners.dragend, true)
-    window.removeEventListener('drop', existingListeners.drop, true)
-  }
-
-  const dragstart = (): void => setWebviewsDragPassthrough(true)
-  const dragend = (): void => setWebviewsDragPassthrough(false)
-  const drop = (): void => setWebviewsDragPassthrough(false)
-
-  window.addEventListener('dragstart', dragstart, true)
-  window.addEventListener('dragend', dragend, true)
-  window.addEventListener('drop', drop, true)
-  // Why: BrowserPane installs process-wide drag listeners so parked webviews
-  // stop swallowing drop targets. We store/remove the previous handlers on
-  // window to keep Vite HMR from stacking duplicates across module reloads.
-  listenerHost[DRAG_LISTENER_KEY] = { dragstart, dragend, drop }
 }
 
 export function destroyPersistentWebview(browserTabId: string): void {
@@ -85,7 +118,7 @@ export function destroyPersistentWebview(browserTabId: string): void {
     window.focus()
   }
   webview.remove()
-  webviewRegistry.delete(browserTabId)
+  unregisterPersistentWebview(browserTabId)
   registeredWebContentsIds.delete(browserTabId)
   parkedAtByTabId.delete(browserTabId)
   clearLiveBrowserUrl(browserTabId)