Skip to content

Commit 7bb3529

Browse files
committed
fix(player): own connection and media resources
1 parent ab96a94 commit 7bb3529

7 files changed

Lines changed: 272 additions & 51 deletions

packages/player/src/hyperframes-player.test.ts

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -418,6 +418,58 @@ describe("HyperframesPlayer parent-frame media", () => {
418418
expect(mockAudio.src).toBe("");
419419
});
420420

421+
it("owns exactly one controls, media, and listener set across ten reconnects", () => {
422+
const reconnectingPlayer = player as PlayerElement & {
423+
readonly iframeElement: HTMLIFrameElement;
424+
readonly paused: boolean;
425+
readonly _parentMedia: unknown[];
426+
shadowRoot: ShadowRoot;
427+
};
428+
reconnectingPlayer.setAttribute("controls", "");
429+
reconnectingPlayer.setAttribute("audio-src", "https://cdn.example.com/narration.mp3");
430+
431+
const windowAdd = vi.spyOn(window, "addEventListener");
432+
const windowRemove = vi.spyOn(window, "removeEventListener");
433+
const iframeAdd = vi.spyOn(reconnectingPlayer.iframeElement, "addEventListener");
434+
const iframeRemove = vi.spyOn(reconnectingPlayer.iframeElement, "removeEventListener");
435+
436+
for (let cycle = 0; cycle < 10; cycle++) {
437+
document.body.appendChild(reconnectingPlayer);
438+
expect(reconnectingPlayer.shadowRoot.querySelectorAll(".hfp-controls")).toHaveLength(1);
439+
expect(reconnectingPlayer._parentMedia).toHaveLength(1);
440+
441+
reconnectingPlayer.remove();
442+
expect(reconnectingPlayer.shadowRoot.querySelectorAll(".hfp-controls")).toHaveLength(0);
443+
expect(reconnectingPlayer._parentMedia).toHaveLength(0);
444+
expect(reconnectingPlayer.paused).toBe(true);
445+
}
446+
447+
document.body.appendChild(reconnectingPlayer);
448+
449+
expect(reconnectingPlayer.shadowRoot.querySelectorAll(".hfp-controls")).toHaveLength(1);
450+
expect(reconnectingPlayer._parentMedia).toHaveLength(1);
451+
expect(
452+
windowAdd.mock.calls.filter(([eventName]) => eventName === "message").length -
453+
windowRemove.mock.calls.filter(([eventName]) => eventName === "message").length,
454+
).toBe(1);
455+
expect(
456+
iframeAdd.mock.calls.filter(([eventName]) => eventName === "load").length -
457+
iframeRemove.mock.calls.filter(([eventName]) => eventName === "load").length,
458+
).toBe(1);
459+
});
460+
461+
it("returns parent-media ownership to the runtime after reconnect", () => {
462+
player.setAttribute("audio-src", "https://cdn.example.com/narration.mp3");
463+
document.body.appendChild(player);
464+
player._promoteToParentProxy?.();
465+
expect(player._audioOwner).toBe("parent");
466+
467+
player.remove();
468+
document.body.appendChild(player);
469+
470+
expect(player._audioOwner).toBe("runtime");
471+
});
472+
421473
it("updates parent media when playback-rate changes after setup", () => {
422474
player.setAttribute("audio-src", "https://cdn.example.com/narration.mp3");
423475
document.body.appendChild(player);

packages/player/src/hyperframes-player.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -177,6 +177,8 @@ class HyperframesPlayer extends HTMLElement {
177177
}
178178

179179
disconnectedCallback() {
180+
this._sendControl("pause");
181+
this._stopIframeMedia();
180182
this.resizeObserver.disconnect();
181183
window.removeEventListener("message", this._onMessage);
182184
this.iframe.removeEventListener("load", this._onIframeLoad);
@@ -187,6 +189,9 @@ class HyperframesPlayer extends HTMLElement {
187189
this.shaderLoader.destroy();
188190
this._media.destroy();
189191
this.controlsApi?.destroy();
192+
this.controlsApi = null;
193+
this._paused = true;
194+
this._ready = false;
190195
}
191196

192197
// fallow-ignore-next-line complexity
@@ -749,6 +754,7 @@ class HyperframesPlayer extends HTMLElement {
749754
}
750755

751756
private _onIframeLoad() {
757+
this._ready = false;
752758
this._directTimelineAdapter = null;
753759
this._directTimelineClock.stop();
754760
this._stopParentTickClock();

packages/player/src/parent-media.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,8 @@ export class ParentMediaManager {
111111
this._entries = [];
112112
this._urlAudioEntry = null;
113113
this._urlAudioSrc = null;
114+
this._audioOwner = "runtime";
115+
this._playbackErrorPosted = false;
114116
}
115117

116118
updateMuted(muted: boolean): void {

packages/player/src/slideshow/hyperframes-slideshow.test.ts

Lines changed: 57 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -394,35 +394,70 @@ describe("<hyperframes-slideshow>", () => {
394394
el.remove();
395395
});
396396

397-
it("mute button applies globally to child players and page media", () => {
398-
const el = document.createElement("hyperframes-slideshow") as any;
399-
el.setAttribute("sound", "");
400-
const player = document.createElement("hyperframes-player") as any;
401-
player.muted = false;
402-
el.appendChild(player);
397+
it("mute and stopMedia affect only media owned by that slideshow", () => {
398+
const bind = (el: any) => {
399+
el.__setControllerForTest({
400+
next: () => {},
401+
prev: () => {},
402+
onChange: () => () => {},
403+
counter: { index: 1, total: 1 },
404+
breadcrumb: [{ id: "main", label: "Main deck" }],
405+
currentSlide: { hotspots: [] },
406+
nextSlide: null,
407+
});
408+
};
409+
const attachPlayerMedia = (slideshow: HTMLElement, id: string) => {
410+
const player = document.createElement("hyperframes-player") as any;
411+
player.id = id;
412+
player.muted = false;
413+
const frame = document.createElement("iframe");
414+
document.body.appendChild(frame);
415+
const frameDoc = frame.contentDocument!;
416+
const video = frameDoc.createElement("video");
417+
video.id = `${id}-video`;
418+
video.pause = vi.fn();
419+
frameDoc.body.appendChild(video);
420+
Object.defineProperty(player, "iframeElement", { configurable: true, value: frame });
421+
slideshow.appendChild(player);
422+
return { player, frame, video };
423+
};
424+
425+
const slideshowA = document.createElement("hyperframes-slideshow") as any;
426+
const slideshowB = document.createElement("hyperframes-slideshow") as any;
427+
slideshowA.setAttribute("sound", "");
428+
slideshowB.setAttribute("sound", "");
429+
const ownedA = attachPlayerMedia(slideshowA, "a");
430+
const ownedB = attachPlayerMedia(slideshowB, "b");
403431
const pageVideo = document.createElement("video");
404-
document.body.append(pageVideo, el);
405-
el.__setControllerForTest({
406-
next: () => {},
407-
prev: () => {},
408-
onChange: () => () => {},
409-
counter: { index: 1, total: 1 },
410-
breadcrumb: [{ id: "main", label: "Main deck" }],
411-
currentSlide: { hotspots: [] },
412-
nextSlide: null,
413-
});
432+
pageVideo.pause = vi.fn();
433+
document.body.append(pageVideo, slideshowA, slideshowB);
434+
bind(slideshowA);
435+
bind(slideshowB);
414436

415-
const muteBtn = el.querySelector("[data-hf-mute]") as HTMLElement;
437+
const muteBtn = slideshowA.querySelector("[data-hf-mute]") as HTMLElement;
416438
muteBtn.click();
417-
expect(player.muted).toBe(true);
418-
expect(pageVideo.muted).toBe(true);
439+
expect(ownedA.player.muted).toBe(true);
440+
expect(ownedA.video.muted).toBe(true);
441+
expect(ownedB.player.muted).toBe(false);
442+
expect(ownedB.video.muted).toBe(false);
443+
expect(pageVideo.muted).toBe(false);
419444

420-
const muteBtnAfter = el.querySelector("[data-hf-mute]") as HTMLElement;
445+
const muteBtnAfter = slideshowA.querySelector("[data-hf-mute]") as HTMLElement;
421446
muteBtnAfter.click();
422-
expect(player.muted).toBe(false);
447+
expect(ownedA.player.muted).toBe(false);
448+
expect(ownedA.video.muted).toBe(false);
449+
expect(ownedB.video.muted).toBe(false);
423450
expect(pageVideo.muted).toBe(false);
424451

425-
el.remove();
452+
slideshowA.stopDocumentMedia();
453+
expect(ownedA.video.pause).toHaveBeenCalledTimes(1);
454+
expect(ownedB.video.pause).not.toHaveBeenCalled();
455+
expect(pageVideo.pause).not.toHaveBeenCalled();
456+
457+
slideshowA.remove();
458+
slideshowB.remove();
459+
ownedA.frame.remove();
460+
ownedB.frame.remove();
426461
pageVideo.remove();
427462
});
428463

packages/player/src/slideshow/hyperframes-slideshow.ts

Lines changed: 27 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import {
1111
type PresenterMediaAction,
1212
type PresenterMediaMessage,
1313
} from "./slideshowPresenter";
14+
import { OwnedMediaRegistry } from "./owned-media-registry";
1415

1516
interface Hotspot {
1617
id: string;
@@ -68,9 +69,18 @@ type PlayerElement = HTMLElement & {
6869
readonly ready: boolean;
6970
};
7071

71-
type SlideshowMediaElement = HTMLMediaElement & {
72-
dataset: DOMStringMap;
73-
};
72+
type SlideshowMediaElement = HTMLMediaElement;
73+
74+
const SLIDESHOW_MEDIA_ACTIONS = [
75+
"play",
76+
"pause",
77+
"seeking",
78+
"seeked",
79+
"ratechange",
80+
"volumechange",
81+
"ended",
82+
"timeupdate",
83+
] as const satisfies readonly PresenterMediaAction[];
7484

7585
/** True when the keydown originated in a text-entry control (typing must never
7686
* navigate the deck). Duck-typed so it works for events from the composition
@@ -202,6 +212,10 @@ export class HyperframesSlideshow extends HTMLElement {
202212
// Bumped whenever autoplay starts or media is stopped (slide change), so a
203213
// pending re-assert from a previous autoplay can't replay a clip we've left.
204214
private autoplayToken = 0;
215+
private readonly ownedMedia = new OwnedMediaRegistry<PresenterMediaAction>(
216+
SLIDESHOW_MEDIA_ACTIONS,
217+
(el, key, action) => this.publishMediaState(el, key, action),
218+
);
205219

206220
/** Whether audio is currently muted. Reflects `data-hf-muted` attribute. */
207221
get muted(): boolean {
@@ -291,6 +305,7 @@ export class HyperframesSlideshow extends HTMLElement {
291305
this.playerObserver.disconnect();
292306
this.playerObserver = null;
293307
}
308+
this.ownedMedia.clear();
294309
this.audienceMediaUnlockButton?.remove();
295310
this.audienceMediaUnlockButton = null;
296311
this.audienceMutedPlaybackKeys.clear();
@@ -545,8 +560,8 @@ export class HyperframesSlideshow extends HTMLElement {
545560
this.wireSlideshowMedia();
546561
if (this.mediaWireInterval === null) {
547562
// Same-origin player iframes can hydrate media after the slideshow binds.
548-
// The dataset guard prevents duplicate listeners, and removed iframe nodes
549-
// are collectable because this component keeps no media element references.
563+
// The owned registry prevents duplicate listeners and releases removed
564+
// iframe nodes through AbortController-backed teardown.
550565
this.mediaWireInterval = setInterval(() => this.wireSlideshowMedia(), 1000);
551566
}
552567
}
@@ -668,22 +683,9 @@ export class HyperframesSlideshow extends HTMLElement {
668683
}
669684

670685
private wireSlideshowMedia(): void {
671-
const actions: PresenterMediaAction[] = [
672-
"play",
673-
"pause",
674-
"seeking",
675-
"seeked",
676-
"ratechange",
677-
"volumechange",
678-
"ended",
679-
"timeupdate",
680-
];
681-
for (const { key, el } of this.mediaEntries()) {
682-
if (el.dataset.hfSlideshowMediaSync === "1") continue;
683-
el.dataset.hfSlideshowMediaSync = "1";
684-
for (const action of actions) {
685-
el.addEventListener(action, () => this.publishMediaState(el, key, action));
686-
}
686+
const added = this.ownedMedia.sync(this.mediaEntries());
687+
for (const el of added) {
688+
el.muted = this._muted || el.defaultMuted;
687689
}
688690
}
689691

@@ -1161,20 +1163,16 @@ export class HyperframesSlideshow extends HTMLElement {
11611163
}
11621164
}
11631165

1164-
const doc = this.ownerDocument;
1165-
for (const el of doc.querySelectorAll("video, audio")) {
1166-
if (el instanceof HTMLMediaElement) el.muted = muted || el.defaultMuted;
1167-
}
1166+
this.ownedMedia.sync(this.mediaEntries());
1167+
this.ownedMedia.setMuted(muted);
11681168
}
11691169

11701170
private stopDocumentMedia(): void {
11711171
// Invalidate any in-flight autoplay re-assert so leaving a slide can't be
11721172
// undone by a pending timeout replaying the clip we just paused.
11731173
this.autoplayToken++;
1174-
const doc = this.ownerDocument;
1175-
for (const el of doc.querySelectorAll("video, audio")) {
1176-
if (el instanceof HTMLMediaElement) el.pause();
1177-
}
1174+
this.ownedMedia.sync(this.mediaEntries());
1175+
this.ownedMedia.pauseAll();
11781176
}
11791177

11801178
/**
Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
import { describe, expect, it, vi } from "vitest";
2+
import { OwnedMediaRegistry } from "./owned-media-registry.js";
3+
4+
describe("OwnedMediaRegistry", () => {
5+
it("installs one abortable listener set and rebinds when the stable key changes", () => {
6+
const video = document.createElement("video");
7+
const onAction = vi.fn();
8+
const registry = new OwnedMediaRegistry(["play", "pause"] as const, onAction);
9+
10+
registry.sync([{ key: "slide-a", el: video }]);
11+
registry.sync([{ key: "slide-a", el: video }]);
12+
video.dispatchEvent(new Event("play"));
13+
expect(onAction).toHaveBeenCalledTimes(1);
14+
expect(onAction).toHaveBeenLastCalledWith(video, "slide-a", "play");
15+
16+
registry.sync([{ key: "slide-b", el: video }]);
17+
video.dispatchEvent(new Event("pause"));
18+
expect(onAction).toHaveBeenCalledTimes(2);
19+
expect(onAction).toHaveBeenLastCalledWith(video, "slide-b", "pause");
20+
21+
registry.sync([]);
22+
video.dispatchEvent(new Event("play"));
23+
expect(onAction).toHaveBeenCalledTimes(2);
24+
});
25+
26+
it("mutates and pauses only the currently owned media", () => {
27+
const owned = document.createElement("video");
28+
const unrelated = document.createElement("video");
29+
owned.pause = vi.fn();
30+
unrelated.pause = vi.fn();
31+
const registry = new OwnedMediaRegistry(["play"] as const, vi.fn());
32+
registry.sync([{ key: "owned", el: owned }]);
33+
34+
registry.setMuted(true);
35+
registry.pauseAll();
36+
37+
expect(owned.muted).toBe(true);
38+
expect(owned.pause).toHaveBeenCalledTimes(1);
39+
expect(unrelated.muted).toBe(false);
40+
expect(unrelated.pause).not.toHaveBeenCalled();
41+
});
42+
43+
it("aborts every listener when the slideshow disconnects", () => {
44+
const audio = document.createElement("audio");
45+
const onAction = vi.fn();
46+
const registry = new OwnedMediaRegistry(["play"] as const, onAction);
47+
registry.sync([{ key: "audio", el: audio }]);
48+
49+
registry.clear();
50+
audio.dispatchEvent(new Event("play"));
51+
52+
expect(onAction).not.toHaveBeenCalled();
53+
});
54+
});

0 commit comments

Comments
 (0)