diff --git a/config/scripts/mobile-web-app-render-harness.mjs b/config/scripts/mobile-web-app-render-harness.mjs index 6582b1217c7..5d5b13b879b 100644 --- a/config/scripts/mobile-web-app-render-harness.mjs +++ b/config/scripts/mobile-web-app-render-harness.mjs @@ -427,9 +427,13 @@ export function installSchedulerRecorder() { // own header and line 1 is this wrapper. const caller = ((new Error('scheduled').stack ?? '').split('\n')[2] ?? '').trim() const container = document.getElementById('terminal-container') - state.scheduled.push({ kind, caller, owned: container !== null }) + // `fired` is what makes "owed" readable: a callback that has not run is still owed, whether + // it was cancelled or is merely waiting, and cancelling never sets it. + const entry = { kind, caller, owned: container !== null, fired: false } + state.scheduled.push(entry) return schedule( (...args) => { + entry.fired = true if (container !== null && !container.isConnected) { state.leaked.push(`${kind} from ${caller}`) } @@ -447,6 +451,53 @@ export function installSchedulerRecorder() { } /** Recorded before anything else runs, so a refusal during the page's own boot is counted. */ +/** + * Every window and document listener the page holds, by target, type and phase. + * + * Identity, not a tally: `addEventListener` with a listener the target already holds is a no-op in + * the DOM, and `removeEventListener` with one it does not hold is too, so counting calls would + * report leaks a browser does not have. The set is the live listeners, which is what a snapshot + * before and after a mount can be compared on. + */ +export function installListenerRecorder() { + const live = new Map() + globalThis.__orcaListeners = { + snapshot: () => + Object.fromEntries( + [...live.entries()] + .map(([key, listeners]) => [key, listeners.size]) + .filter(([, n]) => n > 0) + ) + } + const keyFor = (target, type, options) => { + const where = target === globalThis ? 'window' : target === document ? 'document' : null + if (where === null) { + return null + } + const capture = typeof options === 'object' && options !== null ? !!options.capture : !!options + return `${where} ${type}${capture ? ' capture' : ''}` + } + const add = EventTarget.prototype.addEventListener + const remove = EventTarget.prototype.removeEventListener + EventTarget.prototype.addEventListener = function (type, listener, options) { + const key = keyFor(this, type, options) + if (key !== null && listener) { + if (!live.has(key)) { + live.set(key, new Set()) + } + live.get(key).add(listener) + } + return add.call(this, type, listener, options) + } + EventTarget.prototype.removeEventListener = function (type, listener, options) { + const key = keyFor(this, type, options) + if (key !== null && listener) { + live.get(key)?.delete(listener) + } + return remove.call(this, type, listener, options) + } +} + export function installCspViolationRecorder() { globalThis.__orcaCspViolations = [] document.addEventListener('securitypolicyviolation', (event) => { diff --git a/config/scripts/mobile-web-app-terminal-render-fixture.mjs b/config/scripts/mobile-web-app-terminal-render-fixture.mjs new file mode 100644 index 00000000000..7e92e68c5c4 --- /dev/null +++ b/config/scripts/mobile-web-app-terminal-render-fixture.mjs @@ -0,0 +1,163 @@ +import { mkdir, mkdtemp, rm, writeFile } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { fileURLToPath } from 'node:url' +import { chromium } from 'playwright-core' +import { buildMobileWebAppBundle } from './build-mobile-web-app-bundle.mjs' +import { MOBILE_WEB_APP_ROUTE_ROOT } from './mobile-web-app-route-manifest.mjs' +import { + COLS, + CONTROL_SOURCE, + LAYOUT_SOURCE, + probeRouteSource, + ROWS +} from './mobile-web-app-terminal-probe-route.mjs' +import { + createBundleServer, + installCspViolationRecorder, + installListenerRecorder, + installPageErrorSentinel, + installSchedulerRecorder, + installShellDouble, + readBridgeFaultGrant, + readBridgeProtocolVersion, + readShellCsp +} from './mobile-web-app-render-harness.mjs' + +/** + * The scratch bundle the terminal render check runs against, and the two ways to open a page on it. + * + * Its own module because the check's cases are the thing under review and the server, the browser + * and the scratch route tree are not. Nothing here is module-scoped: the fixture holds what it + * built in the closures it hands back, so two of them could not read each other's browser. + */ + +const projectDir = fileURLToPath(new URL('../..', import.meta.url)) +const mobileDir = join(projectDir, 'mobile') + +export const PROBE_ROUTE = '/h/terminal-probe' +export const CONTROL_ROUTE = '/h/terminal-control' +const PAGE_ROUTE_PATTERNS = [PROBE_ROUTE, CONTROL_ROUTE] +const SHELL_SESSION_ID = 'terminal-render-session' +const SHELL_BUILD_ID = 'terminal-render-build' +const SHELL_HOST = { + id: 'terminal-render-host', + name: 'Terminal Render Host', + endpoint: 'ws://terminal-render', + lastConnected: 1 +} + +/** Builds the bundle, serves it under the shell's own policy, and launches the browser. */ +export async function startTerminalRenderFixture() { + const cspHeader = await readShellCsp() + const bridgeVersion = await readBridgeProtocolVersion() + const faultGrant = await readBridgeFaultGrant() + const scratch = await mkdtemp(join(tmpdir(), 'orca-c75-terminal-render-')) + const appDir = join(scratch, 'app') + const routeDir = join(appDir, MOBILE_WEB_APP_ROUTE_ROOT) + await mkdir(routeDir, { recursive: true }) + await writeFile(join(routeDir, '_layout.tsx'), LAYOUT_SOURCE) + // Extensionless, so the bundler resolves the `.web.tsx` sibling exactly as it would for a real + // route. Naming the `.tsx` would mount the WebView wrapper no browser can render. + await writeFile( + join(routeDir, 'terminal-probe.tsx'), + probeRouteSource(join(mobileDir, 'src', 'terminal', 'TerminalWebView')) + ) + await writeFile(join(routeDir, 'terminal-control.tsx'), CONTROL_SOURCE) + const built = await buildMobileWebAppBundle({ + appDir, + outDir: join(scratch, 'bundle'), + pageRoutes: [ + { pathname: PROBE_ROUTE, grants: [] }, + { pathname: CONTROL_ROUTE, grants: [] } + ] + }) + const { origin, server } = await createBundleServer({ outDir: built.outDir, cspHeader }) + const executablePath = process.env.ORCA_MOBILE_WEB_RENDER_BROWSER + const browser = await chromium.launch({ + headless: true, + ...(executablePath ? { executablePath } : {}) + }) + + async function openPage( + pathname, + { errorSentinel = false, listeners = false, scheduler = false, beforeNavigate } = {} + ) { + const page = await browser.newPage({ viewport: { width: 390, height: 844 } }) + await beforeNavigate?.(page) + if (scheduler) { + await page.addInitScript(installSchedulerRecorder) + } + if (listeners) { + await page.addInitScript(installListenerRecorder) + } + await page.addInitScript(installCspViolationRecorder) + if (errorSentinel) { + await page.addInitScript(installPageErrorSentinel) + } + await page.addInitScript(installShellDouble, { + version: bridgeVersion, + sessionId: SHELL_SESSION_ID, + buildId: SHELL_BUILD_ID, + route: { pathname, params: {} }, + host: SHELL_HOST, + storage: {}, + faultGrant, + grants: [faultGrant], + pageRoutes: PAGE_ROUTE_PATTERNS, + replies: {} + }) + const errors = [] + page.on('pageerror', (error) => errors.push(`${error.name}: ${error.message}`)) + page.on('console', (message) => { + if (message.type() === 'error') { + errors.push(`console.error: ${message.text()}`) + } + }) + await page.goto(`${origin}/`, { waitUntil: 'load' }) + await page.waitForFunction(() => document.documentElement.dataset.orcaWebEntry === 'mounted', { + timeout: 60_000, + polling: 250 + }) + return { errors, page } + } + + async function openTerminal(options) { + const opened = await openPage(PROBE_ROUTE, options) + await opened.page.waitForFunction(() => globalThis.__orcaTerminalReady === true, { + timeout: 60_000, + polling: 100 + }) + return opened + } + + return { + openPage, + openTerminal, + close: async () => { + await browser.close() + server.close() + await rm(scratch, { recursive: true, force: true }) + } + } +} + +/** + * The markup, then `init`, then the engine. + * + * xterm is opened by the document's `init`, not by the mount: the component plants the elements + * and the modules read them, and the terminal appears on the first host command. So the order + * here is the order a session screen uses, and each step is waited for rather than assumed — + * `.xterm` before `init` would time out on a page that was working perfectly. + */ +export async function openProbeTerminal(page) { + await page.locator('#terminal-container').waitFor({ state: 'attached', timeout: 30_000 }) + await page.evaluate( + ([cols, rows]) => globalThis.__orcaTerminalProbe.init(cols, rows, ''), + [COLS, ROWS] + ) + // Attached rather than visible: the replacement surface is hidden until its writes drain, and + // the commit that reveals it is the last step of the same rAF chain `awaitReady` waits on. + await page.locator('#terminal-surface .xterm').waitFor({ state: 'attached', timeout: 30_000 }) + await page.evaluate(() => globalThis.__orcaTerminalProbe.awaitReady()) +} diff --git a/config/scripts/mobile-web-app-terminal-render.test.mjs b/config/scripts/mobile-web-app-terminal-render.test.mjs index 1663abbc36d..181b9c53774 100644 --- a/config/scripts/mobile-web-app-terminal-render.test.mjs +++ b/config/scripts/mobile-web-app-terminal-render.test.mjs @@ -1,35 +1,18 @@ -import { mkdir, mkdtemp, rm, writeFile } from 'node:fs/promises' -import { tmpdir } from 'node:os' -import { join } from 'node:path' -import { fileURLToPath } from 'node:url' import { afterAll, beforeAll, describe, expect, it } from 'vitest' -import { chromium } from 'playwright-core' -import { buildMobileWebAppBundle } from './build-mobile-web-app-bundle.mjs' -import { MOBILE_WEB_APP_ROUTE_ROOT } from './mobile-web-app-route-manifest.mjs' import { mobileWebAppDependenciesPresent } from './mobile-web-app-bundle-dependencies.mjs' import { - COLS, - CONTROL_SOURCE, escapeDenseStream, FIRST_MARKER, LAST_MARKER, - LAYOUT_SOURCE, - MIN_STREAM_BYTES, - probeRouteSource, - ROWS + MIN_STREAM_BYTES } from './mobile-web-app-terminal-probe-route.mjs' import { - createBundleServer, - readRootComputedStyles, - terminalStyleReach, - installCspViolationRecorder, - installPageErrorSentinel, - installSchedulerRecorder, - installShellDouble, - readBridgeFaultGrant, - readBridgeProtocolVersion, - readShellCsp -} from './mobile-web-app-render-harness.mjs' + CONTROL_ROUTE, + openProbeTerminal, + PROBE_ROUTE, + startTerminalRenderFixture +} from './mobile-web-app-terminal-render-fixture.mjs' +import { readRootComputedStyles, terminalStyleReach } from './mobile-web-app-render-harness.mjs' /** * The page's terminal, in a real browser, under the policy the shell sends. @@ -51,125 +34,26 @@ import { * tree. That step retires the moment the session route is registered. */ -const projectDir = fileURLToPath(new URL('../..', import.meta.url)) -const mobileDir = join(projectDir, 'mobile') - -const PROBE_ROUTE = '/h/terminal-probe' -const CONTROL_ROUTE = '/h/terminal-control' -const PAGE_ROUTE_PATTERNS = [PROBE_ROUTE, CONTROL_ROUTE] -const SHELL_SESSION_ID = 'terminal-render-session' -const SHELL_BUILD_ID = 'terminal-render-build' -const SHELL_HOST = { - id: 'terminal-render-host', - name: 'Terminal Render Host', - endpoint: 'ws://terminal-render', - lastConnected: 1 -} - const bundles = mobileWebAppDependenciesPresent() const describeRender = bundles ? describe : describe.skip -let scratch -let server -let browser -let origin -let cspHeader = null -let bridgeVersion = null -let faultGrant = null +let fixture = null let controlCspViolations = [] const stream = escapeDenseStream() +const openPage = (pathname, options) => fixture.openPage(pathname, options) +const openTerminal = (options) => fixture.openTerminal(options) beforeAll(async () => { if (!bundles) { return } - cspHeader = await readShellCsp() - bridgeVersion = await readBridgeProtocolVersion() - faultGrant = await readBridgeFaultGrant() - scratch = await mkdtemp(join(tmpdir(), 'orca-c75-terminal-render-')) - const appDir = join(scratch, 'app') - const routeDir = join(appDir, MOBILE_WEB_APP_ROUTE_ROOT) - await mkdir(routeDir, { recursive: true }) - await writeFile(join(routeDir, '_layout.tsx'), LAYOUT_SOURCE) - // Extensionless, so the bundler resolves the `.web.tsx` sibling exactly as it would for a real - // route. Naming the `.tsx` would mount the WebView wrapper no browser can render. - await writeFile( - join(routeDir, 'terminal-probe.tsx'), - probeRouteSource(join(mobileDir, 'src', 'terminal', 'TerminalWebView')) - ) - await writeFile(join(routeDir, 'terminal-control.tsx'), CONTROL_SOURCE) - const built = await buildMobileWebAppBundle({ - appDir, - outDir: join(scratch, 'bundle'), - pageRoutes: [ - { pathname: PROBE_ROUTE, grants: [] }, - { pathname: CONTROL_ROUTE, grants: [] } - ] - }) - const served = await createBundleServer({ outDir: built.outDir, cspHeader }) - server = served.server - origin = served.origin - const executablePath = process.env.ORCA_MOBILE_WEB_RENDER_BROWSER - browser = await chromium.launch({ headless: true, ...(executablePath ? { executablePath } : {}) }) + fixture = await startTerminalRenderFixture() }, 600_000) afterAll(async () => { - await browser?.close() - server?.close() - if (scratch) { - await rm(scratch, { recursive: true, force: true }) - } + await fixture?.close() }) -async function openPage( - pathname, - { errorSentinel = false, scheduler = false, beforeNavigate } = {} -) { - const page = await browser.newPage({ viewport: { width: 390, height: 844 } }) - await beforeNavigate?.(page) - if (scheduler) { - await page.addInitScript(installSchedulerRecorder) - } - await page.addInitScript(installCspViolationRecorder) - if (errorSentinel) { - await page.addInitScript(installPageErrorSentinel) - } - await page.addInitScript(installShellDouble, { - version: bridgeVersion, - sessionId: SHELL_SESSION_ID, - buildId: SHELL_BUILD_ID, - route: { pathname, params: {} }, - host: SHELL_HOST, - storage: {}, - faultGrant, - grants: [faultGrant], - pageRoutes: PAGE_ROUTE_PATTERNS, - replies: {} - }) - const errors = [] - page.on('pageerror', (error) => errors.push(`${error.name}: ${error.message}`)) - page.on('console', (message) => { - if (message.type() === 'error') { - errors.push(`console.error: ${message.text()}`) - } - }) - await page.goto(`${origin}/`, { waitUntil: 'load' }) - await page.waitForFunction(() => document.documentElement.dataset.orcaWebEntry === 'mounted', { - timeout: 60_000, - polling: 250 - }) - return { errors, page } -} - -async function openTerminal(options) { - const opened = await openPage(PROBE_ROUTE, options) - await opened.page.waitForFunction(() => globalThis.__orcaTerminalReady === true, { - timeout: 60_000, - polling: 100 - }) - return opened -} - /** Violations this page recorded that the control did not, which is the terminal's own account. */ async function terminalCspViolations(page) { const seen = await page.evaluate(() => globalThis.__orcaCspViolations) @@ -182,26 +66,6 @@ function stripAssetPath(entry) { return entry.replace(/ @ .*$/, '') } -/** - * The markup, then `init`, then the engine. - * - * xterm is opened by the document's `init`, not by the mount: the component plants the elements - * and the modules read them, and the terminal appears on the first host command. So the order - * here is the order a session screen uses, and each step is waited for rather than assumed — - * `.xterm` before `init` would time out on a page that was working perfectly. - */ -async function openProbeTerminal(page) { - await page.locator('#terminal-container').waitFor({ state: 'attached', timeout: 30_000 }) - await page.evaluate( - ([cols, rows]) => globalThis.__orcaTerminalProbe.init(cols, rows, ''), - [COLS, ROWS] - ) - // Attached rather than visible: the replacement surface is hidden until its writes drain, and - // the commit that reveals it is the last step of the same rAF chain `awaitReady` waits on. - await page.locator('#terminal-surface .xterm').waitFor({ state: 'attached', timeout: 30_000 }) - await page.evaluate(() => globalThis.__orcaTerminalProbe.awaitReady()) -} - describeRender( 'the terminal on the page', () => { @@ -471,6 +335,100 @@ describeRender( await page.close() }, 300_000) + it('comes back from Reload while the chunk it is waiting on is still in flight', async () => { + // The other end of the case above: the chunk does not fail, it is merely slow — a cold CDN + // edge, a phone on a train. The document is reached by a dynamic import, so the mount is in + // flight while the 15 s readiness watchdog runs out and puts the overlay on the screen, and + // ruling 20 names that overlay's Reload as the way back. Reload is a second mount, so it is + // refused outright unless the first mount's cleanup could give the page back while its + // import was still unresolved — which is what the handle being synchronous is for. + // + // Held past the watchdog rather than mocked past it, because the window under test is the + // one between the claim and the import resolving, and only a real pending request has it. + const HOLD_MS = 20_000 + let held = null + const { page } = await openPage(PROBE_ROUTE, { + beforeNavigate: async (opened) => { + await opened.route('**/*.js', async (route) => { + const response = await route.fetch() + const body = await response.text() + if (held === null && body.includes('terminal runtime error')) { + held = route.request().url() + await new Promise((resolve) => setTimeout(resolve, HOLD_MS)) + } + await route.fulfill({ response, body }) + }) + } + }) + await expect.poll(() => held, { timeout: 60_000 }).not.toBe(null) + // The watchdog, named: the overlay has to be the one the stall raises, not an engine error + // from somewhere else, or Reload would be answering a different question. + await page.waitForFunction( + () => + (globalThis.__orcaTerminalEngineErrors ?? []).some((entry) => + entry.includes('no ready signal') + ), + undefined, + { timeout: 60_000, polling: 100 } + ) + const reload = page.getByText('Reload') + await reload.waitFor({ timeout: 30_000 }) + expect( + await page.evaluate(() => globalThis.__orcaTerminalReady === true), + 'the first mount was still waiting on its chunk when Reload appeared' + ).toBe(false) + + await reload.click() + await page.waitForFunction(() => globalThis.__orcaTerminalReady === true, { + timeout: 60_000, + polling: 100 + }) + await openProbeTerminal(page) + await assertLiveTerminal(page, 'reload-during-import') + await page.unrouteAll({ behavior: 'ignoreErrors' }) + await page.close() + }, 300_000) + + it('leaves the page the listeners it found, across a mount and a dispose', async () => { + // Ruling 20 moved every install into a start function and ruling 21 gave each one a stop, + // and the document installs on `window` and `document` both: the resize refit, the error + // reporter, the tap and gesture listeners the surface modules arm. A stop that forgets one + // does not fail anything visible — the next mount simply adds a second copy, and the page + // accumulates a listener per terminal it has ever shown. + // + // The comparison is drawn across a second mount rather than against the bare page: the + // component mounts as the route does, so there is no moment before the first terminal to + // photograph. Both readings are taken with no terminal on the page, so a mount that leaks + // once leaks again and the two disagree. + const { page } = await openTerminal({ listeners: true }) + await openProbeTerminal(page) + const withNoTerminal = async () => { + await page.evaluate(() => globalThis.__orcaTerminalProbe.setMounted(false)) + await page.locator('#terminal-container').waitFor({ state: 'detached', timeout: 30_000 }) + return page.evaluate(() => globalThis.__orcaListeners.snapshot()) + } + const before = await withNoTerminal() + await page.evaluate(() => { + globalThis.__orcaTerminalReady = false + globalThis.__orcaTerminalProbe.setMounted(true) + }) + await page.waitForFunction(() => globalThis.__orcaTerminalReady === true, { + timeout: 60_000, + polling: 100 + }) + await openProbeTerminal(page) + const whileLive = await page.evaluate(() => globalThis.__orcaListeners.snapshot()) + const after = await withNoTerminal() + + // The precondition: a mount that installed nothing would satisfy the equality below for + // exactly the reason the case exists to refuse. + expect(whileLive, 'the mount installed listeners the dispose has to take back').not.toEqual( + before + ) + expect(after).toEqual(before) + await page.close() + }, 300_000) + it('still reports runtime errors after a first mount spent the non-fatal budget', async () => { // Ruling 21's finding, end to end. `reportEngineError` caps non-fatal notifies at five so a // per-frame thrower cannot flood the host. That counter is the document's, not the mount's: @@ -608,9 +566,14 @@ describeRender( // bump the token it tests itself against — the frame still runs. Nothing but // `cancelDocumentFrames` takes it back. // - // The retry loop is what makes the timing certain. With the surface hidden the grid - // measures zero, so the fit never commits and re-asks for a frame every time, up to its own - // 60-frame cap: at the moment of dispose one is always owed. + // Being owed at the moment of dispose is the whole precondition, so the frame and the + // dispose are put in one task rather than left to overlap: a resize refits through the + // document's own registry, synchronously, and the unmount is requested in the same discrete + // click, which React flushes before the event returns. Nothing the browser serves can run + // in between, so the count the observer reads is what dispose was holding. Left to timing + // instead — a refit on an interval — the retry loop commits on its first attempt whenever + // the grid still measures, and a dispose that lands between two refits owes nothing and + // agrees with an empty leak list for the reason under test. That run was one in five. let documentChunk = null const { page } = await openPage(PROBE_ROUTE, { scheduler: true, @@ -632,12 +595,32 @@ describeRender( await openProbeTerminal(page) expect(documentChunk, 'the document was served as its own chunk').not.toBe(null) - await page.evaluate(() => { - globalThis.__orcaScheduler.watching = true - document.getElementById('terminal-surface').style.display = 'none' - globalThis.dispatchEvent(new Event('resize')) - globalThis.__orcaTerminalProbe.setMounted(false) - }) + await page.evaluate((chunk) => { + const state = globalThis.__orcaScheduler + state.pendingAtDispose = null + state.watching = true + // A microtask, so it runs after the synchronous dispose that emptied the host and before + // any frame the browser has yet to serve: what it reads is what dispose left owed. A + // cancelled frame never runs, so it is still owed here, which is the point. + const observer = new MutationObserver(() => { + if (document.getElementById('terminal-container') || state.pendingAtDispose !== null) { + return + } + state.pendingAtDispose = state.scheduled.filter( + (entry) => entry.kind === 'frame' && !entry.fired && entry.caller.includes(chunk) + ).length + observer.disconnect() + }) + observer.observe(document.body, { childList: true, subtree: true }) + const trigger = document.createElement('button') + document.body.appendChild(trigger) + trigger.addEventListener('click', () => { + globalThis.dispatchEvent(new Event('resize')) + globalThis.__orcaTerminalProbe.setMounted(false) + }) + trigger.click() + trigger.remove() + }, documentChunk) await page.locator('#terminal-container').waitFor({ state: 'detached', timeout: 30_000 }) await page.evaluate(() => { globalThis.__orcaTerminalReady = false @@ -651,13 +634,9 @@ describeRender( await page.evaluate(() => new Promise((resolve) => globalThis.setTimeout(resolve, 3000))) const scheduler = await page.evaluate(() => globalThis.__orcaScheduler) - // The precondition, stated as frames rather than as work of any kind: this case exists - // because the timer one cannot see a frame, so a run where the refit asked for none would - // agree with the empty list below for exactly the reason under test. expect( - scheduler.scheduled.filter( - (entry) => entry.owned && entry.kind === 'frame' && entry.caller.includes(documentChunk) - ).length + scheduler.pendingAtDispose, + 'the document owed at least one frame at the moment it was disposed' ).toBeGreaterThan(0) expect( scheduler.leaked.filter( diff --git a/mobile/src/terminal/TerminalWebView.web.tsx b/mobile/src/terminal/TerminalWebView.web.tsx index 0f29fbf5774..5349cc181bb 100644 --- a/mobile/src/terminal/TerminalWebView.web.tsx +++ b/mobile/src/terminal/TerminalWebView.web.tsx @@ -70,10 +70,36 @@ export const TerminalWebView = forwardRef( if (!host) { return } - void mountTerminalWebDocument(host, (message) => receiveRef.current?.(message)).then( - (mounted) => { + // The handle comes back before the document does, which is what makes the cleanup below + // able to answer for a mount whose import is still in flight. Without it a slow chunk left + // the page claimed by a mount that had already been torn down, and Reload — the way out the + // overlay offers — was refused as a second document. + const reportMountFailure = (error: unknown) => { + // The document is reached by a dynamic import, so its chunk can fail to load — offline, a + // stale hashed filename after a deploy, an evaluation error in a module body. No engine + // ever ran, so no `error` notify is coming. It goes down the document's own reporting + // path, which names the cause in the overlay instead of leaving the readiness watchdog to + // say "no ready after 15s". + receiveRef.current?.({ + type: 'error', + fatal: true, + message: `terminal document failed to load - ${ + error instanceof Error ? error.message : String(error) + }` + }) + } + let mounted + try { + mounted = mountTerminalWebDocument(host, (message) => receiveRef.current?.(message)) + } catch (error) { + // The mount refuses synchronously when the page is already taken, and the refusal is the + // overlay's to show rather than the tree's to crash on. + reportMountFailure(error) + return + } + void mounted.ready.then( + () => { if (cancelled) { - mounted.dispose() return } documentRef.current = mounted @@ -89,24 +115,14 @@ export const TerminalWebView = forwardRef( if (cancelled) { return } - // The document is reached by a dynamic import, so its chunk can fail to load — offline, - // a stale hashed filename after a deploy, an evaluation error in a module body. That is - // a rejected promise and nothing else: no engine ever ran, so no `error` notify is - // coming. It goes down the document's own reporting path, which names the cause in the - // overlay instead of leaving the readiness watchdog to say "no ready after 15s". - receiveRef.current?.({ - type: 'error', - fatal: true, - message: `terminal document failed to load - ${ - error instanceof Error ? error.message : String(error) - }` - }) + reportMountFailure(error) } ) + const live = mounted return () => { cancelled = true - documentRef.current?.dispose() documentRef.current = null + live.dispose() } // Mounted once per generation: re-running this would throw away a live terminal and its // scrollback, and the controller's identity changes with every callback prop. diff --git a/mobile/src/terminal/document/document-parse-time-effects.test.ts b/mobile/src/terminal/document/document-parse-time-effects.test.ts index 87669be3f20..d1db3210f36 100644 --- a/mobile/src/terminal/document/document-parse-time-effects.test.ts +++ b/mobile/src/terminal/document/document-parse-time-effects.test.ts @@ -64,15 +64,6 @@ function moduleSource(name: string): string { return readFileSync(new URL(`./${name}.ts`, import.meta.url), 'utf8') } -/** - * The top-level statements that are not declarations, and the initialisers that run something. - * - * A declaration counts as work when its initialiser calls, constructs, awaits, or reaches into - * `document` or `window`: `const scrollIndicator = document.getElementById(...)` is a declaration - * by shape and a parse-time element read by effect, and it is the exact form that survived a - * remount still holding the first mount's node. Object and regex literals are not work, which is - * why this reads the tree rather than the text. - */ /** A node's own properties, or nothing when it is not one. Read rather than asserted. */ function fieldsOf(node: unknown): [string, unknown][] { return node !== null && typeof node === 'object' && !Array.isArray(node) @@ -150,6 +141,15 @@ function mutableBindings(name: string): string[] { return mutableBindingsIn(name, moduleSource(name)) } +/** + * The top-level statements that are not declarations, and the initialisers that run something. + * + * A declaration counts as work when its initialiser calls, constructs, awaits, or reaches into + * `document` or `window`: `const scrollIndicator = document.getElementById(...)` is a declaration + * by shape and a parse-time element read by effect, and it is the exact form that survived a + * remount still holding the first mount's node. Object and regex literals are not work, which is + * why this reads the tree rather than the text. + */ function parseTimeEffects(name: string): string[] { return parseTimeEffectsIn(name, moduleSource(name)) } diff --git a/mobile/src/terminal/document/page-document-module-order.test.ts b/mobile/src/terminal/document/page-document-module-order.test.ts index d758e56a80c..01611200d1c 100644 --- a/mobile/src/terminal/document/page-document-module-order.test.ts +++ b/mobile/src/terminal/document/page-document-module-order.test.ts @@ -52,7 +52,7 @@ describe('the page entry for the terminal document', () => { // modules; what runs is the call sequence, and the generator writes its own from the same // sources. A module that grows a start function and is not called here would leave the page // with an element nobody read. - const sequence = [...pageEntry.matchAll(/^ {2}(start[A-Za-z]+)\(\)$/gm)].map( + const sequence = [...pageEntry.matchAll(/^ {2,4}(start[A-Za-z]+)\(\)$/gm)].map( (match) => match[1]! ) const emitted = await terminalDocumentStartCalls([ diff --git a/mobile/src/terminal/document/page-document-modules.ts b/mobile/src/terminal/document/page-document-modules.ts index ad27a11cbdd..a9fcfeed0e9 100644 --- a/mobile/src/terminal/document/page-document-modules.ts +++ b/mobile/src/terminal/document/page-document-modules.ts @@ -63,15 +63,31 @@ import { startSurfaceTouchGestures, stopSurfaceTouchGestures } from './surface-t */ export function startPageDocumentModules() { resetTerminalDocumentScope() - startRuntimeConstants() - startSurfaceSwap() - startTextScaling() - startWebglRecovery() - startHostNotify() - startSelectionStateAndEviction() - startTapDispatch() - startSelectionMenuButtons() - startSurfaceTouchGestures() + // Unwound if one of them throws: a start that completed has already taken a listener or + // installed the reporter, and leaving those behind would outlive the mount that never happened. + // Only the starts with an undo need recording; the rest write scope fields the next reset + // overwrites. + const undo: (() => void)[] = [] + try { + startRuntimeConstants() + startSurfaceSwap() + startTextScaling() + startWebglRecovery() + undo.unshift(stopWebglRecovery) + startHostNotify() + undo.unshift(stopHostNotify) + startSelectionStateAndEviction() + startTapDispatch() + undo.unshift(stopTapDispatch) + startSelectionMenuButtons() + startSurfaceTouchGestures() + undo.unshift(stopSurfaceTouchGestures) + } catch (error) { + for (const stop of undo) { + stop() + } + throw error + } } /** diff --git a/mobile/src/terminal/document/page-document-start-unwind.test.ts b/mobile/src/terminal/document/page-document-start-unwind.test.ts new file mode 100644 index 00000000000..c2a557aa50d --- /dev/null +++ b/mobile/src/terminal/document/page-document-start-unwind.test.ts @@ -0,0 +1,60 @@ +// @vitest-environment happy-dom +import { describe, expect, it } from 'vitest' + +/** + * A start sequence that throws leaves nothing of itself behind. + * + * The starts are not all writes to the scope: `startHostNotify` installs the host's error + * reporter and `startTapDispatch` takes four document listeners. If one of the later starts + * throws, the mount fails and its handle releases the page — but the reporter and the listeners + * are already installed, and nothing else would reach them: the next mount's reset nulls the undo + * the install handed back, so the listener would stay for the life of the tab. + * + * The provocation is the document's own markup with the selection menu missing, which is what + * `startSelectionMenuButtons` reads and the only thing it does. + */ +const MARKUP_WITHOUT_THE_MENU = + '
' + + '
' + + '
' + + '
' + +describe('the page start sequence', () => { + it('unwinds the starts that completed when a later one throws', async () => { + document.body.innerHTML = MARKUP_WITHOUT_THE_MENU + const { startPageDocumentModules } = await import('./page-document-modules') + const previous = window.onerror + window.onerror = null + try { + expect(() => startPageDocumentModules()).toThrow() + // `startHostNotify` ran and installed the default reporter, which takes `window.onerror`. + // The unwind is the only thing that gives it back: the next mount's reset nulls the undo it + // handed out, so an install left standing here is permanent. + expect(window.onerror).toBe(null) + } finally { + window.onerror = previous + } + }) + + it('would have installed one, so the null above is a measurement', async () => { + // The precondition. With the menu present the same sequence completes, and the reporter it + // installs is exactly what the case above asserts was taken back. + document.body.innerHTML = MARKUP_WITHOUT_THE_MENU.replace( + '
', + '
' + + '
' + ) + const { startPageDocumentModules, stopPageDocumentModules } = + await import('./page-document-modules') + const previous = window.onerror + window.onerror = null + try { + startPageDocumentModules() + expect(window.onerror).not.toBe(null) + stopPageDocumentModules() + expect(window.onerror).toBe(null) + } finally { + window.onerror = previous + } + }) +}) diff --git a/mobile/src/terminal/terminal-web-document-mount.ts b/mobile/src/terminal/terminal-web-document-mount.ts index b5db2c196ca..5d5748b4440 100644 --- a/mobile/src/terminal/terminal-web-document-mount.ts +++ b/mobile/src/terminal/terminal-web-document-mount.ts @@ -28,6 +28,15 @@ export type TerminalWebDocument = { /** Hands one host command to the document, as `postMessage` does inside the WebView. */ send: (command: TerminalWebViewCommand & { id: number }) => void dispose: () => void + /** + * Settles when the document is live, or rejects with what stopped it. + * + * The handle itself is returned before this: the document is reached by a dynamic import, and a + * caller that had to await the import to get a handle would have nothing to dispose while the + * import was in flight. That is not a corner — a slow chunk is what the readiness watchdog is + * for, and the overlay's Reload is what ruling 20 names as the way out of it. + */ + ready: Promise } const STYLE_ELEMENT_ID = 'orca-terminal-document-style' @@ -96,40 +105,119 @@ function createPageWebglAddon(onFallback: (reason: string) => void) { */ let liveDocument: symbol | null = null -export async function mountTerminalWebDocument( +/** What a mount has built so far, which is nothing until its import resolves. */ +type StartedDocument = { + modules: typeof import('./document/page-document-modules') + onWindowResize: () => void +} + +/** + * The document, mounted. The handle comes back before the document exists. + * + * Synchronous on purpose. The modules arrive through a dynamic import, and the caller's cleanup + * can run while that import is still in flight — a slow chunk, a cold cache, a tab that was + * backgrounded. A caller that had to await the import to get a handle would have nothing to + * dispose in that window, and the claim below would outlive the mount that made it: the next + * mount, the one the error overlay's Reload asks for, would be refused as a second document and + * the terminal would never come back. So the claim and the handle are made here, together, and + * `dispose` answers for whichever state the mount is in when it is called. + */ +export function mountTerminalWebDocument( host: HTMLElement, receive: (message: Record) => void -): Promise { +): TerminalWebDocument { if (liveDocument) { throw new Error('the terminal document is already mounted on this page') } const token = Symbol('orca terminal document') liveDocument = token - try { - return await buildTerminalWebDocument(host, receive, token) - } catch (error) { - // A mount that never completed holds nothing, and the overlay's Reload has to be able to try - // again — the dynamic import below is exactly the step that can fail. Guarded all the same: - // this must not take the page back from a document that is not this one. + let started: StartedDocument | null = null + const release = () => { if (liveDocument === token) { liveDocument = null } + host.innerHTML = '' + // The sheet stays in the head; the class does not, so every rule in it matches nothing + // again the moment the terminal is gone. + host.classList.remove(HOST_CLASS) + } + + try { + ensureDocumentStyle() + host.classList.add(HOST_CLASS) + host.innerHTML = TERMINAL_DOCUMENT_MARKUP + // The WebView's `` declares this before anything runs, and the document's error + // reporter reads it unguarded. Without it the first report throws inside `window.onerror`. + window.__engineErrors = [] + } catch (error) { + // The claim is made before this runs, so it has to come back if the planting fails. + release() throw error } + + const ready = buildTerminalWebDocument(host, receive).then( + (built) => { + if (liveDocument !== token) { + // Disposed while the import was in flight. The page is already someone else's, so this + // releases nothing and starts nothing; the modules it resolved are inert until started. + return + } + started = built + }, + (error: unknown) => { + // The import failed, so nothing was started and the page has to go back — the overlay's + // Reload is a second mount and it must be allowed to make one. + release() + throw error + } + ) + + return { + send: (command) => { + started?.modules.handleMsg(command) + }, + dispose: () => { + // Once, and only by the document that is live. A handle outlives what it built — the + // component holds one in a ref and React may run a cleanup after a later mount has already + // started — so a second call, or a call from a handle whose document has been replaced, + // would tear down the terminal that is on the screen now. Everything below this line is + // shared: the scope, the module sequences, the `window.__engineErrors` array. + if (liveDocument !== token) { + return + } + liveDocument = null + if (started) { + teardownStartedDocument(started) + } + host.innerHTML = '' + host.classList.remove(HOST_CLASS) + }, + ready + } +} + +/** Undoes a document that did start: its listener, its module sequence and its terminals. */ +function teardownStartedDocument({ modules, onWindowResize }: StartedDocument) { + window.removeEventListener('resize', onWindowResize) + modules.stopPageDocumentModules() + const { scope } = modules + // Both terminals, because a swap that never committed leaves two. `beginTerminalSurfaceSwap` + // opens a hidden replacement and `commitTerminalSurfaceSwap` disposes the one it replaced; an + // unmount between the two leaves the committed terminal live with nothing pointing at it. They + // are the same object whenever no swap is open, so the pair is deduplicated. + for (const terminal of new Set([scope.term, scope.committedTerm])) { + try { + terminal?.dispose() + } catch {} + } + scope.term = null + scope.committedTerm = null } async function buildTerminalWebDocument( host: HTMLElement, - receive: (message: Record) => void, - token: symbol -): Promise { - ensureDocumentStyle() - host.classList.add(HOST_CLASS) - host.innerHTML = TERMINAL_DOCUMENT_MARKUP - // The WebView's `` declares this before anything runs, and the document's error reporter - // reads it unguarded. Without it the first report throws inside `window.onerror`. - window.__engineErrors = [] - + receive: (message: Record) => void +): Promise { const documentModules = await import('./document/page-document-modules') const { scope } = documentModules @@ -182,37 +270,5 @@ async function buildTerminalWebDocument( } window.addEventListener('resize', onWindowResize) - return { - send: (command) => { - documentModules.handleMsg(command) - }, - dispose: () => { - // Once, and only by the document that is live. A handle outlives what it built — the - // component holds one in a ref and React may run a cleanup after a later mount has already - // started — so a second call, or a call from a handle whose document has been replaced, - // would tear down the terminal that is on the screen now. Everything below this line is - // shared: the scope, the module sequences, the `window.__engineErrors` array. - if (liveDocument !== token) { - return - } - liveDocument = null - window.removeEventListener('resize', onWindowResize) - documentModules.stopPageDocumentModules() - // Both terminals, because a swap that never committed leaves two. `beginTerminalSurfaceSwap` - // opens a hidden replacement and `commitTerminalSurfaceSwap` disposes the one it replaced; - // an unmount between the two leaves the committed terminal live with nothing pointing at - // it. They are the same object whenever no swap is open, so the pair is deduplicated. - for (const terminal of new Set([scope.term, scope.committedTerm])) { - try { - terminal?.dispose() - } catch {} - } - scope.term = null - scope.committedTerm = null - host.innerHTML = '' - // The sheet stays in the head; the class does not, so every rule in it matches nothing - // again the moment the terminal is gone. - host.classList.remove(HOST_CLASS) - } - } + return { modules: documentModules, onWindowResize } } diff --git a/mobile/src/terminal/terminal-web-document-single-mount.test.ts b/mobile/src/terminal/terminal-web-document-single-mount.test.ts index e516d732911..05944d65ea7 100644 --- a/mobile/src/terminal/terminal-web-document-single-mount.test.ts +++ b/mobile/src/terminal/terminal-web-document-single-mount.test.ts @@ -24,13 +24,15 @@ describe('the page terminal document', () => { const second = document.createElement('div') document.body.appendChild(second) - const mounted = await mountTerminalWebDocument(host, () => {}) - await expect(mountTerminalWebDocument(second, () => {})).rejects.toThrow( + const mounted = mountTerminalWebDocument(host, () => {}) + await mounted.ready + expect(() => mountTerminalWebDocument(second, () => {})).toThrow( 'the terminal document is already mounted on this page' ) mounted.dispose() - const remounted = await mountTerminalWebDocument(second, () => {}) + const remounted = mountTerminalWebDocument(second, () => {}) + await remounted.ready expect(second.querySelector('#terminal-container')).not.toBe(null) remounted.dispose() }) @@ -42,7 +44,8 @@ describe('the page terminal document', () => { // would leave it holding its renderer, its observers and its buffers for the life of the tab. const host = document.createElement('div') document.body.appendChild(host) - const mounted = await mountTerminalWebDocument(host, () => {}) + const mounted = mountTerminalWebDocument(host, () => {}) + await mounted.ready const { scope } = await import('./document/page-document-modules') const disposed: string[] = [] @@ -63,7 +66,8 @@ describe('the page terminal document', () => { // twice is what the deduplication exists to stop. const host = document.createElement('div') document.body.appendChild(host) - const mounted = await mountTerminalWebDocument(host, () => {}) + const mounted = mountTerminalWebDocument(host, () => {}) + await mounted.ready const { scope } = await import('./document/page-document-modules') let disposals = 0 @@ -83,7 +87,8 @@ describe('the page terminal document', () => { // dispose, which is what the next mount does. const host = document.createElement('div') document.body.appendChild(host) - const mounted = await mountTerminalWebDocument(host, () => {}) + const mounted = mountTerminalWebDocument(host, () => {}) + await mounted.ready const { scope } = await import('./document/page-document-modules') mounted.dispose() @@ -107,9 +112,11 @@ describe('the page terminal document', () => { const second = document.createElement('div') document.body.append(first, second) - const stale = await mountTerminalWebDocument(first, () => {}) + const stale = mountTerminalWebDocument(first, () => {}) + await stale.ready stale.dispose() - const live = await mountTerminalWebDocument(second, () => {}) + const live = mountTerminalWebDocument(second, () => {}) + await live.ready const { scope } = await import('./document/page-document-modules') let disposals = 0 @@ -122,7 +129,37 @@ describe('the page terminal document', () => { expect(second.querySelector('#terminal-container')).not.toBe(null) expect(scope.term).not.toBe(null) // And the page is still taken, so the live document is still the one that owns it. - await expect(mountTerminalWebDocument(first, () => {})).rejects.toThrow( + expect(() => mountTerminalWebDocument(first, () => {})).toThrow( + 'the terminal document is already mounted on this page' + ) + live.dispose() + }) + + it('tells two mounts of the same element apart, which a host comparison cannot', async () => { + // Why the claim is a token and not the host. React reuses elements, so the page can hand the + // second mount the very element the first one used — that is the ordinary remount, not a + // corner. A dispose that asked "is this my host?" would answer yes for both handles, and the + // stale one would tear down the live document while leaving the page claimed. + const host = document.createElement('div') + document.body.appendChild(host) + + const stale = mountTerminalWebDocument(host, () => {}) + await stale.ready + stale.dispose() + const live = mountTerminalWebDocument(host, () => {}) + await live.ready + const { scope } = await import('./document/page-document-modules') + + let disposals = 0 + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: dispose is the only member the mount's dispose reaches on a terminal. + scope.term = { dispose: () => (disposals += 1) } as unknown as typeof scope.term + + stale.dispose() + + expect(disposals).toBe(0) + expect(host.querySelector('#terminal-container')).not.toBe(null) + expect(host.classList.contains('orca-terminal-document-host')).toBe(true) + expect(() => mountTerminalWebDocument(host, () => {})).toThrow( 'the terminal document is already mounted on this page' ) live.dispose() @@ -140,11 +177,12 @@ describe('the page terminal document', () => { return '' } }) - await expect(mountTerminalWebDocument(detached, () => {})).rejects.toThrow('orca-mount-failed') + expect(() => mountTerminalWebDocument(detached, () => {})).toThrow('orca-mount-failed') const host = document.createElement('div') document.body.appendChild(host) - const mounted = await mountTerminalWebDocument(host, () => {}) + const mounted = mountTerminalWebDocument(host, () => {}) + await mounted.ready expect(host.querySelector('#terminal-container')).not.toBe(null) mounted.dispose() })