fix(desktop): one preview viewer or recording stopping no longer freezes the others

apps/desktop/src/preview/CdpRelay.test.ts

@@ -16,6 +16,7 @@ const makeTarget = (
   title: () => "Fixture",
   userAgent: () => "Electron",
   setDownloadDirectory,
+  receivesScreencast: () => true,
 });
 
 const makeRelay = (target: CdpRelayTarget, write: (raw: string) => void) =>

apps/desktop/src/preview/CdpRelay.ts

@@ -9,6 +9,8 @@ export interface CdpRelayTarget {
     owner?: string,
   ) => Promise<unknown>;
   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;
   /** The page's own CDP target id; Playwright keys the main frame by it. */
   readonly targetId: () => Promise<string>;
   readonly url: () => string;
@@ -260,8 +262,15 @@ export function createCdpRelayConnection(
         send({ method, params, sessionId });
         return;
       }
-      for (const [id, kind] of sessions)
-        if (kind === "page") send({ method, params, sessionId: id });
+      for (const [id, kind] of sessions) {
+        if (kind !== "page") continue;
+        if (
+          method === "Page.screencastFrame" &&
+          !target.receivesScreencast(id === PAGE_SESSION_ID ? undefined : id)
+        )
+          continue;
+        send({ method, params, sessionId: id });
+      }
     },
   };
 }

apps/desktop/src/preview/DesktopBrowserHost.test.ts

@@ -16,6 +16,15 @@ const encodeJson = Schema.encodeSync(Schema.fromJsonString(Schema.Unknown));
 const decodeCdpReply = Schema.decodeUnknownSync(
   Schema.fromJsonString(Schema.Struct({ id: Schema.Number })),
 );
