diff --git a/apps/desktop/electron/main.ts b/apps/desktop/electron/main.ts index 0f3ad0c6aa..df968812bd 100644 --- a/apps/desktop/electron/main.ts +++ b/apps/desktop/electron/main.ts @@ -540,7 +540,7 @@ import { import { registerWindowControlIpc, windowControlState } from './window-controls' import { createWindowOpenHandler } from './window-open-policy' import { installWindowRendererLifecycle } from './window-renderer-lifecycle' -import { createWindowRevealController } from './window-reveal' +import { wireWindowReveal } from './window-reveal' import { bindGeometryPersistence, computeWindowOptions, @@ -12959,23 +12959,8 @@ function installPreviewGuestPreload() { // though the renderer finished loading. Keep the themed path as the preferred // reveal, then fall back a few seconds after the renderer loads. `show` and // `onRevealed` carry the caller's reveal action and post-visible work; whichever -// path wins runs them exactly once. -function wireWindowReveal(win, { show, onRevealed }: { show?: () => void; onRevealed?: () => void } = {}) { - const controller = createWindowRevealController( - { - isDestroyed: () => win.isDestroyed(), - isVisible: () => win.isVisible(), - show: show ?? (() => win.show()) - }, - { onRevealed } - ) - - win.once('ready-to-show', controller.reveal) - win.webContents.once('did-finish-load', controller.scheduleFallback) - win.on('closed', controller.dispose) - - return controller -} +// path wins runs them exactly once. Callers that pass `onRevealFailed` also get +// the pre-paint failure branch (see window-reveal.ts). // Secondary "session windows" — one extra OS window per chat so a user can // work with multiple chats side by side. The registry guarantees one window @@ -13939,6 +13924,17 @@ function spawnHudWindow(sessionId, profile) { // Compositor overlay adapters (Hyprland float+pin today). Electron // alwaysOnTop is already set; this is the dialect some WMs actually hear. void promoteHudOverlay({ title: HUD_WINDOW_TITLE }) + }, + // #108230: the HUD is born `show: false` + transparent, and its renderer + // lifecycle is deliberately log-only (#81290 — a dead renderer should be + // diagnosable, not resurrected). But a load failure or renderer crash + // BEFORE first paint leaves a hidden window every toggle claims is open. + // Tear it down instead: requestHudClose is bounded, and the 'closed' + // handler below owns the one teardown path (snap shortcut, main-window + // restore, broadcastHudState(false)) so the toggles converge to closed. + onRevealFailed: reason => { + rememberLog(`[renderer:hud] window never revealed; tearing it down (${reason})`) + destroyHudWindow(win) } }) diff --git a/apps/desktop/electron/window-reveal.test.ts b/apps/desktop/electron/window-reveal.test.ts index 5fe28b90f3..e07d5071b0 100644 --- a/apps/desktop/electron/window-reveal.test.ts +++ b/apps/desktop/electron/window-reveal.test.ts @@ -2,7 +2,7 @@ import assert from 'node:assert/strict' import { test } from 'vitest' -import { createWindowRevealController } from './window-reveal' +import { createWindowRevealController, wireWindowReveal } from './window-reveal' function createHarness({ visible = false }: { visible?: boolean } = {}) { let destroyed = false @@ -186,3 +186,174 @@ test('reveals without an onRevealed callback', () => { assert.equal(controller.reveal(), true) assert.equal(shown, true) }) + +// ── Reveal failure branch (#108230) ───────────────────────────────────────── +// +// A window created `show: false` is only revealed by success-shaped events +// (ready-to-show / did-finish-load). When the main frame fails to load or the +// render process dies before first paint, none of those fire — the fallback +// timer is never even scheduled — so the window stays hidden forever while +// callers keep claiming it is open. The failure branch hands such windows to +// `onRevealFailed` exactly once and disarms every other path. + +type FailureHarness = { + emit(event: 'closed' | 'did-finish-load' | 'did-fail-load' | 'render-process-gone', ...args: unknown[]): void + emitReadyToShow(): void + failures: string[] + revealCalls: number + scheduledCallback: (() => void) | null + showCalls: number + visible: () => boolean +} + +function createFailureHarness(): FailureHarness { + const windowListeners = new Map void>>() + const webContentsListeners = new Map void>>() + let visible = false + const failures: string[] = [] + let revealCalls = 0 + let scheduledCallback: (() => void) | null = null + + const win = { + isDestroyed: () => false, + isVisible: () => visible, + show: () => { + visible = true + }, + once: (event: string, listener: (...args: any[]) => void) => { + windowListeners.set(`once:${event}`, [...(windowListeners.get(`once:${event}`) ?? []), listener]) + }, + on: (event: string, listener: (...args: any[]) => void) => { + windowListeners.set(`on:${event}`, [...(windowListeners.get(`on:${event}`) ?? []), listener]) + }, + webContents: { + once: (event: string, listener: (...args: any[]) => void) => { + webContentsListeners.set(`once:${event}`, [...(webContentsListeners.get(`once:${event}`) ?? []), listener]) + }, + on: (event: string, listener: (...args: any[]) => void) => { + webContentsListeners.set(`on:${event}`, [...(webContentsListeners.get(`on:${event}`) ?? []), listener]) + } + } + } + + wireWindowReveal(win, { + onRevealed: () => { + revealCalls += 1 + }, + onRevealFailed: reason => { + failures.push(reason) + }, + setTimer: callback => { + scheduledCallback = callback + + return 1 as unknown as ReturnType + }, + clearTimer: () => {} + }) + + const emit = ( + target: 'window' | 'webContents', + kind: 'once' | 'on', + event: string, + ...args: unknown[] + ) => { + const key = `${kind}:${event}` + const map = target === 'window' ? windowListeners : webContentsListeners + + for (const listener of [...(map.get(key) ?? [])]) { + listener(...args) + } + } + + return { + emit: (event, ...args) => { + if (event === 'closed') { + emit('window', 'on', event, ...args) + } else if (event === 'did-finish-load') { + emit('webContents', 'once', event, ...args) + } else { + emit('webContents', 'on', event, ...args) + } + }, + emitReadyToShow: () => emit('window', 'once', 'ready-to-show'), + failures, + get revealCalls() { + return revealCalls + }, + get scheduledCallback() { + return scheduledCallback + }, + showCalls: 0, + visible: () => visible + } +} + +test('a main-frame load failure before reveal fires onRevealFailed once and disarms the reveal', () => { + const harness = createFailureHarness() + + // The main frame failed: did-finish-load never fires, so no fallback is + // scheduled — without the failure branch nothing would ever happen again. + harness.emit('did-fail-load', {}, -3, 'ABORTED', 'file:///hud', true) + + assert.equal(harness.failures.length, 1) + assert.match(harness.failures[0]!, /main frame/) + + // Disarmed: a late ready-to-show must not show a dead window, and a second + // failure must not re-fire the teardown. + harness.emitReadyToShow() + assert.equal(harness.visible(), false) + assert.equal(harness.revealCalls, 0) + + harness.emit('render-process-gone', {}, { reason: 'oom' }) + assert.equal(harness.failures.length, 1) +}) + +test('a subframe load failure never tears the window down', () => { + const harness = createFailureHarness() + + harness.emit('did-fail-load', {}, -3, 'ABORTED', 'file:///subframe.js', false) + harness.emitReadyToShow() + + assert.deepEqual(harness.failures, []) + assert.equal(harness.visible(), true) +}) + +test('a render-process crash before first paint fires onRevealFailed', () => { + const harness = createFailureHarness() + + harness.emit('render-process-gone', {}, { reason: 'crashed' }) + + assert.equal(harness.failures.length, 1) + assert.match(harness.failures[0]!, /render process gone/) +}) + +test('a crash or load failure AFTER reveal leaves the window alone', () => { + const harness = createFailureHarness() + + harness.emitReadyToShow() + assert.equal(harness.visible(), true) + + // Post-reveal, the window's own lifecycle owns recovery — the reveal + // controller must not resurrect its failure branch for a shown window. + harness.emit('render-process-gone', {}, { reason: 'crashed' }) + harness.emit('did-fail-load', {}, -3, 'ABORTED', 'file:///hud', true) + + assert.deepEqual(harness.failures, []) + assert.equal(harness.revealCalls, 1) +}) + +test('a scheduled fallback is cancelled when the load fails first', () => { + const harness = createFailureHarness() + + // did-finish-load arrived (fallback scheduled at 4s), then the process died + // before first paint: the timer must not later reveal a dead window. + harness.emit('did-finish-load') + assert.ok(harness.scheduledCallback) + + harness.emit('render-process-gone', {}, { reason: 'kill' }) + harness.scheduledCallback?.() + + assert.equal(harness.failures.length, 1) + assert.equal(harness.visible(), false) + assert.equal(harness.revealCalls, 0) +}) diff --git a/apps/desktop/electron/window-reveal.ts b/apps/desktop/electron/window-reveal.ts index 6f2ea48da1..db289a348f 100644 --- a/apps/desktop/electron/window-reveal.ts +++ b/apps/desktop/electron/window-reveal.ts @@ -8,6 +8,8 @@ type TimerHandle = ReturnType type WindowRevealOptions = { onRevealed?: () => void + /** Called once when the window fails to reach first paint (see fail below). */ + onRevealFailed?: (reason: string) => void delayMs?: number setTimer?: (callback: () => void, delayMs: number) => TimerHandle clearTimer?: (timer: TimerHandle) => void @@ -19,6 +21,7 @@ export function createWindowRevealController( window: WindowRevealTarget, { onRevealed = () => {}, + onRevealFailed = () => {}, delayMs = WINDOW_REVEAL_FALLBACK_MS, setTimer = (callback, delay) => setTimeout(callback, delay), clearTimer = timer => clearTimeout(timer) @@ -26,6 +29,7 @@ export function createWindowRevealController( ) { let disposed = false let revealed = false + let failed = false let fallbackTimer: TimerHandle | null = null const cancelFallback = () => { @@ -38,7 +42,7 @@ export function createWindowRevealController( } const reveal = () => { - if (disposed || revealed || window.isDestroyed()) { + if (disposed || revealed || failed || window.isDestroyed()) { return false } @@ -55,7 +59,7 @@ export function createWindowRevealController( } const scheduleFallback = () => { - if (disposed || revealed || fallbackTimer !== null || window.isDestroyed()) { + if (disposed || revealed || failed || fallbackTimer !== null || window.isDestroyed()) { return } @@ -65,6 +69,28 @@ export function createWindowRevealController( }, delayMs) } + /** + * The reveal gate has no success-shaped event left to hang on: the main + * frame failed to load or the render process died before first paint, so + * neither `ready-to-show` nor `did-finish-load` will ever fire — and a + * `show: false` window would stay hidden forever while callers keep + * claiming it is open (#108230). Hand it to `onRevealFailed` once and + * disarm every other path. A window that already revealed keeps its own + * caller-chosen lifecycle: a post-reveal crash is not this branch's + * business. + */ + const fail = (reason: string) => { + if (disposed || revealed || failed || window.isDestroyed()) { + return false + } + + failed = true + cancelFallback() + onRevealFailed(reason) + + return true + } + const dispose = () => { disposed = true cancelFallback() @@ -72,7 +98,84 @@ export function createWindowRevealController( return { dispose, + fail, reveal, scheduleFallback } } + +/** Minimal event-emitter shape wireWindowReveal listens on (a BrowserWindow + * satisfies it; tests pass fakes). */ +type RevealEvents = { + once(event: string, listener: (...args: unknown[]) => void): unknown + on(event: string, listener: (...args: unknown[]) => void): unknown +} + +export type WindowRevealWindow = WindowRevealTarget & + RevealEvents & { + webContents: RevealEvents & { + on( + event: 'did-fail-load', + listener: ( + event: unknown, + errorCode: number, + errorDescription: string, + validatedURL: string, + isMainFrame: boolean + ) => void + ): unknown + on(event: 'render-process-gone', listener: (event: unknown, details: { reason?: string }) => void): unknown + } + } + +/** + * Wire a `show: false` window's reveal to the load lifecycle: reveal on + * `ready-to-show`, arm the bounded fallback on `did-finish-load`, and — only + * for callers that pass `onRevealFailed` — catch the two pre-paint death + * modes (`did-fail-load` on the main frame, `render-process-gone`) so a + * failed window is handed to the caller once instead of staying hidden + * forever. + */ +export function wireWindowReveal( + win: WindowRevealWindow, + { + show, + onRevealed, + onRevealFailed, + ...timing + }: { + show?: () => void + onRevealed?: () => void + onRevealFailed?: (reason: string) => void + } & Omit = {} +) { + const controller = createWindowRevealController( + { + isDestroyed: () => win.isDestroyed(), + isVisible: () => win.isVisible(), + show: show ?? (() => win.show()) + }, + { onRevealed, onRevealFailed, ...timing } + ) + + win.once('ready-to-show', controller.reveal) + win.webContents.once('did-finish-load', controller.scheduleFallback) + win.on('closed', controller.dispose) + + if (onRevealFailed) { + win.webContents.on( + 'did-fail-load', + (_event, errorCode, errorDescription, _validatedURL, isMainFrame) => { + if (isMainFrame) { + controller.fail(`main frame failed to load (${errorCode}: ${errorDescription})`) + } + } + ) + + win.webContents.on('render-process-gone', (_event, details) => { + controller.fail(`render process gone (${details?.reason ?? 'unknown'})`) + }) + } + + return controller +}