fix(player): own connection and media resources (#2152)

## What

Make Player connection resources symmetric and scope slideshow media ownership.

## Why

Reconnect could retain destroyed handles, while slideshow operations scanned and mutated unrelated document media.

## How

Null and recreate connection-owned resources idempotently and introduce an OwnedMediaRegistry with abortable cleanup.

## Test plan

- [x] Player reconnect, media-scope, slideshow, and owned-media registry tests
- [x] Stack-wide lint, format, build, typecheck, and relevant integration gates
This commit is contained in:
James Russo
2026-07-13 16:15:06 -04:00
committed by GitHub
7 changed files with 274 additions and 53 deletions
@@ -418,6 +418,58 @@ describe("HyperframesPlayer parent-frame media", () => {
expect(mockAudio.src).toBe("");
});
it("owns exactly one controls, media, and listener set across ten reconnects", () => {
const reconnectingPlayer = player as PlayerElement & {
readonly iframeElement: HTMLIFrameElement;
readonly paused: boolean;
readonly _parentMedia: unknown[];
shadowRoot: ShadowRoot;
};
reconnectingPlayer.setAttribute("controls", "");
reconnectingPlayer.setAttribute("audio-src", "https://cdn.example.com/narration.mp3");
const windowAdd = vi.spyOn(window, "addEventListener");
const windowRemove = vi.spyOn(window, "removeEventListener");
const iframeAdd = vi.spyOn(reconnectingPlayer.iframeElement, "addEventListener");
const iframeRemove = vi.spyOn(reconnectingPlayer.iframeElement, "removeEventListener");
for (let cycle = 0; cycle < 10; cycle++) {
document.body.appendChild(reconnectingPlayer);
expect(reconnectingPlayer.shadowRoot.querySelectorAll(".hfp-controls")).toHaveLength(1);
expect(reconnectingPlayer._parentMedia).toHaveLength(1);
reconnectingPlayer.remove();
expect(reconnectingPlayer.shadowRoot.querySelectorAll(".hfp-controls")).toHaveLength(0);
expect(reconnectingPlayer._parentMedia).toHaveLength(0);
expect(reconnectingPlayer.paused).toBe(true);
}
document.body.appendChild(reconnectingPlayer);
expect(reconnectingPlayer.shadowRoot.querySelectorAll(".hfp-controls")).toHaveLength(1);
expect(reconnectingPlayer._parentMedia).toHaveLength(1);
expect(
windowAdd.mock.calls.filter(([eventName]) => eventName === "message").length -
windowRemove.mock.calls.filter(([eventName]) => eventName === "message").length,
).toBe(1);
expect(
iframeAdd.mock.calls.filter(([eventName]) => eventName === "load").length -
iframeRemove.mock.calls.filter(([eventName]) => eventName === "load").length,
).toBe(1);
});
it("returns parent-media ownership to the runtime after reconnect", () => {
player.setAttribute("audio-src", "https://cdn.example.com/narration.mp3");
document.body.appendChild(player);
player._promoteToParentProxy?.();
expect(player._audioOwner).toBe("parent");
player.remove();
document.body.appendChild(player);
expect(player._audioOwner).toBe("runtime");
});
it("updates parent media when playback-rate changes after setup", () => {
player.setAttribute("audio-src", "https://cdn.example.com/narration.mp3");
document.body.appendChild(player);
@@ -177,6 +177,8 @@ class HyperframesPlayer extends HTMLElement {
}
disconnectedCallback() {
this._sendControl("pause");
this._stopIframeMedia();
this.resizeObserver.disconnect();
window.removeEventListener("message", this._onMessage);
this.iframe.removeEventListener("load", this._onIframeLoad);
@@ -187,6 +189,9 @@ class HyperframesPlayer extends HTMLElement {
this.shaderLoader.destroy();
this._media.destroy();
this.controlsApi?.destroy();
this.controlsApi = null;
this._paused = true;
this._ready = false;
}
// fallow-ignore-next-line complexity
@@ -755,6 +760,7 @@ class HyperframesPlayer extends HTMLElement {
}
private _onIframeLoad() {
this._ready = false;
this._directTimelineAdapter = null;
this._directTimelineClock.stop();
this._stopParentTickClock();
+2
View File
@@ -111,6 +111,8 @@ export class ParentMediaManager {
this._entries = [];
this._urlAudioEntry = null;
this._urlAudioSrc = null;
this._audioOwner = "runtime";
this._playbackErrorPosted = false;
}
updateMuted(muted: boolean): void {
@@ -394,35 +394,70 @@ describe("<hyperframes-slideshow>", () => {
el.remove();
});
it("mute button applies globally to child players and page media", () => {
const el = document.createElement("hyperframes-slideshow") as any;
el.setAttribute("sound", "");
const player = document.createElement("hyperframes-player") as any;
player.muted = false;
el.appendChild(player);
it("mute and stopMedia affect only media owned by that slideshow", () => {
const bind = (el: any) => {
el.__setControllerForTest({
next: () => {},
prev: () => {},
onChange: () => () => {},
counter: { index: 1, total: 1 },
breadcrumb: [{ id: "main", label: "Main deck" }],
currentSlide: { hotspots: [] },
nextSlide: null,
});
};
const attachPlayerMedia = (slideshow: HTMLElement, id: string) => {
const player = document.createElement("hyperframes-player") as any;
player.id = id;
player.muted = false;
const frame = document.createElement("iframe");
document.body.appendChild(frame);
const frameDoc = frame.contentDocument!;
const video = frameDoc.createElement("video");
video.id = `${id}-video`;
video.pause = vi.fn();
frameDoc.body.appendChild(video);
Object.defineProperty(player, "iframeElement", { configurable: true, value: frame });
slideshow.appendChild(player);
return { player, frame, video };
};
const slideshowA = document.createElement("hyperframes-slideshow") as any;
const slideshowB = document.createElement("hyperframes-slideshow") as any;
slideshowA.setAttribute("sound", "");
slideshowB.setAttribute("sound", "");
const ownedA = attachPlayerMedia(slideshowA, "a");
const ownedB = attachPlayerMedia(slideshowB, "b");
const pageVideo = document.createElement("video");
document.body.append(pageVideo, el);
el.__setControllerForTest({
next: () => {},
prev: () => {},
onChange: () => () => {},
counter: { index: 1, total: 1 },
breadcrumb: [{ id: "main", label: "Main deck" }],
currentSlide: { hotspots: [] },
nextSlide: null,
});
pageVideo.pause = vi.fn();
document.body.append(pageVideo, slideshowA, slideshowB);
bind(slideshowA);
bind(slideshowB);
const muteBtn = el.querySelector("[data-hf-mute]") as HTMLElement;
const muteBtn = slideshowA.querySelector("[data-hf-mute]") as HTMLElement;
muteBtn.click();
expect(player.muted).toBe(true);
expect(pageVideo.muted).toBe(true);
const muteBtnAfter = el.querySelector("[data-hf-mute]") as HTMLElement;
muteBtnAfter.click();
expect(player.muted).toBe(false);
expect(ownedA.player.muted).toBe(true);
expect(ownedA.video.muted).toBe(true);
expect(ownedB.player.muted).toBe(false);
expect(ownedB.video.muted).toBe(false);
expect(pageVideo.muted).toBe(false);
el.remove();
const muteBtnAfter = slideshowA.querySelector("[data-hf-mute]") as HTMLElement;
muteBtnAfter.click();
expect(ownedA.player.muted).toBe(false);
expect(ownedA.video.muted).toBe(false);
expect(ownedB.video.muted).toBe(false);
expect(pageVideo.muted).toBe(false);
slideshowA.stopDocumentMedia();
expect(ownedA.video.pause).toHaveBeenCalledTimes(1);
expect(ownedB.video.pause).not.toHaveBeenCalled();
expect(pageVideo.pause).not.toHaveBeenCalled();
slideshowA.remove();
slideshowB.remove();
ownedA.frame.remove();
ownedB.frame.remove();
pageVideo.remove();
});
@@ -11,6 +11,7 @@ import {
type PresenterMediaAction,
type PresenterMediaMessage,
} from "./slideshowPresenter";
import { OwnedMediaRegistry } from "./owned-media-registry";
interface Hotspot {
id: string;
@@ -68,9 +69,18 @@ type PlayerElement = HTMLElement & {
readonly ready: boolean;
};
type SlideshowMediaElement = HTMLMediaElement & {
dataset: DOMStringMap;
};
type SlideshowMediaElement = HTMLMediaElement;
const SLIDESHOW_MEDIA_ACTIONS = [
"play",
"pause",
"seeking",
"seeked",
"ratechange",
"volumechange",
"ended",
"timeupdate",
] as const satisfies readonly PresenterMediaAction[];
/** True when the keydown originated in a text-entry control (typing must never
* navigate the deck). Duck-typed so it works for events from the composition
@@ -202,6 +212,10 @@ export class HyperframesSlideshow extends HTMLElement {
// Bumped whenever autoplay starts or media is stopped (slide change), so a
// pending re-assert from a previous autoplay can't replay a clip we've left.
private autoplayToken = 0;
private readonly ownedMedia = new OwnedMediaRegistry<PresenterMediaAction>(
SLIDESHOW_MEDIA_ACTIONS,
(el, key, action) => this.publishMediaState(el, key, action),
);
/** Whether audio is currently muted. Reflects `data-hf-muted` attribute. */
get muted(): boolean {
@@ -291,6 +305,7 @@ export class HyperframesSlideshow extends HTMLElement {
this.playerObserver.disconnect();
this.playerObserver = null;
}
this.ownedMedia.clear();
this.audienceMediaUnlockButton?.remove();
this.audienceMediaUnlockButton = null;
this.audienceMutedPlaybackKeys.clear();
@@ -545,8 +560,8 @@ export class HyperframesSlideshow extends HTMLElement {
this.wireSlideshowMedia();
if (this.mediaWireInterval === null) {
// Same-origin player iframes can hydrate media after the slideshow binds.
// The dataset guard prevents duplicate listeners, and removed iframe nodes
// are collectable because this component keeps no media element references.
// The owned registry prevents duplicate listeners and releases removed
// iframe nodes through AbortController-backed teardown.
this.mediaWireInterval = setInterval(() => this.wireSlideshowMedia(), 1000);
}
}
@@ -668,22 +683,9 @@ export class HyperframesSlideshow extends HTMLElement {
}
private wireSlideshowMedia(): void {
const actions: PresenterMediaAction[] = [
"play",
"pause",
"seeking",
"seeked",
"ratechange",
"volumechange",
"ended",
"timeupdate",
];
for (const { key, el } of this.mediaEntries()) {
if (el.dataset.hfSlideshowMediaSync === "1") continue;
el.dataset.hfSlideshowMediaSync = "1";
for (const action of actions) {
el.addEventListener(action, () => this.publishMediaState(el, key, action));
}
const added = this.ownedMedia.sync(this.mediaEntries());
for (const el of added) {
el.muted = this._muted || el.defaultMuted;
}
}
@@ -1161,20 +1163,16 @@ export class HyperframesSlideshow extends HTMLElement {
}
}
const doc = this.ownerDocument;
for (const el of doc.querySelectorAll("video, audio")) {
if (el instanceof HTMLMediaElement) el.muted = muted || el.defaultMuted;
}
this.ownedMedia.sync(this.mediaEntries());
this.ownedMedia.setMuted(muted);
}
private stopDocumentMedia(): void {
// Invalidate any in-flight autoplay re-assert so leaving a slide can't be
// undone by a pending timeout replaying the clip we just paused.
this.autoplayToken++;
const doc = this.ownerDocument;
for (const el of doc.querySelectorAll("video, audio")) {
if (el instanceof HTMLMediaElement) el.pause();
}
this.ownedMedia.sync(this.mediaEntries());
this.ownedMedia.pauseAll();
}
/**
@@ -0,0 +1,54 @@
import { describe, expect, it, vi } from "vitest";
import { OwnedMediaRegistry } from "./owned-media-registry.js";
describe("OwnedMediaRegistry", () => {
it("installs one abortable listener set and rebinds when the stable key changes", () => {
const video = document.createElement("video");
const onAction = vi.fn();
const registry = new OwnedMediaRegistry(["play", "pause"] as const, onAction);
registry.sync([{ key: "slide-a", el: video }]);
registry.sync([{ key: "slide-a", el: video }]);
video.dispatchEvent(new Event("play"));
expect(onAction).toHaveBeenCalledTimes(1);
expect(onAction).toHaveBeenLastCalledWith(video, "slide-a", "play");
registry.sync([{ key: "slide-b", el: video }]);
video.dispatchEvent(new Event("pause"));
expect(onAction).toHaveBeenCalledTimes(2);
expect(onAction).toHaveBeenLastCalledWith(video, "slide-b", "pause");
registry.sync([]);
video.dispatchEvent(new Event("play"));
expect(onAction).toHaveBeenCalledTimes(2);
});
it("mutates and pauses only the currently owned media", () => {
const owned = document.createElement("video");
const unrelated = document.createElement("video");
owned.pause = vi.fn();
unrelated.pause = vi.fn();
const registry = new OwnedMediaRegistry(["play"] as const, vi.fn());
registry.sync([{ key: "owned", el: owned }]);
registry.setMuted(true);
registry.pauseAll();
expect(owned.muted).toBe(true);
expect(owned.pause).toHaveBeenCalledTimes(1);
expect(unrelated.muted).toBe(false);
expect(unrelated.pause).not.toHaveBeenCalled();
});
it("aborts every listener when the slideshow disconnects", () => {
const audio = document.createElement("audio");
const onAction = vi.fn();
const registry = new OwnedMediaRegistry(["play"] as const, onAction);
registry.sync([{ key: "audio", el: audio }]);
registry.clear();
audio.dispatchEvent(new Event("play"));
expect(onAction).not.toHaveBeenCalled();
});
});
@@ -0,0 +1,74 @@
export type OwnedMediaEntry = {
key: string;
el: HTMLMediaElement;
};
type MediaBinding = {
key: string;
controller: AbortController;
};
/**
* Owns the media event listeners and imperative mutations for one slideshow.
* A WeakMap ties each listener lifetime to its media element while the Set
* provides the bounded iteration needed for mute, pause, and deterministic
* teardown. No marker attributes leak ownership into shared DOM.
*/
export class OwnedMediaRegistry<Action extends string> {
private bindings = new WeakMap<HTMLMediaElement, MediaBinding>();
private owned = new Set<HTMLMediaElement>();
constructor(
private readonly actions: readonly Action[],
private readonly onAction: (el: HTMLMediaElement, key: string, action: Action) => void,
) {}
sync(entries: readonly OwnedMediaEntry[]): HTMLMediaElement[] {
const next = new Set<HTMLMediaElement>();
const added: HTMLMediaElement[] = [];
for (const { key, el } of entries) {
next.add(el);
const binding = this.bindings.get(el);
if (binding?.key === key) continue;
binding?.controller.abort();
const AbortControllerCtor = el.ownerDocument.defaultView?.AbortController ?? AbortController;
const controller = new AbortControllerCtor();
this.bindings.set(el, { key, controller });
added.push(el);
for (const action of this.actions) {
el.addEventListener(action, () => this.onAction(el, key, action), {
signal: controller.signal,
});
}
}
for (const el of this.owned) {
if (next.has(el)) continue;
this.bindings.get(el)?.controller.abort();
this.bindings.delete(el);
}
this.owned = next;
return added;
}
setMuted(muted: boolean): void {
for (const el of this.owned) {
el.muted = muted || el.defaultMuted;
}
}
pauseAll(): void {
for (const el of this.owned) {
el.pause();
}
}
clear(): void {
for (const el of this.owned) {
this.bindings.get(el)?.controller.abort();
}
this.owned.clear();
this.bindings = new WeakMap();
}
}