Skip to content

Commit 97cfc07

Browse files
fix(preview): remote viewers recover from stalled first captures (#25381)
1 parent 0a333a6 commit 97cfc07

3 files changed

Lines changed: 56 additions & 6 deletions

File tree

‎apps/mobile/src/features/browser/PreviewStreamWebView.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -405,7 +405,7 @@ function PreviewStreamDocumentView({
405405
onPictureInPicture?.(message, message.detail);
406406
return;
407407
case "status":
408-
clearBootstrapTimer();
408+
if (message.status !== "connecting") clearBootstrapTimer();
409409
if (message.status === "error") {
410410
fail(message.detail ?? "Browser stream failed.");
411411
return;

‎apps/server/src/preview/ServerBrowser.test.ts‎

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,7 @@ function makeSession(
114114
if (method === "Page.getNavigationHistory") return { currentIndex: 0, entries: [{}] };
115115
if (method === "Page.getLayoutMetrics") return { cssVisualViewport: { pageX: 0, pageY: 0 } };
116116
if (method === "Page.captureScreenshot") {
117+
if ((input as { format: string }).format === "jpeg" && heldStill) await heldStill();
117118
// A snapshot sizes its screenshot from the PNG header; viewers get JPEG frames as-is.
118119
return { data: (input as { format: string }).format === "png" ? PNG_HEADER : "ZnJhbWU=" };
119120
}
@@ -384,6 +385,7 @@ let encoderPageOverride: Page | undefined;
384385
const encoderStreams: Array<{ frameRate: number | undefined; bitsPerSecond: number }> = [];
385386
const contextRequests: Array<{ profileId: string; isolated: boolean }> = [];
386387
let contextGate: PromiseWithResolvers<void> | null = null;
388+
let heldStill: (() => Promise<void>) | null = null;
387389
let pageCopy = "";
388390
let keyCopy = "";
389391
let focusedFrame: ReturnType<typeof makeFrame> | null = null;
@@ -568,6 +570,7 @@ beforeEach(() => {
568570
encoderStreams.length = 0;
569571
contextRequests.length = 0;
570572
contextGate = null;
573+
heldStill = null;
571574
contextFailure = null;
572575
pageSetupFailure = null;
573576
initialNavigation = null;
@@ -1130,6 +1133,38 @@ it.live("enforces provider ownership and explicit targets when a session has mul
11301133
).pipe(Effect.provide(layer)),
11311134
);
11321135

1136+
it.effect("a first still that never returns does not stall the tab's later captures", () =>
1137+
Effect.scoped(
1138+
Effect.gen(function* () {
1139+
const { browser, broker, tabId } = yield* ready;
1140+
vi.useFakeTimers({ toFake: ["setTimeout", "clearTimeout"] });
1141+
yield* Effect.addFinalizer(() => Effect.sync(() => vi.useRealTimers()));
1142+
const capturing = Promise.withResolvers<void>();
1143+
const stuck = Promise.withResolvers<void>();
1144+
heldStill = () => {
1145+
heldStill = null;
1146+
capturing.resolve();
1147+
return stuck.promise;
1148+
};
1149+
yield* Effect.addFinalizer(() => Effect.sync(() => stuck.resolve()));
1150+
yield* browser.attachViewer(viewerInput(tabId, true));
1151+
yield* Effect.promise(() => capturing.promise);
1152+
const snapshotting = yield* broker
1153+
.invoke<PreviewAutomationSnapshot>({ scope, tabId, operation: "snapshot", input: {} })
1154+
.pipe(Effect.forkScoped);
1155+
yield* Effect.promise(() => vi.advanceTimersByTimeAsync(2_000));
1156+
const snapshot = yield* Fiber.join(snapshotting);
1157+
expect(snapshot.accessibilityTree).toContain("delete");
1158+
const second = yield* browser.attachViewer(viewerInput(tabId, false));
1159+
const frame = yield* Stream.fromQueue(second.output).pipe(
1160+
Stream.filter((output) => output._tag === "frame"),
1161+
Stream.runHead,
1162+
);
1163+
expect(Option.isSome(frame)).toBe(true);
1164+
}),
1165+
).pipe(Effect.provide(layer)),
1166+
);
1167+
11331168
it.live("streams to a read-only viewer without allowing takeover, input, or viewport changes", () =>
11341169
Effect.scoped(
11351170
Effect.gen(function* () {
@@ -1148,7 +1183,10 @@ it.live("streams to a read-only viewer without allowing takeover, input, or view
11481183
yield* viewer.input(message);
11491184
expect(page.setViewportSize).not.toHaveBeenCalled();
11501185
expect(session.send.mock.calls.some(([method]) => method.startsWith("Input."))).toBe(false);
1151-
const outputs = yield* Queue.takeAll(viewer.output);
1186+
const outputs = yield* Stream.fromQueue(viewer.output).pipe(
1187+
Stream.takeUntil((output) => output._tag === "frame"),
1188+
Stream.runCollect,
1189+
);
11521190
expect(outputs).toContainEqual(
11531191
expect.objectContaining({ _tag: "frame", data: Buffer.from("frame") }),
11541192
);

‎apps/server/src/preview/ServerBrowser.ts‎

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,8 @@ const SCREENCAST_MOTION_WINDOW_MS = 300;
111111
const SCREENCAST_MOTION_QUALITY = 50;
112112
const HOST_RECONNECT_DELAY = "1 second";
113113
const DESKTOP_ATTACH_TIMEOUT = "10 seconds";
114+
// A viewer still is best-effort; a capture Chromium never answers must not hold the capture lock.
115+
const VIEWER_STILL_TIMEOUT_MS = 2_000;
114116
const VIEWER_OUTPUT_LIMIT = 64;
115117
const RECORDING_SCREENCAST = { format: "jpeg", quality: 90, everyNthFrame: 1 } as const;
116118
const RECORDING_BINARY_EXPORT_MIN_BYTES = 8 * 1024 * 1024;
@@ -2945,9 +2947,19 @@ const make = Effect.gen(function* () {
29452947
broadcastControl(tab);
29462948
pushFileChooser(tab);
29472949
const pushStill = async () => {
2948-
const data = await withCaptureLock(tab, () =>
2949-
ServerBrowserPage.captureViewport(session, { format: "jpeg", quality }),
2950-
).catch(() => null);
2950+
const data = await withCaptureLock(tab, async () => {
2951+
let timer: ReturnType<typeof setTimeout> | undefined;
2952+
try {
2953+
return await Promise.race([
2954+
ServerBrowserPage.captureViewport(session, { format: "jpeg", quality }),
2955+
new Promise<null>((resolve) => {
2956+
timer = setTimeout(() => resolve(null), VIEWER_STILL_TIMEOUT_MS);
2957+
}),
2958+
]);
2959+
} finally {
2960+
clearTimeout(timer);
2961+
}
2962+
}).catch(() => null);
29512963
if (data) viewer.push({ _tag: "frame", data: Buffer.from(data, "base64"), ack: Effect.void });
29522964
};
29532965
let settleTimer: ReturnType<typeof setTimeout> | null = null;
@@ -3005,7 +3017,7 @@ const make = Effect.gen(function* () {
30053017
yield* Effect.promise(() => startScreencast(1));
30063018
// An idle page does not repaint for a new screencast, so the viewer
30073019
// starts from a still unless a live frame beat it.
3008-
if (!screencastStarted) yield* Effect.promise(pushStill);
3020+
if (!screencastStarted) yield* Effect.forkScoped(Effect.promise(pushStill));
30093021
return {
30103022
output,
30113023
input: (raw: unknown) =>

0 commit comments

Comments
 (0)