diff --git a/mobile/src/terminal/terminal-web-document-mount-rejection.test.ts b/mobile/src/terminal/terminal-web-document-mount-rejection.test.ts new file mode 100644 index 00000000000..790a73c5356 --- /dev/null +++ b/mobile/src/terminal/terminal-web-document-mount-rejection.test.ts @@ -0,0 +1,78 @@ +// @vitest-environment happy-dom +import { describe, expect, it, vi } from 'vitest' + +/** + * What a mount whose chunk never arrived is allowed to touch on its way out. + * + * The one path where a mount reaches its own cleanup holding a page that belongs to someone else. + * Everywhere else the build reads the claim again after its import and stops, but a rejected + * import never gets that far: the failure arrives at the mount's error handler directly, and by + * then the overlay's Reload may already have built a second document into the same element. A + * release that emptied the host anyway would blank the terminal on the screen and hand the page + * back while its document ran on. + * + * Its own file because making the import fail is the only way to reach this, and the mock has to + * be in place before the mount module is loaded. It fails once, so the second mount gets the real + * modules and can be a live document to protect. + */ + +const { chunk } = vi.hoisted(() => ({ chunk: { failures: 0 } })) +vi.mock('./document/page-document-modules', async (importOriginal) => { + if (chunk.failures === 0) { + chunk.failures += 1 + throw new Error('orca-document-chunk-failed') + } + return importOriginal() +}) + +const { mountTerminalWebDocument } = await import('./terminal-web-document-mount') + +const HOST_CLASS = 'orca-terminal-document-host' + +describe('a page mount whose document chunk failed', () => { + it('leaves the document that replaced it alone', async () => { + const host = document.createElement('div') + document.body.appendChild(host) + let resizeListeners = 0 + const realAdd = window.addEventListener.bind(window) + const realRemove = window.removeEventListener.bind(window) + window.addEventListener = (type, listener, options) => { + resizeListeners += type === 'resize' ? 1 : 0 + realAdd(type, listener, options) + } + window.removeEventListener = (type, listener, options) => { + resizeListeners -= type === 'resize' ? 1 : 0 + realRemove(type, listener, options) + } + + const abandoned = mountTerminalWebDocument(host, () => {}) + abandoned.dispose() + // The same element, as React hands it back on the overlay's Reload. + const live = mountTerminalWebDocument(host, () => {}) + // The message is the mocking layer's, not the one thrown, so the two counters are what say + // which import did what: the abandoned mount's failed, and the live mount's did not. + await expect(abandoned.ready).rejects.toThrow() + expect(chunk.failures, 'the abandoned mount is the one whose chunk failed').toBe(1) + + expect(host.querySelector('#terminal-container')).not.toBe(null) + expect(host.classList.contains(HOST_CLASS)).toBe(true) + // Still claimed, so the release did not hand the page back either. + expect(() => mountTerminalWebDocument(host, () => {})).toThrow( + 'the terminal document is already mounted on this page' + ) + + await live.ready + // The other half of the precondition: the mount that replaced it is a real started document, + // not a second casualty. Its resize listener is the one the start sequence adds. + expect(resizeListeners, 'the live mount started its document').toBe(1) + // And disposing the abandoned handle a second time changes nothing. + abandoned.dispose() + expect(host.querySelector('#terminal-container')).not.toBe(null) + expect(host.classList.contains(HOST_CLASS)).toBe(true) + live.dispose() + window.addEventListener = realAdd + window.removeEventListener = realRemove + expect(host.querySelector('#terminal-container')).toBe(null) + expect(resizeListeners, 'and it took its listener back on the way out').toBe(0) + }) +}) diff --git a/mobile/src/terminal/terminal-web-document-mount.ts b/mobile/src/terminal/terminal-web-document-mount.ts index 8f29a4abc01..93d16e4f2bb 100644 --- a/mobile/src/terminal/terminal-web-document-mount.ts +++ b/mobile/src/terminal/terminal-web-document-mount.ts @@ -169,23 +169,19 @@ export function mountTerminalWebDocument( throw error } - const ready = buildTerminalWebDocument(host, receive, token).then( - (built) => { - 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. A later mount may already - // hold the page, which `release` answers for. - release() - throw error - } - ) + // Adopted by the build itself, in the same turn as the start sequence and the listener it adds, + // rather than when this promise settles. A `.then` runs a microtask later, and a dispose in + // between would find nothing started, skip the teardown and hand the page back with the + // document still running on it. + const ready = buildTerminalWebDocument(host, receive, token, (built) => { + started = built + }).catch((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. A later mount may already + // hold the page, which `release` answers for. + release() + throw error + }) return { send: (command) => { @@ -230,7 +226,7 @@ function teardownStartedDocument({ modules, onWindowResize }: StartedDocument) { } /** - * The document, built and started — or `null` if the page stopped being this mount's. + * Builds and starts the document, and hands it to `adopt` — or returns having done neither. * * 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 @@ -239,15 +235,19 @@ function teardownStartedDocument({ modules, onWindowResize }: StartedDocument) { * 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. + * + * `adopt` rather than a return value for the same reason: what it hands over is what undoes all of + * that, and the caller has to be holding it before this function's turn ends. */ async function buildTerminalWebDocument( host: HTMLElement, receive: (message: Record) => void, - token: symbol -): Promise { + token: symbol, + adopt: (built: StartedDocument) => void +): Promise { const documentModules = await import('./document/page-document-modules') if (liveDocument !== token) { - return null + return } const { scope } = documentModules @@ -300,5 +300,5 @@ async function buildTerminalWebDocument( } window.addEventListener('resize', onWindowResize) - return { modules: documentModules, onWindowResize } + adopt({ 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 af9a371c361..c3f2d58d8c1 100644 --- a/mobile/src/terminal/terminal-web-document-single-mount.test.ts +++ b/mobile/src/terminal/terminal-web-document-single-mount.test.ts @@ -222,6 +222,56 @@ describe('the page terminal document', () => { remounted.dispose() }) + it('tears down a document that started, however late the dispose is', async () => { + // The start sequence and the handle on what undoes it have to land in one turn. They did not: + // the build started the document, installed its listeners and returned, and the assignment + // that recorded it ran a microtask later — so a dispose in between found nothing started, + // skipped the teardown and handed the page back with the document still running on it. The + // dispose here is queued behind the document module import the build awaits, which puts it in + // that window rather than before or after it. + const host = document.createElement('div') + document.body.appendChild(host) + const listeners = new Set() + const realAdd = window.addEventListener.bind(window) + const realRemove = window.removeEventListener.bind(window) + window.addEventListener = (type, listener, options) => { + if (type === 'resize') { + listeners.add(listener) + } + realAdd(type, listener, options) + } + window.removeEventListener = (type, listener, options) => { + if (type === 'resize') { + listeners.delete(listener) + } + realRemove(type, listener, options) + } + + const mounted = mountTerminalWebDocument(host, () => {}) + try { + await import('./document/page-document-modules') + mounted.dispose() + await mounted.ready + } finally { + window.addEventListener = realAdd + window.removeEventListener = realRemove + } + + // The precondition: the document did start, so there was something to tear down. A build that + // bailed at the ownership check would add no listener and satisfy the emptiness below for the + // one reason this case exists to refuse. + expect( + listeners.size + Number(host.querySelector('#terminal-container') === null) + ).toBeGreaterThan(0) + expect(listeners.size, 'the resize listener the started document added').toBe(0) + const { scope } = await import('./document/page-document-modules') + expect(scope.term).toBe(null) + // And the page is free, which a handle that lost track of what it started would not have left. + const remounted = mountTerminalWebDocument(host, () => {}) + await remounted.ready + 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.