From 9d9599c6a6b1b03b16317c2d708711f105024dcb Mon Sep 17 00:00:00 2001 From: Dmitry Gozman Date: Wed, 31 Mar 2021 10:38:05 -0700 Subject: [PATCH] api(video): implement video.saveAs and video.delete (#6005) These methods are safe to call while the page is still open, or when it is already closed. Works in remotely connected browser as well. Also makes video.path() to throw for remotely connected browser. Under the hood migrated Download and Video to use the common Artifact object. --- docs/src/api/class-download.md | 4 +- docs/src/api/class-video.md | 18 ++- src/browserServerImpl.ts | 22 ---- src/client/artifact.ts | 79 ++++++++++++ src/client/connection.ts | 8 +- src/client/download.ts | 67 +++------- src/client/page.ts | 35 ++++-- src/client/video.ts | 43 +++++-- ...oadDispatcher.ts => artifactDispatcher.ts} | 31 +++-- src/dispatchers/browserContextDispatcher.ts | 16 +++ src/dispatchers/pageDispatcher.ts | 17 ++- src/protocol/channels.ts | 74 ++++++----- src/protocol/protocol.yml | 29 ++--- src/protocol/validator.ts | 12 +- src/server/artifact.ts | 117 ++++++++++++++++++ src/server/browser.ts | 25 ++-- src/server/browserContext.ts | 42 ++----- src/server/chromium/crPage.ts | 7 +- src/server/download.ts | 105 +--------------- src/server/firefox/ffPage.ts | 2 + src/server/page.ts | 14 +-- test/browsertype-connect.spec.ts | 9 +- test/download.spec.ts | 10 +- test/screencast.spec.ts | 71 +++++++++++ types/types.d.ts | 20 ++- 25 files changed, 527 insertions(+), 350 deletions(-) create mode 100644 src/client/artifact.ts rename src/dispatchers/{downloadDispatcher.ts => artifactDispatcher.ts} (72%) create mode 100644 src/server/artifact.ts diff --git a/docs/src/api/class-download.md b/docs/src/api/class-download.md index 41859c8731..055dbaefa7 100644 --- a/docs/src/api/class-download.md +++ b/docs/src/api/class-download.md @@ -18,7 +18,7 @@ const path = await download.path(); ```java // wait for download to start -Download download = page.waitForDownload(() -> page.click("a")); +Download download = page.waitForDownload(() -> page.click("a")); // wait for download to complete Path path = download.path(); ``` @@ -73,7 +73,7 @@ Returns download error if any. Will wait for the download to finish if necessary - returns: <[null]|[path]> Returns path to the downloaded file in case of successful download. The method will -wait for the download to finish if necessary. +wait for the download to finish if necessary. The method throws when connected remotely via [`method: BrowserType.connect`]. ## async method: Download.saveAs diff --git a/docs/src/api/class-video.md b/docs/src/api/class-video.md index 275e3b1c01..626a9183cd 100644 --- a/docs/src/api/class-video.md +++ b/docs/src/api/class-video.md @@ -1,6 +1,6 @@ # class: Video -When browser context is created with the `videosPath` option, each page has a video object associated with it. +When browser context is created with the `recordVideo` option, each page has a video object associated with it. ```js console.log(await page.video().path()); @@ -18,8 +18,22 @@ print(await page.video.path()) print(page.video.path()) ``` +## async method: Video.delete + +Deletes the video file. Will wait for the video to finish if necessary. + ## async method: Video.path - returns: <[path]> Returns the file system path this video will be recorded to. The video is guaranteed to be written to the filesystem -upon closing the browser context. \ No newline at end of file +upon closing the browser context. This method throws when connected remotely via [`method: BrowserType.connect`]. + +## async method: Video.saveAs + +Saves the video to a user-specified path. It is safe to call this method while the video +is still in progress, or after the page has closed. This method waits until the page is closed and the video is fully saved. + +### param: Video.saveAs.path +- `path` <[path]> + +Path where the video should be saved. diff --git a/src/browserServerImpl.ts b/src/browserServerImpl.ts index eb035e43cf..b7b2e28214 100644 --- a/src/browserServerImpl.ts +++ b/src/browserServerImpl.ts @@ -17,7 +17,6 @@ import { LaunchServerOptions, Logger } from './client/types'; import { BrowserType } from './server/browserType'; import * as ws from 'ws'; -import fs from 'fs'; import { Browser } from './server/browser'; import { ChildProcess } from 'child_process'; import { EventEmitter } from 'ws'; @@ -30,8 +29,6 @@ import { envObjectToArray } from './client/clientHelper'; import { createGuid } from './utils/utils'; import { SelectorsDispatcher } from './dispatchers/selectorsDispatcher'; import { Selectors } from './server/selectors'; -import { BrowserContext, Video } from './server/browserContext'; -import { StreamDispatcher } from './dispatchers/streamDispatcher'; import { ProtocolLogger } from './server/types'; import { CallMetadata, internalCallMetadata, SdkObject } from './server/instrumentation'; @@ -163,7 +160,6 @@ class ConnectedBrowser extends BrowserDispatcher { } const result = await super.newContext(params, metadata); const dispatcher = result.context as BrowserContextDispatcher; - dispatcher._object.on(BrowserContext.Events.VideoStarted, (video: Video) => this._sendVideo(dispatcher, video)); dispatcher._object._setSelectors(this._selectors); this._contexts.push(dispatcher); return result; @@ -184,24 +180,6 @@ class ConnectedBrowser extends BrowserDispatcher { super._didClose(); } } - - private _sendVideo(contextDispatcher: BrowserContextDispatcher, video: Video) { - video._waitForCallbackOnFinish(async () => { - const readable = fs.createReadStream(video._path); - await new Promise(f => readable.on('readable', f)); - const stream = new StreamDispatcher(this._remoteBrowser!._scope, readable); - this._remoteBrowser!._dispatchEvent('video', { - stream, - context: contextDispatcher, - relativePath: video._relativePath - }); - await new Promise(resolve => { - readable.on('close', resolve); - readable.on('end', resolve); - readable.on('error', resolve); - }); - }); - } } function toProtocolLogger(logger: Logger | undefined): ProtocolLogger | undefined { diff --git a/src/client/artifact.ts b/src/client/artifact.ts new file mode 100644 index 0000000000..358cf84dcf --- /dev/null +++ b/src/client/artifact.ts @@ -0,0 +1,79 @@ +/** + * Copyright (c) Microsoft Corporation. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import * as channels from '../protocol/channels'; +import * as fs from 'fs'; +import { Stream } from './stream'; +import { mkdirIfNeeded } from '../utils/utils'; +import { ChannelOwner } from './channelOwner'; +import { Readable } from 'stream'; + +export class Artifact extends ChannelOwner { + _isRemote = false; + _apiName: string = ''; + + static from(channel: channels.ArtifactChannel): Artifact { + return (channel as any)._object; + } + + async pathAfterFinished(): Promise { + if (this._isRemote) + throw new Error(`Path is not available when using browserType.connect(). Use saveAs() to save a local copy.`); + return this._wrapApiCall(`${this._apiName}.path`, async (channel: channels.ArtifactChannel) => { + return (await channel.pathAfterFinished()).value || null; + }); + } + + async saveAs(path: string): Promise { + return this._wrapApiCall(`${this._apiName}.saveAs`, async (channel: channels.ArtifactChannel) => { + if (!this._isRemote) { + await channel.saveAs({ path }); + return; + } + + const result = await channel.saveAsStream(); + const stream = Stream.from(result.stream); + await mkdirIfNeeded(path); + await new Promise((resolve, reject) => { + stream.stream().pipe(fs.createWriteStream(path)) + .on('finish' as any, resolve) + .on('error' as any, reject); + }); + }); + } + + async failure(): Promise { + return this._wrapApiCall(`${this._apiName}.failure`, async (channel: channels.ArtifactChannel) => { + return (await channel.failure()).error || null; + }); + } + + async createReadStream(): Promise { + return this._wrapApiCall(`${this._apiName}.createReadStream`, async (channel: channels.ArtifactChannel) => { + const result = await channel.stream(); + if (!result.stream) + return null; + const stream = Stream.from(result.stream); + return stream.stream(); + }); + } + + async delete(): Promise { + return this._wrapApiCall(`${this._apiName}.delete`, async (channel: channels.ArtifactChannel) => { + return channel.delete(); + }); + } +} diff --git a/src/client/connection.ts b/src/client/connection.ts index 0460f7e2c4..bebab7e206 100644 --- a/src/client/connection.ts +++ b/src/client/connection.ts @@ -26,7 +26,6 @@ import { Page, BindingCall } from './page'; import { Worker } from './worker'; import { ConsoleMessage } from './consoleMessage'; import { Dialog } from './dialog'; -import { Download } from './download'; import { parseError } from '../protocol/serializers'; import { CDPSession } from './cdpSession'; import { Playwright } from './playwright'; @@ -42,6 +41,7 @@ import { SelectorsOwner } from './selectors'; import { isUnderTest } from '../utils/utils'; import { Android, AndroidSocket, AndroidDevice } from './android'; import { captureStackTrace } from '../utils/stackTrace'; +import { Artifact } from './artifact'; class Root extends ChannelOwner { constructor(connection: Connection) { @@ -156,6 +156,9 @@ export class Connection { case 'AndroidDevice': result = new AndroidDevice(parent, type, guid, initializer); break; + case 'Artifact': + result = new Artifact(parent, type, guid, initializer); + break; case 'BindingCall': result = new BindingCall(parent, type, guid, initializer); break; @@ -191,9 +194,6 @@ export class Connection { case 'Dialog': result = new Dialog(parent, type, guid, initializer); break; - case 'Download': - result = new Download(parent, type, guid, initializer); - break; case 'Electron': result = new Electron(parent, type, guid, initializer); break; diff --git a/src/client/download.ts b/src/client/download.ts index 3db51a7412..c34295fd9b 100644 --- a/src/client/download.ts +++ b/src/client/download.ts @@ -14,81 +14,46 @@ * limitations under the License. */ -import * as channels from '../protocol/channels'; -import { ChannelOwner } from './channelOwner'; import { Readable } from 'stream'; -import { Stream } from './stream'; -import { Browser } from './browser'; -import { BrowserContext } from './browserContext'; -import fs from 'fs'; -import { mkdirIfNeeded } from '../utils/utils'; import * as api from '../../types/types'; +import { Artifact } from './artifact'; -export class Download extends ChannelOwner implements api.Download { - private _browser: Browser | null; +export class Download implements api.Download { + private _url: string; + private _suggestedFilename: string; + private _artifact: Artifact; - static from(download: channels.DownloadChannel): Download { - return (download as any)._object; - } - - constructor(parent: ChannelOwner, type: string, guid: string, initializer: channels.DownloadInitializer) { - super(parent, type, guid, initializer); - this._browser = (parent as BrowserContext)._browser; + constructor(url: string, suggestedFilename: string, artifact: Artifact) { + this._url = url; + this._suggestedFilename = suggestedFilename; + this._artifact = artifact; } url(): string { - return this._initializer.url; + return this._url; } suggestedFilename(): string { - return this._initializer.suggestedFilename; + return this._suggestedFilename; } async path(): Promise { - if (this._browser && this._browser._isRemote) - throw new Error(`Path is not available when using browserType.connect(). Use download.saveAs() to save a local copy.`); - return this._wrapApiCall('download.path', async (channel: channels.DownloadChannel) => { - return (await channel.path()).value || null; - }); + return this._artifact.pathAfterFinished(); } async saveAs(path: string): Promise { - return this._wrapApiCall('download.saveAs', async (channel: channels.DownloadChannel) => { - if (!this._browser || !this._browser._isRemote) { - await channel.saveAs({ path }); - return; - } - - const result = await channel.saveAsStream(); - const stream = Stream.from(result.stream); - await mkdirIfNeeded(path); - await new Promise((resolve, reject) => { - stream.stream().pipe(fs.createWriteStream(path)) - .on('finish' as any, resolve) - .on('error' as any, reject); - }); - }); + return this._artifact.saveAs(path); } async failure(): Promise { - return this._wrapApiCall('download.failure', async (channel: channels.DownloadChannel) => { - return (await channel.failure()).error || null; - }); + return this._artifact.failure(); } async createReadStream(): Promise { - return this._wrapApiCall('download.createReadStream', async (channel: channels.DownloadChannel) => { - const result = await channel.stream(); - if (!result.stream) - return null; - const stream = Stream.from(result.stream); - return stream.stream(); - }); + return this._artifact.createReadStream(); } async delete(): Promise { - return this._wrapApiCall('download.delete', async (channel: channels.DownloadChannel) => { - return channel.delete(); - }); + return this._artifact.delete(); } } diff --git a/src/client/page.ts b/src/client/page.ts index 302bec6882..bc56a9d10d 100644 --- a/src/client/page.ts +++ b/src/client/page.ts @@ -47,6 +47,7 @@ import { isString, isRegExp, isObject, mkdirIfNeeded, headersObjectToArray } fro import { isSafeCloseError } from '../utils/errors'; import { Video } from './video'; import type { ChromiumBrowserContext } from './chromiumBrowserContext'; +import { Artifact } from './artifact'; const fsWriteFileAsync = util.promisify(fs.writeFile.bind(fs)); const mkdirAsync = util.promisify(fs.mkdir); @@ -72,6 +73,7 @@ export class Page extends ChannelOwner(); _workers = new Set(); private _closed = false; + _closedOrCrashedPromise: Promise; private _viewportSize: Size | null; private _routes: { url: URLMatch, handler: RouteHandler }[] = []; @@ -120,7 +122,11 @@ export class Page extends ChannelOwner {}); }); this._channel.on('domcontentloaded', () => this.emit(Events.Page.DOMContentLoaded, this)); - this._channel.on('download', ({ download }) => this.emit(Events.Page.Download, Download.from(download))); + this._channel.on('download', ({ url, suggestedFilename, artifact }) => { + const artifactObject = Artifact.from(artifact); + artifactObject._isRemote = !!this._browserContext._browser && this._browserContext._browser._isRemote; + this.emit(Events.Page.Download, new Download(url, suggestedFilename, artifactObject)); + }); this._channel.on('fileChooser', ({ element, isMultiple }) => this.emit(Events.Page.FileChooser, new FileChooser(this, ElementHandle.from(element), isMultiple))); this._channel.on('frameAttached', ({ frame }) => this._onFrameAttached(Frame.from(frame))); this._channel.on('frameDetached', ({ frame }) => this._onFrameDetached(Frame.from(frame))); @@ -132,7 +138,10 @@ export class Page extends ChannelOwner this._onRequestFinished(Request.from(request), responseEndTiming)); this._channel.on('response', ({ response }) => this.emit(Events.Page.Response, Response.from(response))); this._channel.on('route', ({ route, request }) => this._onRoute(Route.from(route), Request.from(request))); - this._channel.on('video', ({ relativePath }) => this.video()!._setRelativePath(relativePath)); + this._channel.on('video', ({ artifact }) => { + const artifactObject = Artifact.from(artifact); + this._forceVideo()._artifactReady(artifactObject); + }); this._channel.on('webSocket', ({ webSocket }) => this.emit(Events.Page.WebSocket, WebSocket.from(webSocket))); this._channel.on('worker', ({ worker }) => this._onWorker(Worker.from(worker))); @@ -142,6 +151,11 @@ export class Page extends ChannelOwner(f => this.once(Events.Page.Close, f)), + new Promise(f => this.once(Events.Page.Crash, f)), + ]); } private _onRequestFailed(request: Request, responseEndTiming: number, failureText: string | undefined) { @@ -247,16 +261,19 @@ export class Page extends ChannelOwner(func: () => T): T { diff --git a/src/client/video.ts b/src/client/video.ts index a80800ef21..b6f8cfe3a0 100644 --- a/src/client/video.ts +++ b/src/client/video.ts @@ -14,25 +14,48 @@ * limitations under the License. */ -import path from 'path'; import { Page } from './page'; import * as api from '../../types/types'; +import { Artifact } from './artifact'; export class Video implements api.Video { - private _page: Page; - private _pathCallback: ((path: string) => void) | undefined; - private _pathPromise: Promise; + private _artifact: Promise | null = null; + private _artifactCallback = (artifact: Artifact) => {}; + private _isRemote = false; constructor(page: Page) { - this._page = page; - this._pathPromise = new Promise(f => this._pathCallback = f); + const browser = page.context()._browser; + this._isRemote = !!browser && browser._isRemote; + this._artifact = Promise.race([ + new Promise(f => this._artifactCallback = f), + page._closedOrCrashedPromise.then(() => null), + ]); } - _setRelativePath(relativePath: string) { - this._pathCallback!(path.join(this._page.context()._options.recordVideo!.dir, relativePath)); + _artifactReady(artifact: Artifact) { + artifact._isRemote = this._isRemote; + this._artifactCallback(artifact); } - path(): Promise { - return this._pathPromise; + async path(): Promise { + if (this._isRemote) + throw new Error(`Path is not available when using browserType.connect(). Use saveAs() to save a local copy.`); + const artifact = await this._artifact; + if (!artifact) + throw new Error('Page did not produce any video frames'); + return artifact._initializer.absolutePath; + } + + async saveAs(path: string): Promise { + const artifact = await this._artifact; + if (!artifact) + throw new Error('Page did not produce any video frames'); + return artifact.saveAs(path); + } + + async delete(): Promise { + const artifact = await this._artifact; + if (artifact) + await artifact.delete(); } } diff --git a/src/dispatchers/downloadDispatcher.ts b/src/dispatchers/artifactDispatcher.ts similarity index 72% rename from src/dispatchers/downloadDispatcher.ts rename to src/dispatchers/artifactDispatcher.ts index 449522bd30..c5a6e58b2e 100644 --- a/src/dispatchers/downloadDispatcher.ts +++ b/src/dispatchers/artifactDispatcher.ts @@ -14,35 +14,33 @@ * limitations under the License. */ -import { Download } from '../server/download'; import * as channels from '../protocol/channels'; import { Dispatcher, DispatcherScope } from './dispatcher'; import { StreamDispatcher } from './streamDispatcher'; import fs from 'fs'; import * as util from 'util'; import { mkdirIfNeeded } from '../utils/utils'; +import { Artifact } from '../server/artifact'; -export class DownloadDispatcher extends Dispatcher implements channels.DownloadChannel { - constructor(scope: DispatcherScope, download: Download) { - super(scope, download, 'Download', { - url: download.url(), - suggestedFilename: download.suggestedFilename(), +export class ArtifactDispatcher extends Dispatcher implements channels.ArtifactChannel { + constructor(scope: DispatcherScope, artifact: Artifact) { + super(scope, artifact, 'Artifact', { + absolutePath: artifact.localPath(), }); } - async path(): Promise { - const path = await this._object.localPath(); + async pathAfterFinished(): Promise { + const path = await this._object.localPathAfterFinished(); return { value: path || undefined }; } - async saveAs(params: channels.DownloadSaveAsParams): Promise { + async saveAs(params: channels.ArtifactSaveAsParams): Promise { return await new Promise((resolve, reject) => { this._object.saveAs(async (localPath, error) => { if (error !== undefined) { reject(new Error(error)); return; } - try { await mkdirIfNeeded(params.path); await util.promisify(fs.copyFile)(localPath, params.path); @@ -54,21 +52,20 @@ export class DownloadDispatcher extends Dispatcher { + async saveAsStream(): Promise { return await new Promise((resolve, reject) => { this._object.saveAs(async (localPath, error) => { if (error !== undefined) { reject(new Error(error)); return; } - try { const readable = fs.createReadStream(localPath); await new Promise(f => readable.on('readable', f)); const stream = new StreamDispatcher(this._scope, readable); // Resolve with a stream, so that client starts saving the data. resolve({ stream }); - // Block the download until the stream is consumed. + // Block the Artifact until the stream is consumed. await new Promise(resolve => { readable.on('close', resolve); readable.on('end', resolve); @@ -81,8 +78,8 @@ export class DownloadDispatcher extends Dispatcher { - const fileName = await this._object.localPath(); + async stream(): Promise { + const fileName = await this._object.localPathAfterFinished(); if (!fileName) return {}; const readable = fs.createReadStream(fileName); @@ -90,8 +87,8 @@ export class DownloadDispatcher extends Dispatcher { - const error = await this._object.failure(); + async failure(): Promise { + const error = await this._object.failureError(); return { error: error || undefined }; } diff --git a/src/dispatchers/browserContextDispatcher.ts b/src/dispatchers/browserContextDispatcher.ts index b5df9fcb63..f8bbdeffab 100644 --- a/src/dispatchers/browserContextDispatcher.ts +++ b/src/dispatchers/browserContextDispatcher.ts @@ -23,6 +23,8 @@ import { CRBrowserContext } from '../server/chromium/crBrowser'; import { CDPSessionDispatcher } from './cdpSessionDispatcher'; import { RecorderSupplement } from '../server/supplements/recorderSupplement'; import { CallMetadata } from '../server/instrumentation'; +import { ArtifactDispatcher } from './artifactDispatcher'; +import { Artifact } from '../server/artifact'; export class BrowserContextDispatcher extends Dispatcher implements channels.BrowserContextChannel { private _context: BrowserContext; @@ -30,6 +32,20 @@ export class BrowserContextDispatcher extends Dispatcher { + // Note: Video must outlive Page and BrowserContext, so that client can saveAs it + // after closing the context. We use |scope| for it. + const artifactDispatcher = new ArtifactDispatcher(scope, artifact); + this._dispatchEvent('video', { artifact: artifactDispatcher }); + }; + context.on(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: new PageDispatcher(this._scope, page) }); diff --git a/src/dispatchers/pageDispatcher.ts b/src/dispatchers/pageDispatcher.ts index 24ec17014c..580c7599ba 100644 --- a/src/dispatchers/pageDispatcher.ts +++ b/src/dispatchers/pageDispatcher.ts @@ -14,16 +14,15 @@ * limitations under the License. */ -import { BrowserContext, Video } from '../server/browserContext'; +import { BrowserContext } from '../server/browserContext'; import { Frame } from '../server/frames'; import { Request } from '../server/network'; import { Page, Worker } from '../server/page'; import * as channels from '../protocol/channels'; -import { Dispatcher, DispatcherScope, lookupDispatcher, lookupNullableDispatcher } from './dispatcher'; +import { Dispatcher, DispatcherScope, existingDispatcher, lookupDispatcher, lookupNullableDispatcher } from './dispatcher'; import { parseError, serializeError } from '../protocol/serializers'; import { ConsoleMessageDispatcher } from './consoleMessageDispatcher'; import { DialogDispatcher } from './dialogDispatcher'; -import { DownloadDispatcher } from './downloadDispatcher'; import { FrameDispatcher } from './frameDispatcher'; import { RequestDispatcher, ResponseDispatcher, RouteDispatcher, WebSocketDispatcher } from './networkDispatchers'; import { serializeResult, parseArgument } from './jsHandleDispatcher'; @@ -32,6 +31,9 @@ import { FileChooser } from '../server/fileChooser'; import { CRCoverage } from '../server/chromium/crCoverage'; import { JSHandle } from '../server/javascript'; import { CallMetadata } from '../server/instrumentation'; +import { Artifact } from '../server/artifact'; +import { ArtifactDispatcher } from './artifactDispatcher'; +import { Download } from '../server/download'; export class PageDispatcher extends Dispatcher implements channels.PageChannel { private _page: Page; @@ -41,7 +43,6 @@ export class PageDispatcher extends Dispatcher i // If we split pageCreated and pageReady, there should be no main frame during pageCreated. super(scope, page, 'Page', { mainFrame: FrameDispatcher.from(scope, page.mainFrame()), - videoRelativePath: page._video ? page._video._relativePath : undefined, viewportSize: page.viewportSize() || undefined, isClosed: page.isClosed() }, true); @@ -54,7 +55,9 @@ export class PageDispatcher extends Dispatcher i page.on(Page.Events.Crash, () => this._dispatchEvent('crash')); page.on(Page.Events.DOMContentLoaded, () => this._dispatchEvent('domcontentloaded')); page.on(Page.Events.Dialog, dialog => this._dispatchEvent('dialog', { dialog: new DialogDispatcher(this._scope, dialog) })); - page.on(Page.Events.Download, download => this._dispatchEvent('download', { download: new DownloadDispatcher(scope, download) })); + page.on(Page.Events.Download, (download: Download) => { + this._dispatchEvent('download', { url: download.url, suggestedFilename: download.suggestedFilename(), artifact: new ArtifactDispatcher(scope, download.artifact) }); + }); this._page.on(Page.Events.FileChooser, (fileChooser: FileChooser) => this._dispatchEvent('fileChooser', { element: new ElementHandleDispatcher(this._scope, fileChooser.element()), isMultiple: fileChooser.isMultiple() @@ -75,9 +78,11 @@ export class PageDispatcher extends Dispatcher i responseEndTiming: request._responseEndTiming })); page.on(Page.Events.Response, response => this._dispatchEvent('response', { response: new ResponseDispatcher(this._scope, response) })); - page.on(Page.Events.VideoStarted, (video: Video) => this._dispatchEvent('video', { relativePath: video._relativePath })); page.on(Page.Events.WebSocket, webSocket => this._dispatchEvent('webSocket', { webSocket: new WebSocketDispatcher(this._scope, webSocket) })); page.on(Page.Events.Worker, worker => this._dispatchEvent('worker', { worker: new WorkerDispatcher(this._scope, worker) })); + page.on(Page.Events.Video, (artifact: Artifact) => this._dispatchEvent('video', { artifact: existingDispatcher(artifact) })); + if (page._video) + this._dispatchEvent('video', { artifact: existingDispatcher(page._video) }); } async setDefaultNavigationTimeoutNoReply(params: channels.PageSetDefaultNavigationTimeoutNoReplyParams, metadata: CallMetadata): Promise { diff --git a/src/protocol/channels.ts b/src/protocol/channels.ts index d6d40ccbad..37a8cb643e 100644 --- a/src/protocol/channels.ts +++ b/src/protocol/channels.ts @@ -187,13 +187,7 @@ export type RemoteBrowserInitializer = { selectors: SelectorsChannel, }; export interface RemoteBrowserChannel extends Channel { - on(event: 'video', callback: (params: RemoteBrowserVideoEvent) => void): this; } -export type RemoteBrowserVideoEvent = { - context: BrowserContextChannel, - stream: StreamChannel, - relativePath: string, -}; // ----------- Selectors ----------- export type SelectorsInitializer = {}; @@ -595,6 +589,7 @@ export interface BrowserContextChannel extends Channel { on(event: 'close', callback: (params: BrowserContextCloseEvent) => void): this; on(event: 'page', callback: (params: BrowserContextPageEvent) => void): this; on(event: 'route', callback: (params: BrowserContextRouteEvent) => void): this; + on(event: 'video', callback: (params: BrowserContextVideoEvent) => void): this; on(event: 'crBackgroundPage', callback: (params: BrowserContextCrBackgroundPageEvent) => void): this; on(event: 'crServiceWorker', callback: (params: BrowserContextCrServiceWorkerEvent) => void): this; addCookies(params: BrowserContextAddCookiesParams, metadata?: Metadata): Promise; @@ -629,6 +624,9 @@ export type BrowserContextRouteEvent = { route: RouteChannel, request: RequestChannel, }; +export type BrowserContextVideoEvent = { + artifact: ArtifactChannel, +}; export type BrowserContextCrBackgroundPageEvent = { page: PageChannel, }; @@ -799,7 +797,6 @@ export type PageInitializer = { height: number, }, isClosed: boolean, - videoRelativePath?: string, }; export interface PageChannel extends Channel { on(event: 'bindingCall', callback: (params: PageBindingCallEvent) => void): this; @@ -868,7 +865,9 @@ export type PageDialogEvent = { dialog: DialogChannel, }; export type PageDownloadEvent = { - download: DownloadChannel, + url: string, + suggestedFilename: string, + artifact: ArtifactChannel, }; export type PageDomcontentloadedEvent = {}; export type PageFileChooserEvent = { @@ -908,7 +907,7 @@ export type PageRouteEvent = { request: RequestChannel, }; export type PageVideoEvent = { - relativePath: string, + artifact: ArtifactChannel, }; export type PageWebSocketEvent = { webSocket: WebSocketChannel, @@ -2410,49 +2409,48 @@ export type DialogDismissParams = {}; export type DialogDismissOptions = {}; export type DialogDismissResult = void; -// ----------- Download ----------- -export type DownloadInitializer = { - url: string, - suggestedFilename: string, +// ----------- Artifact ----------- +export type ArtifactInitializer = { + absolutePath: string, }; -export interface DownloadChannel extends Channel { - path(params?: DownloadPathParams, metadata?: Metadata): Promise; - saveAs(params: DownloadSaveAsParams, metadata?: Metadata): Promise; - saveAsStream(params?: DownloadSaveAsStreamParams, metadata?: Metadata): Promise; - failure(params?: DownloadFailureParams, metadata?: Metadata): Promise; - stream(params?: DownloadStreamParams, metadata?: Metadata): Promise; - delete(params?: DownloadDeleteParams, metadata?: Metadata): Promise; +export interface ArtifactChannel extends Channel { + pathAfterFinished(params?: ArtifactPathAfterFinishedParams, metadata?: Metadata): Promise; + saveAs(params: ArtifactSaveAsParams, metadata?: Metadata): Promise; + saveAsStream(params?: ArtifactSaveAsStreamParams, metadata?: Metadata): Promise; + failure(params?: ArtifactFailureParams, metadata?: Metadata): Promise; + stream(params?: ArtifactStreamParams, metadata?: Metadata): Promise; + delete(params?: ArtifactDeleteParams, metadata?: Metadata): Promise; } -export type DownloadPathParams = {}; -export type DownloadPathOptions = {}; -export type DownloadPathResult = { +export type ArtifactPathAfterFinishedParams = {}; +export type ArtifactPathAfterFinishedOptions = {}; +export type ArtifactPathAfterFinishedResult = { value?: string, }; -export type DownloadSaveAsParams = { +export type ArtifactSaveAsParams = { path: string, }; -export type DownloadSaveAsOptions = { +export type ArtifactSaveAsOptions = { }; -export type DownloadSaveAsResult = void; -export type DownloadSaveAsStreamParams = {}; -export type DownloadSaveAsStreamOptions = {}; -export type DownloadSaveAsStreamResult = { +export type ArtifactSaveAsResult = void; +export type ArtifactSaveAsStreamParams = {}; +export type ArtifactSaveAsStreamOptions = {}; +export type ArtifactSaveAsStreamResult = { stream: StreamChannel, }; -export type DownloadFailureParams = {}; -export type DownloadFailureOptions = {}; -export type DownloadFailureResult = { +export type ArtifactFailureParams = {}; +export type ArtifactFailureOptions = {}; +export type ArtifactFailureResult = { error?: string, }; -export type DownloadStreamParams = {}; -export type DownloadStreamOptions = {}; -export type DownloadStreamResult = { +export type ArtifactStreamParams = {}; +export type ArtifactStreamOptions = {}; +export type ArtifactStreamResult = { stream?: StreamChannel, }; -export type DownloadDeleteParams = {}; -export type DownloadDeleteOptions = {}; -export type DownloadDeleteResult = void; +export type ArtifactDeleteParams = {}; +export type ArtifactDeleteOptions = {}; +export type ArtifactDeleteResult = void; // ----------- Stream ----------- export type StreamInitializer = {}; diff --git a/src/protocol/protocol.yml b/src/protocol/protocol.yml index ac8f105a7a..368eb0ac78 100644 --- a/src/protocol/protocol.yml +++ b/src/protocol/protocol.yml @@ -380,16 +380,6 @@ RemoteBrowser: browser: Browser selectors: Selectors - events: - - # Video stream blocks owner context from closing until the stream is closed. - # Make sure to close the stream! - video: - parameters: - context: BrowserContext - stream: Stream - relativePath: string - Selectors: type: interface @@ -631,6 +621,10 @@ BrowserContext: route: Route request: Request + video: + parameters: + artifact: Artifact + crBackgroundPage: parameters: page: Page @@ -650,7 +644,6 @@ Page: width: number height: number isClosed: boolean - videoRelativePath: string? commands: @@ -940,7 +933,9 @@ Page: download: parameters: - download: Download + url: string + suggestedFilename: string + artifact: Artifact domcontentloaded: @@ -993,7 +988,7 @@ Page: video: parameters: - relativePath: string + artifact: Artifact webSocket: parameters: @@ -1991,16 +1986,15 @@ Dialog: -Download: +Artifact: type: interface initializer: - url: string - suggestedFilename: string + absolutePath: string commands: - path: + pathAfterFinished: returns: value: string? @@ -2025,7 +2019,6 @@ Download: delete: - Stream: type: interface diff --git a/src/protocol/validator.ts b/src/protocol/validator.ts index 4fc7fa2b3b..761173e66a 100644 --- a/src/protocol/validator.ts +++ b/src/protocol/validator.ts @@ -926,14 +926,14 @@ export function createScheme(tChannel: (name: string) => Validator): Scheme { promptText: tOptional(tString), }); scheme.DialogDismissParams = tOptional(tObject({})); - scheme.DownloadPathParams = tOptional(tObject({})); - scheme.DownloadSaveAsParams = tObject({ + scheme.ArtifactPathAfterFinishedParams = tOptional(tObject({})); + scheme.ArtifactSaveAsParams = tObject({ path: tString, }); - scheme.DownloadSaveAsStreamParams = tOptional(tObject({})); - scheme.DownloadFailureParams = tOptional(tObject({})); - scheme.DownloadStreamParams = tOptional(tObject({})); - scheme.DownloadDeleteParams = tOptional(tObject({})); + scheme.ArtifactSaveAsStreamParams = tOptional(tObject({})); + scheme.ArtifactFailureParams = tOptional(tObject({})); + scheme.ArtifactStreamParams = tOptional(tObject({})); + scheme.ArtifactDeleteParams = tOptional(tObject({})); scheme.StreamReadParams = tObject({ size: tOptional(tNumber), }); diff --git a/src/server/artifact.ts b/src/server/artifact.ts new file mode 100644 index 0000000000..5750b53740 --- /dev/null +++ b/src/server/artifact.ts @@ -0,0 +1,117 @@ +/** + * Copyright (c) Microsoft Corporation. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import fs from 'fs'; +import * as util from 'util'; + +type SaveCallback = (localPath: string, error?: string) => Promise; + +export class Artifact { + private _localPath: string; + private _unaccessibleErrorMessage: string | undefined; + private _finishedCallback: () => void; + private _finishedPromise: Promise; + private _saveCallbacks: SaveCallback[] = []; + private _finished: boolean = false; + private _deleted = false; + private _failureError: string | null = null; + + constructor(localPath: string, unaccessibleErrorMessage?: string) { + this._localPath = localPath; + this._unaccessibleErrorMessage = unaccessibleErrorMessage; + this._finishedCallback = () => {}; + this._finishedPromise = new Promise(f => this._finishedCallback = f); + } + + finishedPromise() { + return this._finishedPromise; + } + + localPath() { + return this._localPath; + } + + async localPathAfterFinished(): Promise { + if (this._unaccessibleErrorMessage) + throw new Error(this._unaccessibleErrorMessage); + await this._finishedPromise; + if (this._failureError) + return null; + return this._localPath; + } + + saveAs(saveCallback: SaveCallback) { + if (this._unaccessibleErrorMessage) + throw new Error(this._unaccessibleErrorMessage); + if (this._deleted) + throw new Error(`File already deleted. Save before deleting.`); + if (this._failureError) + throw new Error(`File not found on disk. Check download.failure() for details.`); + + if (this._finished) { + saveCallback(this._localPath).catch(e => {}); + return; + } + this._saveCallbacks.push(saveCallback); + } + + async failureError(): Promise { + if (this._unaccessibleErrorMessage) + return this._unaccessibleErrorMessage; + await this._finishedPromise; + return this._failureError; + } + + async delete(): Promise { + if (this._unaccessibleErrorMessage) + return; + const fileName = await this.localPathAfterFinished(); + if (this._deleted) + return; + this._deleted = true; + if (fileName) + await util.promisify(fs.unlink)(fileName).catch(e => {}); + } + + async deleteOnContextClose(): Promise { + // Compared to "delete", this method does not wait for the artifact to finish. + // We use it when closing the context to avoid stalling. + if (this._deleted) + return; + this._deleted = true; + if (!this._unaccessibleErrorMessage) + await util.promisify(fs.unlink)(this._localPath).catch(e => {}); + await this.reportFinished('File deleted upon browser context closure.'); + } + + async reportFinished(error?: string) { + if (this._finished) + return; + this._finished = true; + this._failureError = error || null; + + if (error) { + for (const callback of this._saveCallbacks) + await callback('', error); + } else { + for (const callback of this._saveCallbacks) + await callback(this._localPath); + } + this._saveCallbacks = []; + + this._finishedCallback(); + } +} diff --git a/src/server/browser.ts b/src/server/browser.ts index f3effe352d..9303674d62 100644 --- a/src/server/browser.ts +++ b/src/server/browser.ts @@ -15,7 +15,7 @@ */ import * as types from './types'; -import { BrowserContext, Video } from './browserContext'; +import { BrowserContext } from './browserContext'; import { Page } from './page'; import { Download } from './download'; import { ProxySettings } from './types'; @@ -23,6 +23,7 @@ import { ChildProcess } from 'child_process'; import { RecentLogsCollector } from '../utils/debugLogger'; import * as registry from '../utils/registry'; import { SdkObject } from './instrumentation'; +import { Artifact } from './artifact'; export interface BrowserProcess { onclose?: ((exitCode: number | null, signal: string | null) => void); @@ -60,7 +61,7 @@ export abstract class Browser extends SdkObject { private _downloads = new Map(); _defaultContext: BrowserContext | null = null; private _startedClosing = false; - readonly _idToVideo = new Map(); + readonly _idToVideo = new Map(); constructor(options: BrowserOptions) { super(options.rootSdkObject); @@ -89,24 +90,26 @@ export abstract class Browser extends SdkObject { const download = this._downloads.get(uuid); if (!download) return; - download._reportFinished(error); + download.artifact.reportFinished(error); this._downloads.delete(uuid); } _videoStarted(context: BrowserContext, videoId: string, path: string, pageOrError: Promise) { - const video = new Video(context, videoId, path); - this._idToVideo.set(videoId, video); - context.emit(BrowserContext.Events.VideoStarted, video); - pageOrError.then(pageOrError => { - if (pageOrError instanceof Page) - pageOrError.videoStarted(video); + const artifact = new Artifact(path); + this._idToVideo.set(videoId, { context, artifact }); + context.emit(BrowserContext.Events.VideoStarted, artifact); + pageOrError.then(page => { + if (page instanceof Page) { + page._video = artifact; + page.emit(Page.Events.Video, artifact); + } }); } _videoFinished(videoId: string) { - const video = this._idToVideo.get(videoId)!; + const video = this._idToVideo.get(videoId); this._idToVideo.delete(videoId); - video._finish(); + video?.artifact.reportFinished(); } _didClose() { diff --git a/src/server/browserContext.ts b/src/server/browserContext.ts index db666b0407..627efbd937 100644 --- a/src/server/browserContext.ts +++ b/src/server/browserContext.ts @@ -29,40 +29,12 @@ import * as types from './types'; import path from 'path'; import { CallMetadata, internalCallMetadata, SdkObject } from './instrumentation'; -export class Video { - readonly _videoId: string; - readonly _path: string; - readonly _relativePath: string; - readonly _context: BrowserContext; - readonly _finishedPromise: Promise; - private _finishCallback: () => void = () => {}; - private _callbackOnFinish?: () => Promise; - - constructor(context: BrowserContext, videoId: string, p: string) { - this._videoId = videoId; - this._path = p; - this._relativePath = path.relative(context._options.recordVideo!.dir, p); - this._context = context; - this._finishedPromise = new Promise(fulfill => this._finishCallback = fulfill); - } - - async _finish() { - if (this._callbackOnFinish) - await this._callbackOnFinish(); - this._finishCallback(); - } - - _waitForCallbackOnFinish(callback: () => Promise) { - this._callbackOnFinish = callback; - } -} - export abstract class BrowserContext extends SdkObject { static Events = { Close: 'close', Page: 'page', - VideoStarted: 'videostarted', BeforeClose: 'beforeclose', + VideoStarted: 'videostarted', }; readonly _timeoutSettings = new TimeoutSettings(); @@ -121,6 +93,10 @@ export abstract class BrowserContext extends SdkObject { } this._closedStatus = 'closed'; this._downloads.clear(); + for (const [id, video] of this._browser._idToVideo) { + if (video.context === this) + this._browser._idToVideo.delete(id); + } this._closePromiseFulfill!(new Error('Context closed')); this.emit(BrowserContext.Events.Close); } @@ -267,15 +243,15 @@ export abstract class BrowserContext extends SdkObject { // Cleanup. const promises: Promise[] = []; - for (const video of this._browser._idToVideo.values()) { + for (const { context, artifact } of this._browser._idToVideo.values()) { // Wait for the videos to finish. - if (video._context === this) - promises.push(video._finishedPromise); + if (context === this) + promises.push(artifact.finishedPromise()); } for (const download of this._downloads) { // We delete downloads after context closure // so that browser does not write to the download file anymore. - promises.push(download.deleteOnContextClose()); + promises.push(download.artifact.deleteOnContextClose()); } await Promise.all(promises); diff --git a/src/server/chromium/crPage.ts b/src/server/chromium/crPage.ts index 9f71b0404a..1ef1a94f3e 100644 --- a/src/server/chromium/crPage.ts +++ b/src/server/chromium/crPage.ts @@ -863,7 +863,8 @@ class FrameSession { } async _startScreencast(options: types.PageScreencastOptions) { - assert(this._screencastId); + const screencastId = this._screencastId; + assert(screencastId); const gotFirstFrame = new Promise(f => this._client.once('Page.screencastFrame', f)); await this._client.send('Page.startScreencast', { format: 'jpeg', @@ -872,7 +873,9 @@ class FrameSession { maxHeight: options.height, }); // Wait for the first frame before reporting video to the client. - this._crPage._browserContext._browser._videoStarted(this._crPage._browserContext, this._screencastId, options.outputFile, gotFirstFrame.then(() => this._crPage.pageOrError())); + gotFirstFrame.then(() => { + this._crPage._browserContext._browser._videoStarted(this._crPage._browserContext, screencastId, options.outputFile, this._crPage.pageOrError()); + }); } async _stopScreencast(): Promise { diff --git a/src/server/download.ts b/src/server/download.ts index 86f7252d65..138c024f1c 100644 --- a/src/server/download.ts +++ b/src/server/download.ts @@ -15,37 +15,23 @@ */ import path from 'path'; -import fs from 'fs'; -import * as util from 'util'; import { Page } from './page'; import { assert } from '../utils/utils'; - -type SaveCallback = (localPath: string, error?: string) => Promise; +import { Artifact } from './artifact'; export class Download { - private _downloadsPath: string; - private _uuid: string; - private _finishedCallback: () => void; - private _finishedPromise: Promise; - private _saveCallbacks: SaveCallback[] = []; - private _finished: boolean = false; + readonly artifact: Artifact; + readonly url: string; private _page: Page; - private _acceptDownloads: boolean; - private _failure: string | null = null; - private _deleted = false; - private _url: string; private _suggestedFilename: string | undefined; constructor(page: Page, downloadsPath: string, uuid: string, url: string, suggestedFilename?: string) { + const unaccessibleErrorMessage = !page._browserContext._options.acceptDownloads ? 'Pass { acceptDownloads: true } when you are creating your browser context.' : undefined; + this.artifact = new Artifact(path.join(downloadsPath, uuid), unaccessibleErrorMessage); this._page = page; - this._downloadsPath = downloadsPath; - this._uuid = uuid; - this._url = url; + this.url = url; this._suggestedFilename = suggestedFilename; - this._finishedCallback = () => {}; - this._finishedPromise = new Promise(f => this._finishedCallback = f); page._browserContext._downloads.add(this); - this._acceptDownloads = !!this._page._browserContext._options.acceptDownloads; if (suggestedFilename !== undefined) this._page.emit(Page.Events.Download, this); } @@ -56,86 +42,7 @@ export class Download { this._page.emit(Page.Events.Download, this); } - url(): string { - return this._url; - } - suggestedFilename(): string { return this._suggestedFilename!; } - - async localPath(): Promise { - if (!this._acceptDownloads) - throw new Error('Pass { acceptDownloads: true } when you are creating your browser context.'); - const fileName = path.join(this._downloadsPath, this._uuid); - await this._finishedPromise; - if (this._failure) - return null; - return fileName; - } - - saveAs(saveCallback: SaveCallback) { - if (!this._acceptDownloads) - throw new Error('Pass { acceptDownloads: true } when you are creating your browser context.'); - if (this._deleted) - throw new Error('Download already deleted. Save before deleting.'); - if (this._failure) - throw new Error('Download not found on disk. Check download.failure() for details.'); - - if (this._finished) { - saveCallback(path.join(this._downloadsPath, this._uuid)).catch(e => {}); - return; - } - this._saveCallbacks.push(saveCallback); - } - - async failure(): Promise { - if (!this._acceptDownloads) - return 'Pass { acceptDownloads: true } when you are creating your browser context.'; - await this._finishedPromise; - return this._failure; - } - - async delete(): Promise { - if (!this._acceptDownloads) - return; - const fileName = await this.localPath(); - if (this._deleted) - return; - this._deleted = true; - if (fileName) - await util.promisify(fs.unlink)(fileName).catch(e => {}); - } - - async deleteOnContextClose(): Promise { - // Compared to "delete", this method does not wait for the download to finish. - // We use it when closing the context to avoid stalling. - if (this._deleted) - return; - this._deleted = true; - if (this._acceptDownloads) { - const fileName = path.join(this._downloadsPath, this._uuid); - await util.promisify(fs.unlink)(fileName).catch(e => {}); - } - await this._reportFinished('Download deleted upon browser context closure.'); - } - - async _reportFinished(error?: string) { - if (this._finished) - return; - this._finished = true; - this._failure = error || null; - - if (error) { - for (const callback of this._saveCallbacks) - await callback('', error); - } else { - const fullPath = path.join(this._downloadsPath, this._uuid); - for (const callback of this._saveCallbacks) - await callback(fullPath); - } - this._saveCallbacks = []; - - this._finishedCallback(); - } } diff --git a/src/server/firefox/ffPage.ts b/src/server/firefox/ffPage.ts index 1653536551..d0f54e672b 100644 --- a/src/server/firefox/ffPage.ts +++ b/src/server/firefox/ffPage.ts @@ -310,6 +310,8 @@ export class FFPage implements PageDelegate { } didClose() { + if (!this._initializedPage) + this._markAsError(new Error('Page has been closed')); this._session.dispose(); helper.removeEventListeners(this._eventListeners); this._networkManager.dispose(); diff --git a/src/server/page.ts b/src/server/page.ts index 3877148502..a8893acc44 100644 --- a/src/server/page.ts +++ b/src/server/page.ts @@ -23,7 +23,7 @@ import * as network from './network'; import { Screenshotter } from './screenshotter'; import { TimeoutSettings } from '../utils/timeoutSettings'; import * as types from './types'; -import { BrowserContext, Video } from './browserContext'; +import { BrowserContext } from './browserContext'; import { ConsoleMessage } from './console'; import * as accessibility from './accessibility'; import { FileChooser } from './fileChooser'; @@ -32,6 +32,7 @@ import { assert, createGuid, isError } from '../utils/utils'; import { debugLogger } from '../utils/debugLogger'; import { Selectors } from './selectors'; import { CallMetadata, SdkObject } from './instrumentation'; +import { Artifact } from './artifact'; export interface PageDelegate { readonly rawMouse: input.RawMouse; @@ -114,9 +115,9 @@ export class Page extends SdkObject { InternalFrameNavigatedToNewDocument: 'internalframenavigatedtonewdocument', Load: 'load', Popup: 'popup', + Video: 'video', WebSocket: 'websocket', Worker: 'worker', - VideoStarted: 'videostarted', }; private _closedState: 'open' | 'closing' | 'closed' = 'open'; @@ -146,9 +147,9 @@ export class Page extends SdkObject { private _serverRequestInterceptor: network.RouteHandler | undefined; _ownedContext: BrowserContext | undefined; readonly selectors: Selectors; - _video: Video | null = null; readonly uniqueId: string; _pageIsError: Error | undefined; + _video: Artifact | null = null; constructor(delegate: PageDelegate, browserContext: BrowserContext) { super(browserContext); @@ -181,7 +182,7 @@ export class Page extends SdkObject { this.selectors = browserContext.selectors(); } - async reportAsNew(error?: Error) { + reportAsNew(error?: Error) { if (error) { // Initialization error could have happened because of // context/browser closure. Just ignore the page. @@ -478,11 +479,6 @@ export class Page extends SdkObject { await this._delegate.setFileChooserIntercepted(enabled); } - videoStarted(video: Video) { - this._video = video; - this.emit(Page.Events.VideoStarted, video); - } - frameNavigatedToNewDocument(frame: frames.Frame) { this.emit(Page.Events.InternalFrameNavigatedToNewDocument, frame); const url = frame.url(); diff --git a/test/browsertype-connect.spec.ts b/test/browsertype-connect.spec.ts index ee8c07dfc4..4bf48a9339 100644 --- a/test/browsertype-connect.spec.ts +++ b/test/browsertype-connect.spec.ts @@ -242,7 +242,7 @@ describe('connect', (suite, { mode }) => { await page.close(); }); - it('should save videos from remote browser', async ({browserType, remoteServer, testInfo}) => { + it('should saveAs videos from remote browser', async ({browserType, remoteServer, testInfo}) => { const remote = await browserType.connect({ wsEndpoint: remoteServer.wsEndpoint() }); const videosPath = testInfo.outputPath(); const context = await remote.newContext({ @@ -253,8 +253,11 @@ describe('connect', (suite, { mode }) => { await new Promise(r => setTimeout(r, 1000)); await context.close(); - const files = fs.readdirSync(videosPath); - expect(files.some(file => file.endsWith('webm'))).toBe(true); + const savedAsPath = testInfo.outputPath('my-video.webm'); + await page.video().saveAs(savedAsPath); + expect(fs.existsSync(savedAsPath)).toBeTruthy(); + const error = await page.video().path().catch(e => e); + expect(error.message).toContain('Path is not available when using browserType.connect(). Use saveAs() to save a local copy.'); }); it('should be able to connect 20 times to a single server without warnings', async ({browserType, remoteServer, server}) => { diff --git a/test/download.spec.ts b/test/download.spec.ts index f167b0244a..295ccf6fa7 100644 --- a/test/download.spec.ts +++ b/test/download.spec.ts @@ -189,7 +189,7 @@ describe('download event', () => { expect(fs.existsSync(nestedPath)).toBeTruthy(); expect(fs.readFileSync(nestedPath).toString()).toBe('Hello world'); const error = await download.path().catch(e => e); - expect(error.message).toContain('Path is not available when using browserType.connect(). Use download.saveAs() to save a local copy.'); + expect(error.message).toContain('Path is not available when using browserType.connect(). Use saveAs() to save a local copy.'); await browser.close(); }); @@ -216,7 +216,7 @@ describe('download event', () => { const userPath = testInfo.outputPath('download.txt'); await download.delete(); const { message } = await download.saveAs(userPath).catch(e => e); - expect(message).toContain('Download already deleted. Save before deleting.'); + expect(message).toContain('File already deleted. Save before deleting.'); await page.close(); }); @@ -233,7 +233,7 @@ describe('download event', () => { const userPath = testInfo.outputPath('download.txt'); await download.delete(); const { message } = await download.saveAs(userPath).catch(e => e); - expect(message).toContain('Download already deleted. Save before deleting.'); + expect(message).toContain('File already deleted. Save before deleting.'); await browser.close(); }); @@ -413,7 +413,7 @@ describe('download event', () => { page.context().close(), ]); expect(downloadPath).toBe(null); - expect(saveError.message).toContain('Download deleted upon browser context closure.'); + expect(saveError.message).toContain('File deleted upon browser context closure.'); }); it('should close the context without awaiting the download', (test, { browserName, platform }) => { @@ -440,6 +440,6 @@ describe('download event', () => { page.context().close(), ]); expect(downloadPath).toBe(null); - expect(saveError.message).toContain('Download deleted upon browser context closure.'); + expect(saveError.message).toContain('File deleted upon browser context closure.'); }); }); diff --git a/test/screencast.spec.ts b/test/screencast.spec.ts index 356913cc89..59196c9bc4 100644 --- a/test/screencast.spec.ts +++ b/test/screencast.spec.ts @@ -219,6 +219,77 @@ describe('screencast', suite => { expect(fs.existsSync(path)).toBeTruthy(); }); + it('should saveAs video', async ({browser, testInfo}) => { + const videosPath = testInfo.outputPath(''); + const size = { width: 320, height: 240 }; + const context = await browser.newContext({ + recordVideo: { + dir: videosPath, + size + }, + viewport: size, + }); + const page = await context.newPage(); + await page.evaluate(() => document.body.style.backgroundColor = 'red'); + await page.waitForTimeout(1000); + await context.close(); + + const saveAsPath = testInfo.outputPath('my-video.webm'); + await page.video().saveAs(saveAsPath); + expect(fs.existsSync(saveAsPath)).toBeTruthy(); + }); + + it('saveAs should throw when no video frames', async ({browser, browserName, testInfo}) => { + const videosPath = testInfo.outputPath(''); + const size = { width: 320, height: 240 }; + const context = await browser.newContext({ + recordVideo: { + dir: videosPath, + size + }, + viewport: size, + }); + + const page = await context.newPage(); + const [popup] = await Promise.all([ + page.context().waitForEvent('page'), + page.evaluate(() => { + const win = window.open('about:blank'); + win.close(); + }), + ]); + await page.close(); + + const saveAsPath = testInfo.outputPath('my-video.webm'); + const error = await popup.video().saveAs(saveAsPath).catch(e => e); + // WebKit pauses renderer before win.close() and actually writes something. + if (browserName === 'webkit') + expect(fs.existsSync(saveAsPath)).toBeTruthy(); + else + expect(error.message).toContain('Page did not produce any video frames'); + }); + + it('should delete video', async ({browser, testInfo}) => { + const videosPath = testInfo.outputPath(''); + const size = { width: 320, height: 240 }; + const context = await browser.newContext({ + recordVideo: { + dir: videosPath, + size + }, + viewport: size, + }); + const page = await context.newPage(); + const deletePromise = page.video().delete(); + await page.evaluate(() => document.body.style.backgroundColor = 'red'); + await page.waitForTimeout(1000); + await context.close(); + + const videoPath = await page.video().path(); + await deletePromise; + expect(fs.existsSync(videoPath)).toBeFalsy(); + }); + it('should expose video path blank page', async ({browser, testInfo}) => { const videosPath = testInfo.outputPath(''); const size = { width: 320, height: 240 }; diff --git a/types/types.d.ts b/types/types.d.ts index 0cea7d7f32..442266c539 100644 --- a/types/types.d.ts +++ b/types/types.d.ts @@ -9256,7 +9256,8 @@ export interface Download { /** * Returns path to the downloaded file in case of successful download. The method will wait for the download to finish if - * necessary. + * necessary. The method throws when connected remotely via + * [browserType.connect(params)](https://playwright.dev/docs/api/class-browsertype#browsertypeconnectparams). */ path(): Promise; @@ -10225,7 +10226,7 @@ export interface Touchscreen { } /** - * When browser context is created with the `videosPath` option, each page has a video object associated with it. + * When browser context is created with the `recordVideo` option, each page has a video object associated with it. * * ```js * console.log(await page.video().path()); @@ -10233,11 +10234,24 @@ export interface Touchscreen { * */ export interface Video { + /** + * Deletes the video file. Will wait for the video to finish if necessary. + */ + delete(): Promise; + /** * Returns the file system path this video will be recorded to. The video is guaranteed to be written to the filesystem - * upon closing the browser context. + * upon closing the browser context. This method throws when connected remotely via + * [browserType.connect(params)](https://playwright.dev/docs/api/class-browsertype#browsertypeconnectparams). */ path(): Promise; + + /** + * Saves the video to a user-specified path. It is safe to call this method while the video is still in progress, or after + * the page has closed. This method waits until the page is closed and the video is fully saved. + * @param path Path where the video should be saved. + */ + saveAs(path: string): Promise; } /**