+const decodeCdpMessage = Schema.decodeUnknownSync(
+  Schema.fromJsonString(
+    Schema.Struct({
+      id: Schema.optional(Schema.Number),
+      method: Schema.optional(Schema.String),
+      sessionId: Schema.optional(Schema.String),
+    }),
+  ),
+);
 const key = { threadId: "thread-1", tabId: "tab-1" };
 /** Sends a page command whose id the fake debugger records as it receives it. */
 const command = (
@@ -43,6 +52,8 @@ const makeDebuggee = (
   const pending: Array<() => void> = [];
   const focusChanges: Array<() => void> = [];
   const sent: Array<unknown> = [];
+  const methods: Array<string> = [];
+  const rejects: Array<() => void> = [];
   const sendWaiters: Array<() => void> = [];
   const focusEmulation: Array<boolean> = [];
   const debuggee = Object.assign(emitter, {
@@ -67,6 +78,8 @@ const makeDebuggee = (
           return;
         }
         sent.push(params?.id);
+        methods.push(method);
+        rejects.push(() => reject(new Error("Refused")));
         pending.push(() => resolve({ method }));
         sendWaiters.shift()?.();
       }),
@@ -94,6 +107,10 @@ const makeDebuggee = (
       webContents.emit("did-fail-load", {}, code, "Failed", "http://localhost/", true),
     /** Ids of the commands the debugger received, in order. */
     sent,
+    /** Methods of the same commands, in order. */
+    methods,
+    /** Refuses the pending command the debugger received with this id. */
+    refuse: (id: number) => rejects[sent.indexOf(id)]?.(),
     focusEmulation,
     nextSend: () => new Promise<void>((resolve) => sendWaiters.push(resolve)),
     release: () => pending.splice(0).forEach((resolve) => resolve()),
@@ -116,16 +133,24 @@ const takeEvents = (host: DesktopBrowserHost.DesktopBrowserHost["Service"], coun
 /**
  * Ids of the replies the host sends, in order. A screenshot's reply leaves after its turn
  * has passed on, so by then the next waiting screenshot, if any, has been sent. Subscribe
- * before sending commands: a new subscriber releases the tab's relay.
+ * before sending commands: a new subscriber releases the tab's relay. Screencast frames
+ * go to `frames` by the session they were sent to.
  */
-const watchReplies = (host: DesktopBrowserHost.DesktopBrowserHost["Service"]) =>
+const watchReplies = (
+  host: DesktopBrowserHost.DesktopBrowserHost["Service"],
+  frames: Array<string | undefined> = [],
+) =>
   Effect.gen(function* () {
     const replies = yield* Queue.unbounded<number>();
     yield* host.events.pipe(
       Stream.map((line) => decodeEvent(new TextDecoder().decode(line))),
-      Stream.runForEach((event) =>
-        event.type === "cdp" ? Queue.offer(replies, decodeCdpReply(event.message).id) : Effect.void,
-      ),
+      Stream.runForEach((event) => {
+        if (event.type !== "cdp") return Effect.void;
+        const { id, method, sessionId } = decodeCdpMessage(event.message);
+        if (id !== undefined) return Queue.offer(replies, id);
+        if (method === "Page.screencastFrame") frames.push(sessionId);
+        return Effect.void;
+      }),
       Effect.forkScoped,
     );
     yield* Effect.yieldNow;
@@ -1040,6 +1065,175 @@ describe("DesktopBrowserHost", () => {
     }).pipe(Effect.scoped),
   );
 
+  it.effect("keeps a guest's one screencast running until its last streaming session stops", () =>
+    Effect.gen(function* () {
+      const host = yield* DesktopBrowserHost.make;
+      const debuggee = makeDebuggee();
+      host.attach(key, debuggee.tab);
+      const frames: Array<string | undefined> = [];
+      const replies = yield* watchReplies(host, frames);
+      const raw = (message: Record<string, unknown>) =>
+        host.handleCommandLine(encodeJson({ type: "cdp", ...key, message: encodeJson(message) }));
+      const nativeStops = () =>
+        debuggee.methods.filter((method) => method === "Page.stopScreencast").length;
+
+      // A viewer and a recording share Chromium's single screencast for the guest.
+      yield* raw({ id: 0, method: "Target.setAutoAttach", params: { autoAttach: true } });
+      expect(yield* Queue.take(replies)).toBe(0);
+      yield* raw({ id: 1, method: "Target.attachToTarget", params: { targetId: "GUEST" } });
+      yield* raw({ id: 2, method: "Target.attachToTarget", params: { targetId: "GUEST" } });
+      expect([yield* Queue.take(replies), yield* Queue.take(replies)]).toEqual([1, 2]);
+      yield* raw({ id: 3, method: "Page.startScreencast", sessionId: "t3-preview-page-1" });
+      yield* raw({ id: 4, method: "Page.startScreencast", sessionId: "t3-preview-page-2" });
+      yield* settle;
+      debuggee.release();
+      yield* raw({ id: 5, method: "Page.stopScreencast", sessionId: "t3-preview-page-1" });
+      yield* settle;
+      expect(nativeStops(), "stopped the screencast another session still streams").toBe(0);
+      debuggee.emit("Page.screencastFrame", { data: "frame", sessionId: 1 });
+      yield* settle;
+      expect(frames, "sent a stopped or idle session the screencast's frames").toEqual([
+        "t3-preview-page-2",
+      ]);
+
+      yield* raw({ id: 6, method: "Page.stopScreencast", sessionId: "t3-preview-page-2" });
+      yield* settle;
+      expect(nativeStops()).toBe(1);
+    }).pipe(Effect.scoped),
+  );
+
+  it.effect("keeps a session's screencast when another of its starts is refused", () =>
+    Effect.gen(function* () {
+      const host = yield* DesktopBrowserHost.make;
+      const debuggee = makeDebuggee();
+      host.attach(key, debuggee.tab);
+      const frames: Array<string | undefined> = [];
+      const replies = yield* watchReplies(host, frames);
+      const raw = (message: Record<string, unknown>) =>
+        host.handleCommandLine(encodeJson({ type: "cdp", ...key, message: encodeJson(message) }));
+
+      yield* raw({ id: 0, method: "Target.setAutoAttach", params: { autoAttach: true } });
+      expect(yield* Queue.take(replies)).toBe(0);
+      yield* raw({ id: 1, method: "Target.attachToTarget", params: { targetId: "GUEST" } });
+      expect(yield* Queue.take(replies)).toBe(1);
+      yield* raw({
+        id: 2,
+        method: "Page.startScreencast",
+        params: { id: 2 },
+        sessionId: "t3-preview-page-1",
+      });
+      yield* raw({
+        id: 3,
+        method: "Page.startScreencast",
+        params: { id: 3 },
+        sessionId: "t3-preview-page-1",
+      });
+      yield* settle;
+      debuggee.refuse(2);
+      debuggee.release();
+      expect([yield* Queue.take(replies), yield* Queue.take(replies)].toSorted()).toEqual([2, 3]);
+      yield* settle;
+      expect(debuggee.methods).not.toContain("Page.stopScreencast");
+      debuggee.emit("Page.screencastFrame", { data: "frame", sessionId: 1 });
+      yield* settle;
+      expect(frames).toContain("t3-preview-page-1");
+
+      // A refused replacement leaves the session's running stream, shared with another session.
+      yield* raw({ id: 4, method: "Target.attachToTarget", params: { targetId: "GUEST" } });
+      expect(yield* Queue.take(replies)).toBe(4);
+      yield* raw({
+        id: 5,
+        method: "Page.startScreencast",
+        params: { id: 5 },
+        sessionId: "t3-preview-page-2",
+      });
+      yield* raw({
+        id: 6,
+        method: "Page.startScreencast",
+        params: { id: 6 },
+        sessionId: "t3-preview-page-1",
+      });
+      yield* settle;
+      debuggee.refuse(6);
+      debuggee.release();
+      expect([yield* Queue.take(replies), yield* Queue.take(replies)].toSorted()).toEqual([5, 6]);
+      yield* settle;
+      frames.length = 0;
+      debuggee.emit("Page.screencastFrame", { data: "frame", sessionId: 2 });
+      yield* settle;
+      expect(frames).toEqual(["t3-preview-page-1", "t3-preview-page-2"]);
+    }).pipe(Effect.scoped),
+  );
+
+  it.effect("stops a guest's screencast once its last streaming session detaches", () =>
+    Effect.gen(function* () {
+      const host = yield* DesktopBrowserHost.make;
+      const debuggee = makeDebuggee();
+      host.attach(key, debuggee.tab);
+      const replies = yield* watchReplies(host);
+      const raw = (message: Record<string, unknown>) =>
+        host.handleCommandLine(encodeJson({ type: "cdp", ...key, message: encodeJson(message) }));
+      const nativeStops = () =>
+        debuggee.methods.filter((method) => method === "Page.stopScreencast").length;
+
+      yield* raw({ id: 1, method: "Target.attachToTarget", params: { targetId: "GUEST" } });
+      yield* raw({ id: 2, method: "Target.attachToTarget", params: { targetId: "GUEST" } });
+      expect([yield* Queue.take(replies), yield* Queue.take(replies)]).toEqual([1, 2]);
+      yield* raw({ id: 3, method: "Page.startScreencast", sessionId: "t3-preview-page-1" });
+      yield* raw({ id: 4, method: "Page.startScreencast", sessionId: "t3-preview-page-2" });
+      yield* settle;
+      debuggee.release();
+      // Streamers go without stopping, such as viewers that closed.
+      yield* raw({
+        id: 5,
+        method: "Target.detachFromTarget",
+        params: { sessionId: "t3-preview-page-1" },
+      });
+      yield* settle;
+      expect(nativeStops(), "stopped the screencast another session still streams").toBe(0);
+      yield* raw({
+        id: 6,
+        method: "Target.detachFromTarget",
+        params: { sessionId: "t3-preview-page-2" },
+      });
+      const answered = [];
+      for (let reply = 0; reply < 4; reply++) answered.push(yield* Queue.take(replies));
+      expect(answered.toSorted()).toEqual([3, 4, 5, 6]);
+      yield* settle;
+      expect(nativeStops()).toBe(1);
+    }).pipe(Effect.scoped),
+  );
+
+  it.effect("stops a screencast whose start lands after its session detached", () =>
+    Effect.gen(function* () {
+      const host = yield* DesktopBrowserHost.make;
+      const debuggee = makeDebuggee({ deferFocus: true });
+      host.attach(key, debuggee.tab);
+      const replies = yield* watchReplies(host);
+      const raw = (message: Record<string, unknown>) =>
+        host.handleCommandLine(encodeJson({ type: "cdp", ...key, message: encodeJson(message) }));
+
+      yield* raw({ id: 1, method: "Target.attachToTarget", params: { targetId: "GUEST" } });
+      expect(yield* Queue.take(replies)).toBe(1);
+      // The start waits on focus emulation while the viewer closes.
+      yield* raw({ id: 2, method: "Page.startScreencast", sessionId: "t3-preview-page-1" });
+      yield* raw({
+        id: 3,
+        method: "Target.detachFromTarget",
+        params: { sessionId: "t3-preview-page-1" },
+      });
+      expect(yield* Queue.take(replies)).toBe(3);
+      debuggee.settleFocus();
+      yield* settle;
+      debuggee.release();
+      expect(yield* Queue.take(replies)).toBe(2);
+      yield* settle;
+      expect(debuggee.methods.at(-1), "left the late screencast running").toBe(
+        "Page.stopScreencast",
+      );
+    }).pipe(Effect.scoped),
+  );
+
   it.effect("saves a server tab's download under its CDP guid where the server asked", () =>
     Effect.gen(function* () {
       const host = yield* DesktopBrowserHost.make;

apps/desktop/src/preview/DesktopBrowserHost.ts

@@ -75,8 +75,8 @@ interface AttachedTab {
    */
   agentUsed: boolean;
   focusReleasing: boolean;
-  /** The sessions streaming the guest; one stopping leaves the others' screencasts running. */
-  readonly screencasts: Set<string | undefined>;
+  /** The sessions sharing the guest's one Chromium screencast; it stops with the last of them. */
+  readonly screencasts: Map<string | undefined, { started: boolean; pending: number }>;
   readonly onMessage: (
     event: Electron.Event,
     method: string,
@@ -289,9 +289,15 @@ export const make = Effect.gen(function* () {
     dropFocusHolds(tab);
   };
 
+  /** An owner that leaves without stopping would otherwise keep the screencast running unseen. */
+  const leaveScreencast = (tab: AttachedTab, owner: string | undefined) => {
+    if (!tab.screencasts.delete(owner) || tab.screencasts.size > 0) return;
+    void tab.debuggee.debugger.sendCommand("Page.stopScreencast").catch(() => {});
+  };
+
   /** A detached session, such as a closed viewer, waits on nothing it sent. */
   const dropSessionHolds = (tab: AttachedTab, owner: string) => {
-    tab.screencasts.delete(owner);
+    leaveScreencast(tab, owner);
     for (const [pending, holder] of tab.focusPending) {
       if (holder === owner) tab.focusPending.delete(pending);
     }
@@ -330,13 +336,19 @@ export const make = Effect.gen(function* () {
       {
         send: (method, params, sessionId, owner) => {
           // 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);
+          const stream =
+            sessionId === undefined && method === "Page.startScreencast"
+              ? (tab.screencasts.get(owner) ?? { started: false, pending: 0 })
+              : undefined;
+          if (stream) {
+            stream.pending += 1;
+            tab.screencasts.set(owner, stream);
             if (owner !== undefined) streamingOwners.add(owner);
             syncPainting(tab);
           } else if (sessionId === undefined && method === "Page.stopScreencast") {
             tab.screencasts.delete(owner);
             syncPainting(tab);
+            if (tab.screencasts.size > 0) return Promise.resolve({});
           }
           // A person's input through a viewer, or Playwright's own housekeeping, is not the agent's.
           if (
@@ -365,19 +377,42 @@ export const make = Effect.gen(function* () {
                   held ? holdFocus(tab, owner, () => forward(current)) : forward(current),
               )
             : method === "Page.startScreencast"
-              ? 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, owner, forward).then(
+                  (result) => {
+                    if (stream) {
+                      stream.pending -= 1;
+                      stream.started = true;
+                      // A detach's stop can overtake a start still waiting on focus; nobody owns it now.
+                      if (tab.screencasts.get(owner) !== stream && tab.screencasts.size === 0) {
+                        void debuggee.sendCommand("Page.stopScreencast").catch(() => {});
+                      }
+                    }
+                    return result;
+                  },
+                  (cause: unknown) => {
+                    if (stream) stream.pending -= 1;
+                    // A viewer can outlive a refused start; its running or pending stream still holds it.
+                    if (
+                      stream &&
+                      !stream.started &&
+                      stream.pending === 0 &&
+                      tab.relay === relay &&
+                      tab.screencasts.get(owner) === stream
+                    ) {
+                      leaveScreencast(tab, owner);
+                    }
+                    releaseFocusWhenIdle(tab);
+                    syncPainting(tab);
+                    throw cause;
+                  },
+                )
               : holdFocus(tab, owner, forward);
         },
         sessionDetached: (owner) => {
           detachedOwners.add(owner);
           dropSessionHolds(tab, owner);
         },
+        receivesScreencast: (owner) => tab.screencasts.has(owner),
         targetId: () =>
           debuggee
             .sendCommand("Target.getTargetInfo")
@@ -463,7 +498,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 === "Page.downloadWillBegin") {
           const guid = (params as { guid?: unknown } | undefined)?.guid;