From 80020871493e7ba95b59f7ad8bdef755369b2d14 Mon Sep 17 00:00:00 2001 From: Jack Singer Date: Sun, 1 Nov 2026 00:29:42 -0400 Subject: [PATCH 1/8] fix(desktop): bound preview frames awaiting consumer acknowledgement --- apps/desktop/src/preview/CdpRelay.test.ts | 70 +++++++++++++++++++++-- apps/desktop/src/preview/CdpRelay.ts | 34 +++++++++++ 2 files changed, 100 insertions(+), 4 deletions(-) diff --git a/apps/desktop/src/preview/CdpRelay.test.ts b/apps/desktop/src/preview/CdpRelay.test.ts index a2589edf8f7b2..c06c31a51d86f 100644 --- a/apps/desktop/src/preview/CdpRelay.test.ts +++ b/apps/desktop/src/preview/CdpRelay.test.ts @@ -132,8 +132,8 @@ describe("CDP relay", () => { }); describe("CDP relay extra sessions", () => { - it("gives a second page session the page's commands and events", async () => { - const send = vi.fn(async () => ({})); + it("bounds screencast delivery independently for each page session", async () => { + const send = vi.fn(async () => ({})); const written: Array> = []; const relay = makeRelay(makeTarget(send), (raw) => written.push(JSON.parse(raw))); relay.receive(JSON.stringify({ id: 1, method: "Target.setAutoAttach", params: {} })); @@ -165,8 +165,70 @@ describe("CDP relay extra sessions", () => { await settle(); expect(send).toHaveBeenLastCalledWith("Page.startScreencast", {}, undefined, extra); written.length = 0; - relay.event("Page.screencastFrame", { data: "x" }, undefined); - expect(written.map((m) => m["sessionId"])).toEqual(["t3-preview-page", extra]); + const frame = (sessionId = 7) => + relay.event("Page.screencastFrame", { data: "x", sessionId }, undefined); + for (let i = 0; i < 100; i++) frame(); + expect(written).toHaveLength(1); + expect(written.map((m) => m["sessionId"])).toEqual([extra]); + + relay.receive( + JSON.stringify({ + id: 6, + method: "Page.startScreencast", + sessionId: "t3-preview-page", + }), + ); + await settle(); + written.length = 0; + frame(); + expect(written.map((m) => m["sessionId"])).toEqual(["t3-preview-page"]); + const acknowledge = (sessionId: string, frameId = 7) => + relay.receive( + JSON.stringify({ + id: 7, + method: "Page.screencastFrameAck", + params: { sessionId: frameId }, + sessionId, + }), + ); + acknowledge(extra); + await settle(); + written.length = 0; + frame(); + frame(); + expect(written.map((m) => m["sessionId"])).toEqual([extra]); + expect(send.mock.calls.filter(([method]) => method === "Page.screencastFrameAck")).toEqual([]); + + relay.receive(JSON.stringify({ id: 8, method: "Page.stopScreencast", sessionId: extra })); + relay.receive(JSON.stringify({ id: 9, method: "Page.startScreencast", sessionId: extra })); + await settle(); + written.length = 0; + frame(8); + acknowledge(extra); // A late acknowledgement from the previous stream must not release this one. + await settle(); + written.length = 0; + frame(8); + expect(written).toEqual([]); + acknowledge(extra, 8); + await settle(); + written.length = 0; + frame(8); + expect(written.map((m) => m["sessionId"])).toEqual([extra]); + + relay.receive( + JSON.stringify({ + id: 10, + method: "Target.detachFromTarget", + params: { sessionId: extra }, + }), + ); + await settle(); + written.length = 0; + frame(8); + expect(written).toEqual([]); + // Other protocol events still reach the main page session while its frame is pending. + relay.event("Page.loadEventFired", { timestamp: 1 }, undefined); + expect(written.map((m) => m["method"])).toEqual(["Page.loadEventFired"]); }); }); diff --git a/apps/desktop/src/preview/CdpRelay.ts b/apps/desktop/src/preview/CdpRelay.ts index d52b0a3b2ab55..994f35f1feba8 100644 --- a/apps/desktop/src/preview/CdpRelay.ts +++ b/apps/desktop/src/preview/CdpRelay.ts @@ -100,6 +100,8 @@ export function createCdpRelayConnection( let attached = false; /** Relay-created page sessions share one real debugger; child frame sessions retain their own ids. */ const sessions = new Map([[PAGE_SESSION_ID, "page"]]); + // The manager acknowledges native frames immediately; each relay consumer must bound its own queue. + const screencasts = new Map(); let sessionSequence = 0; const newSession = (kind: "browser" | "page") => { const sessionId = `t3-preview-${kind}-${++sessionSequence}`; @@ -161,6 +163,7 @@ export function createCdpRelayConnection( const sessionId = command.params?.["sessionId"]; if (typeof sessionId === "string" && sessionId !== PAGE_SESSION_ID) { sessions.delete(sessionId); + screencasts.delete(sessionId); target.sessionDetached?.(sessionId); } return {}; @@ -191,6 +194,27 @@ export function createCdpRelayConnection( } // Playwright asks for this on every page; the host emulates focus only while it is in use. if (command.method === "Emulation.setFocusEmulationEnabled") return Promise.resolve({}); + if (child === undefined) { + const sessionId = command.sessionId!; + if (command.method === "Page.screencastFrameAck") { + const stream = screencasts.get(sessionId); + if (stream && stream.pendingFrame === command.params?.["sessionId"]) { + stream.pendingFrame = null; + } + return Promise.resolve({}); + } + if (command.method === "Page.stopScreencast") screencasts.delete(sessionId); + if (command.method === "Page.startScreencast") { + const stream = { pendingFrame: null as number | null }; + screencasts.set(sessionId, stream); + return target + .send(command.method, command.params ?? {}, child, owner) + .catch((cause: unknown) => { + if (screencasts.get(sessionId) === stream) screencasts.delete(sessionId); + throw cause; + }); + } + } return target.send(command.method, command.params ?? {}, child, owner); }; @@ -242,6 +266,16 @@ export function createCdpRelayConnection( send({ method, params, sessionId }); return; } + if (method === "Page.screencastFrame") { + const frameId = (params as { sessionId?: unknown } | null)?.sessionId; + if (typeof frameId !== "number") return; + for (const [id, stream] of screencasts) { + if (stream.pendingFrame !== null) continue; + stream.pendingFrame = frameId; + send({ method, params, sessionId: id }); + } + return; + } for (const [id, kind] of sessions) if (kind === "page") send({ method, params, sessionId: id }); }, From e693d312058c808a48f79d53084fd018390b6914 Mon Sep 17 00:00:00 2001 From: Jack Singer Date: Sun, 1 Nov 2026 01:00:19 -0400 Subject: [PATCH 2/8] fix(desktop): preserve committed screencasts after failed restarts --- apps/desktop/src/preview/CdpRelay.test.ts | 55 +++++++++++++- apps/desktop/src/preview/CdpRelay.ts | 23 +++--- .../src/preview/DesktopBrowserHost.test.ts | 76 ++++++++++++++++++- .../desktop/src/preview/DesktopBrowserHost.ts | 36 ++++++--- 4 files changed, 166 insertions(+), 24 deletions(-) diff --git a/apps/desktop/src/preview/CdpRelay.test.ts b/apps/desktop/src/preview/CdpRelay.test.ts index c06c31a51d86f..411ef6714a679 100644 --- a/apps/desktop/src/preview/CdpRelay.test.ts +++ b/apps/desktop/src/preview/CdpRelay.test.ts @@ -132,6 +132,55 @@ describe("CDP relay", () => { }); describe("CDP relay extra sessions", () => { + it.each([ + { committed: true, newerSucceeds: false, olderSucceeds: false }, + { committed: false, newerSucceeds: true, olderSucceeds: false }, + { committed: false, newerSucceeds: false, olderSucceeds: true }, + { committed: false, newerSucceeds: false, olderSucceeds: false }, + ])( + "preserves only committed streams across overlapping starts (%j)", + async ({ committed, newerSucceeds, olderSucceeds }) => { + const send = vi.fn(async () => ({})); + const written: Array> = []; + const relay = makeRelay(makeTarget(send), (raw) => written.push(JSON.parse(raw))); + const command = (id: number, method: string, params = {}) => + relay.receive(JSON.stringify({ id, method, params, sessionId: "t3-preview-page" })); + const frame = (sessionId: number) => + relay.event("Page.screencastFrame", { data: "x", sessionId }, undefined); + const frames = () => + written.filter((message) => message["method"] === "Page.screencastFrame"); + relay.receive(JSON.stringify({ id: 1, method: "Target.setAutoAttach" })); + if (committed) { + command(2, "Page.startScreencast"); + await settle(); + frame(7); + expect(frames()).toHaveLength(1); + } + await settle(); + written.length = 0; + const older = Promise.withResolvers(); + const newer = Promise.withResolvers(); + send.mockImplementationOnce(() => older.promise).mockImplementationOnce(() => newer.promise); + command(3, "Page.startScreencast"); + command(4, "Page.startScreencast"); + frame(8); + expect(frames()).toEqual([]); + if (newerSucceeds) newer.resolve({}); + else newer.reject(new Error("Refused")); + await settle(); + if (olderSucceeds) older.resolve({}); + else older.reject(new Error("Refused")); + await settle(); + if (committed) command(5, "Page.screencastFrameAck", { sessionId: 7 }); + frame(9); + frame(9); + expect(frames()).toHaveLength(committed || newerSucceeds ? 1 : 0); + expect(written.filter((message) => message["error"])).toHaveLength( + Number(!olderSucceeds) + Number(!newerSucceeds), + ); + }, + ); + it("bounds screencast delivery independently for each page session", async () => { const send = vi.fn(async () => ({})); const written: Array> = []; @@ -215,13 +264,17 @@ describe("CDP relay extra sessions", () => { frame(8); expect(written.map((m) => m["sessionId"])).toEqual([extra]); + const restarting = Promise.withResolvers(); + send.mockReturnValueOnce(restarting.promise); + relay.receive(JSON.stringify({ id: 10, method: "Page.startScreencast", sessionId: extra })); relay.receive( JSON.stringify({ - id: 10, + id: 11, method: "Target.detachFromTarget", params: { sessionId: extra }, }), ); + restarting.resolve({}); await settle(); written.length = 0; frame(8); diff --git a/apps/desktop/src/preview/CdpRelay.ts b/apps/desktop/src/preview/CdpRelay.ts index 994f35f1feba8..f1798a94036d5 100644 --- a/apps/desktop/src/preview/CdpRelay.ts +++ b/apps/desktop/src/preview/CdpRelay.ts @@ -102,6 +102,7 @@ export function createCdpRelayConnection( const sessions = new Map([[PAGE_SESSION_ID, "page"]]); // The manager acknowledges native frames immediately; each relay consumer must bound its own queue. const screencasts = new Map(); + const screencastStarts = new Map(); let sessionSequence = 0; const newSession = (kind: "browser" | "page") => { const sessionId = `t3-preview-${kind}-${++sessionSequence}`; @@ -164,6 +165,7 @@ export function createCdpRelayConnection( if (typeof sessionId === "string" && sessionId !== PAGE_SESSION_ID) { sessions.delete(sessionId); screencasts.delete(sessionId); + screencastStarts.delete(sessionId); target.sessionDetached?.(sessionId); } return {}; @@ -203,16 +205,19 @@ export function createCdpRelayConnection( } return Promise.resolve({}); } - if (command.method === "Page.stopScreencast") screencasts.delete(sessionId); + if (command.method === "Page.stopScreencast") { + screencasts.delete(sessionId); + screencastStarts.delete(sessionId); + } if (command.method === "Page.startScreencast") { - const stream = { pendingFrame: null as number | null }; - screencasts.set(sessionId, stream); - return target - .send(command.method, command.params ?? {}, child, owner) - .catch((cause: unknown) => { - if (screencasts.get(sessionId) === stream) screencasts.delete(sessionId); - throw cause; - }); + const start = {}; + screencastStarts.set(sessionId, start); + return target.send(command.method, command.params ?? {}, child, owner).then((result) => { + if (screencastStarts.get(sessionId) === start) { + screencasts.set(sessionId, { pendingFrame: null }); + } + return result; + }); } } return target.send(command.method, command.params ?? {}, child, owner); diff --git a/apps/desktop/src/preview/DesktopBrowserHost.test.ts b/apps/desktop/src/preview/DesktopBrowserHost.test.ts index 3168788bebe9b..d4a1f4c5da6ad 100644 --- a/apps/desktop/src/preview/DesktopBrowserHost.test.ts +++ b/apps/desktop/src/preview/DesktopBrowserHost.test.ts @@ -38,7 +38,7 @@ const makeDebuggee = ( } = {}, ) => { const emitter = new NodeEvents.EventEmitter(); - const pending: Array<() => void> = []; + const pending: Array<{ resolve: () => void; reject: () => void }> = []; const focusChanges: Array<() => void> = []; const sent: Array = []; const sendWaiters: Array<() => void> = []; @@ -65,7 +65,10 @@ const makeDebuggee = ( return; } sent.push(params?.id); - pending.push(() => resolve({ method })); + pending.push({ + resolve: () => resolve({ method }), + reject: () => reject(new Error("Refused")), + }); sendWaiters.shift()?.(); }), }); @@ -85,8 +88,11 @@ const makeDebuggee = ( sent, focusEmulation, nextSend: () => new Promise((resolve) => sendWaiters.push(resolve)), - release: () => pending.splice(0).forEach((resolve) => resolve()), - releaseFirst: () => pending.shift()?.(), + release: () => pending.splice(0).forEach(({ resolve }) => resolve()), + releaseFirst: () => pending.shift()?.resolve(), + releaseLast: () => pending.pop()?.resolve(), + refuseFirst: () => pending.shift()?.reject(), + refuseLast: () => pending.pop()?.reject(), settleFocus: () => focusChanges.splice(0).forEach((resolve) => resolve()), }; }; @@ -616,6 +622,68 @@ describe("DesktopBrowserHost", () => { }).pipe(Effect.scoped), ); + it.effect("preserves committed painting holds across failed and overlapping starts", () => + Effect.gen(function* () { + const host = yield* DesktopBrowserHost.make; + const debuggee = makeDebuggee(); + host.attach(key, debuggee.tab); + const replies = yield* watchReplies(host); + const painting = yield* watchPainting(host); + + yield* command(host, 1, "Page.startScreencast"); + yield* settle; + debuggee.releaseFirst(); + expect(yield* Queue.take(replies)).toBe(1); + yield* command(host, 2, "Page.startScreencast"); + yield* settle; + debuggee.refuseFirst(); + expect(yield* Queue.take(replies)).toBe(2); + expect(painting).toEqual([true]); + + yield* command(host, 3, "Page.stopScreencast"); + yield* settle; + debuggee.releaseFirst(); + expect(yield* Queue.take(replies)).toBe(3); + expect(painting).toEqual([true, false]); + + yield* command(host, 4, "Page.startScreencast"); + yield* command(host, 5, "Page.startScreencast"); + yield* settle; + debuggee.releaseLast(); + expect(yield* Queue.take(replies)).toBe(5); + debuggee.refuseFirst(); + expect(yield* Queue.take(replies)).toBe(4); + expect(painting).toEqual([true, false, true]); + + yield* command(host, 6, "Page.stopScreencast"); + yield* settle; + debuggee.releaseFirst(); + expect(yield* Queue.take(replies)).toBe(6); + expect(painting).toEqual([true, false, true, false]); + + yield* command(host, 7, "Page.startScreencast"); + yield* command(host, 8, "Page.startScreencast"); + yield* settle; + debuggee.refuseLast(); + expect(yield* Queue.take(replies)).toBe(8); + debuggee.refuseFirst(); + expect(yield* Queue.take(replies)).toBe(7); + expect(painting).toEqual([true, false, true, false, true, false]); + + yield* command(host, 9, "Page.startScreencast"); + yield* settle; + yield* command(host, 10, "Page.stopScreencast"); + yield* settle; + debuggee.releaseFirst(); + expect(yield* Queue.take(replies)).toBe(9); + debuggee.releaseFirst(); + expect(yield* Queue.take(replies)).toBe(10); + expect(painting).toEqual([true, false, true, false, true, false, true, false]); + yield* TestClock.adjust("1 minute"); + expect(debuggee.focusEmulation.at(-1)).toBe(false); + }).pipe(Effect.scoped), + ); + it.effect("lets a guest throttle after a person's input through a viewer that then closed", () => Effect.gen(function* () { const host = yield* DesktopBrowserHost.make; diff --git a/apps/desktop/src/preview/DesktopBrowserHost.ts b/apps/desktop/src/preview/DesktopBrowserHost.ts index 5a2b5463fc674..727a6440ff27a 100644 --- a/apps/desktop/src/preview/DesktopBrowserHost.ts +++ b/apps/desktop/src/preview/DesktopBrowserHost.ts @@ -70,7 +70,7 @@ interface AttachedTab { agentUsed: boolean; focusReleasing: boolean; /** The sessions streaming the guest; one stopping leaves the others' screencasts running. */ - readonly screencasts: Set; + readonly screencasts: Map; readonly onMessage: ( event: Electron.Event, method: string, @@ -311,9 +311,11 @@ export const make = Effect.gen(function* () { const relay: CdpRelayConnection = createCdpRelayConnection( { send: (method, params, sessionId, owner) => { + let start: { committed: boolean } | undefined; // A child session's screencast is not the page's, such as a late one on a detached session. if (sessionId === undefined && method === "Page.startScreencast") { - tab.screencasts.add(owner); + start = { committed: tab.screencasts.get(owner)?.committed ?? false }; + tab.screencasts.set(owner, start); if (owner !== undefined) streamingOwners.add(owner); } else if (sessionId === undefined && method === "Page.stopScreencast") { tab.screencasts.delete(owner); @@ -343,13 +345,27 @@ export const make = Effect.gen(function* () { held ? holdFocus(tab, method, owner, () => forward(current)) : forward(current), ) : method === "Page.startScreencast" - ? holdFocus(tab, method, owner, forward).catch((cause: unknown) => { - // A viewer can outlive a refused stream; nothing is streaming for it. - if (sessionId === undefined && tab.relay === relay) tab.screencasts.delete(owner); - releaseFocusWhenIdle(tab); - syncPainting(tab); - throw cause; - }) + ? holdFocus(tab, method, owner, forward).then( + (result) => { + if (start && tab.relay === relay && tab.screencasts.get(owner) === start) { + start.committed = true; + } + return result; + }, + (cause: unknown) => { + if ( + start && + !start.committed && + tab.relay === relay && + tab.screencasts.get(owner) === start + ) { + tab.screencasts.delete(owner); + } + releaseFocusWhenIdle(tab); + syncPainting(tab); + throw cause; + }, + ) : holdFocus(tab, method, owner, forward); }, sessionDetached: (owner) => { @@ -438,7 +454,7 @@ export const make = Effect.gen(function* () { painting: false, agentUsed: false, focusReleasing: false, - screencasts: new Set(), + screencasts: new Map(), onMessage: (_event, method, params, sessionId) => { if (method === "Browser.downloadWillBegin") { const guid = (params as { guid?: unknown } | undefined)?.guid; From 7e9258225c9d56541f6e2e35161f6cfebdb1386c Mon Sep 17 00:00:00 2001 From: maria-rcks Date: Sun, 1 Nov 2026 08:10:55 +0000 Subject: [PATCH 3/8] fix(desktop): preserve screencast backpressure across restarts --- apps/desktop/src/preview/CdpRelay.test.ts | 7 ++++- apps/desktop/src/preview/CdpRelay.ts | 2 +- .../src/preview/DesktopBrowserHost.test.ts | 26 +++++++++++++++++++ .../desktop/src/preview/DesktopBrowserHost.ts | 15 +++++++---- 4 files changed, 43 insertions(+), 7 deletions(-) diff --git a/apps/desktop/src/preview/CdpRelay.test.ts b/apps/desktop/src/preview/CdpRelay.test.ts index 411ef6714a679..2f6292bc259f7 100644 --- a/apps/desktop/src/preview/CdpRelay.test.ts +++ b/apps/desktop/src/preview/CdpRelay.test.ts @@ -134,6 +134,7 @@ describe("CDP relay", () => { describe("CDP relay extra sessions", () => { it.each([ { committed: true, newerSucceeds: false, olderSucceeds: false }, + { committed: true, newerSucceeds: true, olderSucceeds: false }, { committed: false, newerSucceeds: true, olderSucceeds: false }, { committed: false, newerSucceeds: false, olderSucceeds: true }, { committed: false, newerSucceeds: false, olderSucceeds: false }, @@ -171,7 +172,11 @@ describe("CDP relay extra sessions", () => { if (olderSucceeds) older.resolve({}); else older.reject(new Error("Refused")); await settle(); - if (committed) command(5, "Page.screencastFrameAck", { sessionId: 7 }); + if (committed) { + frame(9); + expect(frames()).toEqual([]); + command(5, "Page.screencastFrameAck", { sessionId: 7 }); + } frame(9); frame(9); expect(frames()).toHaveLength(committed || newerSucceeds ? 1 : 0); diff --git a/apps/desktop/src/preview/CdpRelay.ts b/apps/desktop/src/preview/CdpRelay.ts index f1798a94036d5..a19251f9f2095 100644 --- a/apps/desktop/src/preview/CdpRelay.ts +++ b/apps/desktop/src/preview/CdpRelay.ts @@ -214,7 +214,7 @@ export function createCdpRelayConnection( screencastStarts.set(sessionId, start); return target.send(command.method, command.params ?? {}, child, owner).then((result) => { if (screencastStarts.get(sessionId) === start) { - screencasts.set(sessionId, { pendingFrame: null }); + if (!screencasts.has(sessionId)) screencasts.set(sessionId, { pendingFrame: null }); } return result; }); diff --git a/apps/desktop/src/preview/DesktopBrowserHost.test.ts b/apps/desktop/src/preview/DesktopBrowserHost.test.ts index d4a1f4c5da6ad..7be499dd8ad9f 100644 --- a/apps/desktop/src/preview/DesktopBrowserHost.test.ts +++ b/apps/desktop/src/preview/DesktopBrowserHost.test.ts @@ -679,6 +679,32 @@ describe("DesktopBrowserHost", () => { debuggee.releaseFirst(); expect(yield* Queue.take(replies)).toBe(10); expect(painting).toEqual([true, false, true, false, true, false, true, false]); + yield* command(host, 11, "Page.startScreencast"); + yield* command(host, 12, "Page.startScreencast"); + yield* settle; + debuggee.releaseFirst(); + expect(yield* Queue.take(replies)).toBe(11); + debuggee.refuseLast(); + expect(yield* Queue.take(replies)).toBe(12); + expect(painting.at(-1)).toBe(true); + + yield* command(host, 13, "Page.stopScreencast"); + yield* settle; + debuggee.releaseFirst(); + expect(yield* Queue.take(replies)).toBe(13); + expect(painting.at(-1)).toBe(false); + + yield* command(host, 14, "Page.startScreencast"); + yield* command(host, 15, "Page.stopScreencast"); + yield* command(host, 16, "Page.startScreencast"); + yield* settle; + debuggee.releaseFirst(); + expect(yield* Queue.take(replies)).toBe(14); + debuggee.releaseFirst(); + expect(yield* Queue.take(replies)).toBe(15); + debuggee.refuseLast(); + expect(yield* Queue.take(replies)).toBe(16); + expect(painting.at(-1)).toBe(false); yield* TestClock.adjust("1 minute"); expect(debuggee.focusEmulation.at(-1)).toBe(false); }).pipe(Effect.scoped), diff --git a/apps/desktop/src/preview/DesktopBrowserHost.ts b/apps/desktop/src/preview/DesktopBrowserHost.ts index a877122027456..3878182e262d4 100644 --- a/apps/desktop/src/preview/DesktopBrowserHost.ts +++ b/apps/desktop/src/preview/DesktopBrowserHost.ts @@ -70,7 +70,7 @@ interface AttachedTab { agentUsed: boolean; focusReleasing: boolean; /** The sessions streaming the guest; one stopping leaves the others' screencasts running. */ - readonly screencasts: Map; + readonly screencasts: Map; readonly onMessage: ( event: Electron.Event, method: string, @@ -309,10 +309,14 @@ export const make = Effect.gen(function* () { const relay: CdpRelayConnection = createCdpRelayConnection( { send: (method, params, sessionId, owner) => { - let start: { committed: boolean } | undefined; + let start: { committed: boolean; generation: object } | undefined; // A child session's screencast is not the page's, such as a late one on a detached session. if (sessionId === undefined && method === "Page.startScreencast") { - start = { committed: tab.screencasts.get(owner)?.committed ?? false }; + const previous = tab.screencasts.get(owner); + start = { + committed: previous?.committed ?? false, + generation: previous?.generation ?? {}, + }; tab.screencasts.set(owner, start); if (owner !== undefined) streamingOwners.add(owner); } else if (sessionId === undefined && method === "Page.stopScreencast") { @@ -345,8 +349,9 @@ export const make = Effect.gen(function* () { : method === "Page.startScreencast" ? holdFocus(tab, method, owner, forward).then( (result) => { - if (start && tab.relay === relay && tab.screencasts.get(owner) === start) { - start.committed = true; + const current = tab.screencasts.get(owner); + if (start && tab.relay === relay && current?.generation === start.generation) { + current.committed = true; } return result; }, From c7d24da86ccf31d51cec1992bc0985ed0489df42 Mon Sep 17 00:00:00 2001 From: maria-rcks Date: Sun, 1 Nov 2026 08:21:28 +0000 Subject: [PATCH 4/8] fix(desktop): retain screencast slots through pause and overlapping starts --- apps/desktop/src/preview/CdpRelay.test.ts | 44 ++++++++++++------- apps/desktop/src/preview/CdpRelay.ts | 13 +++--- .../src/preview/DesktopBrowserHost.test.ts | 14 ++++++ .../desktop/src/preview/DesktopBrowserHost.ts | 19 ++++---- 4 files changed, 60 insertions(+), 30 deletions(-) diff --git a/apps/desktop/src/preview/CdpRelay.test.ts b/apps/desktop/src/preview/CdpRelay.test.ts index 2f6292bc259f7..bb74c11baad6e 100644 --- a/apps/desktop/src/preview/CdpRelay.test.ts +++ b/apps/desktop/src/preview/CdpRelay.test.ts @@ -132,15 +132,17 @@ describe("CDP relay", () => { }); describe("CDP relay extra sessions", () => { - it.each([ - { committed: true, newerSucceeds: false, olderSucceeds: false }, - { committed: true, newerSucceeds: true, olderSucceeds: false }, - { committed: false, newerSucceeds: true, olderSucceeds: false }, - { committed: false, newerSucceeds: false, olderSucceeds: true }, - { committed: false, newerSucceeds: false, olderSucceeds: false }, - ])( + it.each( + [ + { committed: true, newerSucceeds: false, olderSucceeds: false }, + { committed: true, newerSucceeds: true, olderSucceeds: false }, + { committed: false, newerSucceeds: true, olderSucceeds: false }, + { committed: false, newerSucceeds: false, olderSucceeds: true }, + { committed: false, newerSucceeds: false, olderSucceeds: false }, + ].flatMap((scenario) => [true, false].map((olderFirst) => ({ ...scenario, olderFirst }))), + )( "preserves only committed streams across overlapping starts (%j)", - async ({ committed, newerSucceeds, olderSucceeds }) => { + async ({ committed, newerSucceeds, olderSucceeds, olderFirst }) => { const send = vi.fn(async () => ({})); const written: Array> = []; const relay = makeRelay(makeTarget(send), (raw) => written.push(JSON.parse(raw))); @@ -166,12 +168,19 @@ describe("CDP relay extra sessions", () => { command(4, "Page.startScreencast"); frame(8); expect(frames()).toEqual([]); - if (newerSucceeds) newer.resolve({}); - else newer.reject(new Error("Refused")); - await settle(); - if (olderSucceeds) older.resolve({}); - else older.reject(new Error("Refused")); - await settle(); + for (const [pending, succeeds] of olderFirst + ? ([ + [older, olderSucceeds], + [newer, newerSucceeds], + ] as const) + : ([ + [newer, newerSucceeds], + [older, olderSucceeds], + ] as const)) { + if (succeeds) pending.resolve({}); + else pending.reject(new Error("Refused")); + await settle(); + } if (committed) { frame(9); expect(frames()).toEqual([]); @@ -179,7 +188,7 @@ describe("CDP relay extra sessions", () => { } frame(9); frame(9); - expect(frames()).toHaveLength(committed || newerSucceeds ? 1 : 0); + expect(frames()).toHaveLength(committed || newerSucceeds || olderSucceeds ? 1 : 0); expect(written.filter((message) => message["error"])).toHaveLength( Number(!olderSucceeds) + Number(!newerSucceeds), ); @@ -258,6 +267,11 @@ describe("CDP relay extra sessions", () => { await settle(); written.length = 0; frame(8); + expect(written).toEqual([]); + acknowledge(extra); + await settle(); + written.length = 0; + frame(8); acknowledge(extra); // A late acknowledgement from the previous stream must not release this one. await settle(); written.length = 0; diff --git a/apps/desktop/src/preview/CdpRelay.ts b/apps/desktop/src/preview/CdpRelay.ts index a19251f9f2095..1b298222850e8 100644 --- a/apps/desktop/src/preview/CdpRelay.ts +++ b/apps/desktop/src/preview/CdpRelay.ts @@ -101,7 +101,7 @@ export function createCdpRelayConnection( /** Relay-created page sessions share one real debugger; child frame sessions retain their own ids. */ const sessions = new Map([[PAGE_SESSION_ID, "page"]]); // The manager acknowledges native frames immediately; each relay consumer must bound its own queue. - const screencasts = new Map(); + const screencasts = new Map(); const screencastStarts = new Map(); let sessionSequence = 0; const newSession = (kind: "browser" | "page") => { @@ -206,15 +206,18 @@ export function createCdpRelayConnection( return Promise.resolve({}); } if (command.method === "Page.stopScreencast") { - screencasts.delete(sessionId); + const stream = screencasts.get(sessionId); + if (stream) stream.active = false; screencastStarts.delete(sessionId); } if (command.method === "Page.startScreencast") { - const start = {}; + const start = screencastStarts.get(sessionId) ?? {}; screencastStarts.set(sessionId, start); return target.send(command.method, command.params ?? {}, child, owner).then((result) => { if (screencastStarts.get(sessionId) === start) { - if (!screencasts.has(sessionId)) screencasts.set(sessionId, { pendingFrame: null }); + const stream = screencasts.get(sessionId); + if (stream) stream.active = true; + else screencasts.set(sessionId, { pendingFrame: null, active: true }); } return result; }); @@ -275,7 +278,7 @@ export function createCdpRelayConnection( const frameId = (params as { sessionId?: unknown } | null)?.sessionId; if (typeof frameId !== "number") return; for (const [id, stream] of screencasts) { - if (stream.pendingFrame !== null) continue; + if (!stream.active || stream.pendingFrame !== null) continue; stream.pendingFrame = frameId; send({ method, params, sessionId: id }); } diff --git a/apps/desktop/src/preview/DesktopBrowserHost.test.ts b/apps/desktop/src/preview/DesktopBrowserHost.test.ts index 7be499dd8ad9f..714a380b6e63f 100644 --- a/apps/desktop/src/preview/DesktopBrowserHost.test.ts +++ b/apps/desktop/src/preview/DesktopBrowserHost.test.ts @@ -705,6 +705,20 @@ describe("DesktopBrowserHost", () => { debuggee.refuseLast(); expect(yield* Queue.take(replies)).toBe(16); expect(painting.at(-1)).toBe(false); + yield* command(host, 17, "Page.startScreencast"); + yield* command(host, 18, "Page.startScreencast"); + yield* settle; + debuggee.refuseLast(); + expect(yield* Queue.take(replies)).toBe(18); + debuggee.releaseFirst(); + expect(yield* Queue.take(replies)).toBe(17); + expect(painting.at(-1)).toBe(true); + + yield* command(host, 19, "Page.stopScreencast"); + yield* settle; + debuggee.releaseFirst(); + expect(yield* Queue.take(replies)).toBe(19); + expect(painting.at(-1)).toBe(false); yield* TestClock.adjust("1 minute"); expect(debuggee.focusEmulation.at(-1)).toBe(false); }).pipe(Effect.scoped), diff --git a/apps/desktop/src/preview/DesktopBrowserHost.ts b/apps/desktop/src/preview/DesktopBrowserHost.ts index 3878182e262d4..915daeac7b994 100644 --- a/apps/desktop/src/preview/DesktopBrowserHost.ts +++ b/apps/desktop/src/preview/DesktopBrowserHost.ts @@ -70,7 +70,7 @@ interface AttachedTab { agentUsed: boolean; focusReleasing: boolean; /** The sessions streaming the guest; one stopping leaves the others' screencasts running. */ - readonly screencasts: Map; + readonly screencasts: Map; readonly onMessage: ( event: Electron.Event, method: string, @@ -309,14 +309,11 @@ export const make = Effect.gen(function* () { const relay: CdpRelayConnection = createCdpRelayConnection( { send: (method, params, sessionId, owner) => { - let start: { committed: boolean; generation: object } | undefined; + let start: { committed: boolean; pending: number } | undefined; // A child session's screencast is not the page's, such as a late one on a detached session. if (sessionId === undefined && method === "Page.startScreencast") { - const previous = tab.screencasts.get(owner); - start = { - committed: previous?.committed ?? false, - generation: previous?.generation ?? {}, - }; + start = tab.screencasts.get(owner) ?? { committed: false, pending: 0 }; + start.pending++; tab.screencasts.set(owner, start); if (owner !== undefined) streamingOwners.add(owner); } else if (sessionId === undefined && method === "Page.stopScreencast") { @@ -349,15 +346,17 @@ export const make = Effect.gen(function* () { : method === "Page.startScreencast" ? holdFocus(tab, method, owner, forward).then( (result) => { - const current = tab.screencasts.get(owner); - if (start && tab.relay === relay && current?.generation === start.generation) { - current.committed = true; + if (start && tab.relay === relay && tab.screencasts.get(owner) === start) { + start.pending--; + start.committed = true; } return result; }, (cause: unknown) => { + if (start) start.pending--; if ( start && + start.pending === 0 && !start.committed && tab.relay === relay && tab.screencasts.get(owner) === start From 852b103575a901c904d098df447702d4d3d76373 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Sat, 14 Nov 2026 13:21:26 +0000 Subject: [PATCH 5/8] fix(desktop): keep a screencast frame's slot across restarts Restarting a screencast must not release an unacknowledged frame, or a consumer that keeps restarting gets frames without ever acknowledging one. Co-Authored-By: Claude Opus 5.5 --- apps/desktop/src/preview/CdpRelay.test.ts | 10 ++++++++-- apps/desktop/src/preview/CdpRelay.ts | 15 +++++---------- .../src/preview/DesktopBrowserHost.test.ts | 7 +++++++ 3 files changed, 20 insertions(+), 12 deletions(-) diff --git a/apps/desktop/src/preview/CdpRelay.test.ts b/apps/desktop/src/preview/CdpRelay.test.ts index 9b6aa21efa754..f55562a2ad9a8 100644 --- a/apps/desktop/src/preview/CdpRelay.test.ts +++ b/apps/desktop/src/preview/CdpRelay.test.ts @@ -202,11 +202,17 @@ describe("CDP relay extra sessions", () => { frame(3); expect(delivered()).toEqual([]); expect(send.mock.calls.some(([method]) => method === "Page.screencastFrameAck")).toBe(false); - // A restart drops the old stream's unacknowledged frame. - relay.receive(JSON.stringify({ id: 7, method: "Page.startScreencast", sessionId: extra })); + // A restart keeps the unacknowledged frame's slot until its own acknowledgement. + relay.receive(JSON.stringify({ id: 7, method: "Page.stopScreencast", sessionId: extra })); + relay.receive(JSON.stringify({ id: 8, method: "Page.startScreencast", sessionId: extra })); await settle(); delivered(); frame(4); + expect(delivered()).toEqual([]); + ack(extra, 2); + await settle(); + delivered(); + frame(5); expect(delivered()).toEqual([extra]); // Other events still reach a session while its frame is unacknowledged. relay.event("Page.loadEventFired", { timestamp: 1 }, undefined); diff --git a/apps/desktop/src/preview/CdpRelay.ts b/apps/desktop/src/preview/CdpRelay.ts index 6c8968f1cfa1d..268ab59624543 100644 --- a/apps/desktop/src/preview/CdpRelay.ts +++ b/apps/desktop/src/preview/CdpRelay.ts @@ -205,17 +205,12 @@ export function createCdpRelayConnection( } // Playwright asks for this on every page; the host emulates focus only while it is in use. if (command.method === "Emulation.setFocusEmulationEnabled") return Promise.resolve({}); - if (child === undefined) { - const sessionId = command.sessionId!; - if (command.method === "Page.screencastFrameAck") { - if (unackedFrames.get(sessionId) === command.params?.["sessionId"]) { - unackedFrames.delete(sessionId); - } - return Promise.resolve({}); - } - if (command.method === "Page.startScreencast" || command.method === "Page.stopScreencast") { - unackedFrames.delete(sessionId); + if (command.method === "Page.screencastFrameAck" && child === undefined) { + // A restart keeps the slot too, or restarting would let frames past an unacknowledged one. + if (unackedFrames.get(command.sessionId!) === command.params?.["sessionId"]) { + unackedFrames.delete(command.sessionId!); } + return Promise.resolve({}); } // The shared debugger keeps Runtime enabled after an earlier connection, and a repeated // enable reports no existing contexts; Playwright waits for them, so reset once per connection. diff --git a/apps/desktop/src/preview/DesktopBrowserHost.test.ts b/apps/desktop/src/preview/DesktopBrowserHost.test.ts index d6f25d66aa810..cf6c7736f81d3 100644 --- a/apps/desktop/src/preview/DesktopBrowserHost.test.ts +++ b/apps/desktop/src/preview/DesktopBrowserHost.test.ts @@ -1157,6 +1157,13 @@ describe("DesktopBrowserHost", () => { debuggee.refuse(6); debuggee.release(); expect([yield* Queue.take(replies), yield* Queue.take(replies)].toSorted()).toEqual([5, 6]); + yield* raw({ + id: 7, + method: "Page.screencastFrameAck", + params: { sessionId: 1 }, + sessionId: "t3-preview-page-1", + }); + expect(yield* Queue.take(replies)).toBe(7); yield* settle; frames.length = 0; debuggee.emit("Page.screencastFrame", { data: "frame", sessionId: 2 }); From e74b8e12406bc80f97f285aacbaf53eacdeba9d4 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Sat, 14 Nov 2026 13:33:03 +0000 Subject: [PATCH 6/8] fix(desktop): send a burst's last screencast frame on acknowledgement Frames that arrived while a session's frame was unacknowledged were dropped, and Chromium sends none until the next repaint, so a viewer could stay on a mid-transition frame. Keep the newest skipped frame per session and send it when the matching acknowledgement arrives. Co-Authored-By: Claude Opus 5.5 --- apps/desktop/src/preview/CdpRelay.test.ts | 37 +++++++++++---------- apps/desktop/src/preview/CdpRelay.ts | 39 ++++++++++++++++------- 2 files changed, 48 insertions(+), 28 deletions(-) diff --git a/apps/desktop/src/preview/CdpRelay.test.ts b/apps/desktop/src/preview/CdpRelay.test.ts index f55562a2ad9a8..bf6ff13b49d02 100644 --- a/apps/desktop/src/preview/CdpRelay.test.ts +++ b/apps/desktop/src/preview/CdpRelay.test.ts @@ -182,24 +182,31 @@ describe("CDP relay extra sessions", () => { }), ); const delivered = () => { - const sessions = written.map((m) => m["sessionId"]); + const frames = written + .filter((m) => m["method"]) + .map((m) => [m["sessionId"], (m["params"] as { sessionId?: number }).sessionId]); written.length = 0; - return sessions; + return frames; }; frame(1); - expect(delivered()).toEqual(["t3-preview-page", extra]); - // A session that has not acknowledged its frame gets no more, however many arrive. + expect(delivered()).toEqual([ + ["t3-preview-page", 1], + [extra, 1], + ]); + // A session that has not acknowledged its frame gets only the newest one, on its ack. for (let i = 2; i < 100; i++) frame(i); expect(delivered()).toEqual([]); ack(extra, 1); await settle(); - delivered(); - frame(2); - expect(delivered()).toEqual([extra]); - ack(extra, 1); + expect(delivered()).toEqual([[extra, 99]]); + ack(extra, 99); await settle(); - delivered(); - frame(3); + expect(delivered()).toEqual([]); + frame(100); + expect(delivered()).toEqual([[extra, 100]]); + ack(extra, 99); + await settle(); + frame(101); expect(delivered()).toEqual([]); expect(send.mock.calls.some(([method]) => method === "Page.screencastFrameAck")).toBe(false); // A restart keeps the unacknowledged frame's slot until its own acknowledgement. @@ -207,16 +214,14 @@ describe("CDP relay extra sessions", () => { relay.receive(JSON.stringify({ id: 8, method: "Page.startScreencast", sessionId: extra })); await settle(); delivered(); - frame(4); + frame(102); expect(delivered()).toEqual([]); - ack(extra, 2); + ack(extra, 100); await settle(); - delivered(); - frame(5); - expect(delivered()).toEqual([extra]); + expect(delivered()).toEqual([[extra, 102]]); // Other events still reach a session while its frame is unacknowledged. relay.event("Page.loadEventFired", { timestamp: 1 }, undefined); - expect(delivered()).toEqual(["t3-preview-page", extra]); + expect(delivered().map(([session]) => session)).toEqual(["t3-preview-page", extra]); }); }); diff --git a/apps/desktop/src/preview/CdpRelay.ts b/apps/desktop/src/preview/CdpRelay.ts index 268ab59624543..273ad1a7c7bcf 100644 --- a/apps/desktop/src/preview/CdpRelay.ts +++ b/apps/desktop/src/preview/CdpRelay.ts @@ -105,6 +105,12 @@ export function createCdpRelayConnection( const sessions = new Map([[PAGE_SESSION_ID, "page"]]); // The manager acks native frames at once, so each page session waits on its own ack instead. const unackedFrames = new Map(); + // The newest frame skipped meanwhile, sent on the ack; Chromium sends none until the next repaint. + const skippedFrames = new Map>(); + const sendFrame = (sessionId: string, params: Record) => { + unackedFrames.set(sessionId, params["sessionId"] as number); + send({ method: "Page.screencastFrame", params, sessionId }); + }; let sessionSequence = 0; const newSession = (kind: "browser" | "page") => { const sessionId = `t3-preview-${kind}-${++sessionSequence}`; @@ -173,6 +179,7 @@ export function createCdpRelayConnection( if (typeof sessionId === "string" && sessionId !== PAGE_SESSION_ID) { sessions.delete(sessionId); unackedFrames.delete(sessionId); + skippedFrames.delete(sessionId); target.sessionDetached?.(sessionId); } return {}; @@ -207,8 +214,16 @@ export function createCdpRelayConnection( if (command.method === "Emulation.setFocusEmulationEnabled") return Promise.resolve({}); if (command.method === "Page.screencastFrameAck" && child === undefined) { // A restart keeps the slot too, or restarting would let frames past an unacknowledged one. - if (unackedFrames.get(command.sessionId!) === command.params?.["sessionId"]) { - unackedFrames.delete(command.sessionId!); + const sessionId = command.sessionId!; + if (unackedFrames.get(sessionId) === command.params?.["sessionId"]) { + unackedFrames.delete(sessionId); + const skipped = skippedFrames.get(sessionId); + skippedFrames.delete(sessionId); + if ( + skipped && + target.receivesScreencast(sessionId === PAGE_SESSION_ID ? undefined : sessionId) + ) + sendFrame(sessionId, skipped); } return Promise.resolve({}); } @@ -272,19 +287,19 @@ export function createCdpRelayConnection( send({ method, params, sessionId }); return; } - const frameId = - method === "Page.screencastFrame" - ? (params as { sessionId?: unknown } | null)?.sessionId - : undefined; - for (const [id, kind] of sessions) { - if (kind !== "page") continue; - if (method === "Page.screencastFrame") { - if (typeof frameId !== "number" || unackedFrames.has(id)) continue; + if (method === "Page.screencastFrame") { + const frame = params as Record | null; + if (typeof frame?.["sessionId"] !== "number") return; + for (const [id, kind] of sessions) { + if (kind !== "page") continue; if (!target.receivesScreencast(id === PAGE_SESSION_ID ? undefined : id)) continue; - unackedFrames.set(id, frameId); + if (unackedFrames.has(id)) skippedFrames.set(id, frame); + else sendFrame(id, frame); } - send({ method, params, sessionId: id }); + return; } + for (const [id, kind] of sessions) + if (kind === "page") send({ method, params, sessionId: id }); }, }; } From de2746c3aac632f20a5c9f056147e1159f81df5d Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Sat, 14 Nov 2026 14:17:17 +0000 Subject: [PATCH 7/8] fix(desktop): pace native screencast frames to relay consumers The preview manager acknowledged every native screencast frame on arrival, so Chromium kept pushing full-size frames into the main process at full rate while recording, even though the relay delivered only one per consumer acknowledgement. The relay now acknowledges a native frame once it delivers it, and leaves a frame every session is busy for unacknowledged until a consumer is ready, so Chromium produces at the consumer's pace. Co-Authored-By: Claude Opus 5.5 --- apps/desktop/src/preview/CdpRelay.test.ts | 11 ++++- apps/desktop/src/preview/CdpRelay.ts | 45 ++++++++++++++----- .../desktop/src/preview/DesktopBrowserHost.ts | 4 ++ apps/desktop/src/preview/Manager.ts | 21 ++------- 4 files changed, 51 insertions(+), 30 deletions(-) diff --git a/apps/desktop/src/preview/CdpRelay.test.ts b/apps/desktop/src/preview/CdpRelay.test.ts index bf6ff13b49d02..b52c5056691e9 100644 --- a/apps/desktop/src/preview/CdpRelay.test.ts +++ b/apps/desktop/src/preview/CdpRelay.test.ts @@ -9,6 +9,7 @@ import { const makeTarget = ( send: CdpRelayTarget["send"], setDownloadDirectory: CdpRelayTarget["setDownloadDirectory"] = () => {}, + ackScreencastFrame: CdpRelayTarget["ackScreencastFrame"] = () => {}, ): CdpRelayTarget => ({ send, targetId: async () => "GUEST-TARGET", @@ -17,6 +18,7 @@ const makeTarget = ( userAgent: () => "Electron", setDownloadDirectory, receivesScreencast: () => true, + ackScreencastFrame, }); const makeRelay = (target: CdpRelayTarget, write: (raw: string) => void) => @@ -140,7 +142,11 @@ describe("CDP relay extra sessions", () => { it("gives a second page session the page's commands and events", async () => { const send = vi.fn(async () => ({})); const written: Array> = []; - const relay = makeRelay(makeTarget(send), (raw) => written.push(JSON.parse(raw))); + const nativeAcks: Array = []; + const relay = makeRelay( + makeTarget(send, undefined, (frameId) => nativeAcks.push(frameId)), + (raw) => written.push(JSON.parse(raw)), + ); relay.receive(JSON.stringify({ id: 1, method: "Target.setAutoAttach", params: {} })); relay.receive(JSON.stringify({ id: 2, method: "Target.attachToBrowserTarget" })); await settle(); @@ -196,9 +202,12 @@ describe("CDP relay extra sessions", () => { // A session that has not acknowledged its frame gets only the newest one, on its ack. for (let i = 2; i < 100; i++) frame(i); expect(delivered()).toEqual([]); + // Chromium waits on the newest frame while every session is busy; it is acknowledged once delivered. + expect(nativeAcks.at(-1)).toBe(98); ack(extra, 1); await settle(); expect(delivered()).toEqual([[extra, 99]]); + expect(nativeAcks.at(-1)).toBe(99); ack(extra, 99); await settle(); expect(delivered()).toEqual([]); diff --git a/apps/desktop/src/preview/CdpRelay.ts b/apps/desktop/src/preview/CdpRelay.ts index 273ad1a7c7bcf..563b66b2b3f0b 100644 --- a/apps/desktop/src/preview/CdpRelay.ts +++ b/apps/desktop/src/preview/CdpRelay.ts @@ -11,6 +11,8 @@ export interface CdpRelayTarget { readonly sessionDetached?: (owner: string) => void; /** Whether a page session, by owner as `send` names it, gets the page's screencast frames. */ readonly receivesScreencast: (owner: string | undefined) => boolean; + /** Acknowledges a native screencast frame, letting Chromium send the next one. */ + readonly ackScreencastFrame: (frameId: number) => void; /** The page's own CDP target id; Playwright keys the main frame by it. */ readonly targetId: () => Promise; readonly url: () => string; @@ -103,10 +105,17 @@ export function createCdpRelayConnection( let downloadEventsEnabled = false; /** Relay-created page sessions share one real debugger; child frame sessions retain their own ids. */ const sessions = new Map([[PAGE_SESSION_ID, "page"]]); - // The manager acks native frames at once, so each page session waits on its own ack instead. + // Each page session holds one unacknowledged frame and keeps the newest one skipped meanwhile. const unackedFrames = new Map(); - // The newest frame skipped meanwhile, sent on the ack; Chromium sends none until the next repaint. const skippedFrames = new Map>(); + // A frame every session is busy for stays unacknowledged natively, so Chromium waits for a consumer. + let heldNativeFrame: number | null = null; + const releaseNativeFrame = (frameId: unknown, delivered: boolean) => { + if (heldNativeFrame === null || heldNativeFrame !== frameId) return; + if (!delivered && [...skippedFrames.values()].some((f) => f["sessionId"] === frameId)) return; + heldNativeFrame = null; + target.ackScreencastFrame(frameId); + }; const sendFrame = (sessionId: string, params: Record) => { unackedFrames.set(sessionId, params["sessionId"] as number); send({ method: "Page.screencastFrame", params, sessionId }); @@ -179,7 +188,9 @@ export function createCdpRelayConnection( if (typeof sessionId === "string" && sessionId !== PAGE_SESSION_ID) { sessions.delete(sessionId); unackedFrames.delete(sessionId); + const skipped = skippedFrames.get(sessionId); skippedFrames.delete(sessionId); + releaseNativeFrame(skipped?.["sessionId"], false); target.sessionDetached?.(sessionId); } return {}; @@ -219,11 +230,11 @@ export function createCdpRelayConnection( unackedFrames.delete(sessionId); const skipped = skippedFrames.get(sessionId); skippedFrames.delete(sessionId); - if ( - skipped && - target.receivesScreencast(sessionId === PAGE_SESSION_ID ? undefined : sessionId) - ) - sendFrame(sessionId, skipped); + const delivered = + skipped !== undefined && + target.receivesScreencast(sessionId === PAGE_SESSION_ID ? undefined : sessionId); + if (delivered) sendFrame(sessionId, skipped); + releaseNativeFrame(skipped?.["sessionId"], delivered); } return Promise.resolve({}); } @@ -288,14 +299,26 @@ export function createCdpRelayConnection( return; } if (method === "Page.screencastFrame") { - const frame = params as Record | null; - if (typeof frame?.["sessionId"] !== "number") return; + const frame = (params ?? {}) as Record; + const frameId = frame["sessionId"]; + if (typeof frameId !== "number") return; + if (heldNativeFrame !== null) target.ackScreencastFrame(heldNativeFrame); + heldNativeFrame = null; + let held = false; + let delivered = false; for (const [id, kind] of sessions) { if (kind !== "page") continue; if (!target.receivesScreencast(id === PAGE_SESSION_ID ? undefined : id)) continue; - if (unackedFrames.has(id)) skippedFrames.set(id, frame); - else sendFrame(id, frame); + if (unackedFrames.has(id)) { + skippedFrames.set(id, frame); + held = true; + } else { + sendFrame(id, frame); + delivered = true; + } } + if (held && !delivered) heldNativeFrame = frameId; + else target.ackScreencastFrame(frameId); return; } for (const [id, kind] of sessions) diff --git a/apps/desktop/src/preview/DesktopBrowserHost.ts b/apps/desktop/src/preview/DesktopBrowserHost.ts index 296be2fa6709f..e0326b7b76ec1 100644 --- a/apps/desktop/src/preview/DesktopBrowserHost.ts +++ b/apps/desktop/src/preview/DesktopBrowserHost.ts @@ -421,6 +421,10 @@ export const make = Effect.gen(function* () { dropSessionHolds(tab, owner); }, receivesScreencast: (owner) => tab.screencasts.has(owner), + ackScreencastFrame: (frameId) => + void debuggee + .sendCommand("Page.screencastFrameAck", { sessionId: frameId }) + .catch(() => {}), targetId: () => debuggee .sendCommand("Target.getTargetInfo") diff --git a/apps/desktop/src/preview/Manager.ts b/apps/desktop/src/preview/Manager.ts index 2c199e358fa5b..b7835def18b7e 100644 --- a/apps/desktop/src/preview/Manager.ts +++ b/apps/desktop/src/preview/Manager.ts @@ -1108,28 +1108,13 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function ), scope, ); - const handleDebuggerMessage = Effect.fnUntraced(function* ( - method: string, - params: Record, - ) { - if (method === "Page.screencastFrame") { - const sessionId = params["sessionId"]; - if (typeof sessionId === "number") { - yield* attemptPromise( - { - operation: "ackScreencastFrame", - webContentsId: wc.id, - }, - () => wcDebugger.sendCommand("Page.screencastFrameAck", { sessionId }), - ).pipe(Effect.ignore); - } - } + const handleDebuggerMessage = Effect.fnUntraced(function* (method: string) { if (method === "Runtime.consoleAPICalled" || method === "Runtime.exceptionThrown") { yield* Queue.offer(consoleReleases, undefined); } }); - const onMessage: BrowserControlSession["onMessage"] = (_event, method, params) => { - runFork(handleDebuggerMessage(method, params)); + const onMessage: BrowserControlSession["onMessage"] = (_event, method) => { + runFork(handleDebuggerMessage(method)); }; const control: BrowserControlSession = { webContentsId: wc.id, From 4abf7407ac6596100859c9ecc78a6f1683348a45 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Sat, 14 Nov 2026 14:33:09 +0000 Subject: [PATCH 8/8] fix(desktop): pace Chromium's screencast to the consumers' acknowledgements Chromium gives every frame of a screencast run the same id and keeps sending while fewer than its in-flight limit are unacknowledged. Acking a held frame whenever a newer one arrived kept it under that limit, so it never paused. Leave frames every session is busy for unacknowledged and release one per consumer acknowledgement, or all of them once no session holds one. Co-Authored-By: Claude Opus 5.5 --- apps/desktop/src/preview/CdpRelay.test.ts | 8 ++++-- apps/desktop/src/preview/CdpRelay.ts | 34 ++++++++++++----------- 2 files changed, 23 insertions(+), 19 deletions(-) diff --git a/apps/desktop/src/preview/CdpRelay.test.ts b/apps/desktop/src/preview/CdpRelay.test.ts index b52c5056691e9..564b4dc57d6f4 100644 --- a/apps/desktop/src/preview/CdpRelay.test.ts +++ b/apps/desktop/src/preview/CdpRelay.test.ts @@ -202,15 +202,17 @@ describe("CDP relay extra sessions", () => { // A session that has not acknowledged its frame gets only the newest one, on its ack. for (let i = 2; i < 100; i++) frame(i); expect(delivered()).toEqual([]); - // Chromium waits on the newest frame while every session is busy; it is acknowledged once delivered. - expect(nativeAcks.at(-1)).toBe(98); + // Frames every session is busy for stay unacknowledged, so Chromium pauses at its in-flight limit. + expect(nativeAcks).toEqual([1]); ack(extra, 1); await settle(); expect(delivered()).toEqual([[extra, 99]]); - expect(nativeAcks.at(-1)).toBe(99); + // Each consumer ack lets Chromium send one more frame. + expect(nativeAcks).toEqual([1, 2]); ack(extra, 99); await settle(); expect(delivered()).toEqual([]); + expect(nativeAcks).toEqual([1, 2, 3]); frame(100); expect(delivered()).toEqual([[extra, 100]]); ack(extra, 99); diff --git a/apps/desktop/src/preview/CdpRelay.ts b/apps/desktop/src/preview/CdpRelay.ts index 563b66b2b3f0b..e9f3cf509b446 100644 --- a/apps/desktop/src/preview/CdpRelay.ts +++ b/apps/desktop/src/preview/CdpRelay.ts @@ -108,13 +108,12 @@ export function createCdpRelayConnection( // Each page session holds one unacknowledged frame and keeps the newest one skipped meanwhile. const unackedFrames = new Map(); const skippedFrames = new Map>(); - // A frame every session is busy for stays unacknowledged natively, so Chromium waits for a consumer. - let heldNativeFrame: number | null = null; - const releaseNativeFrame = (frameId: unknown, delivered: boolean) => { - if (heldNativeFrame === null || heldNativeFrame !== frameId) return; - if (!delivered && [...skippedFrames.values()].some((f) => f["sessionId"] === frameId)) return; - heldNativeFrame = null; - target.ackScreencastFrame(frameId); + // Chromium keeps a few frames in flight and reuses one frame id per run, so pacing it means + // leaving frames every session is busy for unacknowledged and releasing one per consumer ack. + const heldNativeFrames: Array = []; + const releaseHeldFramesWithoutHolders = () => { + if (skippedFrames.size === 0) + for (const frameId of heldNativeFrames.splice(0)) target.ackScreencastFrame(frameId); }; const sendFrame = (sessionId: string, params: Record) => { unackedFrames.set(sessionId, params["sessionId"] as number); @@ -188,9 +187,8 @@ export function createCdpRelayConnection( if (typeof sessionId === "string" && sessionId !== PAGE_SESSION_ID) { sessions.delete(sessionId); unackedFrames.delete(sessionId); - const skipped = skippedFrames.get(sessionId); skippedFrames.delete(sessionId); - releaseNativeFrame(skipped?.["sessionId"], false); + releaseHeldFramesWithoutHolders(); target.sessionDetached?.(sessionId); } return {}; @@ -230,14 +228,20 @@ export function createCdpRelayConnection( unackedFrames.delete(sessionId); const skipped = skippedFrames.get(sessionId); skippedFrames.delete(sessionId); - const delivered = + if ( skipped !== undefined && - target.receivesScreencast(sessionId === PAGE_SESSION_ID ? undefined : sessionId); - if (delivered) sendFrame(sessionId, skipped); - releaseNativeFrame(skipped?.["sessionId"], delivered); + target.receivesScreencast(sessionId === PAGE_SESSION_ID ? undefined : sessionId) + ) + sendFrame(sessionId, skipped); + const released = heldNativeFrames.shift(); + if (released !== undefined) target.ackScreencastFrame(released); } return Promise.resolve({}); } + if (command.method === "Page.stopScreencast" && child === undefined) { + skippedFrames.delete(command.sessionId!); + releaseHeldFramesWithoutHolders(); + } // The shared debugger keeps Runtime enabled after an earlier connection, and a repeated // enable reports no existing contexts; Playwright waits for them, so reset once per connection. if (command.method === "Runtime.enable" && child === undefined) { @@ -302,8 +306,6 @@ export function createCdpRelayConnection( const frame = (params ?? {}) as Record; const frameId = frame["sessionId"]; if (typeof frameId !== "number") return; - if (heldNativeFrame !== null) target.ackScreencastFrame(heldNativeFrame); - heldNativeFrame = null; let held = false; let delivered = false; for (const [id, kind] of sessions) { @@ -317,7 +319,7 @@ export function createCdpRelayConnection( delivered = true; } } - if (held && !delivered) heldNativeFrame = frameId; + if (held && !delivered) heldNativeFrames.push(frameId); else target.ackScreencastFrame(frameId); return; }