fix(mobile): stop a page mount that lost its claim before it writes the scope

Round 5 on #21809.

F1 (blocking). `buildTerminalWebDocument` had no token, so after its `await
import(...)` the whole body ran whatever had happened in the meantime: it
overwrote the six seams, called `startPageDocumentModules` and added the resize
listener, and only then did the caller's `.then` read the claim and throw the
result away. Everything after that await is shared — the seams are fields on a
module-singleton scope, and the start sequence resets that scope and installs
the document's listeners — so a mount disposed while its chunk was in flight was
writing over a mount that owns the page. The claim is now re-read the instant
the import lands, before any of it, and the build returns null.

`ready` for such a mount resolves rather than rejecting. Nothing failed: the
caller asked for the terminal and then asked for it to go away, and the chunk
arriving afterwards is not something for the error overlay to name. Before this
it rejected with a TypeError from `startSelectionMenuButtons` reaching for an
emptied host.

F2. The rejection handler called `release()` unconditionally, emptying a host the
mount may no longer own. It now releases only when the page is still its own.

Pins, both red first. In happy-dom: mount, dispose, then await ready — no
listener, timer or frame added while it resolves, the six seams unchanged,
`terminalGeneration` unmoved because the start sequence never ran, and the page
free for the next mount. Without the fix that case rejects with the
`startSelectionMenuButtons` TypeError. In the browser, the Reload-while-in-flight
case now reads the page's listeners with no terminal on it and compares them
against a page that mounted once and disposed once; without the fix the
abandoned mount leaves `window error` and `window resize` behind, because the
second mount's scope reset nulls the first mount's reporter undo.

