Unbind screen:changed hook on last release

Ensure the highway_3d control removes its screen:changed listener when the last reference is released to avoid listener/closure leaks across plugin reloads. Added a best-effort off() call and clears _pcScreenHook so future acquires re-bind correctly. Tests updated: mock feedBack on/off implemented, helpers added (screenHooks, fireScreenChanged), and a new test verifies the subscription is removed on final _pcRelease and re-subscribed on re-acquire.
This commit is contained in:
Kyle 2026-07-18 22:57:12 -04:00
parent 048cb1a66e
commit 4788b3e080
2 changed files with 50 additions and 2 deletions

View File

@ -4259,6 +4259,19 @@
_pcRefs = Math.max(0, _pcRefs - 1);
if (_pcRefs > 0) return;
if (_pcRetryTimer) { clearTimeout(_pcRetryTimer); _pcRetryTimer = 0; }
// Drop the screen:changed subscription too, not just the DOM. The
// refcount guard inside the hook makes a stale one harmless, but the
// listener and its closure would otherwise outlive the control for the
// page's lifetime — and a plugin re-load (new ?v=) evaluates this file
// again, binding another hook to the same bus while the old one stays.
// _pcBindScreenHook re-binds on the next acquire.
if (_pcScreenHook) {
try {
const bus = window.feedBack;
if (bus && typeof bus.off === 'function') bus.off('screen:changed', _pcScreenHook);
} catch (e) { /* best-effort: a host without off() just keeps the no-op hook */ }
_pcScreenHook = null;
}
_pcTeardownDom();
}

View File

@ -118,6 +118,7 @@ function load({ store: initialStore } = {}) {
customVideoName: '',
}, initialStore);
const bus = {};
const listeners = new Set();
const emit = (key) => { for (const fn of listeners) fn(key); };
const writes = [];
@ -137,7 +138,19 @@ function load({ store: initialStore } = {}) {
getElementById: () => null,
},
window: {
feedBack: { ui: { playerControlSlot: () => dom.slot } },
feedBack: {
ui: { playerControlSlot: () => dom.slot },
// The real bus is an EventTarget wrapper exposing on/off. Modelled
// here so the screen:changed subscription — and its removal — are
// observable.
on: (ev, fn) => { (bus[ev] || (bus[ev] = [])).push(fn); },
off: (ev, fn) => {
const l = bus[ev];
if (!l) return;
const i = l.indexOf(fn);
if (i >= 0) l.splice(i, 1);
},
},
h3dBgSetStyle: (v) => { writes.push(['style', v]); store.style = v; emit('style'); },
h3dBgSetReactive: (v) => { writes.push(['reactive', v]); store.reactive = v; emit('reactive'); },
h3dBgSetIntensity: (v) => { writes.push(['intensity', v]); store.intensity = v; emit('intensity'); },
@ -155,7 +168,9 @@ function load({ store: initialStore } = {}) {
+ ' get refs() { return _pcRefs; } })',
sandbox,
);
return { api, dom, store, emit, writes, timers, sandbox, listenerCount: () => listeners.size };
const fireScreenChanged = () => (bus['screen:changed'] || []).slice().forEach((fn) => fn());
const screenHooks = () => (bus['screen:changed'] || []).length;
return { api, dom, store, emit, writes, timers, sandbox, listenerCount: () => listeners.size, fireScreenChanged, screenHooks };
}
test('mounts one control into the player-control slot', () => {
@ -184,6 +199,26 @@ test('multiple renderer instances share a single control', () => {
assert.equal(api.el, null);
});
test('the last release unbinds the screen:changed hook', () => {
const ctl = load();
ctl.api._pcAcquire();
assert.equal(ctl.screenHooks(), 1, 'acquire should subscribe once');
ctl.api._pcAcquire();
ctl.api._pcRelease();
assert.equal(ctl.screenHooks(), 1, 'a partial release must keep the hook');
ctl.api._pcRelease();
assert.equal(ctl.screenHooks(), 0, 'the hook outlived the control');
// And re-acquiring must re-subscribe exactly once, not zero times (the
// bind is guarded on _pcScreenHook, so failing to null it would leave the
// control permanently deaf to chrome rebuilds).
ctl.api._pcAcquire();
assert.equal(ctl.screenHooks(), 1, 're-acquire did not re-subscribe');
ctl.api._pcRelease();
});
test('teardown unsubscribes from the settings bus', () => {
const ctl = load();
ctl.api._pcAcquire();