From 640e990431f0e8caa5d6fcbb914c936ecbbe8b4e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miguel=20=C3=81ngel?= Date: Sun, 27 Sep 2026 00:40:07 +0000 Subject: [PATCH] fix(player): a reloaded preview waits for its assets once, not twice (#4546) * fix(player): a reloaded preview waits for its assets once, not twice * fix(studio): keep the preview's comment share at its base * refactor(player): record the ready document where ready is sent * fix(player): a source change stops the previous film's audio right away * fix(player): the next film no longer replays the previous film's audio * fix(player): an audio-src track sharing the film's URL survives a film change * fix(core): first-frame readiness counts media a script adds before load * fix(player): media a script adds before load still holds the first paint * fix(player): hosts see a load's readiness only after the player has handled it * fix(player): a slow first film no longer reports a missing timeline * test(player): pin the connected flag with a mid-insertion load --- .../player/src/hyperframes-player.test.ts | 120 +++++++++++++++++- packages/player/src/hyperframes-player.ts | 45 +++++-- packages/player/src/parent-media.test.ts | 43 ++++++- packages/player/src/parent-media.ts | 13 +- .../src/player/components/Player.test.ts | 23 ++++ .../studio/src/player/components/Player.tsx | 6 +- 6 files changed, 229 insertions(+), 21 deletions(-) diff --git a/packages/player/src/hyperframes-player.test.ts b/packages/player/src/hyperframes-player.test.ts index 84f86e58a2..e514eef2fc 100644 --- a/packages/player/src/hyperframes-player.test.ts +++ b/packages/player/src/hyperframes-player.test.ts @@ -131,6 +131,9 @@ describe("HyperframesPlayer parent-frame media", () => { seek: (t: number) => void; _audioOwner?: "runtime" | "parent"; _promoteToParentProxy?: () => void; + _ready?: boolean; + _assetsReady?: boolean; + _parentTickRaf?: number | null; }; let player: PlayerElement; @@ -256,6 +259,23 @@ describe("HyperframesPlayer parent-frame media", () => { expect(mockAudio.pause).toHaveBeenCalled(); }); + it("hands audio back to the iframe and pauses the proxy as soon as the source changes", () => { + player.setAttribute("audio-src", "https://cdn.example.com/narration.mp3"); + document.body.appendChild(player); + player._promoteToParentProxy?.(); + player._ready = true; + player._assetsReady = true; + player.play(); + expect(player._parentTickRaf).not.toBeNull(); + mockAudio.pause.mockClear(); + + player.setAttribute("src", "next-composition.html"); + + expect(player._audioOwner).toBe("runtime"); + expect(mockAudio.pause).toHaveBeenCalled(); + expect(player._parentTickRaf).toBeNull(); + }); + function dispatchAutoplayBlockedFromPlayerFrame(player: HTMLElement): HTMLMediaElement { const iframe = player.shadowRoot?.querySelector("iframe"); if (!(iframe instanceof HTMLIFrameElement)) throw new Error("expected player iframe"); @@ -461,10 +481,11 @@ describe("HyperframesPlayer parent-frame media", () => { windowAdd.mock.calls.filter(([eventName]) => eventName === "message").length - windowRemove.mock.calls.filter(([eventName]) => eventName === "message").length, ).toBe(1); + // The one load listener is added in the constructor, before these spies. expect( iframeAdd.mock.calls.filter(([eventName]) => eventName === "load").length - iframeRemove.mock.calls.filter(([eventName]) => eventName === "load").length, - ).toBe(1); + ).toBe(0); }); it("returns parent-media ownership to the runtime after reconnect", () => { @@ -3310,6 +3331,78 @@ describe("HyperframesPlayer asset-ready gate", () => { data: { source: "hf-preview", ...data }, } as unknown as MessageEvent); + it("ignores its iframe's blank-document load that arrives before it is connected", () => { + const player = document.createElement("hyperframes-player") as PlayerInternal & { + probe: { start(): void }; + }; + const start = vi.spyOn(player.probe, "start"); + // Mid-insertion the element already reads as connected; connectedCallback has not run yet. + Object.defineProperty(player, "isConnected", { get: () => true, configurable: true }); + + player.iframe.dispatchEvent(new Event("load")); + + expect(start).not.toHaveBeenCalled(); + }); + + it("handles its iframe's load before a host's load listener runs", async () => { + const player = document.createElement("hyperframes-player") as PlayerInternal & { + _readyDocument: Document | null; + }; + const readyAtHostLoad: boolean[] = []; + player.iframe.addEventListener("load", () => readyAtHostLoad.push(player._ready)); + document.body.appendChild(player); + await vi.waitFor(() => expect(readyAtHostLoad).toHaveLength(1)); + player._ready = true; + player._readyDocument = document.implementation.createHTMLDocument("previous document"); + + player.iframe.dispatchEvent(new Event("load")); + + expect(readyAtHostLoad[1]).toBe(false); + player.remove(); + }); + + it("paints a document before its load when nothing first-frame is pending", async () => { + const player = await createConnectedPlayer(); + player._ready = false; + const doc = player.iframe.contentDocument!; + Object.defineProperty(doc, "readyState", { get: () => "interactive", configurable: true }); + const holdLoad = (e: Event) => e.stopImmediatePropagation(); + doc.defaultView!.addEventListener("load", holdLoad, { capture: true }); + try { + post(player, { type: "timeline", durationInFrames: 60 }); + + await vi.waitFor(() => expect(player.painted).toBe(true)); + } finally { + doc.defaultView!.removeEventListener("load", holdLoad, { capture: true }); + delete (doc as { readyState?: unknown }).readyState; + player.remove(); + } + }); + + it("stays unpainted for first-frame media a script adds before the document's load", async () => { + const player = await createConnectedPlayer(); + player._ready = false; + const doc = player.iframe.contentDocument!; + let readyState: DocumentReadyState = "interactive"; + Object.defineProperty(doc, "readyState", { get: () => readyState, configurable: true }); + post(player, { type: "timeline", durationInFrames: 60 }); + + const video = doc.createElement("video"); + video.setAttribute("data-start", "0"); + Object.defineProperty(video, "readyState", { value: 0, configurable: true }); + doc.body.appendChild(video); + readyState = "complete"; + doc.defaultView!.dispatchEvent(new Event("load")); + player._onIframeLoad(); + await new Promise((resolve) => setTimeout(resolve, 300)); + + expect(player._ready).toBe(true); + expect(player.assetsReady).toBe(false); + expect(player.painted).toBe(false); + delete (doc as { readyState?: unknown }).readyState; + player.remove(); + }); + it("holds an opaque-origin composition until its runtime posts assets-ready", async () => { const player = await createConnectedPlayer(); player._ready = false; @@ -3380,6 +3473,31 @@ describe("HyperframesPlayer asset-ready gate", () => { player.remove(); }); + it("keeps a same-origin handshake when that document's load event arrives after ready", async () => { + const player = await createConnectedPlayer(); + player._ready = false; + const doc = document.implementation.createHTMLDocument("composition"); + Object.defineProperty(player.iframe, "contentDocument", { get: () => doc, configurable: true }); + post(player, { type: "timeline", durationInFrames: 60 }); + await vi.waitFor(() => expect(player.assetsReady).toBe(true)); + + const settled = vi.fn(); + player.addEventListener("assetsready", settled); + player._onIframeLoad(); + await new Promise((resolve) => setTimeout(resolve, 50)); + + expect(player._ready).toBe(true); + expect(player.assetsReady).toBe(true); + expect(settled).not.toHaveBeenCalled(); + + const next = document.implementation.createHTMLDocument("next composition"); + Object.defineProperty(player.iframe, "contentDocument", { get: () => next }); + player._onIframeLoad(); + expect(player._ready).toBe(false); + + player.remove(); + }); + it("does not wait on an opaque-origin runtime that already settled its assets", async () => { const player = await createConnectedPlayer(); player._ready = false; diff --git a/packages/player/src/hyperframes-player.ts b/packages/player/src/hyperframes-player.ts index 0136881e16..078126e9c3 100644 --- a/packages/player/src/hyperframes-player.ts +++ b/packages/player/src/hyperframes-player.ts @@ -115,6 +115,8 @@ class HyperframesPlayer extends HTMLElement { private probe: CompositionProbe; private _ready = false; + private _readyDocument: Document | null = null; + private _connected = false; private _assetsReady = false; private _painted = false; private _pendingPlay = false; @@ -199,13 +201,15 @@ class HyperframesPlayer extends HTMLElement { this.resizeObserver = new ResizeObserver(() => this._rescale()); this._onMessage = this._onMessage.bind(this); this._onIframeLoad = this._onIframeLoad.bind(this); + // Before any host can listen; _onIframeLoad skips the blank-document load before connect. + this.iframe.addEventListener("load", this._onIframeLoad); } connectedCallback() { + this._connected = true; this._applySandboxOriginPolicy(); this.resizeObserver.observe(this); window.addEventListener("message", this._onMessage); - this.iframe.addEventListener("load", this._onIframeLoad); if (this.hasAttribute("controls")) this._setupControls(); if (this.hasAttribute("poster")) this.posterEl = setupPoster(this.shadow, this.getAttribute("poster"), this.posterEl); @@ -224,11 +228,11 @@ class HyperframesPlayer extends HTMLElement { } disconnectedCallback() { + this._connected = false; this._sendControl("pause"); this._stopIframeMedia(); this.resizeObserver.disconnect(); window.removeEventListener("message", this._onMessage); - this.iframe.removeEventListener("load", this._onIframeLoad); this.probe.stop(); this._directTimelineClock.stop(); this._stopParentTickClock(); @@ -869,7 +873,6 @@ class HyperframesPlayer extends HTMLElement { // replaced, where it can only end in a delivery timeout rather than the immediate, // explanatory rejection the caller gets from every other navigating path. this._abandonComposition("Shader options changed before runtime data was applied"); - if (getShaderModeFromElement(this) !== "player") this.shaderLoader.reset(); if (this.hasAttribute("srcdoc")) { this.iframe.srcdoc = prepareSrcdocForElement(this, this.getAttribute("srcdoc") || ""); return; @@ -1095,6 +1098,13 @@ class HyperframesPlayer extends HTMLElement { }, ASSETS_LOADING_SHOW_DELAY_MS); } + private _hasPendingFirstFrameAssets(doc: Document): boolean { + const { pendingMedia, pendingImages, fontsLoading } = scanPendingCompositionAssets(doc, { + scope: FIRST_FRAME_READINESS_SCOPE, + }); + return pendingMedia.length > 0 || pendingImages.length > 0 || fontsLoading; + } + /** Timeout diagnostic. Re-scans since some assets may have resolved by * now. Compute can cause the timeout, so it's reported too. A hidden * document can starve paint-and-idle of frames for the full 8s — that's @@ -1141,11 +1151,21 @@ class HyperframesPlayer extends HTMLElement { * data deliveries end here. A queued play is the caller's, so only its owners clear it. */ private _abandonComposition(reason: string): void { this._ready = false; + this._readyDocument = null; this._invalidateAssetsWait(); + this._releaseDocument(); this._runtimeBridgeReady = false; this._rejectAllRuntimeDataDeliveries(reason); } + private _releaseDocument(): void { + this._directTimelineAdapter = null; + this._directTimelineClock.stop(); + this._stopParentTickClock(); + this.shaderLoader.reset(); + this._media.resetForIframeLoad(); + } + /** Abandons any in-flight asset wait — every `_ready = false` site calls * this first, so a stale wait's settle can't apply to what comes next. */ private _invalidateAssetsWait(): void { @@ -1164,6 +1184,7 @@ class HyperframesPlayer extends HTMLElement { } private _dispatchReady(): void { + this._readyDocument = this._getSameOriginIframeDocument(); const detail = { duration: this._duration, compositionWidth: this._compositionWidth, @@ -1260,22 +1281,24 @@ class HyperframesPlayer extends HTMLElement { } private _onIframeLoad() { + if (!this._connected) return; // The runtime posts its timeline at DOMContentLoaded, before `load`, and every - // host-initiated navigation clears `_ready` first. So a ready opaque-origin player already - // holds this document's handshake; a paused runtime would never post it again. - if (this._ready && this._getSameOriginIframeDocument() === null) return; + // host-initiated navigation clears `_ready` first. So a ready player already holds this + // document's handshake (an opaque origin reads as null); a paused runtime never posts it again. + const doc = this._getSameOriginIframeDocument(); + if (this._ready && doc === this._readyDocument) { + // Its asset wait scanned at DOMContentLoaded; a script may have added first-frame media since. + if (doc && this._hasPendingFirstFrameAssets(doc)) this._waitForAssetsReady(doc); + return; + } this._ready = false; // The runtime installs its bridge at DOMContentLoaded, posts `ready`, and only then does the // iframe's load event fire. Do not erase that authoritative handshake here: doing so strands // retained data set after load until a second `ready` that never comes. Source setters and // sandbox-policy reloads already clear bridge readiness before starting a navigation. - this._directTimelineAdapter = null; - this._directTimelineClock.stop(); - this._stopParentTickClock(); this._invalidateAssetsWait(); - this.shaderLoader.reset(); - this._media.resetForIframeLoad(); + this._releaseDocument(); this.probe.start(); } diff --git a/packages/player/src/parent-media.test.ts b/packages/player/src/parent-media.test.ts index ab1da00347..7a548a50b3 100644 --- a/packages/player/src/parent-media.test.ts +++ b/packages/player/src/parent-media.test.ts @@ -1,4 +1,4 @@ -import { describe, it, expect } from "vitest"; +import { afterEach, describe, it, expect } from "vitest"; import { ParentMediaManager, type ProxyEntry } from "./parent-media"; // A fake media element whose paused state is driven by play()/pause() stubs. @@ -163,6 +163,47 @@ describe("ParentMediaManager audio-src proxy lifecycle", () => { }); }); +describe("ParentMediaManager across documents", () => { + afterEach(() => document.body.replaceChildren()); + + it("drops the previous document's proxies on reset and keeps the audio-src one", () => { + const mgr = makeManager(); + mgr.setupFromUrl("https://example.test/narration.mp3"); + const track = document.createElement("audio"); + track.setAttribute("src", "https://example.test/old-film.mp3"); + track.setAttribute("data-start", "0"); + track.preload = "auto"; + document.body.appendChild(track); + mgr.setupFromIframe(document); + expect(mgr.entries).toHaveLength(2); + const oldProxy = mgr.entries[1].el; + + mgr.resetForIframeLoad(); + + expect(mgr.entries.map((m) => m.el.src)).toEqual(["https://example.test/narration.mp3"]); + expect(oldProxy.getAttribute("src")).toBe(""); + }); + + it("keeps the audio-src track after a reset when the old document shared its URL", () => { + const mgr = makeManager(); + const track = document.createElement("audio"); + track.setAttribute("src", "https://example.test/narration.mp3"); + track.setAttribute("data-start", "2"); + track.setAttribute("data-duration", "3"); + track.preload = "auto"; + document.body.appendChild(track); + mgr.setupFromIframe(document); + mgr.setupFromUrl("https://example.test/narration.mp3"); + expect(mgr.entries).toHaveLength(1); + + mgr.resetForIframeLoad(); + + expect(mgr.entries.map((m) => [m.el.src, m.start, m.duration])).toEqual([ + ["https://example.test/narration.mp3", 0, Infinity], + ]); + }); +}); + describe("ParentMediaManager clip window", () => { it("plays a proxy inside its clip window and pauses it at the clip end instant", () => { const mgr = makeManager({ isPaused: false, owner: "parent" }); diff --git a/packages/player/src/parent-media.ts b/packages/player/src/parent-media.ts index f5cec41909..2b49f9e0bc 100644 --- a/packages/player/src/parent-media.ts +++ b/packages/player/src/parent-media.ts @@ -94,6 +94,10 @@ export class ParentMediaManager { const wasPromoted = this._audioOwner === "parent"; this._audioOwner = "runtime"; this.pauseAll(); + for (const m of this._entries) if (m !== this._urlAudioEntry) m.el.src = ""; + this._entries = this._urlAudioEntry ? [this._urlAudioEntry] : []; + if (this._urlAudioSrc && !this._urlAudioEntry) + this._urlAudioEntry = this._createEntry(this._urlAudioSrc, "audio", 0, Infinity); this.teardownObserver(); if (wasPromoted) { this._dispatchEvent( @@ -300,13 +304,10 @@ export class ParentMediaManager { if (this._urlAudioSrc === audioSrc && this._urlAudioEntry) return; this.teardownUrlAudio(); const entry = this._createEntry(audioSrc, "audio", 0, Infinity); - // `_createEntry` returns null when a proxy for this URL already exists - // (e.g. the composition already adopted the same media). In that case we do - // not own a proxy, so leave the tracking cleared rather than recording a - // src with no entry — otherwise teardown would target nothing and the - // no-op guard would never engage. + // Null when the composition already has a proxy for this URL: that one stays the + // composition's, and a reset creates ours once that document's proxies are gone. this._urlAudioEntry = entry; - this._urlAudioSrc = entry ? audioSrc : null; + this._urlAudioSrc = audioSrc; // If the parent already owns playback, bring the fresh proxy online so a // mid-playback swap is not silent until the next play tick. if (entry && this._audioOwner === "parent" && !this._isPaused()) { diff --git a/packages/studio/src/player/components/Player.test.ts b/packages/studio/src/player/components/Player.test.ts index 8a49d7b92a..b120f6ed36 100644 --- a/packages/studio/src/player/components/Player.test.ts +++ b/packages/studio/src/player/components/Player.test.ts @@ -277,6 +277,29 @@ describe("ready to show", () => { expect(onReadyToShowChange).toHaveBeenLastCalledWith(false); }); + it("can show a document whose player was ready and painted before its load event", async () => { + const onReadyToShowChange = vi.fn(); + const { player } = await mountPlayer({ onReadyToShowChange }); + const el = Object.assign(player as TestHyperframesPlayer, { ready: true, painted: true }); + + act(() => { + el.dispatchEvent(new Event("ready")); + el.dispatchEvent(new Event("painted")); + el.iframeElement.dispatchEvent(new Event("load")); + }); + await twoFrames(); + expect(onReadyToShowChange).toHaveBeenLastCalledWith(true); + + for (const state of [ + { ready: false, painted: true }, + { ready: true, painted: false }, + ]) { + Object.assign(el, state); + act(() => void el.iframeElement.dispatchEvent(new Event("load"))); + expect(onReadyToShowChange).toHaveBeenLastCalledWith(false); + } + }); + it("marks the preview booted when it can show and play, not at ready", async () => { usePlayerStore.setState({ previewBooted: false }); const { player } = await mountPlayer({}); diff --git a/packages/studio/src/player/components/Player.tsx b/packages/studio/src/player/components/Player.tsx index 87284f2b5b..9bf9eb6fde 100644 --- a/packages/studio/src/player/components/Player.tsx +++ b/packages/studio/src/player/components/Player.tsx @@ -39,6 +39,8 @@ interface PlayerProps { interface HyperframesPlayerElement extends HTMLElement { iframeElement: HTMLIFrameElement; + ready?: boolean; + painted?: boolean; } const MEDIA_HAVE_FUTURE_DATA = 3; @@ -245,8 +247,8 @@ export const Player = forwardRef( setLoaded(true); setPreviewError(null); setShaderTransitionLoading(false); - setCompositionLoading(true); - setPainted(false); + setCompositionLoading(!player.ready); + setPainted(Boolean(player.painted)); // Reveal animation on reload (hot-reload, composition switch) if (loadCountRef.current > 1) { container.classList.remove("preview-revealing");