The listener snapshot helper is shared with the mount-and-dispose case rather
than written twice.

Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
This commit is contained in:
Jinwoo-H
2026-09-20 17:31:40 -04:00
parent db356997a9
commit dc6d577096
3 changed files with 112 additions and 16 deletions
@@ -201,6 +201,13 @@ describeRender(
*
* So the assertions are about the live DOM and the live paths, not about readiness.
*/
/** The listeners the page holds with no terminal on it, which is what two mounts can differ by. */
async function listenersWithNoTerminal(page) {
await page.evaluate(() => globalThis.__orcaTerminalProbe.setMounted(false))
await page.locator('#terminal-container').waitFor({ state: 'detached', timeout: 30_000 })
return page.evaluate(() => globalThis.__orcaListeners.snapshot())
}
async function assertLiveTerminal(page, label) {
await page.locator('#terminal-surface .xterm').waitFor({ state: 'attached', timeout: 30_000 })
expect(
@@ -348,6 +355,7 @@ describeRender(
const HOLD_MS = 20_000
let held = null
const { page } = await openPage(PROBE_ROUTE, {
listeners: true,
beforeNavigate: async (opened) => {
await opened.route('**/*.js', async (route) => {
const response = await route.fetch()
@@ -385,6 +393,18 @@ describeRender(
})
await openProbeTerminal(page)
await assertLiveTerminal(page, 'reload-during-import')
// And the mount the Reload abandoned has to have come to nothing. Its chunk arrives while
// the second mount is running on the same scope, so a build that resumed without re-reading
// the claim would install its listeners into this page and reset the live mount's scope,
// nulling the undo that takes the error reporter off. Read against a page that mounted once
// and disposed once: the abandoned mount is the only difference between them, so zero
// difference is the abandoned mount having touched nothing.
const afterAbandoned = await listenersWithNoTerminal(page)
const control = await openTerminal({ listeners: true })
await openProbeTerminal(control.page)
expect(afterAbandoned).toEqual(await listenersWithNoTerminal(control.page))
await control.page.close()
await page.unrouteAll({ behavior: 'ignoreErrors' })
await page.close()
}, 300_000)
@@ -402,12 +422,7 @@ describeRender(
// 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()
const before = await listenersWithNoTerminal(page)
await page.evaluate(() => {
globalThis.__orcaTerminalReady = false
globalThis.__orcaTerminalProbe.setMounted(true)
@@ -418,7 +433,7 @@ describeRender(
})
await openProbeTerminal(page)
const whileLive = await page.evaluate(() => globalThis.__orcaListeners.snapshot())
const after = await withNoTerminal()
const after = await listenersWithNoTerminal(page)
// The precondition: a mount that installed nothing would satisfy the equality below for
// exactly the reason the case exists to refuse.
@@ -35,6 +35,11 @@ export type TerminalWebDocument = {
* 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.
*
* A mount disposed before its import landed resolves rather than rejecting. Nothing failed:
* the caller asked for the terminal and then asked for it to go away, and the chunk arriving
* afterwards is not an error to report. The caller learns which it got from `dispose` being
* the thing it called, not from this.
*/
ready: Promise<void>
}
@@ -155,19 +160,23 @@ export function mountTerminalWebDocument(
throw error
}
const ready = buildTerminalWebDocument(host, receive).then(
const ready = buildTerminalWebDocument(host, receive, token).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.
if (built === null) {
// Disposed while the import was in flight, and the build stopped at the await without
// touching anything. Nothing to keep and nothing to give back.
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()
// Reload is a second mount and it must be allowed to make one. Only if the page is still
// this mount's: a later mount may already hold it, and emptying its host would take the
// terminal that is on the screen.
if (liveDocument === token) {
release()
}
throw error
}
)
@@ -214,11 +223,26 @@ function teardownStartedDocument({ modules, onWindowResize }: StartedDocument) {
scope.committedTerm = null
}
/**
* The document, built and started — or `null` if the page stopped being this mount's.
*
* The token is read again the instant the import lands, before anything below it runs. Every
* statement after this point writes shared state: the six seams are fields on a module-singleton
* scope, `startPageDocumentModules` resets that scope and installs listeners, and the resize
* listener outlives the host. A mount disposed while its chunk was in flight owns none of it, and
* running the body anyway would plant its elements' listeners into a page a later mount is using
* and reset that mount's scope out from under it. Checking only when this resolves is too late:
* by then the writes have happened and the caller can do nothing but discard the result.
*/
async function buildTerminalWebDocument(
host: HTMLElement,
receive: (message: Record<string, unknown>) => void
): Promise<StartedDocument> {
receive: (message: Record<string, unknown>) => void,
token: symbol
): Promise<StartedDocument | null> {
const documentModules = await import('./document/page-document-modules')
if (liveDocument !== token) {
return null
}
const { scope } = documentModules
// Ruling 19 reaches `window.onerror` too: the WebView's document owns its page and may take
@@ -1,5 +1,5 @@
// @vitest-environment happy-dom
import { beforeEach, describe, expect, it } from 'vitest'
import { beforeEach, describe, expect, it, vi } from 'vitest'
import { mountTerminalWebDocument } from './terminal-web-document-mount'
/**
@@ -165,6 +165,63 @@ describe('the page terminal document', () => {
live.dispose()
})
it('touches nothing when it is disposed before its chunk lands', async () => {
// The window the synchronous handle opened. `dispose` can now run while the import is still
// unresolved, so the build resumes on a page it no longer owns — and everything after its
// await writes shared state: the six seams are fields on one module scope, and
// `startPageDocumentModules` resets that scope and installs the document's listeners. A mount
// that checked only when it resolved would have done all of that first and then discarded the
// result, leaving the listeners behind and a later mount's scope reset out from under it.
const host = document.createElement('div')
document.body.appendChild(host)
const { scope } = await import('./document/page-document-modules')
const seamsBefore = {
postToHost: scope.postToHost,
installErrorReporter: scope.installErrorReporter,
paintDocumentBackground: scope.paintDocumentBackground,
createTerminal: scope.createTerminal,
createUnicode11Addon: scope.createUnicode11Addon,
createWebglAddon: scope.createWebglAddon
}
const generationBefore = scope.terminalGeneration
const mounted = mountTerminalWebDocument(host, () => {})
mounted.dispose()
// Armed after the dispose, so anything they catch is the resuming build and nothing else.
const listeners = vi.spyOn(EventTarget.prototype, 'addEventListener')
const timers = vi.spyOn(globalThis, 'setTimeout')
const frames = vi.spyOn(globalThis, 'requestAnimationFrame')
try {
// Resolves: the caller asked for the terminal and then asked for it to go away, so the
// chunk landing afterwards is not a failure to report to the error overlay.
await expect(mounted.ready).resolves.toBeUndefined()
} finally {
listeners.mockRestore()
timers.mockRestore()
frames.mockRestore()
}
expect(listeners).not.toHaveBeenCalled()
expect(timers).not.toHaveBeenCalled()
expect(frames).not.toHaveBeenCalled()
expect({
postToHost: scope.postToHost,
installErrorReporter: scope.installErrorReporter,
paintDocumentBackground: scope.paintDocumentBackground,
createTerminal: scope.createTerminal,
createUnicode11Addon: scope.createUnicode11Addon,
createWebglAddon: scope.createWebglAddon
}).toEqual(seamsBefore)
// `startPageDocumentModules` resets the scope, which carries this forward by one. Unchanged
// is the start sequence never having run.
expect(scope.terminalGeneration).toBe(generationBefore)
// And the page is free, which is what the overlay's Reload needs.
const remounted = mountTerminalWebDocument(host, () => {})
await remounted.ready
expect(host.querySelector('#terminal-container')).not.toBe(null)
remounted.dispose()
})
it('gives the page back when the mount itself fails, so Reload can try again', async () => {
// The overlay's Reload path. A mount that threw holds nothing, and a flag left set would
// refuse every later attempt — the document's chunk failing to load is exactly that case.