fix(screencast): acknowledge frames after callbacks

packages/isomorphic/protocolMetainfo.ts

@@ -298,6 +298,7 @@ export const methodMetainfo = new Map<string, MethodMetainfo>([
   ['Page.screencastShowActions', { title: 'Show actions', group: 'configuration', }],
   ['Page.screencastHideActions', { title: 'Remove actions', group: 'configuration', }],
   ['Page.screencastStart', { title: 'Start screencast', group: 'configuration', }],
+  ['Page.screencastFrameAck', { internal: true, }],
   ['Page.screencastStop', { title: 'Stop screencast', group: 'configuration', }],
   ['Page.updateSubscription', { internal: true, }],
   ['Page.setDockTile', { internal: true, }],

packages/playwright-core/src/client/channels.d.ts

@@ -3934,6 +3934,7 @@ export interface PageChannel extends PageEventTarget, Channel {
   screencastShowActions(params: PageScreencastShowActionsParams, options: TimeoutOptions): Promise<PageScreencastShowActionsResult>;
   screencastHideActions(params: PageScreencastHideActionsParams, options: TimeoutOptions): Promise<PageScreencastHideActionsResult>;
   screencastStart(params: PageScreencastStartParams, options: TimeoutOptions): Promise<PageScreencastStartResult>;
+  screencastFrameAck(params: PageScreencastFrameAckParams, options: TimeoutOptions): Promise<PageScreencastFrameAckResult>;
   screencastStop(params: PageScreencastStopParams, options: TimeoutOptions): Promise<PageScreencastStopResult>;
   updateSubscription(params: PageUpdateSubscriptionParams, options: TimeoutOptions): Promise<PageUpdateSubscriptionResult>;
   setDockTile(params: PageSetDockTileParams, options: TimeoutOptions): Promise<PageSetDockTileResult>;
@@ -3976,6 +3977,7 @@ export type PageRouteEvent = {
   route: RouteChannel,
 };
 export type PageScreencastFrameEvent = {
+  frameId: number,
   data: Binary,
   timestamp: number,
   viewportWidth: number,
@@ -4533,6 +4535,13 @@ export type PageScreencastStartOptions = {
 export type PageScreencastStartResult = {
   artifact?: ArtifactChannel,
 };
+export type PageScreencastFrameAckParams = {
+  frameId: number,
+};
+export type PageScreencastFrameAckOptions = {
+
+};
+export type PageScreencastFrameAckResult = void;
 export type PageScreencastStopParams = {};
 export type PageScreencastStopOptions = {};
 export type PageScreencastStopResult = void;

packages/playwright-core/src/client/screencast.ts

@@ -30,8 +30,12 @@ export class Screencast implements api.Screencast {
 
   constructor(page: Page) {
     this._page = page;
-    this._page._channel.on('screencastFrame', ({ data, timestamp, viewportWidth, viewportHeight }) => {
-      void this._onFrame?.({ data, timestamp, viewportWidth, viewportHeight });
+    this._page._channel.on('screencastFrame', async ({ frameId, data, timestamp, viewportWidth, viewportHeight }) => {
+      try {
+        await this._onFrame?.({ data, timestamp, viewportWidth, viewportHeight });
+      } finally {
+        await this._page._channel.screencastFrameAck({ frameId }, kNoTimeout).catch(() => {});
+      }
     });
   }
 

packages/playwright-core/src/server/bidi/bidiPage.ts

@@ -16,6 +16,7 @@
 
 import { debugLogger } from '@utils/debugLogger';
 import { eventsHelper } from '@utils/eventsHelper';
+import { monotonicTime } from '@isomorphic/time';
 import * as dialog from '../dialog';
 import * as dom from '../dom';
 import * as js from '../javascript';
@@ -58,7 +59,8 @@ export class BidiPage implements PageDelegate {
   private readonly _fragmentNavigations = new Set<string>();
   private readonly _failedNavigations = new Map<string, string>();
   private _screencastTimer: NodeJS.Timeout | undefined;
-  private _waitingForScreenshot = false;
+  private _screencastGeneration = 0;
+  private _screencastRunning = false;
 
   constructor(browserContext: BidiBrowserContext, bidiSession: BidiSession, opener: BidiPage | null) {
     this._session = bidiSession;
@@ -589,19 +591,18 @@ export class BidiPage implements PageDelegate {
   }
 
   startScreencast(options: { width: number, height: number, quality: number }) {
-    if (this._screencastTimer)
+    if (this._screencastRunning)
       return;
 
-    this._waitingForScreenshot = false;
-    this._screencastTimer = setInterval(async () => {
-      if (this._waitingForScreenshot)
-        return;
+    this._screencastRunning = true;
+    const generation = ++this._screencastGeneration;
+    const captureFrame = async () => {
       if (this._session.isDisposed()) {
         this.stopScreencast();
         return;
       }
 
-      this._waitingForScreenshot = true;
+      const startTime = monotonicTime();
       const payload = await this._session.sendMayFail('browsingContext.captureScreenshot', {
         context: this._session.sessionId,
         format: {
@@ -612,20 +613,24 @@ export class BidiPage implements PageDelegate {
       if (payload) {
         const buffer = Buffer.from(payload.data, 'base64');
         const { width, height } = jpegDimensions(buffer);
-        this._page.screencast.onScreencastFrame({
+        await this._page.screencast.onScreencastFrame({
           buffer,
           frameSwapWallTime: Date.now(),
           viewportWidth: width,
           viewportHeight: height,
         });
       }
-      this._waitingForScreenshot = false;
-    }, 40);
+      if (!this._screencastRunning || generation !== this._screencastGeneration)
+        return;
+      this._screencastTimer = setTimeout(captureFrame, Math.max(0, 40 - (monotonicTime() - startTime)));
+    };
+    void captureFrame();
   }
 
   stopScreencast() {
+    this._screencastRunning = false;
     if (this._screencastTimer) {
-      clearInterval(this._screencastTimer);
+      clearTimeout(this._screencastTimer);
       this._screencastTimer = undefined;
     }
   }

packages/playwright-core/src/server/channels.d.ts

@@ -3935,6 +3935,7 @@ export interface PageChannel extends PageEventTarget, Channel {
   screencastShowActions(params: PageScreencastShowActionsParams, progress: Progress): Promise<PageScreencastShowActionsResult>;
   screencastHideActions(params: PageScreencastHideActionsParams, progress: Progress): Promise<PageScreencastHideActionsResult>;
   screencastStart(params: PageScreencastStartParams, progress: Progress): Promise<PageScreencastStartResult>;
+  screencastFrameAck(params: PageScreencastFrameAckParams, progress: Progress): Promise<PageScreencastFrameAckResult>;
   screencastStop(params: PageScreencastStopParams, progress: Progress): Promise<PageScreencastStopResult>;
   updateSubscription(params: PageUpdateSubscriptionParams, progress: Progress): Promise<PageUpdateSubscriptionResult>;
   setDockTile(params: PageSetDockTileParams, progress: Progress): Promise<PageSetDockTileResult>;
@@ -3977,6 +3978,7 @@ export type PageRouteEvent = {
   route: RouteChannel,
 };
 export type PageScreencastFrameEvent = {
+  frameId: number,
   data: Binary,
   timestamp: number,
   viewportWidth: number,
@@ -4534,6 +4536,13 @@ export type PageScreencastStartOptions = {
 export type PageScreencastStartResult = {
   artifact?: ArtifactChannel,
 };
+export type PageScreencastFrameAckParams = {
+  frameId: number,
+};
+export type PageScreencastFrameAckOptions = {
+
+};
+export type PageScreencastFrameAckResult = void;
 export type PageScreencastStopParams = {};
 export type PageScreencastStopOptions = {};
 export type PageScreencastStopResult = void;

packages/playwright-core/src/server/chromium/crPage.ts

@@ -892,12 +892,12 @@ class FrameSession {
 
   _onScreencastFrame(payload: Protocol.Page.screencastFramePayload) {
     const buffer = Buffer.from(payload.data, 'base64');
-    this._page.screencast.onScreencastFrame({
+    void this._page.screencast.onScreencastFrame({
       buffer,
       frameSwapWallTime: payload.metadata.timestamp ? payload.metadata.timestamp * 1000 : Date.now(),
       viewportWidth: payload.metadata.deviceWidth,
       viewportHeight: payload.metadata.deviceHeight,
-    }, () => {
+    }).then(() => {
       this._client._sendMayFail('Page.screencastFrameAck', { sessionId: payload.sessionId });
     });
   }

packages/playwright-core/src/server/dispatchers/pageDispatcher.ts

@@ -16,6 +16,7 @@
 
 import { renderTitleForCall } from '@isomorphic/protocolFormatter';
 import { deserializeURLMatch, urlMatches } from '@isomorphic/urlMatch';
+import { ManualPromise } from '@isomorphic/manualPromise';
 import { Page, Worker } from '../page';
 import { Dispatcher } from './dispatcher';
 import { parseError, serializeError } from '../errors';
@@ -65,6 +66,8 @@ export class PageDispatcher extends Dispatcher<Page, channels.PageChannel, Brows
   private _jsCoverageActive = false;
   private _cssCoverageActive = false;
   private _screencastClient: ScreencastClient | undefined;
+  private _screencastFrameId = 0;
+  private _screencastFrameAcks = new Map<number, ManualPromise<void>>();
   private _videoRecorder: VideoRecorder | undefined;
 
   static from(parentScope: BrowserContextDispatcher, page: Page): PageDispatcher {
@@ -397,10 +400,15 @@ export class PageDispatcher extends Dispatcher<Page, channels.PageChannel, Brows
 
     if (params.sendFrames) {
       this._screencastClient = {
-        onFrame: (frame: ScreencastFrame) => {
-          this._dispatchEvent('screencastFrame', { data: frame.buffer, timestamp: frame.frameSwapWallTime, viewportWidth: frame.viewportWidth, viewportHeight: frame.viewportHeight });
+        onFrame: async (frame: ScreencastFrame) => {
+          const frameId = ++this._screencastFrameId;
+          const promise = new ManualPromise<void>();
+          this._screencastFrameAcks.set(frameId, promise);
+          this._dispatchEvent('screencastFrame', { frameId, data: frame.buffer, timestamp: frame.frameSwapWallTime, viewportWidth: frame.viewportWidth, viewportHeight: frame.viewportHeight });
+          await promise;
         },
-        dispose: () => {},
+        gracefulClose: () => this._clearScreencastFrameAcks(),
+        dispose: () => this._clearScreencastFrameAcks(),
         size: params.size,
         quality: params.quality,
       };
@@ -415,6 +423,14 @@ export class PageDispatcher extends Dispatcher<Page, channels.PageChannel, Brows
     return { artifact: artifact ? createVideoDispatcher(this.parentScope(), artifact) : undefined };
   }
 
+  async screencastFrameAck(params: channels.PageScreencastFrameAckParams): Promise<channels.PageScreencastFrameAckResult> {
+    const promise = this._screencastFrameAcks.get(params.frameId);
+    if (!promise)
+      return;
+    this._screencastFrameAcks.delete(params.frameId);
+    promise.resolve();
+  }
+
   async screencastStop(params: channels.PageScreencastStopParams, progress?: Progress): Promise<channels.PageScreencastStopResult> {
     if (this._videoRecorder) {
       await this._videoRecorder.stop();
@@ -423,8 +439,16 @@ export class PageDispatcher extends Dispatcher<Page, channels.PageChannel, Brows
 
     const client = this._screencastClient;
     this._screencastClient = undefined;
-    if (client)
+    if (client) {
+      client.dispose();
       this._page.screencast.removeClient(client);
+    }
+  }
+
+  private _clearScreencastFrameAcks() {
+    for (const promise of this._screencastFrameAcks.values())
+      promise.resolve();
+    this._screencastFrameAcks.clear();
   }
 
   async startJSCoverage(params: channels.PageStartJSCoverageParams, progress: Progress): Promise<void> {

packages/playwright-core/src/server/firefox/ffPage.ts

@@ -545,12 +545,12 @@ export class FFPage implements PageDelegate {
 
   private _onScreencastFrame(event: Protocol.Page.screencastFramePayload) {
     const buffer = Buffer.from(event.data, 'base64');
-    this._page.screencast.onScreencastFrame({
+    void this._page.screencast.onScreencastFrame({
       buffer,
       frameSwapWallTime: event.timestamp * 1000, // timestamp is in seconds, we need to convert to milliseconds.
       viewportWidth: event.deviceWidth,
       viewportHeight: event.deviceHeight,
-    }, () => {
+    }).then(() => {
       this._session.sendMayFail('Page.screencastFrameAck');
     });
   }

packages/playwright-core/src/server/screencast.ts

@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 
-import { ManualPromise } from '@isomorphic/manualPromise';
+import { LongStandingScope } from '@isomorphic/manualPromise';
 import { renderTitleForCall } from '@isomorphic/protocolFormatter';
 import { debugLogger } from '@utils/debugLogger';
 import { Page } from './page';
@@ -43,7 +43,7 @@ type ActionOptions = {
 
 export class Screencast implements InstrumentationListener {
   readonly page: Page;
-  private _clients = new Map<ScreencastClient, ManualPromise<void>>();
+  private _clients = new Map<ScreencastClient, LongStandingScope>();
   private _actions: ActionOptions | undefined;
   private _size: types.Size | undefined;
   private _lastFrame: types.ScreencastFrame | undefined;
@@ -55,6 +55,8 @@ export class Screencast implements InstrumentationListener {
 
   async handlePageOrContextClose() {
     const clients = [...this._clients.keys()];
+    for (const scope of this._clients.values())
+      scope.reject(new Error('Screencast closed'));
     this._clients.clear();
     for (const client of clients) {
       if (client.gracefulClose)
@@ -63,6 +65,8 @@ export class Screencast implements InstrumentationListener {
   }
 
   dispose() {
+    for (const scope of this._clients.values())
+      scope.reject(new Error('Screencast disposed'));
     for (const client of this._clients.keys())
       client.dispose();
     this._clients.clear();
@@ -79,7 +83,7 @@ export class Screencast implements InstrumentationListener {
 
   addClient(client: ScreencastClient): { size: types.Size } {
     const isFirst = this._clients.size === 0;
-    this._clients.set(client, new ManualPromise<void>());
+    this._clients.set(client, new LongStandingScope());
     if (isFirst) {
       this._startScreencast(client.size, client.quality);
     } else if (this._lastFrame) {
@@ -96,12 +100,12 @@ export class Screencast implements InstrumentationListener {
   }
 
   removeClient(client: ScreencastClient) {
-    const disconnected = this._clients.get(client);
-    if (!disconnected)
+    const scope = this._clients.get(client);
+    if (!scope)
       return;
     this._clients.delete(client);
     // A departing client must not block frame acks for the remaining clients.
-    disconnected.resolve();
+    scope.reject(new Error('Screencast client removed'));
     if (!this._clients.size)
       this._stopScreencast();
   }
@@ -135,24 +139,22 @@ export class Screencast implements InstrumentationListener {
     this.page.delegate.stopScreencast();
   }
 
-  onScreencastFrame(frame: types.ScreencastFrame, ack?: () => void) {
+  async onScreencastFrame(frame: types.ScreencastFrame) {
     this._lastFrame = frame;
+    let syncAck = false;
     const asyncResults: Promise<void>[] = [];
-    for (const [client, disconnected] of this._clients) {
+    for (const [client, scope] of this._clients) {
       const result = client.onFrame(frame);
-      if (!result)
-        continue;
-      asyncResults.push(Promise.race([result.catch(() => {}), disconnected]));
-    }
-    if (ack) {
-      // Ack when any client resolves (OR logic). This ensures that even if
-      // tracing throttles its response, other clients (like video) that resolve
-      // immediately keep frames flowing.
-      if (!asyncResults.length)
-        ack();
+      if (result)
+        asyncResults.push(scope.safeRace(result.catch(() => {})));
       else
-        Promise.race(asyncResults).then(ack);
+        syncAck = true;
     }
+    // Ack when any client resolves (OR logic). This ensures that even if
+    // tracing throttles its response, other clients (like video) that resolve
+    // immediately keep frames flowing.
+    if (!syncAck && asyncResults.length)
+      await Promise.race(asyncResults);
   }
 
   async onBeforeInputAction(sdkObject: SdkObject, metadata: CallMetadata, point?: types.Point, box?: types.Rect): Promise<void> {

packages/playwright-core/src/server/webkit/wkPage.ts

@@ -954,7 +954,7 @@ export class WKPage implements PageDelegate {
   private _onScreencastFrame(event: Protocol.Screencast.screencastFramePayload) {
     const generation = this._screencastGeneration;
     const buffer = Buffer.from(event.data, 'base64');
-    this._page.screencast.onScreencastFrame({
+    void this._page.screencast.onScreencastFrame({
       buffer,
       frameSwapWallTime: event.timestamp
         // timestamp is in seconds, we need to convert to milliseconds.
@@ -965,7 +965,7 @@ export class WKPage implements PageDelegate {
         : Date.now(),
       viewportWidth: event.deviceWidth,
       viewportHeight: event.deviceHeight,
-    }, () => {
+    }).then(() => {
       this._pageProxySession.sendMayFail('Screencast.screencastFrameAck', { generation });
     });
   }

packages/protocol/spec/page.yml

@@ -586,6 +586,11 @@ Page:
       returns:
         artifact: Artifact?
 
+    screencastFrameAck:
+      internal: true
+      parameters:
+        frameId: int
+
     screencastStop:
       title: Stop screencast
       group: configuration
@@ -717,6 +722,7 @@ Page:
 
     screencastFrame:
       parameters:
+        frameId: int
         data: binary
         timestamp: float
         viewportWidth: int