diff --git a/.changes/electron-window-shown-on-load.md b/.changes/electron-window-shown-on-load.md new file mode 100644 index 000000000..49dc2ef01 --- /dev/null +++ b/.changes/electron-window-shown-on-load.md @@ -0,0 +1,8 @@ +--- +type: perf +area: electron +--- + +On Linux the desktop app's window no longer sometimes appears about a second +late at launch: it now opens as soon as the app has loaded, showing the +loading screen until the dashboard is ready. diff --git a/apps/electron-backend-e2e/src/journeys/journey-renderer-gate-client.ts b/apps/electron-backend-e2e/src/journeys/journey-renderer-gate-client.ts index 91387fc9d..61c0b2957 100644 --- a/apps/electron-backend-e2e/src/journeys/journey-renderer-gate-client.ts +++ b/apps/electron-backend-e2e/src/journeys/journey-renderer-gate-client.ts @@ -12,6 +12,8 @@ export interface JourneyRendererGateState { readonly gatedEpochMs: number | null; readonly gatedMethod: string | null; readonly passThroughLoads: number; + /** `did-finish-load` events kept from the app's listeners on about:blank. */ + readonly didFinishLoadHeldOnBlank: number; /** `ready-to-show` events dropped while the window was on about:blank. */ readonly readyToShowHeldOnBlank: number; readonly releasedEpochMs: number | null; diff --git a/apps/electron-backend-e2e/src/performance/journey-renderer-gate-client.spec.ts b/apps/electron-backend-e2e/src/performance/journey-renderer-gate-client.spec.ts index e203d4540..2cbae397d 100644 --- a/apps/electron-backend-e2e/src/performance/journey-renderer-gate-client.spec.ts +++ b/apps/electron-backend-e2e/src/performance/journey-renderer-gate-client.spec.ts @@ -14,6 +14,7 @@ function gate( blankLoadedEpochMs: 1_050, errors: [], gatedEpochMs: 1_020, + didFinishLoadHeldOnBlank: 1, gatedMethod: 'loadFile', passThroughLoads: 0, readyToShowHeldOnBlank: 1, diff --git a/apps/electron-backend-e2e/src/performance/journey-renderer-gate.cjs b/apps/electron-backend-e2e/src/performance/journey-renderer-gate.cjs index be80ae151..baca328c4 100644 --- a/apps/electron-backend-e2e/src/performance/journey-renderer-gate.cjs +++ b/apps/electron-backend-e2e/src/performance/journey-renderer-gate.cjs @@ -22,6 +22,11 @@ * therefore drops `ready-to-show` while the window is on `about:blank`; * Electron emits it again for the real document's first paint, because the * window is still hidden, which is the moment production sees. + * The app also shows its window at the main frame's `did-finish-load` + * when that comes first, so the gate keeps the app's `did-finish-load` + * listeners (those registered before the gated load) away from the + * about:blank load too. Electron's own listener that resolves + * `loadURL(about:blank)` is registered later and still runs. * * With `ipcMain` passed in, the gate also keeps the listeners registered * with `ipcMain.handle` for `TAPPED_IPC_CHANNELS`, so the test can call a @@ -53,6 +58,38 @@ function holdReadyToShowWhileBlank(window, state) { }; } +function holdDidFinishLoadWhileBlank(window, state) { + const contents = window.webContents; + if ( + !contents || + typeof contents.emit !== 'function' || + typeof contents.rawListeners !== 'function' + ) { + return; + } + const appListeners = contents.rawListeners('did-finish-load'); + const originalEmit = contents.emit; + contents.emit = function gatedContentsEmit(eventName, ...args) { + if (eventName !== 'did-finish-load' || !isShowingBlank(window)) { + return originalEmit.call(this, eventName, ...args); + } + state.didFinishLoadHeldOnBlank += 1; + const attached = this.rawListeners(eventName); + const held = appListeners.filter((listener) => + attached.includes(listener) + ); + for (const listener of held) this.removeListener(eventName, listener); + try { + return originalEmit.call(this, eventName, ...args); + } finally { + // Raw listeners keep their `once` wrappers, so a re-added once + // listener still fires once for the real document. + for (const listener of held) + this.prependListener(eventName, listener); + } + }; +} + function tapIpcHandlers(ipcMain, channels) { const handlers = new Map(); const originalHandle = ipcMain.handle; @@ -78,6 +115,7 @@ function installJourneyRendererGate(BrowserWindow, target, options = {}) { errors: [], gatedEpochMs: null, gatedMethod: null, + didFinishLoadHeldOnBlank: 0, passThroughLoads: 0, readyToShowHeldOnBlank: 0, releasedEpochMs: null, @@ -128,6 +166,7 @@ function installJourneyRendererGate(BrowserWindow, target, options = {}) { state.gatedEpochMs = now(); state.gatedMethod = method; holdReadyToShowWhileBlank(this, state); + holdDidFinishLoadWhileBlank(this, state); try { await this.webContents.loadURL(BLANK_URL); state.blankLoadedEpochMs = now(); diff --git a/apps/electron-backend-e2e/src/performance/journey-renderer-gate.spec.ts b/apps/electron-backend-e2e/src/performance/journey-renderer-gate.spec.ts index a6e44978d..8eb336dee 100644 --- a/apps/electron-backend-e2e/src/performance/journey-renderer-gate.spec.ts +++ b/apps/electron-backend-e2e/src/performance/journey-renderer-gate.spec.ts @@ -4,6 +4,7 @@ import test from 'node:test'; interface GateState { blankLoadedEpochMs: number | null; + didFinishLoadHeldOnBlank: number; errors: string[]; gatedEpochMs: number | null; gatedMethod: string | null; @@ -155,13 +156,13 @@ test('records a failed about:blank navigation and still loads after release', as function createEmittingBrowserWindow(log: string[]) { class EmittingBrowserWindow extends EventEmitter { url = ''; - webContents = { + webContents = Object.assign(new EventEmitter(), { getURL: () => this.url, loadURL: async (url: string) => { this.url = url; log.push(`webContents.loadURL:${url}`); }, - }; + }); async loadFile(file: string): Promise { this.url = `file:///${file}`; log.push(`loadFile:${file}`); @@ -201,6 +202,47 @@ test('holds ready-to-show while the window shows about:blank, then lets the real assert.equal(api.state.readyToShowHeldOnBlank, 1); }); +test('keeps did-finish-load of about:blank from the app listeners, not from later ones', async () => { + const log: string[] = []; + const EmittingBrowserWindow = createEmittingBrowserWindow(log); + const api = gateModule.installJourneyRendererGate( + EmittingBrowserWindow as unknown as { + prototype: Record; + }, + {}, + { timeoutMs: 60_000 } + ); + const window = new EmittingBrowserWindow(); + // The app shows its window at the first did-finish-load. + window.webContents.once('did-finish-load', () => + log.push('app:did-finish-load') + ); + const load = window.loadFile('index.html'); + await settle(); + // Registered after the gated load, like Electron's own listener that + // resolves loadURL(about:blank). + window.webContents.on('did-finish-load', () => + log.push('electron:did-finish-load') + ); + window.webContents.emit('did-finish-load'); + assert.equal(api.state.didFinishLoadHeldOnBlank, 1); + + api.release(); + await load; + window.webContents.emit('did-finish-load'); + window.webContents.emit('did-finish-load'); + + assert.deepEqual(log, [ + 'webContents.loadURL:about:blank', + 'electron:did-finish-load', + 'loadFile:index.html', + 'app:did-finish-load', + 'electron:did-finish-load', + 'electron:did-finish-load', + ]); + assert.equal(api.state.didFinishLoadHeldOnBlank, 1); +}); + test('taps ipcMain.handle for the counters channel and passes registrations through', async () => { const registered: string[] = []; const ipcMain: FakeIpcMain = { diff --git a/apps/electron-backend-e2e/src/performance/launch-journey-record.spec.ts b/apps/electron-backend-e2e/src/performance/launch-journey-record.spec.ts index 02bfe3d21..f2084d099 100644 --- a/apps/electron-backend-e2e/src/performance/launch-journey-record.spec.ts +++ b/apps/electron-backend-e2e/src/performance/launch-journey-record.spec.ts @@ -112,6 +112,7 @@ function measurement( blankLoadedEpochMs: 1_050, errors: [], gatedEpochMs: 1_020, + didFinishLoadHeldOnBlank: 1, gatedMethod: 'loadFile', passThroughLoads: 0, readyToShowHeldOnBlank: 1, @@ -185,6 +186,7 @@ test('maps the probe, IPC capture and main counters to exact counters and spawn- 'main.startupPhases': 9, }); assert.equal(record.evidence['rendererGateReadyToShowHeldOnBlank'], 1); + assert.equal(record.evidence['rendererGateDidFinishLoadHeldOnBlank'], 1); assert.deepEqual(record.evidence['epochs'], { firstCard: 2_600.04, firstCardPaint: 2_650, diff --git a/apps/electron-backend-e2e/src/performance/launch-journey-record.ts b/apps/electron-backend-e2e/src/performance/launch-journey-record.ts index 2349648fe..f0ec0b053 100644 --- a/apps/electron-backend-e2e/src/performance/launch-journey-record.ts +++ b/apps/electron-backend-e2e/src/performance/launch-journey-record.ts @@ -185,6 +185,8 @@ export function toLaunchIterationRecord( mainCountersAtRead: mainCounters.counters, rendererGateReadyToShowHeldOnBlank: measurement.gate.readyToShowHeldOnBlank, + rendererGateDidFinishLoadHeldOnBlank: + measurement.gate.didFinishLoadHeldOnBlank, ipcCallsByMethod: ipc.callsByMethod, ipcSerialDepth: serialDepth, ipcTimelineAmbiguousCompletions: ipc.ambiguousTimelineCompletions, diff --git a/apps/electron-backend/src/app/app.spec.ts b/apps/electron-backend/src/app/app.spec.ts index 7e597144d..f8f12e5bd 100644 --- a/apps/electron-backend/src/app/app.spec.ts +++ b/apps/electron-backend/src/app/app.spec.ts @@ -75,11 +75,14 @@ type MockMainWindow = { maximize: jest.Mock; on: jest.Mock void]>; once: jest.Mock void]>; + removeListener: jest.Mock void]>; setFullScreen: jest.Mock; setMenu: jest.Mock; show: jest.Mock; webContents: { on: jest.Mock void]>; + once: jest.Mock void]>; + removeListener: jest.Mock void]>; openDevTools: jest.Mock; setWindowOpenHandler: jest.Mock; getZoomLevel: jest.Mock; @@ -98,12 +101,18 @@ function createMockMainWindow(): MockMainWindow { maximize: jest.fn(), on: jest.fn void]>(), once: jest.fn void]>(), + removeListener: jest.fn void]>(), isDestroyed: jest.fn().mockReturnValue(false), setFullScreen: jest.fn(), setMenu: jest.fn(), show: jest.fn(), webContents: { on: jest.fn void]>(), + once: jest.fn void]>(), + removeListener: jest.fn< + void, + [string, (...args: unknown[]) => void] + >(), openDevTools: jest.fn(), setWindowOpenHandler: jest.fn(), getZoomLevel: jest.fn().mockReturnValue(0), @@ -366,6 +375,30 @@ describe('Electron app security helpers', () => { expect(mainWindow.show).toHaveBeenCalledTimes(1); }); + it('shows the window at did-finish-load when ready-to-show has not come yet', () => { + storeStartupWindowMode('maximized'); + const mainWindow = createWindowViaOnReady(); + + expect(BrowserWindow).toHaveBeenCalledWith( + expect.objectContaining({ + show: false, + backgroundColor: '#1f1f23', + }) + ); + const [loadHandler] = mainWindow.webContents.once.mock.calls + .filter(([eventName]) => eventName === 'did-finish-load') + .map(([, handler]) => handler); + loadHandler(); + + expect(mainWindow.maximize).toHaveBeenCalledTimes(1); + expect(mainWindow.show).toHaveBeenCalledTimes(1); + + // The later ready-to-show is a no-op. + fireReadyToShow(mainWindow); + expect(mainWindow.show).toHaveBeenCalledTimes(1); + expect(mainWindow.maximize).toHaveBeenCalledTimes(1); + }); + it('creates the window fullscreen when the stored mode says so', () => { storeStartupWindowMode('fullscreen'); diff --git a/apps/electron-backend/src/app/app.ts b/apps/electron-backend/src/app/app.ts index 09b927e6c..0573c4bba 100644 --- a/apps/electron-backend/src/app/app.ts +++ b/apps/electron-backend/src/app/app.ts @@ -15,6 +15,10 @@ import { trace, traceStartupPhase, } from './services/debug-trace'; +import { + MAIN_WINDOW_BACKGROUND_COLOR, + showMainWindowWhenLoaded, +} from './services/main-window-first-show'; import { attachMainWindowPerformanceCounters } from './services/performance-counters'; import { STARTUP_WINDOW_MODE, @@ -512,6 +516,9 @@ export default class App { width: width, height: height, show: false, + // The splash colour: the window can be shown before its first + // paint (main-window-first-show.ts). + backgroundColor: MAIN_WINDOW_BACKGROUND_COLOR, webPreferences: getMainWindowWebPreferences(), ...savedWindowBounds, // Fullscreen is a constructor option: the window is created @@ -543,15 +550,17 @@ export default class App { App.mainWindow.center(); } - // if main window is ready to show, close the splash window and show the main window - App.mainWindow.once('ready-to-show', () => { + // Shown at ready-to-show or did-finish-load, whichever comes first + // (see main-window-first-show.ts). + const mainWindow = App.mainWindow; + showMainWindowWhenLoaded(mainWindow, () => { // maximize() on a hidden window shows it (Electron docs), so it - // has to wait for ready-to-show like show() does — any earlier - // and a blank window flashes before the renderer paints. + // waits for the document like show() does — any earlier and a + // blank window flashes before the splash is there. if (startupWindowMode === 'maximized') { - App.mainWindow.maximize(); + mainWindow.maximize(); } - App.mainWindow.show(); + mainWindow.show(); // macOS ignores the constructor's `fullscreen` while the window // is hidden — an NSWindow can only toggle fullscreen once it is // on screen — so the request is repeated after show() wherever @@ -562,9 +571,9 @@ export default class App { // asking for it again. if ( startupWindowMode === 'fullscreen' && - !App.mainWindow.isFullScreen() + !mainWindow.isFullScreen() ) { - requestFullScreen(App.mainWindow, true); + requestFullScreen(mainWindow, true); } }); diff --git a/apps/electron-backend/src/app/services/main-window-first-show.spec.ts b/apps/electron-backend/src/app/services/main-window-first-show.spec.ts new file mode 100644 index 000000000..9ce24d815 --- /dev/null +++ b/apps/electron-backend/src/app/services/main-window-first-show.spec.ts @@ -0,0 +1,71 @@ +import { EventEmitter } from 'events'; +import { + type FirstShowWindow, + showMainWindowWhenLoaded, +} from './main-window-first-show'; + +function createWindow(): FirstShowWindow & + EventEmitter & { + webContents: EventEmitter; + destroyed: boolean; + } { + const window = Object.assign(new EventEmitter(), { + destroyed: false, + webContents: new EventEmitter(), + isDestroyed(): boolean { + return window.destroyed; + }, + }); + return window; +} + +describe('showMainWindowWhenLoaded', () => { + it('shows the window at did-finish-load when ready-to-show has not fired', () => { + // The Linux race: the hidden window gets no frame for its first + // paint, so ready-to-show (and the splash's animation frame) would + // wait about a second after the document has loaded. + const window = createWindow(); + const show = jest.fn(); + showMainWindowWhenLoaded(window, show); + + window.webContents.emit('did-finish-load'); + + expect(show).toHaveBeenCalledTimes(1); + }); + + it('shows the window at ready-to-show when that comes first', () => { + const window = createWindow(); + const show = jest.fn(); + showMainWindowWhenLoaded(window, show); + + window.emit('ready-to-show'); + + expect(show).toHaveBeenCalledTimes(1); + }); + + it('shows the window only once and detaches the other listener', () => { + const window = createWindow(); + const show = jest.fn(); + showMainWindowWhenLoaded(window, show); + + window.webContents.emit('did-finish-load'); + window.emit('ready-to-show'); + // A reload loads the document again; the window is already shown. + window.webContents.emit('did-finish-load'); + + expect(show).toHaveBeenCalledTimes(1); + expect(window.listenerCount('ready-to-show')).toBe(0); + expect(window.webContents.listenerCount('did-finish-load')).toBe(0); + }); + + it('does not show a window that was destroyed before it loaded', () => { + const window = createWindow(); + const show = jest.fn(); + showMainWindowWhenLoaded(window, show); + + window.destroyed = true; + window.emit('ready-to-show'); + + expect(show).not.toHaveBeenCalled(); + }); +}); diff --git a/apps/electron-backend/src/app/services/main-window-first-show.ts b/apps/electron-backend/src/app/services/main-window-first-show.ts new file mode 100644 index 000000000..b7c0ff38d --- /dev/null +++ b/apps/electron-backend/src/app/services/main-window-first-show.ts @@ -0,0 +1,52 @@ +/** + * When the hidden main window is first shown. + * + * The window is created with `show: false` and used to be shown on + * `ready-to-show` only. That event needs the window's first visually + * non-empty paint, and a hidden window does not always get a frame for it: + * on Linux under X11, when the startup scripts run before that frame, the + * next one comes about a second later. Until then nothing is on screen and + * the renderer gets no animation frames, so the splash that `main.ts` + * removes in a `requestAnimationFrame` stays even after the dashboard has + * rendered (J1 on the CI runner: about 450 ms later to the first card, in + * roughly one launch out of three, see docs/architecture/performance-journeys.md). + * + * The window is therefore shown at whichever comes first: `ready-to-show` + * or the main frame's `did-finish-load`. At `did-finish-load` the inline + * splash is parsed and styled, and the window's `backgroundColor` matches + * it, so showing before the first paint does not flash. + */ + +/** Matches `#initial-splash` in `apps/web/src/index.html`. */ +export const MAIN_WINDOW_BACKGROUND_COLOR = '#1f1f23'; + +type OnceEmitter = { + once(event: string, listener: () => void): unknown; + removeListener(event: string, listener: () => void): unknown; +}; + +export type FirstShowWindow = OnceEmitter & { + isDestroyed(): boolean; + readonly webContents: OnceEmitter; +}; + +/** Calls `show` once, at `ready-to-show` or `did-finish-load`, whichever comes first. */ +export function showMainWindowWhenLoaded( + window: FirstShowWindow, + show: () => void +): void { + let shown = false; + const showOnce = (): void => { + if (shown) { + return; + } + shown = true; + window.removeListener('ready-to-show', showOnce); + window.webContents.removeListener('did-finish-load', showOnce); + if (!window.isDestroyed()) { + show(); + } + }; + window.once('ready-to-show', showOnce); + window.webContents.once('did-finish-load', showOnce); +}