fix(inspect): hold a route change until a starting video settles (#294)

* fix(inspect): keep a replaced video in place until its pipeline has started

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.

* fix(inspect): hold a route change until a starting video settles

A route change within the workspace detaches the Inspect page, and WebKit
(GStreamer) deadlocks when a video still starting is detached. Route
changes at loadstart + 0-7 ms hung 4-5 of 120 fresh pages (14 of 120 with
no page code releasing anything, so the detach itself is the trigger).

Inspect now exposes leaveGuard(): a promise while a video on the page is
still starting (current or replaced). The shell renders the next route
once it settles (metadata, error, or the 15 s bound); the latest route
wins. Route changes at loadstart + 0-7 ms now hang 0 of 120.

Not changed, with numbers in TESTING.md D62:
- A replaced video still shows above the new preview until it settles.
  Same-slot stacking changes the video's own box and hung again
  (absolute 5/120, absolute + opacity 0 8/120, one-cell grid 4/120).
- A real page leave cannot wait. Navigating away hung 8/120 and reload
  22/120 even with no page-side release, so it stays a known limitation.
This commit is contained in:
Vito
2026-10-23 03:51:38 -07:00
committed by GitHub
parent 4f1c6e236b
commit b50bf862f0
4 changed files with 104 additions and 8 deletions
+17 -4
View File
@@ -1684,8 +1684,20 @@ mapped differences"):
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
15 s) it is released as above and only then is its URL revoked. For that moment (the
time to metadata, or at most 15 s for a video that never settles) the old video and the
new preview both show, the new one below it: a known, brief overlap. Stacking them in one
slot instead hung again, because it changes the video's own box: the old video
absolutely positioned in its own box hung 5 of 120 pages (8 of 120 with `opacity: 0`),
and a one-cell grid 4 of 120. Only `opacity: 0` on the video, left in the flow, did not
hang (0 of 120), and alone it would leave an empty hole above the new preview. A route
change within the workspace waits the same way: the shell renders the next route once a
video still starting on Inspect settles (`leaveGuard`; the latest route wins). Route
changes at `loadstart` + 0–7 ms hung 4–5 of 120 pages before and 0 of 120 after.
A real page leave cannot wait. With no page code releasing anything, navigating
away hung 8 of 120 pages and reload 22 of 120 (3 and 8 of 120 with the page's own
release), so that WebKit teardown stays a known limitation; dispose still releases 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
@@ -1696,8 +1708,9 @@ mapped differences"):
`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
ms), and `leaving for another route while a video starts waits for it to settle`. 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
+48
View File
@@ -577,6 +577,54 @@ test('a video replaced while still starting is reset once it settles', async ({
}
});
test('leaving for another route while a video starts waits for it to settle', async ({ context, browserName }) => {
test.setTimeout(180000);
// A WebKit (GStreamer) video detached before its pipeline had started deadlocked the page (TESTING.md D62); a
// route change detaches the whole page, so the shell renders the next route once the video reports metadata or an
// error. The route changes from the video's own `loadstart`, or a few milliseconds after it.
for (let attempt = 0; attempt < 15; attempt++) {
const page = await context.newPage();
try {
if (attempt === 0) await startCoverage(page, browserName);
await page.addInitScript(() => {
const load = HTMLMediaElement.prototype.load;
HTMLMediaElement.prototype.load = function () {
if (!this.hasAttribute('src') && window.__starting?.node === this) window.__starting.reset = { readyState: this.readyState, failed: this.error !== null, connected: this.isConnected };
return load.call(this);
};
});
await openInspect(page);
await page.evaluate(delay => {
const slot = document.querySelector('#local-preview');
new MutationObserver((records, observer) => {
const video = slot.querySelector('video');
if (!video) return;
observer.disconnect();
const leave = () => {
window.__starting = { node: video, readyState: video.readyState };
location.hash = '#/overview';
};
video.addEventListener('loadstart', () => { if (delay) setTimeout(leave, delay); else leave(); }, { once: true });
}).observe(slot, { childList: true });
}, attempt % 5);
await choose(page, localFixture('core-tags.webm', 'video/webm'));
await expect(main(page).getByRole('heading', { level: 1, name: 'Overview', exact: true })).toBeVisible({ timeout: 8000 });
const { readyState, reset } = await page.evaluate(() => ({ readyState: window.__starting.readyState, reset: window.__starting.reset }));
if (attempt % 5 === 0) expect(readyState).toBe(0);
// The video was reset and left the page only once it had settled.
expect(reset.readyState >= 1 || reset.failed).toBe(true);
expect(reset.connected).toBe(true);
// The page still answers.
await page.locator('aside.sidebar nav').getByRole('link', { name: 'Inspect a file', exact: true }).click();
await chooser(page).setInputFiles(parityPng('f02.png'), { timeout: 8000 });
await expect(preview(page).locator('img.local-file-preview')).toBeVisible({ timeout: 8000 });
if (attempt === 0) await saveCoverage(page, browserName);
} finally {
await page.close();
}
}
});
test('closing the viewer on a video that just started never blocks the page', async ({ context, browserName }) => {
test.setTimeout(180000);
// The viewer's video teardown resets its element; on WebKit a reset of a still-starting pipeline blocked the page.
+15 -1
View File
@@ -132,9 +132,23 @@ function mountWorkspace() {
refreshCounts(NAV_COUNT_ROUTES);
let viewScope;
let firstRender = true;
// D62: a page whose teardown must wait (Inspect, while a video is still starting) holds the next route until it
// settles; the latest route wins.
let leaveGuard = null, renderToken = 0;
scope.cleanup(() => viewScope?.dispose());
function render(route) {
const token = ++renderToken;
const hold = leaveGuard?.();
if (hold) {
hold.then(() => { if (token === renderToken && !scope.signal.aborted) renderNow(route); });
return;
}
renderNow(route);
}
function renderNow(route) {
leaveGuard = null;
// A shared page dialog never stays open over the next route.
closeWorkspaceDialog();
viewScope?.dispose();
@@ -240,7 +254,7 @@ function mountWorkspace() {
if (route.id === 'inspect') {
// The concept's Inspect a file (app.js renderMetadata): a local file, read only in this browser.
description.textContent = route.description;
mountInspect(main, { parentScope: view, api, sender: createPromptSender({ scope: view }) });
({ leaveGuard } = mountInspect(main, { parentScope: view, api, sender: createPromptSender({ scope: view }) }));
return;
}
if (isHandoff(route)) {
+24 -3
View File
@@ -64,8 +64,10 @@ function releaseMedia(preview) {
/**
* Mounts the page into `main` below the heading the shell rendered. `sender` is the shared prompt sender; `api` is
* only handed to the viewer (the local adapter never calls it). Returns `{dispose}`; the page also ends with
* `parentScope`.
* only handed to the viewer (the local adapter never calls it). Returns `{dispose, leaveGuard}` (`{dispose}` when
* `parentScope` has already ended); the page also ends with `parentScope`. `leaveGuard()` is a promise while a video
* on the page is still starting (D62): the shell renders the next route once it settles, since detaching it then
* deadlocked WebKit.
*/
export function mountInspect(main, { parentScope, api, sender }) {
const scope = createScope();
@@ -110,10 +112,12 @@ export function mountInspect(main, { parentScope, api, sender }) {
entry.waiting.abort(); clearTimeout(entry.timer);
releaseMedia(entry.preview);
if (entry.revoke) URL.revokeObjectURL(url);
entry.done();
}
function retire(preview, url) {
const waiting = new AbortController();
const entry = { preview, waiting, revoke: false, timer: setTimeout(() => finishRetiring(url), STARTING_VIDEO_MS) };
entry.finished = new Promise(resolve => { entry.done = resolve; });
retiring.set(url, entry);
preview.listeners.abort();
preview.node.inert = true; preview.node.setAttribute('aria-hidden', 'true');
@@ -262,5 +266,22 @@ export function mountInspect(main, { parentScope, api, sender }) {
if (open.getAttribute('aria-disabled') === 'true' || !entry || session.state.phase !== 'ready') return;
viewer.open({ items: [entry], selectedKey: entry.key, trigger: open });
});
return { dispose };
/** Settles when no video on the page is still starting; `null` when none is. */
function leaveGuard() {
if (scope.signal.aborted) return null;
const waits = [];
for (const entry of retiring.values()) waits.push(entry.finished);
if (media && isStarting(media.node)) {
const { node } = media;
waits.push(new Promise(resolve => {
const settled = new AbortController();
const done = () => { settled.abort(); clearTimeout(timer); resolve(); };
const timer = setTimeout(done, STARTING_VIDEO_MS);
for (const type of ['loadedmetadata', 'error']) node.addEventListener(type, done, { once: true, signal: settled.signal });
scope.signal.addEventListener('abort', done, { once: true, signal: settled.signal });
}));
}
return waits.length ? Promise.all(waits) : null;
}
return { dispose, leaveGuard };
}