refactor(desktop): centralize auxiliary window lifecycle

apps/desktop/scripts/workhub-browser-presentation-smoke.mjs

@@ -63,8 +63,8 @@ async function run() {
   await main.loadURL(url);
   let owner;
   let ownerParent;
-  let parentResolver;
-  const views = new BrowserViewManager({ create: (id) => new BrowserViewController(parentResolver(id), id, () => {}) });
+  let parentForSession;
+  const views = new BrowserViewManager({ create: (id) => new BrowserViewController(parentForSession(id), id, () => {}) });
   const presentation = createWorkHubPresentation({
     mainWindow: () => main.isDestroyed() ? undefined : main,
     ensureMainWindow: async () => main,
@@ -75,11 +75,14 @@ async function run() {
   presentation.attachMainWindow(main);
   registerBrowserIpc({
     mainWindowController: {
-      getBrowserViews: () => views,
-      ownsRenderer: (wc) => !wc.isDestroyed() && (wc === owner || (!main.isDestroyed() && wc === main.webContents)),
+      getBrowserViews: (resolve) => {
+        parentForSession = resolve;
+        return views;
+      },
       isMainRenderer: (wc) => !main.isDestroyed() && wc === main.webContents,
-      browserParentForRenderer: (wc) => wc === owner ? ownerParent : main.contentView,
-      setBrowserViewParentResolver: (resolve) => { parentResolver = resolve; },
+    },
+    auxiliaryWindowRegistry: {
+      rendererParent: (wc) => wc === owner ? ownerParent : undefined,
     },
     isHostActive: (ref) => ref.hostId === scope.hostId && ref.targetEpoch === scope.targetEpoch,
   });
@@ -164,8 +167,13 @@ async function run() {
     await capture('background-restored', true);
     await command(owner, 'dock');
     await capture('redocked', true);
-    await command(owner, 'detach');
-    main.close();
+    // Leave the conversation docked so the close handler exercises its
+    // production path for moving the live view to the floating window.
+    // Dispatch the close handlers directly before destroying the synthetic
+    // fixture window; BrowserWindow.close() can block in Xvfb while reparenting
+    // WebContentsView children during the native close handshake.
+    main.emit('close');
+    main.destroy();
     await wait(100);
     assert.ok(controller.hasParent(ownerParent));
     assert.equal(controller.state().hasPage, true);

apps/desktop/src/main/__tests__/auxiliary-window-registry.test.ts

@@ -0,0 +1,67 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import assert from 'node:assert/strict';
+import { EventEmitter } from 'node:events';
+import { test } from 'node:test';
+import { createAuxiliaryWindowRegistry } from '../auxiliary-window-registry.js';
+
+test('registry applies declarations, reveal mode, destroy, and renderer parents', () => {
+  class FakeWindow extends EventEmitter {
+    readonly webContents = new EventEmitter();
+    destroyed = false;
+    visible = false;
+    minimized = false;
+    shown = 0;
+    shownInactive = 0;
+    focused = 0;
+
+    constructor(readonly options: Electron.BrowserWindowConstructorOptions) { super(); }
+    isDestroyed() { return this.destroyed; }
+    isVisible() { return this.visible; }
+    isMinimized() { return this.minimized; }
+    show() { this.shown += 1; this.visible = true; }
+    showInactive() { this.shownInactive += 1; this.visible = true; }
+    focus() { this.focused += 1; }
+    restore() { this.minimized = false; }
+    destroy() { this.destroyed = true; this.emit('closed'); }
+  }
+  const registry = createAuxiliaryWindowRegistry(() => ({ BrowserWindow: FakeWindow } as never));
+  const overlay = registry.create('permission-overlay', { title: 'Maka' }) as unknown as FakeWindow;
+
+  assert.deepEqual(overlay.options, { title: 'Maka', show: false });
+
+  registry.show('permission-overlay', overlay as never, 'hidden');
+  assert.equal(overlay.shownInactive, 0);
+  registry.show('permission-overlay', overlay as never, 'active');
+  assert.equal(overlay.shownInactive, 1);
+  overlay.visible = false;
+  registry.focus(overlay as never, 'active');
+  assert.deepEqual({ shown: overlay.shown, focused: overlay.focused }, { shown: 1, focused: 1 });
+
+  const contents = Object.assign(new EventEmitter(), { isDestroyed: () => false });
+  const parent = { kind: 'workhub-container' };
+  registry.registerRenderer(contents as never, parent as never);
+  assert.equal(registry.rendererParent(contents as never), parent);
+  contents.emit('destroyed');
+  assert.equal(registry.rendererParent(contents as never), undefined);
+
+  registry.destroy(overlay as never);
+  assert.equal(overlay.destroyed, true);
+});

apps/desktop/src/main/__tests__/browser-ipc-main.test.ts

@@ -88,6 +88,7 @@ test('browser IPC isolates owned renderer documents and their native parents', a
   class FakeWindow extends EventEmitter {
     visible = true;
     minimized = false;
+    contentView: Electron.View | undefined;
     isVisible(): boolean { return this.visible; }
     isMinimized(): boolean { return this.minimized; }
     isDestroyed(): boolean { return false; }
@@ -115,13 +116,13 @@ test('browser IPC isolates owned renderer documents and their native parents', a
     const workHub = new FakeRenderer('workhub-document-frame', 1.25);
     const mainWindow = new FakeWindow();
     const floatingWindow = new FakeWindow();
+    const mainParent = { getVisible: () => true } as unknown as Electron.View;
+    mainWindow.contentView = mainParent;
     windows.set(main, mainWindow);
     windows.set(workHub, mainWindow);
     let workHubVisible = true;
-    const mainParent = { getVisible: () => true } as unknown as Electron.View;
     const workHubParent = { getVisible: () => workHubVisible } as unknown as Electron.View;
     const owned = new Map<Electron.WebContents, Electron.View>([
-      [main as unknown as Electron.WebContents, mainParent],
       [workHub as unknown as Electron.WebContents, workHubParent],
     ]);
     let hostActive = true;
@@ -152,14 +153,18 @@ test('browser IPC isolates owned renderer documents and their native parents', a
       },
     };
     const mainWindowController = {
-      getBrowserViews: () => manager,
+      getBrowserViews: (resolve: typeof parentResolver) => {
+        parentResolver = resolve;
+        return manager;
+      },
       isMainRenderer: (contents: Electron.WebContents) => contents === main as unknown as Electron.WebContents,
-      ownsRenderer: (contents: Electron.WebContents) => !contents.isDestroyed() && owned.has(contents),
-      browserParentForRenderer: (contents: Electron.WebContents) => owned.get(contents),
-      setBrowserViewParentResolver: (resolve: typeof parentResolver) => { parentResolver = resolve; },
     };
     const browserIpc = registerBrowserIpc({
       mainWindowController: mainWindowController as never,
+      auxiliaryWindowRegistry: {
+        rendererParent: (contents: Electron.WebContents) =>
+          !contents.isDestroyed() ? owned.get(contents) : undefined,
+      },
       isHostActive: (candidate) => hostActive && candidate.hostId === scope.hostId && candidate.targetEpoch === scope.targetEpoch,
     });
 

apps/desktop/src/main/__tests__/workhub-presentation.test.ts

@@ -502,13 +502,20 @@ test('rejects unowned/subframe IPC and buffers navigation until main subscribes'
   h.controller.dispose();
 });
 
-test('application broadcasts reach registered auxiliaries once and stop after release or destruction', async () => {
+test('application broadcasts include registered auxiliary renderers', async () => {
   const entry = fileURLToPath(new URL('../../../src/main/main-window.ts', import.meta.url));
   const output = await build({ entryPoints: [entry], bundle: false, write: false, format: 'cjs', platform: 'node', define: { 'import.meta.dirname': JSON.stringify('/app/dist/main') } });
   const module = { exports: {} as { createMainWindowController: typeof createMainWindowController } };
+  const auxiliaries = new Set<Electron.WebContents>();
   runInNewContext(output.outputFiles[0]!.text, {
     module, exports: module.exports, process,
-    require: () => ({ createWindowRevealGate: () => ({}) }),
+    require: (specifier: string) => specifier.endsWith('/auxiliary-window-registry.js')
+      ? {
+        auxiliaryWindowRegistry: {
+          renderers: () => [...auxiliaries],
+        },
+      }
+      : { createWindowRevealGate: () => ({}) },
   });
   const controller = module.exports.createMainWindowController({
     workspaceRoot: '/workspace', e2eFixture: null, revealMode: 'hidden',
@@ -520,20 +527,10 @@ test('application broadcasts reach registered auxiliaries once and stop after re
     isDestroyed: () => false,
     send: (channel: string) => { messages.push(channel); },
   }) as unknown as Electron.WebContents;
-  const parent = {} as Electron.View;
-  const release = controller.registerAuxiliaryRenderer(renderer, parent);
-  assert.equal(controller.ownsRenderer(renderer), true);
-  assert.equal(controller.browserParentForRenderer(renderer), parent);
+  auxiliaries.add(renderer);
   controller.send('settings:changed');
   assert.deepEqual(messages, ['settings:changed']);
-  release();
-  controller.send('settings:changed');
-  assert.deepEqual(messages, ['settings:changed']);
-  assert.equal(controller.ownsRenderer(renderer), false);
-  assert.equal(controller.browserParentForRenderer(renderer), undefined);
-  controller.registerAuxiliaryRenderer(renderer);
-  renderer.emit('destroyed');
-  assert.equal(controller.ownsRenderer(renderer), false);
+  auxiliaries.delete(renderer);
   controller.send('settings:changed');
   assert.deepEqual(messages, ['settings:changed']);
 });

apps/desktop/src/main/auxiliary-window-registry.ts

@@ -0,0 +1,96 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import { createRequire } from 'node:module';
+import type { BrowserWindow, BrowserWindowConstructorOptions, View, WebContents } from 'electron';
+import { focusWindow, showWindowInactive, type WindowRevealMode } from './window-reveal.js';
+
+function loadElectron(): typeof import('electron') {
+  try {
+    return (0, eval)('require')('electron') as typeof import('electron');
+  } catch {
+    return createRequire(process.execPath)('electron') as typeof import('electron');
+  }
+}
+
+const declarations = {
+  'permission-overlay': {
+    size: {},
+    reveal: showWindowInactive,
+  },
+  workhub: {
+    size: {},
+    reveal: showWindowInactive,
+  },
+  'cursor-overlay': {
+    size: {},
+    reveal: showWindowInactive,
+  },
+  pip: {
+    size: {},
+    reveal: showWindowInactive,
+  },
+} as const;
+
+export type AuxiliaryWindowId = keyof typeof declarations;
+
+export function createAuxiliaryWindowRegistry(
+  electron: () => typeof import('electron') = loadElectron,
+) {
+  const rendererParents = new Map<WebContents, View>();
+
+  return {
+    create(id: AuxiliaryWindowId, options: BrowserWindowConstructorOptions): BrowserWindow {
+      const declaration = declarations[id];
+      const window = new (electron().BrowserWindow)({
+        ...declaration.size,
+        ...options,
+        show: false,
+      });
+      return window;
+    },
+    show(id: AuxiliaryWindowId, window: BrowserWindow | undefined, mode: WindowRevealMode): void {
+      declarations[id].reveal(window ?? null, mode);
+    },
+    focus(window: BrowserWindow | undefined, mode: WindowRevealMode): void {
+      focusWindow(window ?? null, mode);
+    },
+    destroy(window: BrowserWindow | undefined): void {
+      if (window && !window.isDestroyed()) window.destroy();
+    },
+    registerRenderer(contents: WebContents, parent: View): () => void {
+      if (contents.isDestroyed()) return () => undefined;
+      rendererParents.set(contents, parent);
+      const release = (): void => {
+        rendererParents.delete(contents);
+        contents.removeListener('destroyed', release);
+      };
+      contents.once('destroyed', release);
+      return release;
+    },
+    rendererParent(contents: WebContents): View | undefined {
+      return contents.isDestroyed() ? undefined : rendererParents.get(contents);
+    },
+    renderers(): IterableIterator<WebContents> {
+      return rendererParents.keys();
+    },
+  };
+}
+
+export const auxiliaryWindowRegistry = createAuxiliaryWindowRegistry();