chore: update video api and implementation

docs/src/api/class-video.md

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

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([options])](https://playwright.dev/docs/api/class-video#video-stop), or to access the video file when
-   * using the `recordVideo` context option.
+   * [video.stop()](https://playwright.dev/docs/api/class-video#video-stop), or to access the video file when using the
+   * `recordVideo` context option.
    */
   video(): Video;
 
@@ -21835,13 +21835,14 @@ export interface Tracing {
  * ```
  *
  * Alternatively, you can use [video.start([options])](https://playwright.dev/docs/api/class-video#video-start) and
- * [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.
+ * [video.stop()](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({ path: 'video.webm' });
+ * await page.video().stop();
+ * await page.video().saveAs('video.webm');
  * ```
  *
  */
@@ -21897,16 +21898,11 @@ export interface Video {
 
   /**
    * Stops video recording started with
-   * [video.start([options])](https://playwright.dev/docs/api/class-video#video-start) and either saves or discards the
-   * video file.
-   * @param options
+   * [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.
    */
-  stop(options?: {
-    /**
-     * Path where the video should be saved.
-     */
-    path?: string;
-  }): Promise<void>;
+  stop(): Promise<void>;
 }
 
 /**

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

@@ -99,7 +99,7 @@ export class Page extends ChannelOwner<channels.PageChannel> implements api.Page
 
   readonly _bindings = new Map<string, (source: structs.BindingSource, ...args: any[]) => any>();
   readonly _timeoutSettings: TimeoutSettings;
-  private _video: Video | null = null;
+  private _video: Video;
   readonly _opener: Page | null;
   private _closeReason: string | undefined;
   _closeWasCalled: boolean = false;
@@ -133,6 +133,7 @@ export class Page extends ChannelOwner<channels.PageChannel> implements api.Page
     this._viewportSize = initializer.viewportSize;
     this._closed = initializer.isClosed;
     this._opener = Page.fromNullable(initializer.opener);
+    this._video = new Video(this, this._connection, initializer.video ? Artifact.from(initializer.video) : undefined);
 
     this._channel.on('bindingCall', ({ binding }) => this._onBinding(BindingCall.from(binding)));
     this._channel.on('close', () => this._onClose());
@@ -147,10 +148,6 @@ export class Page extends ChannelOwner<channels.PageChannel> implements api.Page
     this._channel.on('locatorHandlerTriggered', ({ uid }) => this._onLocatorHandlerTriggered(uid));
     this._channel.on('route', ({ route }) => this._onRoute(Route.from(route)));
     this._channel.on('webSocketRoute', ({ webSocketRoute }) => this._onWebSocketRoute(WebSocketRoute.from(webSocketRoute)));
-    this._channel.on('video', ({ artifact }) => {
-      const artifactObject = Artifact.from(artifact);
-      this._forceVideo()._artifactReady(artifactObject);
-    });
     this._channel.on('viewportSizeChanged', ({ viewportSize }) => this._viewportSize = viewportSize);
     this._channel.on('webSocket', ({ webSocket }) => this.emit(Events.Page.WebSocket, WebSocket.from(webSocket)));
     this._channel.on('worker', ({ worker }) => this._onWorker(Worker.from(worker)));
@@ -282,17 +279,8 @@ export class Page extends ChannelOwner<channels.PageChannel> implements api.Page
     this._timeoutSettings.setDefaultTimeout(timeout);
   }
 
-  private _forceVideo(): Video {
-    if (!this._video)
-      this._video = new Video(this, this._connection);
-    return this._video;
-  }
-
   video(): Video {
-    // Note: we are creating Video object lazily, because we do not know
-    // BrowserContextOptions when constructing the page - it is assigned
-    // too late during launchPersistentContext.
-    return this._forceVideo();
+    return this._video;
   }
 
   async $(selector: string, options?: { strict?: boolean }): Promise<ElementHandle<SVGElement | HTMLElement> | null> {

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

@@ -14,68 +14,48 @@
  * limitations under the License.
  */
 
-import { ManualPromise } from '../utils/isomorphic/manualPromise';
 import { Artifact } from './artifact';
 
 import type { Connection } from './connection';
 import type { Page } from './page';
 import type * as api from '../../types/types';
 
 export class Video implements api.Video {
-  private _artifact: Promise<Artifact | null> | null = null;
-  private _artifactReadyPromise: ManualPromise<Artifact>;
+  private _artifact: Artifact | undefined;
   private _isRemote = false;
   private _page: Page;
-  private _path: string | undefined;
 
-  constructor(page: Page, connection: Connection) {
+  constructor(page: Page, connection: Connection, artifact: Artifact | undefined) {
     this._page = page;
     this._isRemote = connection.isRemote();
-    this._artifactReadyPromise = new ManualPromise<Artifact>();
-    this._artifact = page._closedOrCrashedScope.safeRace(this._artifactReadyPromise);
-  }
-
-  _artifactReady(artifact: Artifact) {
-    this._artifactReadyPromise.resolve(artifact);
+    this._artifact = artifact;
   }
 
   async start(options: { size?: { width: number, height: number } } = {}): Promise<void> {
     const result = await this._page._channel.videoStart(options);
-    this._path = result.path;
-    this._artifactReadyPromise = new ManualPromise<Artifact>();
-    this._artifact = this._page._closedOrCrashedScope.safeRace(this._artifactReadyPromise);
+    this._artifact = Artifact.from(result.artifact);
   }
 
-  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 stop(): Promise<void> {
+    await this._page._channel.videoStop();
   }
 
   async path(): Promise<string> {
     if (this._isRemote)
       throw new Error(`Path is not available when connecting remotely. Use saveAs() to save a local copy.`);
-    if (this._path)
-      return this._path;
-
-    const artifact = await this._artifact;
-    if (!artifact)
-      throw new Error('Page did not produce any video frames');
-    return artifact._initializer.absolutePath;
+    if (!this._artifact)
+      throw new Error('Video recording has not been started.');
+    return this._artifact._initializer.absolutePath;
   }
 
   async saveAs(path: string): Promise<void> {
-    const artifact = await this._artifact;
-    if (!artifact)
-      throw new Error('Page did not produce any video frames');
-    return await artifact.saveAs(path);
+    if (!this._artifact)
+      throw new Error('Video recording has not been started.');
+    return await this._artifact.saveAs(path);
   }
 
   async delete(): Promise<void> {
-    const artifact = await this._artifact;
-    if (artifact)
-      await artifact.delete();
+    if (this._artifact)
+      await this._artifact.delete();
   }
 }

packages/playwright-core/src/protocol/validator.ts

@@ -928,9 +928,6 @@ scheme.BrowserContextRouteEvent = tObject({
 scheme.BrowserContextWebSocketRouteEvent = tObject({
   webSocketRoute: tChannel(['WebSocketRoute']),
 });
-scheme.BrowserContextVideoEvent = tObject({
-  artifact: tChannel(['Artifact']),
-});
 scheme.BrowserContextServiceWorkerEvent = tObject({
   worker: tChannel(['Worker']),
 });
@@ -1167,6 +1164,7 @@ scheme.PageInitializer = tObject({
   })),
   isClosed: tBoolean,
   opener: tOptional(tChannel(['Page'])),
+  video: tOptional(tChannel(['Artifact'])),
 });
 scheme.PageBindingCallEvent = tObject({
   binding: tChannel(['BindingCall']),
@@ -1203,9 +1201,6 @@ scheme.PageRouteEvent = tObject({
 scheme.PageWebSocketRouteEvent = tObject({
   webSocketRoute: tChannel(['WebSocketRoute']),
 });
-scheme.PageVideoEvent = tObject({
-  artifact: tChannel(['Artifact']),
-});
 scheme.PageWebSocketEvent = tObject({
   webSocket: tChannel(['WebSocket']),
 });
@@ -1510,7 +1505,7 @@ scheme.PageVideoStartParams = tObject({
   })),
 });
 scheme.PageVideoStartResult = tObject({
-  path: tString,
+  artifact: tChannel(['Artifact']),
 });
 scheme.PageVideoStopParams = tOptional(tObject({}));
 scheme.PageVideoStopResult = tOptional(tObject({}));

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

@@ -151,16 +151,11 @@ export abstract class Browser extends SdkObject {
     this._downloads.delete(uuid);
   }
 
-  _videoStarted(context: BrowserContext, videoId: string, path: string, pageOrError: Promise<Page | Error>) {
-    const artifact = new Artifact(context, path);
-    this._idToVideo.set(videoId, { context, artifact });
-    pageOrError.then(page => {
-      if (page instanceof Page) {
-        page.video = artifact;
-        page.emitOnContext(BrowserContext.Events.VideoStarted, artifact);
-        page.emit(Page.Events.Video, artifact);
-      }
-    });
+  _videoStarted(page: Page, videoId: string, path: string) {
+    const artifact = new Artifact(page.browserContext, path);
+    page.video = artifact;
+    this._idToVideo.set(videoId, { context: page.browserContext, artifact });
+    return artifact;
   }
 
   _takeVideo(videoId: string): Artifact | undefined {

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

@@ -63,7 +63,6 @@ const BrowserContextEvent = {
   RequestFulfilled: 'requestfulfilled',
   RequestContinued: 'requestcontinued',
   BeforeClose: 'beforeclose',
-  VideoStarted: 'videostarted',
   RecorderEvent: 'recorderevent',
 } as const;
 
@@ -80,7 +79,6 @@ export type BrowserContextEventMap = {
   [BrowserContextEvent.RequestFulfilled]: [request: network.Request];
   [BrowserContextEvent.RequestContinued]: [request: network.Request];
   [BrowserContextEvent.BeforeClose]: [];
-  [BrowserContextEvent.VideoStarted]: [artifact: Artifact];
   [BrowserContextEvent.RecorderEvent]: [event: { event: 'actionAdded' | 'actionUpdated' | 'signalAdded', data: any, page: Page, code: string }];
 };
 

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

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

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

@@ -32,7 +32,7 @@ export class ArtifactDispatcher extends Dispatcher<Artifact, channels.ArtifactCh
     return ArtifactDispatcher.fromNullable(parentScope, artifact)!;
   }
 
-  static fromNullable(parentScope: DispatcherScope, artifact: Artifact): ArtifactDispatcher | undefined {
+  static fromNullable(parentScope: DispatcherScope, artifact: Artifact | undefined): ArtifactDispatcher | undefined {
     if (!artifact)
       return undefined;
     const result = parentScope.connection.existingDispatcher<ArtifactDispatcher>(artifact);

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

@@ -37,7 +37,6 @@ import { RecorderApp } from '../recorder/recorderApp';
 import { ElementHandleDispatcher } from './elementHandlerDispatcher';
 import { JSHandleDispatcher } from './jsHandleDispatcher';
 
-import type { Artifact } from '../artifact';
 import type { ConsoleMessage } from '../console';
 import type { Dialog } from '../dialog';
 import type { Request, Response, RouteHandler } from '../network';
@@ -97,18 +96,6 @@ export class BrowserContextDispatcher extends Dispatcher<BrowserContext, channel
     // Note: when launching persistent context, or connecting to an existing browser,
     // dispatcher is created very late, so we can already have pages, videos and everything else.
 
-    const onVideo = (artifact: Artifact) => {
-      // Note: Video must outlive Page and BrowserContext, so that client can saveAs it
-      // after closing the context. We use |scope| for it.
-      const artifactDispatcher = ArtifactDispatcher.from(parentScope, artifact);
-      this._dispatchEvent('video', { artifact: artifactDispatcher });
-    };
-    this.addObjectListener(BrowserContext.Events.VideoStarted, onVideo);
-    for (const video of context._browser._idToVideo.values()) {
-      if (video.context === context)
-        onVideo(video.artifact);
-    }
-
     for (const page of context.pages())
       this._dispatchEvent('page', { page: PageDispatcher.from(this, page) });
     this.addObjectListener(BrowserContext.Events.Page, page => {

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

@@ -80,7 +80,8 @@ export class PageDispatcher extends Dispatcher<Page, channels.PageChannel, Brows
       mainFrame,
       viewportSize: page.emulatedSize()?.viewport,
       isClosed: page.isClosed(),
-      opener: PageDispatcher.fromNullable(parentScope, page.opener())
+      opener: PageDispatcher.fromNullable(parentScope, page.opener()),
+      video: page.video ? createVideoDispatcher(parentScope, page.video) : undefined,
     });
 
     this.adopt(mainFrame);
@@ -114,9 +115,6 @@ export class PageDispatcher extends Dispatcher<Page, channels.PageChannel, Brows
     this.addObjectListener(Page.Events.LocatorHandlerTriggered, (uid: number) => this._dispatchEvent('locatorHandlerTriggered', { uid }));
     this.addObjectListener(Page.Events.WebSocket, webSocket => this._dispatchEvent('webSocket', { webSocket: new WebSocketDispatcher(this, webSocket) }));
     this.addObjectListener(Page.Events.Worker, worker => this._dispatchEvent('worker', { worker: new WorkerDispatcher(this, worker) }));
-    this.addObjectListener(Page.Events.Video, (artifact: Artifact) => this._dispatchEvent('video', { artifact: ArtifactDispatcher.from(parentScope, artifact) }));
-    if (page.video)
-      this._dispatchEvent('video', { artifact: ArtifactDispatcher.from(this.parentScope(), page.video) });
     // Ensure client knows about all frames.
     const frames = page.frameManager.frames();
     for (let i = 1; i < frames.length; i++)
@@ -339,7 +337,8 @@ export class PageDispatcher extends Dispatcher<Page, channels.PageChannel, Brows
   }
 
   async videoStart(params: channels.PageVideoStartParams, progress: Progress): Promise<channels.PageVideoStartResult> {
-    return await this._page.screencast.startExplicitVideoRecording(params);
+    const artifact = await this._page.screencast.startExplicitVideoRecording(params);
+    return { artifact: createVideoDispatcher(this.parentScope(), artifact) };
   }
 
   async videoStop(params: channels.PageVideoStopParams, progress: Progress): Promise<channels.PageVideoStopResult> {
@@ -481,3 +480,9 @@ export class BindingCallDispatcher extends Dispatcher<SdkObject, channels.Bindin
     this._dispose();
   }
 }
+
+function createVideoDispatcher(parentScope: BrowserContextDispatcher, video: Artifact): ArtifactDispatcher {
+  // Note: Video must outlive Page and BrowserContext, so that client can saveAs it
+  // after closing the context. We use |scope| for it.
+  return ArtifactDispatcher.from(parentScope.parentScope(), video);
+}