fix(screencast): replace ScreencastStopped event with async path() (#3612)

browsers.json

@@ -13,7 +13,7 @@
     },
     {
       "name": "webkit",
-      "revision": "1330",
+      "revision": "1332",
       "download": true
     }
   ]

src/server/browserContext.ts

@@ -31,11 +31,19 @@ import { DebugController } from './debug/debugController';
 import { isDebugMode } from '../utils/utils';
 
 export class Screencast {
-  readonly path: string;
   readonly page: Page;
+  private readonly _path: string;
+  _finishCallback: () => void = () => {};
+  private readonly _finishedPromise: Promise<void>;
   constructor(path: string, page: Page) {
-    this.path = path;
+    this._path = path;
     this.page = page;
+    this._finishedPromise = new Promise(fulfill => this._finishCallback = fulfill);
+  }
+
+  async path(): Promise<string | null> {
+    await this._finishedPromise;
+    return this._path;
   }
 }
 
@@ -58,6 +66,7 @@ export abstract class BrowserContext extends EventEmitter {
   private _closePromiseFulfill: ((error: Error) => void) | undefined;
   readonly _permissions = new Map<string, string[]>();
   readonly _downloads = new Set<Download>();
+  readonly _idToScreencast = new Map<string, Screencast>();
   readonly _browser: Browser;
 
   constructor(browser: Browser, options: types.BrowserContextOptions, isPersistentContext: boolean) {

src/server/firefox/ffPage.ts

@@ -267,10 +267,8 @@ export class FFPage implements PageDelegate {
 
   _onScreencastStopped(event: Protocol.Page.screencastStoppedPayload) {
     const screencast = this._idToScreencast.get(event.uid);
-    if (!screencast)
-      return;
     this._idToScreencast.delete(event.uid);
-    this._browserContext.emit(BrowserContext.Events.ScreencastStopped, screencast);
+    screencast!._finishCallback();
   }
 
   async exposeBinding(binding: PageBinding) {

src/server/webkit/protocol.ts

@@ -6818,6 +6818,16 @@ Left=1, Right=2, Middle=4, Back=8, Forward=16, None=0.
       uuid: string;
       error: string;
     }
+    export type screencastFinishedPayload = {
+      /**
+       * Unique identifier of the context.
+       */
+      browserContextId: ContextID;
+      /**
+       * Unique identifier of the screencast.
+       */
+      screencastId: Screencast.ScreencastId;
+    }
     
     export type enableParameters = {
     }
@@ -7835,12 +7845,16 @@ Left=1, Right=2, Middle=4, Back=8, Forward=16, None=0.
   }
   
   export module Screencast {
+    /**
+     * Unique identifier of the screencast.
+     */
+    export type ScreencastId = string;
     
     
     /**
      * Starts recoring video to speified file.
      */
-    export type startVideoRecordingParameters = {
+    export type startParameters = {
       /**
        * Output file location.
        */
@@ -7849,14 +7863,18 @@ Left=1, Right=2, Middle=4, Back=8, Forward=16, None=0.
       height: number;
       scale?: number;
     }
-    export type startVideoRecordingReturnValue = {
+    export type startReturnValue = {
+      /**
+       * Unique identifier of the screencast.
+       */
+      screencastId: ScreencastId;
     }
     /**
      * Stops recoding video. Returns after the file has been closed.
      */
-    export type stopVideoRecordingParameters = {
+    export type stopParameters = {
     }
-    export type stopVideoRecordingReturnValue = {
+    export type stopReturnValue = {
     }
   }
   
@@ -8411,6 +8429,7 @@ Left=1, Right=2, Middle=4, Back=8, Forward=16, None=0.
     "Playwright.downloadCreated": Playwright.downloadCreatedPayload;
     "Playwright.downloadFilenameSuggested": Playwright.downloadFilenameSuggestedPayload;
     "Playwright.downloadFinished": Playwright.downloadFinishedPayload;
+    "Playwright.screencastFinished": Playwright.screencastFinishedPayload;
     "Runtime.executionContextCreated": Runtime.executionContextCreatedPayload;
     "ScriptProfiler.trackingStart": ScriptProfiler.trackingStartPayload;
     "ScriptProfiler.trackingUpdate": ScriptProfiler.trackingUpdatePayload;
@@ -8689,8 +8708,8 @@ Left=1, Right=2, Middle=4, Back=8, Forward=16, None=0.
     "Runtime.enableControlFlowProfiler": Runtime.enableControlFlowProfilerParameters;
     "Runtime.disableControlFlowProfiler": Runtime.disableControlFlowProfilerParameters;
     "Runtime.getBasicBlocks": Runtime.getBasicBlocksParameters;
-    "Screencast.startVideoRecording": Screencast.startVideoRecordingParameters;
-    "Screencast.stopVideoRecording": Screencast.stopVideoRecordingParameters;
+    "Screencast.start": Screencast.startParameters;
+    "Screencast.stop": Screencast.stopParameters;
     "ScriptProfiler.startTracking": ScriptProfiler.startTrackingParameters;
     "ScriptProfiler.stopTracking": ScriptProfiler.stopTrackingParameters;
     "ServiceWorker.getInitializationInfo": ServiceWorker.getInitializationInfoParameters;
@@ -8972,8 +8991,8 @@ Left=1, Right=2, Middle=4, Back=8, Forward=16, None=0.
     "Runtime.enableControlFlowProfiler": Runtime.enableControlFlowProfilerReturnValue;
     "Runtime.disableControlFlowProfiler": Runtime.disableControlFlowProfilerReturnValue;
     "Runtime.getBasicBlocks": Runtime.getBasicBlocksReturnValue;
-    "Screencast.startVideoRecording": Screencast.startVideoRecordingReturnValue;
-    "Screencast.stopVideoRecording": Screencast.stopVideoRecordingReturnValue;
+    "Screencast.start": Screencast.startReturnValue;
+    "Screencast.stop": Screencast.stopReturnValue;
     "ScriptProfiler.startTracking": ScriptProfiler.startTrackingReturnValue;
     "ScriptProfiler.stopTracking": ScriptProfiler.stopTrackingReturnValue;
     "ServiceWorker.getInitializationInfo": ServiceWorker.getInitializationInfoReturnValue;

src/server/webkit/wkBrowser.ts

@@ -62,6 +62,7 @@ export class WKBrowser extends Browser {
       helper.addEventListener(this._browserSession, 'Playwright.downloadCreated', this._onDownloadCreated.bind(this)),
       helper.addEventListener(this._browserSession, 'Playwright.downloadFilenameSuggested', this._onDownloadFilenameSuggested.bind(this)),
       helper.addEventListener(this._browserSession, 'Playwright.downloadFinished', this._onDownloadFinished.bind(this)),
+      helper.addEventListener(this._browserSession, 'Playwright.screencastFinished', this._onScreencastFinished.bind(this)),
       helper.addEventListener(this._browserSession, kPageProxyMessageReceived, this._onPageProxyMessageReceived.bind(this)),
     ];
   }
@@ -124,6 +125,13 @@ export class WKBrowser extends Browser {
     this._downloadFinished(payload.uuid, payload.error);
   }
 
+  _onScreencastFinished(payload: Protocol.Playwright.screencastFinishedPayload) {
+    const context = this._contexts.get(payload.browserContextId);
+    if (!context)
+      return;
+    context._screencastFinished(payload.screencastId);
+  }
+
   _onPageProxyCreated(event: Protocol.Playwright.pageProxyCreatedPayload) {
     const pageProxyId = event.pageProxyId;
     let context: WKBrowserContext | null = null;
@@ -326,6 +334,12 @@ export class WKBrowserContext extends BrowserContext {
       await (page._delegate as WKPage).updateRequestInterception();
   }
 
+  _screencastFinished(uid: string) {
+    const screencast = this._idToScreencast.get(uid);
+    this._idToScreencast.delete(uid);
+    screencast!._finishCallback();
+  }
+
   async _doClose() {
     assert(this._browserContextId);
     await this._browser._browserSession.send('Playwright.deleteContext', { browserContextId: this._browserContextId });

src/server/webkit/wkPage.ts

@@ -718,13 +718,15 @@ export class WKPage implements PageDelegate {
       throw new Error('Already recording');
     this._recordingVideoFile = options.outputFile;
     try {
-      await this._pageProxySession.send('Screencast.startVideoRecording', {
+      const {screencastId} = await this._pageProxySession.send('Screencast.start', {
         file: options.outputFile,
         width: options.width,
         height: options.height,
         scale: options.scale,
-      });
-      this._browserContext.emit(BrowserContext.Events.ScreencastStarted, new Screencast(options.outputFile, this._page));
+      }) as any;
+      const screencast = new Screencast(options.outputFile, this._page);
+      this._browserContext._idToScreencast.set(screencastId, screencast);
+      this._browserContext.emit(BrowserContext.Events.ScreencastStarted, screencast);
     } catch (e) {
       this._recordingVideoFile = null;
       throw e;
@@ -734,10 +736,8 @@ export class WKPage implements PageDelegate {
   async stopScreencast(): Promise<void> {
     if (!this._recordingVideoFile)
       throw new Error('No video recording in progress');
-    const fileName = this._recordingVideoFile;
     this._recordingVideoFile = null;
-    await this._pageProxySession.send('Screencast.stopVideoRecording');
-    this._browserContext.emit(BrowserContext.Events.ScreencastStopped, new Screencast(fileName, this._initializedPage!));
+    await this._pageProxySession.send('Screencast.stop');
   }
 
   async takeScreenshot(format: string, documentRect: types.Rect | undefined, viewportRect: types.Rect | undefined, quality: number | undefined): Promise<Buffer> {

test/screencast.spec.ts

@@ -21,6 +21,7 @@ import { Page } from '..';
 import fs from 'fs';
 import path from 'path';
 import url from 'url';
+import { tmpdir } from 'os';
 
 
 declare global {
@@ -265,18 +266,17 @@ it.fail(options.CHROMIUM)('should fire start/stop events when page created/close
   await context._enableScreencast({width: 640, height: 480, dir: tmpDir});
   expect(context._screencastOptions).toBeTruthy();
 
-  const [startEvent, newPage] = await Promise.all([
+  const [screencast, newPage] = await Promise.all([
     new Promise(resolve => context.on('screencaststarted', resolve)) as Promise<any>,
     context.newPage(),
   ]);
-  expect(startEvent.page === newPage).toBe(true);
-  expect(startEvent.path).toBeTruthy();
+  expect(screencast.page === newPage).toBe(true);
 
-  const [stopEvent] = await Promise.all([
-    new Promise(resolve => context.on('screencaststopped', resolve)) as Promise<any>,
+  const [videoFile] = await Promise.all([
+    screencast.path(),
     newPage.close(),
   ]);
-  expect(stopEvent.page === newPage).toBe(true);
+  expect(path.dirname(videoFile)).toBe(tmpDir);
   await context.close();
 });
 
@@ -290,7 +290,7 @@ it.fail(options.CHROMIUM)('should fire start event for popups', async({browser,
 
   const page = await context.newPage();
   await page.mainFrame().goto(server.EMPTY_PAGE);
-  const [startEvent, popup] = await Promise.all([
+  const [screencast, popup] = await Promise.all([
     new Promise(resolve => context.on('screencaststarted', resolve)) as Promise<any>,
     new Promise(resolve => context.on('page', resolve)) as Promise<any>,
     page.mainFrame()._evaluateExpression(() => {
@@ -298,7 +298,7 @@ it.fail(options.CHROMIUM)('should fire start event for popups', async({browser,
       win.close();
     }, true)
   ]);
-  expect(startEvent.path).toBeTruthy();
-  expect(startEvent.page === popup).toBe(true);
+  expect(screencast.page === popup).toBe(true);
+  expect(path.dirname(await screencast.path())).toBe(tmpDir);
   await context.close();
 });