From 014aa9b4d3bc2b6d002e233ad723eb23970d5817 Mon Sep 17 00:00:00 2001 From: Simon Knott Date: Tue, 21 Jul 2026 12:04:31 +0200 Subject: [PATCH 1/2] fix(screencast): acknowledge frames after callbacks Re: https://github.com/microsoft/playwright-python/issues/3145 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2848e8f0-1e34-4996-a6d6-7e48c00bb8f3 --- packages/isomorphic/protocolMetainfo.ts | 1 + .../playwright-core/src/client/channels.d.ts | 9 ++++ .../playwright-core/src/client/screencast.ts | 8 +++- .../src/server/bidi/bidiPage.ts | 27 +++++++----- .../playwright-core/src/server/channels.d.ts | 9 ++++ .../src/server/chromium/crPage.ts | 4 +- .../src/server/dispatchers/pageDispatcher.ts | 32 ++++++++++++-- .../src/server/firefox/ffPage.ts | 4 +- .../playwright-core/src/server/screencast.ts | 40 +++++++++--------- .../src/server/webkit/wkPage.ts | 4 +- packages/protocol/spec/page.yml | 6 +++ packages/protocol/src/validator.ts | 5 +++ tests/library/screencast.spec.ts | 42 +++++++++++++++++++ 13 files changed, 149 insertions(+), 42 deletions(-) diff --git a/packages/isomorphic/protocolMetainfo.ts b/packages/isomorphic/protocolMetainfo.ts index cf93e3f62f25d..c8897fdf9f33a 100644 --- a/packages/isomorphic/protocolMetainfo.ts +++ b/packages/isomorphic/protocolMetainfo.ts @@ -298,6 +298,7 @@ export const methodMetainfo = new Map([ ['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, }], diff --git a/packages/playwright-core/src/client/channels.d.ts b/packages/playwright-core/src/client/channels.d.ts index ee239a9c51efc..2d8fb3f168770 100644 --- a/packages/playwright-core/src/client/channels.d.ts +++ b/packages/playwright-core/src/client/channels.d.ts @@ -3934,6 +3934,7 @@ export interface PageChannel extends PageEventTarget, Channel { screencastShowActions(params: PageScreencastShowActionsParams, options: TimeoutOptions): Promise; screencastHideActions(params: PageScreencastHideActionsParams, options: TimeoutOptions): Promise; screencastStart(params: PageScreencastStartParams, options: TimeoutOptions): Promise; + screencastFrameAck(params: PageScreencastFrameAckParams, options: TimeoutOptions): Promise; screencastStop(params: PageScreencastStopParams, options: TimeoutOptions): Promise; updateSubscription(params: PageUpdateSubscriptionParams, options: TimeoutOptions): Promise; setDockTile(params: PageSetDockTileParams, options: TimeoutOptions): Promise; @@ -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; diff --git a/packages/playwright-core/src/client/screencast.ts b/packages/playwright-core/src/client/screencast.ts index 162ac25d6cdf2..6ae7a2ddfe9ed 100644 --- a/packages/playwright-core/src/client/screencast.ts +++ b/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(() => {}); + } }); } diff --git a/packages/playwright-core/src/server/bidi/bidiPage.ts b/packages/playwright-core/src/server/bidi/bidiPage.ts index b8fcdb1191f11..33825079447d9 100644 --- a/packages/playwright-core/src/server/bidi/bidiPage.ts +++ b/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(); private readonly _failedNavigations = new Map(); 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; } } diff --git a/packages/playwright-core/src/server/channels.d.ts b/packages/playwright-core/src/server/channels.d.ts index b66eb1753d7ef..1bfeedb0a0cad 100644 --- a/packages/playwright-core/src/server/channels.d.ts +++ b/packages/playwright-core/src/server/channels.d.ts @@ -3935,6 +3935,7 @@ export interface PageChannel extends PageEventTarget, Channel { screencastShowActions(params: PageScreencastShowActionsParams, progress: Progress): Promise; screencastHideActions(params: PageScreencastHideActionsParams, progress: Progress): Promise; screencastStart(params: PageScreencastStartParams, progress: Progress): Promise; + screencastFrameAck(params: PageScreencastFrameAckParams, progress: Progress): Promise; screencastStop(params: PageScreencastStopParams, progress: Progress): Promise; updateSubscription(params: PageUpdateSubscriptionParams, progress: Progress): Promise; setDockTile(params: PageSetDockTileParams, progress: Progress): Promise; @@ -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; diff --git a/packages/playwright-core/src/server/chromium/crPage.ts b/packages/playwright-core/src/server/chromium/crPage.ts index aee1443787dc8..b462b0b226eeb 100644 --- a/packages/playwright-core/src/server/chromium/crPage.ts +++ b/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 }); }); } diff --git a/packages/playwright-core/src/server/dispatchers/pageDispatcher.ts b/packages/playwright-core/src/server/dispatchers/pageDispatcher.ts index ede502ef9bbc9..dcf92c5a970be 100644 --- a/packages/playwright-core/src/server/dispatchers/pageDispatcher.ts +++ b/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>(); private _videoRecorder: VideoRecorder | undefined; static from(parentScope: BrowserContextDispatcher, page: Page): PageDispatcher { @@ -397,10 +400,15 @@ export class PageDispatcher extends Dispatcher { - 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(); + 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 { + 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 { if (this._videoRecorder) { await this._videoRecorder.stop(); @@ -423,8 +439,16 @@ export class PageDispatcher extends Dispatcher { diff --git a/packages/playwright-core/src/server/firefox/ffPage.ts b/packages/playwright-core/src/server/firefox/ffPage.ts index 4eecb45561e4e..86c5031367bcb 100644 --- a/packages/playwright-core/src/server/firefox/ffPage.ts +++ b/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'); }); } diff --git a/packages/playwright-core/src/server/screencast.ts b/packages/playwright-core/src/server/screencast.ts index 085d20b28f70d..b545b5e73191a 100644 --- a/packages/playwright-core/src/server/screencast.ts +++ b/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>(); + private _clients = new Map(); 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()); + 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[] = []; - 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 { diff --git a/packages/playwright-core/src/server/webkit/wkPage.ts b/packages/playwright-core/src/server/webkit/wkPage.ts index 5258751640416..55f3c41c4ea58 100644 --- a/packages/playwright-core/src/server/webkit/wkPage.ts +++ b/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 }); }); } diff --git a/packages/protocol/spec/page.yml b/packages/protocol/spec/page.yml index 0d132d013a939..30a1c94422bee 100644 --- a/packages/protocol/spec/page.yml +++ b/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 diff --git a/packages/protocol/src/validator.ts b/packages/protocol/src/validator.ts index 5c080081f7e09..f459614a70278 100644 --- a/packages/protocol/src/validator.ts +++ b/packages/protocol/src/validator.ts @@ -2264,6 +2264,7 @@ scheme.PageRouteEvent = tObject({ route: tChannel(['Route']), }); scheme.PageScreencastFrameEvent = tObject({ + frameId: tInt, data: tBinary, timestamp: tFloat, viewportWidth: tInt, @@ -2624,6 +2625,10 @@ scheme.PageScreencastStartParams = tObject({ scheme.PageScreencastStartResult = tObject({ artifact: tOptional(tChannel(['Artifact'])), }); +scheme.PageScreencastFrameAckParams = tObject({ + frameId: tInt, +}); +scheme.PageScreencastFrameAckResult = tOptional(tObject({})); scheme.PageScreencastStopParams = tOptional(tObject({})); scheme.PageScreencastStopResult = tOptional(tObject({})); scheme.PageUpdateSubscriptionParams = tObject({ diff --git a/tests/library/screencast.spec.ts b/tests/library/screencast.spec.ts index 45bc637cebd98..7dea583816f0f 100644 --- a/tests/library/screencast.spec.ts +++ b/tests/library/screencast.spec.ts @@ -54,6 +54,47 @@ test('screencast.start delivers frames via onFrame callback', async ({ browser, await context.close(); }); +test('applies backpressure while async onFrame callback is pending', async ({ browser, server, trace }) => { + test.skip(trace === 'on', 'trace recording acknowledges screencast frames independently'); + + const context = await browser.newContext({ viewport: { width: 500, height: 400 } }); + const page = await context.newPage(); + + let releaseCallback: () => void; + const callbackDone = new Promise(f => releaseCallback = f); + let frameCount = 0; + await page.screencast.start({ + onFrame: async () => { + ++frameCount; + await callbackDone; + }, + }); + await page.goto(server.EMPTY_PAGE); + await page.evaluate(() => document.body.style.backgroundColor = 'red'); + + for (let i = 0; i < 3; ++i) { + await page.evaluate(() => { + document.body.style.backgroundColor = document.body.style.backgroundColor === 'red' ? 'blue' : 'red'; + }); + await ensureSomeFrames(page); + } + const framesWhileBlocked = frameCount; + expect(framesWhileBlocked).toBeGreaterThan(0); + expect(framesWhileBlocked).toBeLessThan(10); + + releaseCallback!(); + await expect.poll(async () => { + await page.evaluate(() => { + document.body.style.backgroundColor = document.body.style.backgroundColor === 'red' ? 'blue' : 'red'; + }); + await ensureSomeFrames(page); + return frameCount; + }).toBeGreaterThan(framesWhileBlocked); + + await page.screencast.stop(); + await context.close(); +}); + test('onFrame receives viewport size', async ({ browser, server, trace, browserName, isMac, headless }) => { test.skip(trace === 'on', 'trace=on has different screencast image configuration'); test.fixme(browserName === 'firefox' && isMac && !headless, 'wrong frame size in headed Firefox on Mac'); @@ -155,6 +196,7 @@ test('start/stop twice without path creates two files in artifactsDir', async ({ test('start should work when recordVideo is set', async ({ browser }, testInfo) => { test.slow(); + const autoDir = testInfo.outputPath('auto'); const manualDir = testInfo.outputPath('manual'); const context = await browser.newContext({ From aca67828250f01eea84e45bbb8368c87724fc9f6 Mon Sep 17 00:00:00 2001 From: Simon Knott Date: Tue, 21 Jul 2026 15:00:12 +0200 Subject: [PATCH 2/2] test(screencast): assert callback backpressure stalls Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2848e8f0-1e34-4996-a6d6-7e48c00bb8f3 --- tests/library/screencast.spec.ts | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/tests/library/screencast.spec.ts b/tests/library/screencast.spec.ts index 7dea583816f0f..31c4a86213afc 100644 --- a/tests/library/screencast.spec.ts +++ b/tests/library/screencast.spec.ts @@ -62,25 +62,32 @@ test('applies backpressure while async onFrame callback is pending', async ({ br let releaseCallback: () => void; const callbackDone = new Promise(f => releaseCallback = f); + let firstFrame: () => void; + const firstFrameReceived = new Promise(f => firstFrame = f); let frameCount = 0; + let lastFrameTimestamp = 0; await page.screencast.start({ onFrame: async () => { ++frameCount; + lastFrameTimestamp = Date.now(); + firstFrame(); await callbackDone; }, }); await page.goto(server.EMPTY_PAGE); - await page.evaluate(() => document.body.style.backgroundColor = 'red'); - - for (let i = 0; i < 3; ++i) { - await page.evaluate(() => { + await page.evaluate(() => { + const animate = () => { document.body.style.backgroundColor = document.body.style.backgroundColor === 'red' ? 'blue' : 'red'; - }); - await ensureSomeFrames(page); - } + requestAnimationFrame(animate); + }; + requestAnimationFrame(animate); + }); + await firstFrameReceived; + await expect.poll(() => Date.now() - lastFrameTimestamp).toBeGreaterThan(1000); + const framesWhileBlocked = frameCount; - expect(framesWhileBlocked).toBeGreaterThan(0); - expect(framesWhileBlocked).toBeLessThan(10); + await ensureSomeFrames(page); + expect(frameCount).toBe(framesWhileBlocked); releaseCallback!(); await expect.poll(async () => {