diff --git a/apps/desktop/src/preview/DesktopBrowserHost.test.ts b/apps/desktop/src/preview/DesktopBrowserHost.test.ts index 08fefd8d733f5..35ff239b48797 100644 --- a/apps/desktop/src/preview/DesktopBrowserHost.test.ts +++ b/apps/desktop/src/preview/DesktopBrowserHost.test.ts @@ -493,20 +493,21 @@ describe("DesktopBrowserHost", () => { params: { sessionId: "t3-preview-page-1" }, }); expect(yield* Queue.take(replies)).toBe(3); - expect(painting).toEqual([true, false]); + expect(painting).toEqual([true]); // The agent's screenshot waits its turn; painting lets the abandoned one finish first. yield* command(host, 4, "Page.captureScreenshot"); yield* settle; - expect(painting).toEqual([true, false, true]); + expect(painting).toEqual([true]); debuggee.release(); expect(yield* Queue.take(replies)).toBe(2); yield* settle; expect(debuggee.sent).toEqual([undefined, 4]); + expect(painting).toEqual([true]); debuggee.release(); expect(yield* Queue.take(replies)).toBe(4); yield* settle; - expect(painting).toEqual([true, false, true, false]); + expect(painting).toEqual([true, false]); }).pipe(Effect.scoped), ); @@ -531,14 +532,19 @@ describe("DesktopBrowserHost", () => { expect(yield* Queue.take(replies)).toBe(2); yield* raw({ id: 3, method: "Page.captureScreenshot", sessionId: "t3-preview-page-1" }); yield* settle; - expect(painting).toEqual([true, false, true]); + expect(painting).toEqual([true]); yield* raw({ id: 4, method: "Target.detachFromTarget", params: { sessionId: "t3-preview-page-1" }, }); expect(yield* Queue.take(replies)).toBe(4); - expect(painting).toEqual([true, false, true, false]); + + // The agent's answers late; the viewer's is dropped unsent, so nothing is left to paint. + debuggee.release(); + yield* settle; + expect(debuggee.sent).toEqual([1]); + expect(painting).toEqual([true, false]); yield* TestClock.adjust("1 minute"); expect(debuggee.focusEmulation).toEqual([true, false]); }).pipe(Effect.scoped), @@ -568,7 +574,7 @@ describe("DesktopBrowserHost", () => { }).pipe(Effect.scoped), ); - it.effect("stops painting for a screenshot still queued when the agent's turn ends", () => + it.effect("holds no focus for a screenshot still queued when the agent's turn ends", () => Effect.gen(function* () { const host = yield* DesktopBrowserHost.make; const debuggee = makeDebuggee(); @@ -583,15 +589,18 @@ describe("DesktopBrowserHost", () => { yield* settle; yield* turn(false); yield* settle; - expect(painting).toEqual([true, false]); + expect(painting).toEqual([true]); - // The first answers late; the second still goes in turn, but holds nothing. + // The first answers late; the second still goes in turn, painting but holding no focus. debuggee.releaseFirst(); yield* settle; expect(debuggee.sent).toEqual([1, 2]); - expect(painting).toEqual([true, false]); yield* TestClock.adjust("1 minute"); expect(debuggee.focusEmulation).toEqual([true, false]); + expect(painting).toEqual([true]); + debuggee.release(); + yield* settle; + expect(painting).toEqual([true, false]); }).pipe(Effect.scoped), ); @@ -838,7 +847,7 @@ describe("DesktopBrowserHost", () => { }).pipe(Effect.scoped), ); - it.effect("stops painting for a screenshot Chromium never answers", () => + it.effect("stops painting for a screenshot Chromium never answers once its document goes", () => Effect.gen(function* () { const host = yield* DesktopBrowserHost.make; const debuggee = makeDebuggee(); @@ -847,32 +856,34 @@ describe("DesktopBrowserHost", () => { const turn = (active: boolean) => host.handleCommandLine(encodeJson({ type: "turn", threadId: key.threadId, active })); - // The server gave up on it, and the agent's turn ends. + // The server gave up on it, and the agent's turn ends; Chromium still holds the capture. yield* turn(true); yield* command(host, 1, "Page.captureScreenshot"); yield* settle; yield* turn(false); yield* settle; - expect(painting).toEqual([true, false]); + expect(painting).toEqual([true]); // The page loads a new document, which also frees the screenshot turn. debuggee.navigate(); + expect(painting).toEqual([true, false]); yield* command(host, 2, "Page.captureScreenshot"); yield* settle; debuggee.navigate(); yield* settle; expect(painting).toEqual([true, false, true, false]); - // The server releases the relay. + // The server releases the relay; Chromium still holds the capture. yield* command(host, 3, "Page.captureScreenshot"); yield* settle; yield* host.handleCommandLine(encodeJson({ type: "release", ...key })); yield* settle; - expect(painting).toEqual([true, false, true, false, true, false]); + expect(painting).toEqual([true, false, true, false, true]); // The guest detaches; a replacement starts with no screenshot pending. const replacement = makeDebuggee(); host.attach(key, replacement.tab); + expect(painting).toEqual([true, false, true, false, true, false]); yield* command(host, 4, "Page.captureScreenshot"); yield* settle; host.detach(key); @@ -881,6 +892,50 @@ describe("DesktopBrowserHost", () => { }).pipe(Effect.scoped), ); + // A capture Chromium was sent holds its wake lock until a painted frame answers it. + it.effect("paints for a screenshot already sent until Chromium answers it, past the turn", () => + Effect.gen(function* () { + const host = yield* DesktopBrowserHost.make; + const debuggee = makeDebuggee(); + host.attach(key, debuggee.tab); + const painting = yield* watchPainting(host); + const turn = (active: boolean) => + host.handleCommandLine(encodeJson({ type: "turn", threadId: key.threadId, active })); + + yield* turn(true); + yield* command(host, 1, "Page.captureScreenshot"); + yield* settle; + expect(debuggee.sent).toEqual([1]); + yield* turn(false); + yield* settle; + expect(painting).toEqual([true]); + + debuggee.release(); + yield* settle; + expect(painting).toEqual([true, false]); + }).pipe(Effect.scoped), + ); + + it.effect("paints for a screenshot already sent until Chromium answers it, past a release", () => + Effect.gen(function* () { + const host = yield* DesktopBrowserHost.make; + const debuggee = makeDebuggee(); + host.attach(key, debuggee.tab); + const painting = yield* watchPainting(host); + + yield* command(host, 1, "Page.captureScreenshot"); + yield* settle; + expect(debuggee.sent).toEqual([1]); + yield* host.handleCommandLine(encodeJson({ type: "release", ...key })); + yield* settle; + expect(painting).toEqual([true]); + + debuggee.release(); + yield* settle; + expect(painting).toEqual([true, false]); + }).pipe(Effect.scoped), + ); + it.effect("paints a streamed guest until its last screencast ends", () => 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 30f252e3a677e..65b20d4496f57 100644 --- a/apps/desktop/src/preview/DesktopBrowserHost.ts +++ b/apps/desktop/src/preview/DesktopBrowserHost.ts @@ -44,7 +44,7 @@ interface AttachedTab { readonly debuggee: DesktopBrowserTabDebugger; relay: CdpRelayConnection | null; screenshotTurn: object | null; - /** Each holds focus and painting while `held` and whoever queued it still waits on it. */ + /** Each keeps the guest painting while whoever queued it still waits, and focused while `held`. */ readonly waitingScreenshots: Array<{ readonly start: () => void; readonly live: () => boolean; @@ -57,10 +57,9 @@ interface AttachedTab { focusChange: Promise | null; focusUses: number; /** Each pending command, by the extra session waiting on it; the main connection's have none. */ - readonly focusPending: Map< - object, - { readonly owner: string | undefined; readonly paints: boolean } - >; + readonly focusPending: Map; + /** Screenshots Chromium was sent and has not answered; each keeps its wake lock until then. */ + readonly capturing: Set; /** Whether the client keeps the guest painting, as published on `painting`. */ painting: boolean; /** @@ -138,7 +137,7 @@ export const make = Effect.gen(function* () { tab: AttachedTab, relay: CdpRelayConnection, ownerLive: () => boolean, - /** Sends only while `current` holds; holds focus and painting only while `held`. */ + /** Sends only while `current` holds; holds focus only while `held`. */ capture: (current: () => boolean, held: boolean) => Promise, ) => new Promise((resolve, reject) => { @@ -226,38 +225,42 @@ export const make = Effect.gen(function* () { const active = tabs.get(keyOf(tab.key)) === tab && (tab.screencasts.size > 0 || - waitingHolds(tab) || - [...tab.focusPending.values()].some((hold) => hold.paints)); + tab.capturing.size > 0 || + tab.waitingScreenshots.some((waiting) => waiting.live())); if (tab.painting === active) return; tab.painting = active; runFork(PubSub.publish(painting, { key: tab.key, active })); }; /** A command may wait on the page's next frame, so the guest stays focused until it settles. */ - const holdFocus = ( - tab: AttachedTab, - method: string, - owner: string | undefined, - work: () => Promise, - ) => { + const holdFocus = (tab: AttachedTab, owner: string | undefined, work: () => Promise) => { const pending = {}; - tab.focusPending.set(pending, { owner, paints: method === "Page.captureScreenshot" }); + tab.focusPending.set(pending, owner); tab.focusUses += 1; setFocusEmulation(tab, true); releaseFocusWhenIdle(tab); - syncPainting(tab); return work().finally(() => { if (!tab.focusPending.delete(pending)) return; tab.focusUses += 1; releaseFocusWhenIdle(tab); + }); + }; + + /** Chromium answers a capture, and frees its wake lock, only once the guest paints a frame. */ + const paintUntilAnswered = (tab: AttachedTab, send: () => Promise) => { + const sent = {}; + tab.capturing.add(sent); + syncPainting(tab); + return send().finally(() => { + tab.capturing.delete(sent); syncPainting(tab); }); }; /** * Chromium can leave a command unanswered, so the screenshot guard's release points - * also end every hold: a released relay has no one waiting, and a new document answers - * nothing pending from the old one. A released relay's screencast has no viewer to stop it. + * also end every focus hold: a released relay has no one waiting, and a new document + * answers nothing pending from the old one. A released relay's screencast has no viewer. */ const dropFocusHolds = (tab: AttachedTab) => { tab.focusPending.clear(); @@ -275,8 +278,8 @@ export const make = Effect.gen(function* () { /** A detached session, such as a closed viewer, waits on nothing it sent. */ const dropSessionHolds = (tab: AttachedTab, owner: string) => { tab.screencasts.delete(owner); - for (const [pending, hold] of tab.focusPending) { - if (hold.owner === owner) tab.focusPending.delete(pending); + for (const [pending, holder] of tab.focusPending) { + if (holder === owner) tab.focusPending.delete(pending); } releaseFocusWhenIdle(tab); syncPainting(tab); @@ -284,7 +287,7 @@ export const make = Effect.gen(function* () { /** * A command the server stopped waiting on, such as a timed-out screenshot or evaluation, - * may never settle; the turn's end ends every hold. The agent also sends on its own extra + * may never settle; the turn's end ends every focus hold. The agent also sends on its own extra * session, so this cannot spare other sessions; a viewer's screencast still holds. */ const endAgentTurn = (threadId: string) => { @@ -292,7 +295,7 @@ export const make = Effect.gen(function* () { for (const tab of tabs.values()) { if (tab.key.threadId !== threadId) continue; tab.agentUsed = false; - // Still sent in turn, so the queue keeps moving, but no longer kept painting. + // Still sent in turn, so the queue keeps moving, but no longer kept focused. for (const waiting of tab.waitingScreenshots) waiting.held = false; dropFocusHolds(tab); } @@ -315,8 +318,10 @@ export const make = Effect.gen(function* () { if (sessionId === undefined && method === "Page.startScreencast") { tab.screencasts.add(owner); if (owner !== undefined) streamingOwners.add(owner); + syncPainting(tab); } else if (sessionId === undefined && method === "Page.stopScreencast") { tab.screencasts.delete(owner); + syncPainting(tab); } // A person's input through a viewer, or Playwright's own housekeeping, is not the agent's. if ( @@ -329,9 +334,11 @@ export const make = Effect.gen(function* () { const forward = (current?: () => boolean) => { const change = tab.focusChange; const run = () => - current === undefined || current() + current === undefined ? send(method, params, sessionId) - : Promise.reject(new Error("The page this screenshot was for has moved on.")); + : current() + ? paintUntilAnswered(tab, () => send(method, params, sessionId)) + : Promise.reject(new Error("The page this screenshot was for has moved on.")); return change === null ? run() : change.then(run); }; return method === "Page.captureScreenshot" @@ -340,17 +347,17 @@ export const make = Effect.gen(function* () { relay, () => owner === undefined || !detachedOwners.has(owner), (current, held) => - held ? holdFocus(tab, method, owner, () => forward(current)) : forward(current), + held ? holdFocus(tab, owner, () => forward(current)) : forward(current), ) : method === "Page.startScreencast" - ? holdFocus(tab, method, owner, forward).catch((cause: unknown) => { + ? holdFocus(tab, 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); + : holdFocus(tab, owner, forward); }, sessionDetached: (owner) => { detachedOwners.add(owner); @@ -417,6 +424,7 @@ export const make = Effect.gen(function* () { focusChange: null, focusUses: 0, focusPending: new Map(), + capturing: new Set(), painting: false, agentUsed: false, focusReleasing: false, @@ -431,7 +439,8 @@ export const make = Effect.gen(function* () { // A screenshot pending when the page loads a new document, a reload included, can // stay unanswered. A crashed page lands here once it reloads, well after its renderer died. onNewDocument: () => { - // Before the next screenshot starts, so its own hold survives. + // Before the next screenshot starts, so its own holds survive. + tab.capturing.clear(); dropFocusHolds(tab); if (tab.screenshotTurn !== null) nextScreenshot(tab); },