diff --git a/CHANGELOG.md b/CHANGELOG.md index 50c0a3d18..5bae06ad2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,7 +14,7 @@ of current runtime behavior, see [`docs/CURRENT-STATE.md`](docs/CURRENT-STATE.md ## [Unreleased] - Split Cockpit camera/controller, instruments, briefing, signals and layout into focused modules with explicit application operations. -- Give Display portal moves cancellable focus/scroll restoration and terminal cleanup. +- Give Display portal moves cancellable focus/scroll restoration and stop Cockpit work before asynchronous UI teardown. - Separate Context controls, mode transitions and layer restoration; release tab listeners and suppress late panel/search feedback after disposal. diff --git a/docs/CURRENT-STATE.md b/docs/CURRENT-STATE.md index f17e4448d..a383d4012 100644 --- a/docs/CURRENT-STATE.md +++ b/docs/CURRENT-STATE.md @@ -7,7 +7,9 @@ layer, terrain and rendering operations are supplied by composition, retaining the same tracked identity, ground acquisition, motion correction and cadence. The Display portal owns group anchors, focus/scroll restoration and listeners. Superseded portal frames cannot repaint old state or steal focus after disposal; -retained Cockpit actions cannot restart a disposed controller. +retained Cockpit actions cannot restart a disposed controller. Input, subscriptions +and queued panel work stop before asynchronous layer restoration; final camera +and portal cleanup follows that restoration. ## Context coordination diff --git a/scripts/qa-ui-disposal.mjs b/scripts/qa-ui-disposal.mjs index 6e75e8e26..2808adfa0 100644 --- a/scripts/qa-ui-disposal.mjs +++ b/scripts/qa-ui-disposal.mjs @@ -75,6 +75,33 @@ try { return unsubscribe?.(); }; } + const cockpit = ui.cockpitView; + const portal = ui._cockpitDisplayPortal; + const cockpitListenerCount = cockpit._listenerRemovers.length; + counts.cockpit = 0; + cockpit._listenerRemovers = cockpit._listenerRemovers.map( + (remove) => () => { + counts.cockpit++; + return remove(); + }, + ); + const portalHomes = portal.records.map((record) => ({ + group: record.group, + home: record.anchor.parentNode, + anchor: record.anchor, + })); + portal.setActive(true); + let releaseRestoration; + const restorationGate = new Promise((resolve) => { + releaseRestoration = resolve; + }); + const restoreForDisposal = ui._contextControls.restoreForDisposal.bind( + ui._contextControls, + ); + ui._contextControls.restoreForDisposal = async () => { + await restorationGate; + return restoreForDisposal(); + }; const resizeHandler = ui._windowResizeHandler; const removeEventListener = window.removeEventListener; counts.resize = 0; @@ -83,7 +110,21 @@ try { return removeEventListener.call(this, type, callback, options); }; try { - await ui.dispose(); + const disposal = ui.dispose(); + const focusBefore = document.activeElement; + const stoppedBeforeRestoration = + cockpit.destroyed && + portal.stopped && + cockpit._listenerRemovers.length === 0 && + portal.frames.size === 0 && + cockpit.enter() === false && + cockpit.navigateContext(1) === false; + await new Promise((resolve) => + requestAnimationFrame(() => requestAnimationFrame(resolve)), + ); + const noDeferredFocus = document.activeElement === focusBefore; + releaseRestoration(); + await disposal; const once = JSON.stringify(counts); await ui.dispose(); window.dispatchEvent(new Event('resize')); @@ -91,6 +132,23 @@ try { requestAnimationFrame(() => requestAnimationFrame(resolve)), ); return { + cockpitStoppedBeforeRestoration: + stoppedBeforeRestoration && noDeferredFocus, + cockpitReleased: + cockpit.disposed && + cockpit.destroyed && + counts.cockpit === cockpitListenerCount && + cockpitListenerCount > 0 && + portal.destroyed && + portal.listeners.signal.aborted && + portal.frames.size === 0 && + ui._cockpitDisplayPortal === null && + portalHomes.length === 4 && + portalHomes.every( + (record) => + record.group.parentNode === record.home && + !record.anchor.isConnected, + ), connected: Boolean( cctvUnsubscribe && resizeHandler && @@ -116,6 +174,7 @@ try { idempotent: once === JSON.stringify(counts), }; } finally { + releaseRestoration(); window.removeEventListener = removeEventListener; } }); @@ -143,6 +202,14 @@ try { 'Radio, Location and CCTV controls are disposed with the UI', result.controlsReleased, ); + check( + 'Cockpit actions and queued portal work stop before layer restoration', + result.cockpitStoppedBeforeRestoration, + ); + check( + 'Cockpit cleanup releases listeners and returns all Display groups home', + result.cockpitReleased, + ); check('repeated UI disposal is inert', result.idempotent); check( 'disposal and subsequent resize produce no uncaught browser errors', diff --git a/src/cockpitMarkup.test.mjs b/src/cockpitMarkup.test.mjs index 7e2cd1647..adfea26bc 100644 --- a/src/cockpitMarkup.test.mjs +++ b/src/cockpitMarkup.test.mjs @@ -653,7 +653,7 @@ test('Cockpit Display portals shared HUD, Detection, Parameters, and 3D controls ); assert.match( CockpitDisplayPortal.prototype.destroy.toString(), - /setActive\(false, \{ settle: false \}\)[\s\S]*?listeners\.abort\(\)[\s\S]*?record\.anchor\.remove\(\)/, + /this\.stop\(\)[\s\S]*?record\.anchor\.after\(record\.group\)[\s\S]*?record\.anchor\.remove\(\)/, ); assert.doesNotMatch(ui, /_cycleCockpitHud|_cockpitModels3dToggle|_cockpitDetectionToggle/); assert.equal((html.match(/id="style-buttons"/g) || []).length, 1); diff --git a/src/ui.js b/src/ui.js index 66f0b5955..caae0a196 100644 --- a/src/ui.js +++ b/src/ui.js @@ -5130,6 +5130,8 @@ export class StyleManager { this._locationControls?.destroy(); this._cctvControls?.destroy(); this._radioControls?.destroy(); + this.cockpitView?.stop(); + this._cockpitDisplayPortal?.stop(); this._visualEffects.stop(); this._styleParameters?.destroy(); for (const control of this._panelDisclosureControls || []) control.destroy(); diff --git a/src/ui/cockpitController.js b/src/ui/cockpitController.js index 4f5cc33fc..b5f9077a8 100644 --- a/src/ui/cockpitController.js +++ b/src/ui/cockpitController.js @@ -63,6 +63,7 @@ export class CockpitViewController { this.viewer = viewer; this.services = services; this.destroyed = false; + this.disposed = false; this.active = false; this.trackedEntity = null; this.trackedEntityWasShown = true; @@ -407,10 +408,10 @@ export class CockpitViewController { return handleSignalClick.call(this, event); } - dispose() { + /** Revoke new work before the shell awaits layer restoration. */ + stop() { if (this.destroyed) return; this.destroyed = true; - this.exit({ restoreTracking: false }); this.regionalBriefAbort?.abort(); this.regionalBriefAbort = null; this.regionalBriefRequestToken += 1; @@ -421,4 +422,11 @@ export class CockpitViewController { for (const removeListener of this._listenerRemovers.splice(0)) removeListener?.(); } + + dispose() { + if (this.disposed) return; + this.disposed = true; + this.stop(); + this.exit({ restoreTracking: false }); + } } diff --git a/src/ui/cockpitController.test.mjs b/src/ui/cockpitController.test.mjs index 4f8018cbf..e92be00a2 100644 --- a/src/ui/cockpitController.test.mjs +++ b/src/ui/cockpitController.test.mjs @@ -141,3 +141,26 @@ test('an obsolete regional briefing cannot publish after Cockpit disposal', asyn env.restore(); } }); + +test('stopping Cockpit revokes camera actions before deferred disposal releases rendering', () => { + const env = environment(); + try { + const released = []; + const owner = new CockpitViewController(env.viewer, { + services: { + releaseContinuousRender: (reason) => released.push(reason), + }, + }); + owner.active = true; + owner.stop(); + assert.equal(env.callbacks.size, 0); + assert.equal(owner.update(), false); + assert.equal(owner.navigateContext(1), false); + assert.deepEqual(released, []); + owner.dispose(); + assert.deepEqual(released, ['cockpit']); + assert.equal(owner.active, false); + } finally { + env.restore(); + } +}); diff --git a/src/ui/cockpitDisplayPortal.js b/src/ui/cockpitDisplayPortal.js index 50b13d8b6..8c653c6f6 100644 --- a/src/ui/cockpitDisplayPortal.js +++ b/src/ui/cockpitDisplayPortal.js @@ -6,6 +6,7 @@ export class CockpitDisplayPortal { this.layout = layout; this.active = false; this.destroyed = false; + this.stopped = false; this.restoreOwner = null; this.generation = 0; this.frames = new Set(); @@ -49,7 +50,7 @@ export class CockpitDisplayPortal { _frame(generation, callback) { const frame = requestAnimationFrame(() => { this.frames.delete(frame); - if (!this.destroyed && generation === this.generation) callback(); + if (!this.stopped && generation === this.generation) callback(); }); this.frames.add(frame); } @@ -61,7 +62,7 @@ export class CockpitDisplayPortal { } setActive(active, { settle = true } = {}) { - if (this.destroyed) return; + if (this.stopped) return; const nextActive = active === true; if (this.active === nextActive) return; this._cancelFrames(); @@ -102,13 +103,25 @@ export class CockpitDisplayPortal { this.layout(); } - destroy() { - if (this.destroyed) return; - this.setActive(false, { settle: false }); - this.destroyed = true; + stop() { + if (this.stopped) return; + this.stopped = true; this._cancelFrames(); this.listeners.abort(); - for (const record of this.records) record.anchor.remove(); + } + + destroy() { + if (this.destroyed) return; + this.stop(); + this.destroyed = true; + this.active = false; + for (const record of this.records) { + if (record.anchor.parentNode) record.anchor.after(record.group); + record.anchor.remove(); + } + this.cockpitPanel?.classList.remove('uses-shared-display-controls'); + if (this.standardPanel) + this.standardPanel.scrollTop = this.standardScrollTop; this.records = []; this.restoreOwner = null; } diff --git a/src/ui/cockpitDisplayPortal.test.mjs b/src/ui/cockpitDisplayPortal.test.mjs index 219508ced..c9837891c 100644 --- a/src/ui/cockpitDisplayPortal.test.mjs +++ b/src/ui/cockpitDisplayPortal.test.mjs @@ -19,7 +19,7 @@ function fixture() { this.children = []; this.parentNode = null; this.scrollTop = 0; - this.classList = { toggle() {} }; + this.classList = { toggle() {}, remove() {} }; } append(node) { node.remove(); @@ -181,3 +181,22 @@ test('disposal restores groups and revokes mode listeners and queued focus work' f.restore(); } }); + +test('a stopped portal waits for cleanup without running queued focus work', () => { + const f = fixture(); + try { + const owner = f.create(); + owner.setActive(true); + const queued = [...f.frames.values()]; + owner.stop(); + queued.forEach((callback) => callback()); + owner.setActive(false); + assert.equal(f.group.parentNode, f.slot); + assert.equal(f.frames.size, 0); + assert.equal(f.focusCalls, 0); + owner.destroy(); + assert.deepEqual(f.standard.children, [f.group, f.sibling]); + } finally { + f.restore(); + } +}); diff --git a/src/ui/cockpitLayout.js b/src/ui/cockpitLayout.js index 058c69c99..cb6af30e1 100644 --- a/src/ui/cockpitLayout.js +++ b/src/ui/cockpitLayout.js @@ -20,6 +20,7 @@ export function scheduleContextLayout() { return; this.contextLayoutFrame = requestAnimationFrame(() => { this.contextLayoutFrame = null; + if (this.destroyed) return; this.syncContextLayout(); this.syncSignalLayout(); }); diff --git a/src/ui/cockpitTrackingController.js b/src/ui/cockpitTrackingController.js index 5a919ec46..4170333a8 100644 --- a/src/ui/cockpitTrackingController.js +++ b/src/ui/cockpitTrackingController.js @@ -247,7 +247,7 @@ export function exit({ restoreTracking = true } = {}) { ? this.entry : this.entry || this.entryFocusOrigin; this.entryFocusOrigin = null; - if (restoreTarget?.isConnected && !restoreTarget.hidden) { + if (!this.destroyed && restoreTarget?.isConnected && !restoreTarget.hidden) { restoreTarget.focus({ preventScroll: true }); } return true;