From 4788b3e080b9782a0874dd4bde87d35bccc1c8a2 Mon Sep 17 00:00:00 2001
From: Kyle <kyle.j.t@live.co.uk>
Date: Sat, 18 Jul 2026 22:57:12 -0400
Subject: [PATCH] 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.
---
 plugins/highway_3d/screen.js                  | 13 +++++++
 .../tests/background_control.test.js          | 39 ++++++++++++++++++-
 2 files changed, 50 insertions(+), 2 deletions(-)

diff --git a/plugins/highway_3d/screen.js b/plugins/highway_3d/screen.js
index 482185f..bdeee2d 100644
--- a/plugins/highway_3d/screen.js
+++ b/plugins/highway_3d/screen.js
@@ -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();
     }
 
diff --git a/plugins/highway_3d/tests/background_control.test.js b/plugins/highway_3d/tests/background_control.test.js
index 537fe47..abd5d30 100644
--- a/plugins/highway_3d/tests/background_control.test.js
+++ b/plugins/highway_3d/tests/background_control.test.js
@@ -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();