Merge remote-tracking branch 'origin/main' into fix/inspect-webkit-video-replace

WebKit (GStreamer) deadlocks the page when a video leaves the page or the
viewport before its pipeline has started: the main thread changes the
pipeline state and waits for the source's streaming thread, which waits on
the main thread in a synchronous bus handler (native stacks of the hung WPE
web process in the pinned image). Removing, hiding or merely pushing the
video below the fold all hung; none of pause(), removing src, load() or the
revoke was the cause on its own.

A video replaced while still starting now stays where it is, inert and
aria-hidden, with the new preview inserted after it, and the session's
revoke of its URL waits. Once it reports metadata or an error (or after
15 s) it is paused, loses its src, is reset with load() and removed, and
only then is its URL revoked. Every released video is reset with load(), so
no decoder is kept per replaced video.

The loadstart replacement spec now runs on WebKit too (15 fresh pages,
replaced at loadstart + 0-4 ms). TESTING.md D62 records the cause.

TESTING.md

@@ -1627,19 +1627,27 @@ mapped differences"):
   `image/png,image/jpeg,image/webp` plus `video/*,.mp4,.mov,.m4v,.webm,.mkv,.avi,.wmv`. A
   video previews in the same `#local-preview` slot as `video.local-file-preview` with
   native `controls`, `playsInline`, `preload="metadata"` and `muted`, never autoplays
-  inline, and is paused, has its `src` removed and is removed from the page on replacement
-  or dispose, before its URL is revoked. Each preview owns its listeners, so the page never
-  pins a released node. A settled video (metadata read, or failed) is then reset with
-  `load()` at once; one still starting is reset only once it reports metadata or an error,
-  because WebKit (GStreamer) blocked the page when such a pipeline was reset (`replacing a
-  video that just started loading never blocks the page`, `a video replaced while still
-  starting is reset once it settles`). A WebKit load cut off by the revoked URL usually
-  stalls and never settles; a fallback then only drops the waiting listeners. The viewer's
-  own video teardown was checked the same way (`closing the viewer on a video that just
-  started never blocks the page`, 15 fresh pages on WebKit) and needed no change.
-  Follow-up: on WebKit, a video released exactly at its `loadstart` sometimes blocks the
-  page, with or without any reset and also before the deferred reset (2 of 24 runs in the
-  pinned image), so the `loadstart` spec is skipped on WebKit until that is investigated.
+  inline. On replacement or dispose it is paused, has its `src` removed, is reset with
+  `load()` (which frees its decoder) and is removed from the page, before its URL is
+  revoked. Each preview owns its listeners, so the page never pins a released node.
+  A video replaced while still starting (a source, but neither metadata nor an error yet)
+  stays where it is, inert and `aria-hidden`, with the new preview inserted after it, and
+  the session's revoke of its URL waits; once it reports metadata or an error (or after
+  15 s) it is released as above and only then is its URL revoked. Page leave and dispose
+  release it at once. Cause, found with native stacks of the hung WPE web process in the
+  pinned image: WebKit (GStreamer) deadlocks when a video leaves the page or the viewport
+  before its pipeline has started. The main thread changes the pipeline state and waits
+  for the source's streaming thread, which is itself waiting on the main thread in a
+  synchronous bus handler. Removing the video, hiding it, or only pushing it below the
+  fold with a taller preview inserted before it all hung (7 to 21 of 120 fresh pages each,
+  replaced at `loadstart` + 0–7 ms), and none of the earlier steps (`pause()`, removing
+  `src`, `load()`, revoking the URL) was the cause on its own; pushing it out after
+  `loadedmetadata` hung 0 of 180. Specs: `replacing a video that just started loading
+  never blocks the page`, and `a video replaced while still starting is reset once it
+  settles`, which now runs on WebKit too (15 fresh pages, replaced at `loadstart` + 0–4
+  ms). The viewer's own video teardown was checked the same way (`closing the viewer on a
+  video that just started never blocks the page`, 15 fresh pages on WebKit) and needed no
+  change.
 - **D63** (status line): one `p.small.spaced-16` in `#local-preview`, for states the
   concept never shows: "Drop one file at a time.", "Folders cannot be inspected. Choose
   one image or video file.", "This file type cannot be previewed or inspected here.

