fix: stop Cockpit work before asynchronous UI teardown

This commit is contained in:
Sameh Khamis committed 2026-09-13 02:46:57 -07:00
1 parent 947fc79182
commit 152d3e2352
11 files changed
+150 -15

No files matched your search

+1 -1
View File
@@ -14,7 +14,7 @@ of current runtime behavior, see [`docs/CURRENT-STATE.md`](docs/CURRENT-STATE.md
## [Unreleased] ## [Unreleased]
- Split Cockpit camera/controller, instruments, briefing, signals and layout into focused modules with explicit application operations. - 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. - Separate Context controls, mode transitions and layer restoration; release tab listeners and suppress late panel/search feedback after disposal.
+3 -1
View File
@@ -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 same tracked identity, ground acquisition, motion correction and cadence.
The Display portal owns group anchors, focus/scroll restoration and listeners. The Display portal owns group anchors, focus/scroll restoration and listeners.
Superseded portal frames cannot repaint old state or steal focus after disposal; 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 ## Context coordination
+68 -1
View File
@@ -75,6 +75,33 @@ try {
return unsubscribe?.(); 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 resizeHandler = ui._windowResizeHandler;
const removeEventListener = window.removeEventListener; const removeEventListener = window.removeEventListener;
counts.resize = 0; counts.resize = 0;
@@ -83,7 +110,21 @@ try {
return removeEventListener.call(this, type, callback, options); return removeEventListener.call(this, type, callback, options);
}; };
try { 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); const once = JSON.stringify(counts);
await ui.dispose(); await ui.dispose();
window.dispatchEvent(new Event('resize')); window.dispatchEvent(new Event('resize'));
@@ -91,6 +132,23 @@ try {
requestAnimationFrame(() => requestAnimationFrame(resolve)), requestAnimationFrame(() => requestAnimationFrame(resolve)),
); );
return { 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( connected: Boolean(
cctvUnsubscribe && cctvUnsubscribe &&
resizeHandler && resizeHandler &&
@@ -116,6 +174,7 @@ try {
idempotent: once === JSON.stringify(counts), idempotent: once === JSON.stringify(counts),
}; };
} finally { } finally {
releaseRestoration();
window.removeEventListener = removeEventListener; window.removeEventListener = removeEventListener;
} }
}); });
@@ -143,6 +202,14 @@ try {
'Radio, Location and CCTV controls are disposed with the UI', 'Radio, Location and CCTV controls are disposed with the UI',
result.controlsReleased, 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('repeated UI disposal is inert', result.idempotent);
check( check(
'disposal and subsequent resize produce no uncaught browser errors', 'disposal and subsequent resize produce no uncaught browser errors',
+1 -1
View File
@@ -653,7 +653,7 @@ test('Cockpit Display portals shared HUD, Detection, Parameters, and 3D controls
); );
assert.match( assert.match(
CockpitDisplayPortal.prototype.destroy.toString(), 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.doesNotMatch(ui, /_cycleCockpitHud|_cockpitModels3dToggle|_cockpitDetectionToggle/);
assert.equal((html.match(/id="style-buttons"/g) || []).length, 1); assert.equal((html.match(/id="style-buttons"/g) || []).length, 1);
+2
View File
@@ -5130,6 +5130,8 @@ export class StyleManager {
this._locationControls?.destroy(); this._locationControls?.destroy();
this._cctvControls?.destroy(); this._cctvControls?.destroy();
this._radioControls?.destroy(); this._radioControls?.destroy();
this.cockpitView?.stop();
this._cockpitDisplayPortal?.stop();
this._visualEffects.stop(); this._visualEffects.stop();
this._styleParameters?.destroy(); this._styleParameters?.destroy();
for (const control of this._panelDisclosureControls || []) control.destroy(); for (const control of this._panelDisclosureControls || []) control.destroy();
+10 -2
View File
@@ -63,6 +63,7 @@ export class CockpitViewController {
this.viewer = viewer; this.viewer = viewer;
this.services = services; this.services = services;
this.destroyed = false; this.destroyed = false;
this.disposed = false;
this.active = false; this.active = false;
this.trackedEntity = null; this.trackedEntity = null;
this.trackedEntityWasShown = true; this.trackedEntityWasShown = true;
@@ -407,10 +408,10 @@ export class CockpitViewController {
return handleSignalClick.call(this, event); return handleSignalClick.call(this, event);
} }
dispose() { /** Revoke new work before the shell awaits layer restoration. */
stop() {
if (this.destroyed) return; if (this.destroyed) return;
this.destroyed = true; this.destroyed = true;
this.exit({ restoreTracking: false });
this.regionalBriefAbort?.abort(); this.regionalBriefAbort?.abort();
this.regionalBriefAbort = null; this.regionalBriefAbort = null;
this.regionalBriefRequestToken += 1; this.regionalBriefRequestToken += 1;
@@ -421,4 +422,11 @@ export class CockpitViewController {
for (const removeListener of this._listenerRemovers.splice(0)) for (const removeListener of this._listenerRemovers.splice(0))
removeListener?.(); removeListener?.();
} }
dispose() {
if (this.disposed) return;
this.disposed = true;
this.stop();
this.exit({ restoreTracking: false });
}
} }
+23
View File
@@ -141,3 +141,26 @@ test('an obsolete regional briefing cannot publish after Cockpit disposal', asyn
env.restore(); 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();
}
});
+20 -7
View File
@@ -6,6 +6,7 @@ export class CockpitDisplayPortal {
this.layout = layout; this.layout = layout;
this.active = false; this.active = false;
this.destroyed = false; this.destroyed = false;
this.stopped = false;
this.restoreOwner = null; this.restoreOwner = null;
this.generation = 0; this.generation = 0;
this.frames = new Set(); this.frames = new Set();
@@ -49,7 +50,7 @@ export class CockpitDisplayPortal {
_frame(generation, callback) { _frame(generation, callback) {
const frame = requestAnimationFrame(() => { const frame = requestAnimationFrame(() => {
this.frames.delete(frame); this.frames.delete(frame);
if (!this.destroyed && generation === this.generation) callback(); if (!this.stopped && generation === this.generation) callback();
}); });
this.frames.add(frame); this.frames.add(frame);
} }
@@ -61,7 +62,7 @@ export class CockpitDisplayPortal {
} }
setActive(active, { settle = true } = {}) { setActive(active, { settle = true } = {}) {
if (this.destroyed) return; if (this.stopped) return;
const nextActive = active === true; const nextActive = active === true;
if (this.active === nextActive) return; if (this.active === nextActive) return;
this._cancelFrames(); this._cancelFrames();
@@ -102,13 +103,25 @@ export class CockpitDisplayPortal {
this.layout(); this.layout();
} }
destroy() { stop() {
if (this.destroyed) return; if (this.stopped) return;
this.setActive(false, { settle: false }); this.stopped = true;
this.destroyed = true;
this._cancelFrames(); this._cancelFrames();
this.listeners.abort(); 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.records = [];
this.restoreOwner = null; this.restoreOwner = null;
} }
+20 -1
View File
@@ -19,7 +19,7 @@ function fixture() {
this.children = []; this.children = [];
this.parentNode = null; this.parentNode = null;
this.scrollTop = 0; this.scrollTop = 0;
this.classList = { toggle() {} }; this.classList = { toggle() {}, remove() {} };
} }
append(node) { append(node) {
node.remove(); node.remove();
@@ -181,3 +181,22 @@ test('disposal restores groups and revokes mode listeners and queued focus work'
f.restore(); 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();
}
});
+1
View File
@@ -20,6 +20,7 @@ export function scheduleContextLayout() {
return; return;
this.contextLayoutFrame = requestAnimationFrame(() => { this.contextLayoutFrame = requestAnimationFrame(() => {
this.contextLayoutFrame = null; this.contextLayoutFrame = null;
if (this.destroyed) return;
this.syncContextLayout(); this.syncContextLayout();
this.syncSignalLayout(); this.syncSignalLayout();
}); });
+1 -1
View File
@@ -247,7 +247,7 @@ export function exit({ restoreTracking = true } = {}) {
? this.entry ? this.entry
: this.entry || this.entryFocusOrigin; : this.entry || this.entryFocusOrigin;
this.entryFocusOrigin = null; this.entryFocusOrigin = null;
if (restoreTarget?.isConnected && !restoreTarget.hidden) { if (!this.destroyed && restoreTarget?.isConnected && !restoreTarget.hidden) {
restoreTarget.focus({ preventScroll: true }); restoreTarget.focus({ preventScroll: true });
} }
return true; return true;