chore: follow up to video api changes

docs/src/api/class-video.md

@@ -28,36 +28,31 @@ Alternatively, you can use [`method: Video.start`] and [`method: Video.stop`] to
 ```js
 await page.video().start();
 // ... perform actions ...
-await page.video().stop();
-await page.video().saveAs('video.webm');
+await page.video().stop({ path: 'video.webm' });
 ```
 
 ```java
 page.video().start();
 // ... perform actions ...
-page.video().stop();
-page.video().saveAs(Paths.get("video.webm"));
+page.video().stop(new Video.StopOptions().setPath(Paths.get("video.webm")));
 ```
 
 ```python async
 await page.video.start()
 # ... perform actions ...
-await page.video.stop()
-await page.video.save_as("video.webm")
+await page.video.stop(path="video.webm")
 ```
 
 ```python sync
 page.video.start()
 # ... perform actions ...
-page.video.stop()
-page.video.save_as("video.webm")
+page.video.stop(path="video.webm")
 ```
 
 ```csharp
 await page.Video.StartAsync();
 // ... perform actions ...
-await page.Video.StopAsync();
-await page.Video.SaveAsAsync("video.webm");
+await page.Video.StopAsync(new() { Path = "video.webm" });
 ```
 
 ## async method: Video.delete
@@ -145,4 +140,10 @@ Optional dimensions of the recorded video. If not specified the size will be equ
 ## async method: Video.stop
 * since: v1.59
 
-Stops video recording started with [`method: Video.start`]. Use [`method: Video.path`] or [`method: Video.saveAs`] methods to access the video file.
+Stops video recording started with [`method: Video.start`].
+
+### option: Video.stop.path
+* since: v1.59
+- `path` <[path]>
+
+Path where the video should be saved.

packages/playwright-client/types/types.d.ts