tests/browser/workspace-inspect.spec.mjs

@@ -481,15 +481,13 @@ test('replacing a video that just started loading never blocks the page', async
 });
 
 test('a video replaced while still starting is reset once it settles', async ({ context, browserName }) => {
-    // A WebKit (GStreamer) video released exactly at its `loadstart` sometimes blocks the page, with or without any
-    // reset and also with the code before the deferred reset (2 of 24 runs); tracked in TESTING.md (D62). WebKit's
-    // no-hang guard is `replacing a video that just started loading never blocks the page`.
-    test.skip(browserName === 'webkit', 'WebKit release at loadstart is a tracked follow-up (D62)');
-    test.setTimeout(120000);
-    // The video is replaced from its own `loadstart` (readyState HAVE_NOTHING). It leaves the page with no src before
-    // its URL is revoked, and is reset (load() with no src, which frees its decoder) only once it reports metadata or
-    // an error, so the page never blocks. Fresh pages share one browser process.
-    for (let attempt = 0; attempt < 5; attempt++) {
+    test.setTimeout(180000);
+    // The video is replaced from its own `loadstart` (readyState HAVE_NOTHING), or a few milliseconds after it. On
+    // WebKit (GStreamer) a video that left the page or the viewport before its pipeline had started deadlocked the
+    // page (TESTING.md D62), so it stays in place, inert and still playing its URL, with the new preview after it.
+    // Once it reports metadata or an error it is paused, loses its src, is reset (load() with no src, which frees its
+    // decoder) and leaves the page, and only then is its URL revoked. Fresh pages share one browser process.
+    for (let attempt = 0; attempt < 15; attempt++) {
         const page = await context.newPage();
         try {
             if (attempt === 0) await startCoverage(page, browserName);
@@ -510,10 +508,12 @@ test('a video replaced while still starting is reset once it settles', async ({
                     const video = slot.querySelector('video');
                     if (!video) return;
                     observer.disconnect();
-                    video.addEventListener('loadstart', () => {
+                    const replace = () => {
                         window.__starting = { node: video, url: video.getAttribute('src'), readyState: video.readyState };
                         input.files = window.__pngFiles; input.dispatchEvent(new Event('change', { bubbles: true }));
-                    }, { once: true });
+                        window.__starting.replaced = { connected: video.isConnected, src: video.hasAttribute('src'), inert: video.inert, resets: video.__resets ?? 0 };
+                    };
+                    video.addEventListener('loadstart', () => { if (window.__delay) setTimeout(replace, window.__delay); else replace(); }, { once: true });
                 }).observe(slot, { childList: true });
             });
             const png = parityPng('f02.png');
@@ -522,13 +522,17 @@ test('a video replaced while still starting is reset once it settles', async ({
                 transfer.items.add(new File([new Uint8Array(bytes)], 'f02.png', { type: 'image/png' }));
                 window.__pngFiles = transfer.files;
             }, [...png.buffer]);
+            await page.evaluate(delay => { window.__delay = delay; }, attempt % 5);
             await choose(page, localFixture('core-tags.webm', 'video/webm'));
             await expect(preview(page).locator('img.local-file-preview')).toBeVisible({ timeout: 8000 });
             await expect(preview(page).locator('video')).toHaveCount(0);
-            // Released while starting, and never reset synchronously: the URL went once the node had no src.
-            expect(await page.evaluate(() => ({ readyState: window.__starting.readyState, revoked: window.__starting.revoked })))
-                .toEqual({ readyState: 0, revoked: { src: false, connected: false, resets: 0 } });
-            await expect.poll(() => page.evaluate(() => window.__starting.node.__resets ?? 0), { timeout: 15000 }).toBe(1);
+            // Replaced while starting (always so at `loadstart` itself): kept in place and untouched. A video that had
+            // settled is released at once. Either way it is reset once, and its URL goes last.
+            await expect.poll(() => page.evaluate(() => window.__starting.revoked ?? null)).toEqual({ src: false, connected: false, resets: 1 });
+            const { readyState, replaced } = await page.evaluate(() => ({ readyState: window.__starting.readyState, replaced: window.__starting.replaced }));
+            if (attempt % 5 === 0) expect(readyState).toBe(0);
+            expect(replaced).toEqual(readyState === 0 ? { connected: true, src: true, inert: true, resets: 0 } : { connected: false, src: false, inert: false, resets: 1 });
+            expect(await page.evaluate(() => window.__starting.node.__resets)).toBe(1);
             // The page still answers.
             await chooser(page).setInputFiles(parityPng('f02.png', 'after.png'), { timeout: 8000 });
             await expect(preview(page).locator('img.local-file-preview')).toBeVisible({ timeout: 8000 });

tools/vendor/frontend-coverage-baseline.json

@@ -296,7 +296,7 @@
   "workspace/inspect.mjs": {
     "lines": 98,
     "branches": 92,
-    "functions": 92,
+    "functions": 93,
     "statements": 98
   },
   "workspace/inspection-state.mjs": {

web/js/workspace/inspect.mjs

@@ -39,36 +39,24 @@ function element(tag, className, text) {
 
 // The status shown for the selection; AVI/WMV's note and an empty status keep the toast's "metadata is available".
 const TOASTABLE = new Set(['', LOCAL_SESSION_MESSAGES.containerOnly]);
-// How long a released video still starting may take to settle before its deferred reset is dropped.
-const DEFERRED_RESET_MS = 15000;
+// How long a replaced video still starting may stay in place before it is released anyway.
+const STARTING_VIDEO_MS = 15000;
 
-/**
- * Resets a released video once it settles. Removing `src` alone never aborts a load already under way, and WebKit
- * (GStreamer) blocked the page when a pipeline still starting was reset, so load() waits for metadata or an error.
- * The fallback only drops the waiting listeners (a WebKit load cut off by the revoked URL stalls and never settles),
- * so the detached node is never pinned.
- */
-function resetWhenSettled(node) {
-    const waiting = new AbortController();
-    const fallback = setTimeout(() => waiting.abort(), DEFERRED_RESET_MS);
-    const reset = () => { waiting.abort(); clearTimeout(fallback); node.load(); };
-    for (const type of ['loadedmetadata', 'error']) node.addEventListener(type, reset, { once: true, signal: waiting.signal });
+/** A video whose pipeline is still starting: it has a source but neither metadata nor an error yet. */
+function isStarting(node) {
+    return node instanceof HTMLMediaElement && node.hasAttribute('src') && node.readyState < HTMLMediaElement.HAVE_METADATA && node.error === null;
 }
 
 /**
- * Stops a preview from holding its blob: URL: its own listeners end, media is paused and its src dropped before the
- * session revokes the URL, and the node leaves the page. A settled video (metadata read, or failed) is then reset with
- * load() to free its decoder at once; one still starting is reset as soon as it settles.
+ * Stops a preview from holding its blob: URL: its own listeners end, media is paused, its src dropped and it is reset
+ * with load() (which frees its decoder), and the node leaves the page, all before its URL is revoked.
  */
 function releaseMedia(preview) {
     if (!preview) return;
     const { node, listeners } = preview;
     listeners.abort();
     if (node instanceof HTMLMediaElement) {
-        const settled = node.readyState >= HTMLMediaElement.HAVE_METADATA || node.error !== null;
-        node.pause(); node.removeAttribute('src');
-        if (settled) node.load();
-        else resetWhenSettled(node);
+        node.pause(); node.removeAttribute('src'); node.load();
     } else node.removeAttribute('src');
     node.remove();
 }
@@ -108,7 +96,33 @@ export function mountInspect(main, { parentScope, api, sender }) {
     // The session ends after the viewer has closed and the inline media is gone, so no element holds its URL then.
     // The page owns page leave for the same reason: the session listens for no `pagehide` of its own.
     const sessionScope = createScope();
-    const session = createLocalSession({ parentScope: sessionScope, pageTarget: null });
+    // Replaced videos still starting, by URL. WebKit (GStreamer) deadlocked the page when a video left the page or
+    // the viewport before its pipeline had started, so such a video stays where it is, inert, until it reports
+    // metadata or an error; only then is it released and its URL, which the session asked to revoke, revoked.
+    const retiring = new Map();
+    function finishRetiring(url) {
+        const entry = retiring.get(url);
+        if (!entry) return;
+        retiring.delete(url);
+        entry.waiting.abort(); clearTimeout(entry.timer);
+        releaseMedia(entry.preview);
+        if (entry.revoke) URL.revokeObjectURL(url);
+    }
+    function retire(preview, url) {
+        const waiting = new AbortController();
+        const entry = { preview, waiting, revoke: false, timer: setTimeout(() => finishRetiring(url), STARTING_VIDEO_MS) };
+        retiring.set(url, entry);
+        preview.listeners.abort();
+        preview.node.inert = true; preview.node.setAttribute('aria-hidden', 'true');
+        for (const type of ['loadedmetadata', 'error']) preview.node.addEventListener(type, () => finishRetiring(url), { once: true, signal: waiting.signal });
+    }
+    const finishAllRetiring = () => { for (const url of [...retiring.keys()]) finishRetiring(url); };
+    const revokeObjectURL = url => {
+        const entry = retiring.get(url);
+        if (entry) entry.revoke = true;
+        else URL.revokeObjectURL(url);
+    };
+    const session = createLocalSession({ parentScope: sessionScope, pageTarget: null, revokeObjectURL });
     const viewer = createMediaViewer({ api, container: main, parentScope: scope, inspectionController: session.controller, sender,
         adapter: localFileAdapter, onRestoreFocus: () => { if (open.isConnected) open.focus({ preventScroll: true }); } });
 
@@ -153,7 +167,9 @@ export function mountInspect(main, { parentScope, api, sender }) {
     function showMedia(state) {
         const url = state.preview?.url ?? null;
         if (url === mediaUrl) return;
-        releaseMedia(media); media = null; mediaUrl = url;
+        if (media && isStarting(media.node) && !retiring.has(mediaUrl)) retire(media, mediaUrl);
+        else releaseMedia(media);
+        media = null; mediaUrl = url;
         if (!url) return;
         const video = state.preview.media === 'video';
         const node = element(video ? 'video' : 'img', 'local-file-preview');
@@ -173,7 +189,10 @@ export function mountInspect(main, { parentScope, api, sender }) {
         }, { once: true, signal: current.listeners.signal });
         node.src = url;
         media = current;
-        preview.prepend(node);
+        // A video still starting keeps its place, so the new preview goes after it and never moves it.
+        const anchor = [...retiring.values()].map(entry => entry.preview.node).filter(item => item.parentNode === preview).at(-1);
+        if (anchor) anchor.after(node);
+        else preview.prepend(node);
     }
     scope.cleanup(session.subscribe(state => {
         showMedia(state);
@@ -190,6 +209,7 @@ export function mountInspect(main, { parentScope, api, sender }) {
     // viewer (on abort), drops the media, then ends the session.
     scope.cleanup(() => viewer.dispose());
     scope.cleanup(() => { releaseMedia(media); media = null; mediaUrl = null; });
+    scope.cleanup(finishAllRetiring);
     scope.cleanup(() => sessionScope.dispose());
 
     // Page leave: the viewer closes and the inline media goes before the session revokes the URL, and the chooser is
@@ -198,6 +218,7 @@ export function mountInspect(main, { parentScope, api, sender }) {
         clearRejection();
         viewer.close();
         releaseMedia(media); media = null; mediaUrl = null;
+        finishAllRetiring();
         session.clear();
         chosen = null; input.value = '';
     }

web/js/workspace/local-session.mjs

@@ -70,7 +70,7 @@ function chosenFile(input) {
 }
 
 export function createLocalSession({
-    parentScope, createObjectURL = file => URL.createObjectURL(file), revokeObjectURL = url => URL.revokeObjectURL(url),
+    parentScope, createObjectURL = file => URL.createObjectURL(file), revokeObjectURL = URL.revokeObjectURL.bind(URL),
     schedulerYield = defaultSchedulerYield(), setTimer = setTimeout, clearTimer = clearTimeout,
     pageTarget = globalThis.window, extractFacts, pngMetadata,
 } = {}) {