diff --git a/apps/desktop/e2e/fixtures.ts b/apps/desktop/e2e/fixtures.ts index fee8e41970325..787be421886b7 100644 --- a/apps/desktop/e2e/fixtures.ts +++ b/apps/desktop/e2e/fixtures.ts @@ -654,10 +654,10 @@ export async function waitForAppReady(fixture: MockBackendFixture | NoProviderFi // On Electron 40.x, ready-to-show may never fire (electron/electron#51972) // and the window stays hidden even though the DOM is rendered. The main - // process has a TEST_WORKER_INDEX-gated fallback that force-shows the - // window, but the DOM can be ready before that fires. Poll until the - // window is actually visible so interactions (click, screenshot) don't - // hit a hidden surface. + // process reveals it anyway — immediately under TEST_WORKER_INDEX, and via + // wireWindowReveal's post-load fallback in production — but the DOM can be + // ready before that lands. Poll until the window is actually visible so + // interactions (click, screenshot) don't hit a hidden surface. if (app) { const deadline = Date.now() + timeoutMs diff --git a/apps/desktop/electron/main.ts b/apps/desktop/electron/main.ts index 1658e3f132658..ed61fabb26012 100644 --- a/apps/desktop/electron/main.ts +++ b/apps/desktop/electron/main.ts @@ -136,7 +136,6 @@ import { } from './hardening' import { createLinkTitleWindow, guardLinkTitleSession, readLinkTitleWindowTitle } from './link-title-window' import { ensureMainWindow } from './main-window-lifecycle' -import { createMainWindowRevealController } from './main-window-reveal' import { oauthGuardMayHardFail, oauthSessionIsLive, @@ -211,6 +210,7 @@ import { formatBlockerMessage, formatProbeFailedMessage, scanVenvBlockers } from import { fetchMarketplaceThemes, searchMarketplaceThemes } from './vscode-marketplace' import { createWakeIndicatorWindowController } from './wake-indicator-window' import { readWindowBelow } from './window-below' +import { createWindowRevealController } from './window-reveal' import { computeWindowOptions, debounce, @@ -8726,6 +8726,31 @@ function wireCommonWindowHandlers(win, { zoom = true }: { zoom?: boolean } = {}) }) } +// Every window we open starts with `show: false` so the renderer's first themed +// paint lands before it appears, and `ready-to-show` is what reveals it. +// Electron 40 can drop that event entirely (electron/electron#51972) on +// Linux/Wayland, remote displays and VMs, leaving the window hidden forever even +// 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 +} + // 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 // per sessionId (re-opening focuses the existing window) and self-cleans on @@ -8779,11 +8804,7 @@ function spawnSecondaryWindow({ sessionId, watch }: { sessionId?: string; watch? win.setWindowButtonPosition?.(WINDOW_BUTTON_POSITION) } - win.once('ready-to-show', () => { - if (!win.isDestroyed()) { - win.show() - } - }) + wireWindowReveal(win) win.on('enter-full-screen', () => sendWindowStateChanged(true)) win.on('leave-full-screen', () => sendWindowStateChanged(false)) @@ -8861,11 +8882,7 @@ function createInstanceWindow() { win.setWindowButtonPosition?.(WINDOW_BUTTON_POSITION) } - win.once('ready-to-show', () => { - if (!win.isDestroyed()) { - win.show() - } - }) + wireWindowReveal(win) // Per-window fullscreen chrome: send this window its own titlebar inset so its // traffic lights hide/show independently of the primary. @@ -8983,11 +9000,7 @@ function spawnPetOverlayWindow(bounds) { // owns its window-fit + scale, and inheriting zoom would crop the sprite. wireCommonWindowHandlers(win, zoomWiringForWindowKind('petOverlay')) - win.once('ready-to-show', () => { - if (!win.isDestroyed()) { - win.showInactive() - } - }) + wireWindowReveal(win, { show: () => win.showInactive() }) win.on('closed', () => { if (petOverlayWindow === win) { @@ -9232,17 +9245,16 @@ function spawnHudWindow(sessionId) { win.on('moved', schedulePersistHudState) win.on('resized', schedulePersistHudState) - win.once('ready-to-show', () => { - if (win.isDestroyed()) { - return - } - - win.show() - win.focus() - - // Step the app aside: the HUD IS the surface now. - if (hudRestoreMainWindow && mainWindow && !mainWindow.isDestroyed()) { - mainWindow.hide() + wireWindowReveal(win, { + show: () => { + win.show() + win.focus() + }, + onRevealed: () => { + // Step the app aside: the HUD IS the surface now. + if (hudRestoreMainWindow && mainWindow && !mainWindow.isDestroyed()) { + mainWindow.hide() + } } }) @@ -9455,11 +9467,15 @@ function repositionQuickEntryWindow(win) { function showQuickEntryWindow() { if (!quickEntryWindow || quickEntryWindow.isDestroyed()) { - quickEntryWindow = spawnQuickEntryWindow() - quickEntryWindow.once('ready-to-show', () => { - if (!quickEntryWindow?.isDestroyed()) { - quickEntryWindow.show() - quickEntryWindow.focus() + // Reveal the window this call created, not whatever `quickEntryWindow` + // points at by the time the event lands. + const win = spawnQuickEntryWindow() + quickEntryWindow = win + + wireWindowReveal(win, { + show: () => { + win.show() + win.focus() } }) @@ -9582,7 +9598,7 @@ function createWindow() { mainWindow.maximize() } - const revealController = createMainWindowRevealController(createdMainWindow, { + const revealController = wireWindowReveal(createdMainWindow, { onRevealed: () => { // Persist geometry as soon as the window is visible so a crash before the // first clean resize/move/close still captures the restored bounds (#56726). @@ -9609,8 +9625,6 @@ function createWindow() { } }) - mainWindow.once('ready-to-show', revealController.reveal) - // Under Playwright testing, instantly show the window: `ready-to-show` // doesn't fire in some testing envs, and the suite can't wait out the // production fallback. @@ -9637,7 +9651,6 @@ function createWindow() { // the closed wrapper remains truthy, so clear only the window this callback owns. mainWindow.on('closed', () => { - revealController.dispose() closePetOverlay() wakeIndicatorController.close() @@ -9747,11 +9760,6 @@ function createWindow() { startHermes().catch(error => rememberLog(error.stack || error.message)) mainWindow.webContents.once('did-finish-load', () => { - // Electron 40 can omit ready-to-show even after the renderer has loaded, - // leaving the primary window hidden indefinitely. Give the normal themed - // paint path four seconds to win, then reveal the same window as a fallback. - revealController.scheduleFallback() - // Zoom restore is handled by wireCommonWindowHandlers (shared with session // windows); no need to reapply it here. broadcastBootProgress() diff --git a/apps/desktop/electron/main-window-reveal.test.ts b/apps/desktop/electron/window-reveal.test.ts similarity index 64% rename from apps/desktop/electron/main-window-reveal.test.ts rename to apps/desktop/electron/window-reveal.test.ts index e42768d383c41..5fe28b90f3a16 100644 --- a/apps/desktop/electron/main-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 { createMainWindowRevealController } from './main-window-reveal' +import { createWindowRevealController } from './window-reveal' function createHarness({ visible = false }: { visible?: boolean } = {}) { let destroyed = false @@ -13,7 +13,7 @@ function createHarness({ visible = false }: { visible?: boolean } = {}) { let scheduledDelay: number | null = null let clearCalls = 0 - const controller = createMainWindowRevealController( + const controller = createWindowRevealController( { isDestroyed: () => destroyed, isVisible: () => isVisible, @@ -128,3 +128,61 @@ test('does not reveal a destroyed window', () => { harness.controller.scheduleFallback() assert.equal(harness.scheduledCallback, null) }) + +// The pet overlay reveals with showInactive() and the HUD with show() + focus(), +// so the fallback has to run the caller's action rather than a plain show(). +test('the fallback runs the caller reveal action, not a plain show', () => { + const actions: string[] = [] + let scheduled: (() => void) | null = null + + const controller = createWindowRevealController( + { + isDestroyed: () => false, + isVisible: () => false, + show: () => actions.push('showInactive') + }, + { + onRevealed: () => actions.push('revealed'), + setTimer: callback => { + scheduled = callback + + return 1 as unknown as ReturnType + }, + clearTimer: () => {} + } + ) + + controller.scheduleFallback() + scheduled?.() + + assert.deepEqual(actions, ['showInactive', 'revealed']) +}) + +// The HUD hides the main window from onRevealed — whichever path wins, that +// side effect has to happen exactly once. +test('onRevealed side effects run once when both paths fire', () => { + const harness = createHarness() + + harness.controller.scheduleFallback() + harness.controller.reveal() + harness.scheduledCallback?.() + harness.controller.reveal() + + assert.equal(harness.revealCalls, 1) +}) + +// Session and instance windows have no post-visible work to do. +test('reveals without an onRevealed callback', () => { + let shown = false + + const controller = createWindowRevealController({ + isDestroyed: () => false, + isVisible: () => shown, + show: () => { + shown = true + } + }) + + assert.equal(controller.reveal(), true) + assert.equal(shown, true) +}) diff --git a/apps/desktop/electron/main-window-reveal.ts b/apps/desktop/electron/window-reveal.ts similarity index 79% rename from apps/desktop/electron/main-window-reveal.ts rename to apps/desktop/electron/window-reveal.ts index 40206ea0177f6..6f2ea48da100c 100644 --- a/apps/desktop/electron/main-window-reveal.ts +++ b/apps/desktop/electron/window-reveal.ts @@ -1,4 +1,4 @@ -type MainWindowRevealTarget = { +type WindowRevealTarget = { isDestroyed: () => boolean isVisible: () => boolean show: () => void @@ -6,23 +6,23 @@ type MainWindowRevealTarget = { type TimerHandle = ReturnType -type MainWindowRevealOptions = { - onRevealed: () => void +type WindowRevealOptions = { + onRevealed?: () => void delayMs?: number setTimer?: (callback: () => void, delayMs: number) => TimerHandle clearTimer?: (timer: TimerHandle) => void } -export const MAIN_WINDOW_REVEAL_FALLBACK_MS = 4_000 +export const WINDOW_REVEAL_FALLBACK_MS = 4_000 -export function createMainWindowRevealController( - window: MainWindowRevealTarget, +export function createWindowRevealController( + window: WindowRevealTarget, { - onRevealed, - delayMs = MAIN_WINDOW_REVEAL_FALLBACK_MS, + onRevealed = () => {}, + delayMs = WINDOW_REVEAL_FALLBACK_MS, setTimer = (callback, delay) => setTimeout(callback, delay), clearTimer = timer => clearTimeout(timer) - }: MainWindowRevealOptions + }: WindowRevealOptions = {} ) { let disposed = false let revealed = false