@@ -4808,8 +4808,8 @@ export interface Page {
   /**
    * Video object associated with this page. Can be used to control video recording with
    * [video.start([options])](https://playwright.dev/docs/api/class-video#video-start) and
-   * [video.stop()](https://playwright.dev/docs/api/class-video#video-stop), or to access the video file when using the
-   * `recordVideo` context option.
+   * [video.stop([options])](https://playwright.dev/docs/api/class-video#video-stop), or to access the video file when
+   * using the `recordVideo` context option.
    */
   video(): Video;
 
@@ -21835,14 +21835,13 @@ export interface Tracing {
  * ```
  *
  * Alternatively, you can use [video.start([options])](https://playwright.dev/docs/api/class-video#video-start) and
- * [video.stop()](https://playwright.dev/docs/api/class-video#video-stop) to record video manually. This approach is
- * mutually exclusive with the `recordVideo` option.
+ * [video.stop([options])](https://playwright.dev/docs/api/class-video#video-stop) to record video manually. This
+ * approach is mutually exclusive with the `recordVideo` option.
  *
  * ```js
  * await page.video().start();
  * // ... perform actions ...
- * await page.video().stop();
- * await page.video().saveAs('video.webm');
+ * await page.video().stop({ path: 'video.webm' });
  * ```
  *
  */
@@ -21898,11 +21897,15 @@ export interface Video {
 
   /**
    * Stops video recording started with
-   * [video.start([options])](https://playwright.dev/docs/api/class-video#video-start). Use
-   * [video.path()](https://playwright.dev/docs/api/class-video#video-path) or
-   * [video.saveAs(path)](https://playwright.dev/docs/api/class-video#video-save-as) methods to access the video file.
+   * [video.start([options])](https://playwright.dev/docs/api/class-video#video-start).
+   * @param options
    */
-  stop(): Promise<void>;
+  stop(options?: {
+    /**
+     * Path where the video should be saved.
+     */
+    path?: string;
+  }): Promise<void>;
 }
 
 /**

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

@@ -36,8 +36,12 @@ export class Video implements api.Video {
     this._artifact = Artifact.from(result.artifact);
   }
 
-  async stop(): Promise<void> {
-    await this._page._channel.videoStop();
+  async stop(options: { path?: string } = {}): Promise<void> {
+    await this._page._wrapApiCall(async () => {
+      await this._page._channel.videoStop();
+      if (options.path)
+        await this.saveAs(options.path);
+    });
   }
 
   async path(): Promise<string> {

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

@@ -438,7 +438,7 @@ class FrameSession {
     }
 
     let videoOptions: types.VideoOptions | undefined;
-    if (this._isMainFrame() && hasUIWindow)
+    if (this._isMainFrame() && hasUIWindow && !this._page.isStorageStatePage)
       videoOptions = this._crPage._page.screencast.launchAutomaticVideoRecorder();
 
     let lifecycleEventsEnabled: Promise<any>;

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

@@ -97,14 +97,16 @@ export class FFPage implements PageDelegate {
     ];
 
     const promises: Promise<any>[] = [];
-
-    const videoOptions = this._page.screencast.launchAutomaticVideoRecorder();
+    const videoOptions = this._page.isStorageStatePage ? undefined : this._page.screencast.launchAutomaticVideoRecorder();
     if (videoOptions)
       promises.push(this._page.screencast.startVideoRecording(videoOptions));
-    promises.push(this.addInitScript(new InitScript(''), UTILITY_WORLD_NAME));
     promises.push(new Promise(f => this._session.once('Page.ready', f)));
-
     Promise.all(promises).then(() => this._reportAsNew(), error => this._reportAsNew(error));
+
+    // Ideally, we somehow ensure that utility world is created before Page.ready arrives, but currently it is racy.
+    // Even worse, sometimes this protocol call never returns, for example when popup opens a dialog synchronously.
+    // Therefore, we can end up with an initialized page without utility world, although very unlikely.
+    this.addInitScript(new InitScript(''), UTILITY_WORLD_NAME).catch(e => this._reportAsNew(e));
   }
 
   _reportAsNew(error?: Error) {

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

@@ -61,7 +61,7 @@ export class Screencast {
   // and it is equally important to send Screencast.startScreencast before sending Target.resume.
   launchAutomaticVideoRecorder(): types.VideoOptions | undefined {
     const recordVideo = this._page.browserContext._options.recordVideo;
-    if (!recordVideo || this._page.isStorageStatePage)
+    if (!recordVideo)
       return;
     // validateBrowserContextOptions ensures correct video size.
     return this._launchVideoRecorder(recordVideo.dir, recordVideo.size!);

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

@@ -847,7 +847,7 @@ export class WKPage implements PageDelegate {
 
   private async _initializeVideoRecording() {
     const screencast = this._page.screencast;
-    const videoOptions = screencast.launchAutomaticVideoRecorder();
+    const videoOptions = this._page.isStorageStatePage ? undefined : screencast.launchAutomaticVideoRecorder();
     if (videoOptions)
       await screencast.startVideoRecording(videoOptions);
   }

packages/playwright-core/types/types.d.ts

@@ -4808,8 +4808,8 @@ export interface Page {
   /**
    * Video object associated with this page. Can be used to control video recording with
    * [video.start([options])](https://playwright.dev/docs/api/class-video#video-start) and
-   * [video.stop()](https://playwright.dev/docs/api/class-video#video-stop), or to access the video file when using the
-   * `recordVideo` context option.
+   * [video.stop([options])](https://playwright.dev/docs/api/class-video#video-stop), or to access the video file when
+   * using the `recordVideo` context option.
    */
   video(): Video;
 
@@ -21835,14 +21835,13 @@ export interface Tracing {
  * ```
  *
  * Alternatively, you can use [video.start([options])](https://playwright.dev/docs/api/class-video#video-start) and
- * [video.stop()](https://playwright.dev/docs/api/class-video#video-stop) to record video manually. This approach is
- * mutually exclusive with the `recordVideo` option.
+ * [video.stop([options])](https://playwright.dev/docs/api/class-video#video-stop) to record video manually. This
+ * approach is mutually exclusive with the `recordVideo` option.
  *
  * ```js
  * await page.video().start();
  * // ... perform actions ...
- * await page.video().stop();
- * await page.video().saveAs('video.webm');
+ * await page.video().stop({ path: 'video.webm' });
  * ```
  *
  */
@@ -21898,11 +21897,15 @@ export interface Video {
 
   /**
    * Stops video recording started with
-   * [video.start([options])](https://playwright.dev/docs/api/class-video#video-start). Use
-   * [video.path()](https://playwright.dev/docs/api/class-video#video-path) or
-   * [video.saveAs(path)](https://playwright.dev/docs/api/class-video#video-save-as) methods to access the video file.
+   * [video.start([options])](https://playwright.dev/docs/api/class-video#video-start).
+   * @param options
    */
-  stop(): Promise<void>;
+  stop(options?: {
+    /**
+     * Path where the video should be saved.
+     */
+    path?: string;
+  }): Promise<void>;
 }
 
 /**

tests/library/video.spec.ts

@@ -830,14 +830,12 @@ it.describe('screencast', () => {
     const videoPath2 = await page.video().path();
     expect(videoPath2).toBeDefined();
     expect(videoPath2).not.toEqual(videoPath1);
-    await page.video().stop();
-    expectFrames(videoPath2, size, isAlmostGray);
-
     const videoPath3 = testInfo.outputPath('video3.webm');
-    await page.video().saveAs(videoPath3);
+    await page.video().stop({ path: videoPath3 });
     const contents2 = fs.readFileSync(videoPath2).toString('base64');
     const contents3 = fs.readFileSync(videoPath3).toString('base64');
     expect(contents2 === contents3).toBeTruthy();
+    expectFrames(videoPath3, size, isAlmostGray);
 
     await context.close();
   });
@@ -851,8 +849,7 @@ it.describe('screencast', () => {
     const page = await context.newPage();
     const error = await page.video().start().catch(e => e);
     expect(error.message).toContain('Video is already being recorded');
-    await page.video().stop();
-    await page.video().saveAs(testInfo.outputPath('video.webm'));
+    await page.video().stop({ path: testInfo.outputPath('video.webm') });
     await context.close();
   });
 
@@ -895,9 +892,8 @@ it.describe('screencast', () => {
     const context = await browser.newContext({ viewport: size });
     const page = await context.newPage();
     await page.video().start({ size });
-    await page.video().stop();
     const videoPath = testInfo.outputPath('empty-video.webm');
-    await page.video().saveAs(videoPath);
+    await page.video().stop({ path: videoPath });
     await context.close();
     expectFrames(videoPath, size, isAlmostWhite);
   });