mirror of
https://github.com/heygen-com/hyperframes.git
synced 2026-09-03 23:03:09 +00:00
fix(core): evict cached MediaElementSource on src mutation, soften enforcement-point claims
This commit is contained in:
@@ -34,6 +34,21 @@ import type { RuntimeJson } from "./types";
|
||||
* `web-audio` here, correctly or not. Nothing in this codebase feeds
|
||||
* `createMediaElementSource` from a `srcObject` element today, so this is
|
||||
* recorded as a boundary rather than fixed.
|
||||
*
|
||||
* Second known gap, same shape: `isCorsSilenced` judges the RAW url string —
|
||||
* the same-origin URL the author wrote, or whatever the browser resolved into
|
||||
* `currentSrc` — not wherever a server-side redirect chain actually lands.
|
||||
* A same-origin URL that 302s to a cross-origin CDN reads as `web-audio` here
|
||||
* and gets a real `createMediaElementSource` node; whether that node is
|
||||
* silent then depends on the redirect target's CORS headers, which this
|
||||
* classifier never sees (following the chain to inspect the final response
|
||||
* would turn a pure, synchronous verdict — needed on every schedule call —
|
||||
* into an async fetch). A cross-origin URL that redirects back to same-origin
|
||||
* has the opposite miss: classified `decode-only` and sent down the fetch
|
||||
* fallback when Web Audio capture would have worked fine either way. Not
|
||||
* fixed for the same reason as `srcObject` — no caller in this codebase
|
||||
* routes media through a redirecting URL today — but worth knowing before
|
||||
* trusting this classifier's verdict for one that does.
|
||||
*/
|
||||
export type WebAudioMediaRoute =
|
||||
/** Same-origin, CORS-opted-in, or a scheme the check doesn't apply to. */
|
||||
|
||||
@@ -202,6 +202,58 @@ describe("WebAudioTransport", () => {
|
||||
expect(scheduled).not.toBeNull();
|
||||
expect(mock.ctx.createMediaElementSource).toHaveBeenCalledWith(el);
|
||||
});
|
||||
|
||||
// R2 finding: `_mediaElementSources` is keyed by element identity, not by
|
||||
// asset. A cache hit alone said nothing about the element's CURRENT
|
||||
// resource, so a pooled element reused for a new clip kept handing back
|
||||
// the OLD (same-origin) node — and its stale `web-audio` verdict — after
|
||||
// `src` moved to a cross-origin asset with no `crossorigin` opt-in. Only
|
||||
// `destroy()` ever cleared the cache, so this silenced the element for the
|
||||
// rest of the session.
|
||||
it("stops returning the cached node once the same element's src moves cross-origin", async () => {
|
||||
const { transport, mock, gen: gen1 } = setupTransport(100);
|
||||
vi.spyOn(console, "info").mockImplementation(() => {});
|
||||
const el = document.createElement("audio");
|
||||
el.setAttribute("src", "/assets/vo.mp3");
|
||||
|
||||
const first = await transport.scheduleMediaElementPlayback(el, 0, 0, 0, 1, gen1, 1);
|
||||
expect(first).not.toBeNull();
|
||||
expect(mock.ctx.createMediaElementSource).toHaveBeenCalledTimes(1);
|
||||
|
||||
transport.stopAll();
|
||||
// `stopAll()` itself disconnects the transient graph (see "disconnects
|
||||
// the transient graph on stop but keeps the cached native source
|
||||
// reusable" above) without evicting the cache — clear the spy so the
|
||||
// assertion below is about the FIX's own eviction, not that call.
|
||||
mock.mediaElementSourceNode.disconnect.mockClear();
|
||||
el.setAttribute("src", "https://cdn.example.com/reused-clip.mp3");
|
||||
const gen2 = transport.startGeneration();
|
||||
const second = await transport.scheduleMediaElementPlayback(el, 0, 0, 0, 1, gen2, 1);
|
||||
|
||||
expect(second).toBeNull();
|
||||
// The one-way door means a fresh node can't be built either — the
|
||||
// fix's job is to stop HANDING BACK the stale one, not to conjure a
|
||||
// new node over a src that was never eligible.
|
||||
expect(mock.ctx.createMediaElementSource).toHaveBeenCalledTimes(1);
|
||||
expect(mock.mediaElementSourceNode.disconnect).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("keeps returning the cached node when a reused element's src stays eligible", async () => {
|
||||
const { transport, mock, gen: gen1 } = setupTransport(100);
|
||||
const el = document.createElement("audio");
|
||||
el.setAttribute("src", "/assets/vo.mp3");
|
||||
|
||||
await transport.scheduleMediaElementPlayback(el, 0, 0, 0, 1, gen1, 1);
|
||||
transport.stopAll();
|
||||
mock.mediaElementSourceNode.disconnect.mockClear();
|
||||
el.setAttribute("src", "/assets/other-same-origin-clip.mp3");
|
||||
const gen2 = transport.startGeneration();
|
||||
const second = await transport.scheduleMediaElementPlayback(el, 0, 0, 0, 1, gen2, 1);
|
||||
|
||||
expect(second).not.toBeNull();
|
||||
expect(mock.ctx.createMediaElementSource).toHaveBeenCalledTimes(1);
|
||||
expect(mock.mediaElementSourceNode.disconnect).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
it("tracks play generation for async race prevention", () => {
|
||||
|
||||
@@ -255,13 +255,47 @@ export class WebAudioTransport {
|
||||
* element away from its native output, so the question has to be settled
|
||||
* before it, and there is no undo afterwards.
|
||||
*
|
||||
* `init.ts` routes on the same verdict before ever calling in; this stays the
|
||||
* enforcement point so a direct caller (studio, player) cannot reopen the
|
||||
* one-way door.
|
||||
* `init.ts` routes on the same verdict before ever calling in, and
|
||||
* `AudioRow.tsx`'s standalone preview player classifies over its own
|
||||
* throwaway `AudioContext` before its own `createMediaElementSource` call —
|
||||
* this method is A enforcement point, not THE enforcement point; every
|
||||
* caller that can reach `createMediaElementSource` is expected to classify
|
||||
* first. What this method DOES own is the cache below: `_mediaElementSources`
|
||||
* is keyed by element identity, not by asset, so a cache hit alone says
|
||||
* nothing about the element's CURRENT resource. Reclassifying on every call
|
||||
* — cache hit included — means a `src` mutation an outer caller missed (a
|
||||
* pooled element swapped from a same-origin clip to a cross-origin one
|
||||
* without going through a fresh generation) can't leave a stale
|
||||
* `web-audio` verdict silently attached to the new resource.
|
||||
*/
|
||||
private acquireMediaElementSource(el: HTMLMediaElement): MediaElementAudioSourceNode | null {
|
||||
const cached = this._mediaElementSources.get(el);
|
||||
if (cached) return cached;
|
||||
if (cached) {
|
||||
// The node itself doesn't change identity on a src swap, but its
|
||||
// eligibility can: the Web Audio spec's tainted-origin check runs
|
||||
// against the element's CURRENT underlying resource, not the one that
|
||||
// was current when the node was built. A same-origin-to-cross-origin
|
||||
// mutation on this element would otherwise keep returning the old
|
||||
// (now-silent) node forever — `destroy()` was the only thing that ever
|
||||
// cleared this cache, so a long-lived element that changed sources
|
||||
// stayed silenced for the rest of the session (the R2 finding this
|
||||
// block exists to close). The node is still a one-way door — it can't
|
||||
// be un-created, and the element's native output is gone either way —
|
||||
// so disconnecting it just stops it feeding a graph that no longer
|
||||
// matches the asset; the caller falls back to the decode-only path.
|
||||
const route = classifyWebAudioMediaRoute(el);
|
||||
if (route.kind !== "web-audio") {
|
||||
try {
|
||||
cached.disconnect();
|
||||
} catch {
|
||||
// Already torn down.
|
||||
}
|
||||
this._mediaElementSources.delete(el);
|
||||
reportWebAudioMediaRoute(el, route);
|
||||
return null;
|
||||
}
|
||||
return cached;
|
||||
}
|
||||
if (!this._ctx) return null;
|
||||
const route = classifyWebAudioMediaRoute(el);
|
||||
if (route.kind !== "web-audio") {
|
||||
|
||||
Reference in New Issue
Block a user