diff --git a/docs/src/api/class-download.md b/docs/src/api/class-download.md index 7464ebbb88..e4aee1c139 100644 --- a/docs/src/api/class-download.md +++ b/docs/src/api/class-download.md @@ -62,6 +62,14 @@ downloaded content. If [`option: acceptDownloads`] is not set, download events a not performed and user has no access to the downloaded files. ::: +## async method: Download._cancel + +**Chromium-only** Cancels a download. +Will not fail if the download is already finished or canceled. +Upon successful cancellations, `download.failure()` would resolve to `'canceled'`. + +Currently **experimental** and may subject to further changes. + ## async method: Download.createReadStream * langs: java, js, csharp - returns: <[null]|[Readable]> diff --git a/src/client/artifact.ts b/src/client/artifact.ts index 358cf84dcf..5291a34767 100644 --- a/src/client/artifact.ts +++ b/src/client/artifact.ts @@ -71,6 +71,12 @@ export class Artifact extends ChannelOwner { + return this._wrapApiCall(`${this._apiName}.cancel`, async (channel: channels.ArtifactChannel) => { + return channel.cancel(); + }); + } + async delete(): Promise { return this._wrapApiCall(`${this._apiName}.delete`, async (channel: channels.ArtifactChannel) => { return channel.delete(); diff --git a/src/client/download.ts b/src/client/download.ts index 2b4c8ead04..0f530d5184 100644 --- a/src/client/download.ts +++ b/src/client/download.ts @@ -60,6 +60,10 @@ export class Download implements api.Download { return this._artifact.createReadStream(); } + async _cancel(): Promise { + return this._artifact.cancel(); + } + async delete(): Promise { return this._artifact.delete(); } diff --git a/src/dispatchers/artifactDispatcher.ts b/src/dispatchers/artifactDispatcher.ts index 0d7bc5d008..ebd5e915b1 100644 --- a/src/dispatchers/artifactDispatcher.ts +++ b/src/dispatchers/artifactDispatcher.ts @@ -89,6 +89,10 @@ export class ArtifactDispatcher extends Dispatcher { + await this._object.cancel(); + } + async delete(): Promise { await this._object.delete(); this._dispose(); diff --git a/src/protocol/channels.ts b/src/protocol/channels.ts index ac662bf859..31cba1d2b9 100644 --- a/src/protocol/channels.ts +++ b/src/protocol/channels.ts @@ -2480,6 +2480,7 @@ export interface ArtifactChannel extends Channel { saveAsStream(params?: ArtifactSaveAsStreamParams, metadata?: Metadata): Promise; failure(params?: ArtifactFailureParams, metadata?: Metadata): Promise; stream(params?: ArtifactStreamParams, metadata?: Metadata): Promise; + cancel(params?: ArtifactCancelParams, metadata?: Metadata): Promise; delete(params?: ArtifactDeleteParams, metadata?: Metadata): Promise; } export type ArtifactPathAfterFinishedParams = {}; @@ -2509,6 +2510,9 @@ export type ArtifactStreamOptions = {}; export type ArtifactStreamResult = { stream?: StreamChannel, }; +export type ArtifactCancelParams = {}; +export type ArtifactCancelOptions = {}; +export type ArtifactCancelResult = void; export type ArtifactDeleteParams = {}; export type ArtifactDeleteOptions = {}; export type ArtifactDeleteResult = void; diff --git a/src/protocol/protocol.yml b/src/protocol/protocol.yml index 8b372e33bf..71b61827f1 100644 --- a/src/protocol/protocol.yml +++ b/src/protocol/protocol.yml @@ -2043,6 +2043,8 @@ Artifact: returns: stream: Stream? + cancel: + delete: diff --git a/src/protocol/validator.ts b/src/protocol/validator.ts index 1ae820a8b1..3ba41f2852 100644 --- a/src/protocol/validator.ts +++ b/src/protocol/validator.ts @@ -960,6 +960,7 @@ export function createScheme(tChannel: (name: string) => Validator): Scheme { scheme.ArtifactSaveAsStreamParams = tOptional(tObject({})); scheme.ArtifactFailureParams = tOptional(tObject({})); scheme.ArtifactStreamParams = tOptional(tObject({})); + scheme.ArtifactCancelParams = tOptional(tObject({})); scheme.ArtifactDeleteParams = tOptional(tObject({})); scheme.StreamReadParams = tObject({ size: tOptional(tNumber), diff --git a/src/server/artifact.ts b/src/server/artifact.ts index adf8f1b81c..b13da3e13b 100644 --- a/src/server/artifact.ts +++ b/src/server/artifact.ts @@ -15,13 +15,16 @@ */ import fs from 'fs'; +import { assert } from '../utils/utils'; import { SdkObject } from './instrumentation'; type SaveCallback = (localPath: string, error?: string) => Promise; +type CancelCallback = () => Promise; export class Artifact extends SdkObject { private _localPath: string; private _unaccessibleErrorMessage: string | undefined; + private _cancelCallback: CancelCallback | undefined; private _finishedCallback: () => void; private _finishedPromise: Promise; private _saveCallbacks: SaveCallback[] = []; @@ -29,10 +32,11 @@ export class Artifact extends SdkObject { private _deleted = false; private _failureError: string | null = null; - constructor(parent: SdkObject, localPath: string, unaccessibleErrorMessage?: string) { + constructor(parent: SdkObject, localPath: string, unaccessibleErrorMessage?: string, cancelCallback?: CancelCallback) { super(parent, 'artifact'); this._localPath = localPath; this._unaccessibleErrorMessage = unaccessibleErrorMessage; + this._cancelCallback = cancelCallback; this._finishedCallback = () => {}; this._finishedPromise = new Promise(f => this._finishedCallback = f); } @@ -76,6 +80,11 @@ export class Artifact extends SdkObject { return this._failureError; } + async cancel(): Promise { + assert(this._cancelCallback !== undefined); + return this._cancelCallback(); + } + async delete(): Promise { if (this._unaccessibleErrorMessage) return; diff --git a/src/server/browserContext.ts b/src/server/browserContext.ts index 6e08002b5a..71c7a01e3f 100644 --- a/src/server/browserContext.ts +++ b/src/server/browserContext.ts @@ -152,6 +152,7 @@ export abstract class BrowserContext extends SdkObject { abstract _doUpdateRequestInterception(): Promise; abstract _doClose(): Promise; abstract _onClosePersistent(): Promise; + abstract _doCancelDownload(uuid: string): Promise; async cookies(urls: string | string[] | undefined = []): Promise { if (urls && !Array.isArray(urls)) diff --git a/src/server/chromium/crBrowser.ts b/src/server/chromium/crBrowser.ts index c28ddb824b..de158fd2ca 100644 --- a/src/server/chromium/crBrowser.ts +++ b/src/server/chromium/crBrowser.ts @@ -479,6 +479,16 @@ export class CRBrowserContext extends BrowserContext { } } + async _doCancelDownload(guid: string) { + // The upstream CDP method is implemented in a way that no explicit error would be given + // regarding the requested `guid`, even if the download is in a state not suitable for + // cancellation (finished, cancelled, etc.) or the guid is invalid at all. + await this._browser._session.send('Browser.cancelDownload', { + guid: guid, + browserContextId: this._browserContextId, + }); + } + backgroundPages(): Page[] { const result: Page[] = []; for (const backgroundPage of this._browser._backgroundPages.values()) { diff --git a/src/server/download.ts b/src/server/download.ts index 2e80a6402f..847c8cf061 100644 --- a/src/server/download.ts +++ b/src/server/download.ts @@ -27,7 +27,9 @@ export class Download { 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(page, path.join(downloadsPath, uuid), unaccessibleErrorMessage); + this.artifact = new Artifact(page, path.join(downloadsPath, uuid), unaccessibleErrorMessage, () => { + return this._page._browserContext._doCancelDownload(uuid); + }); this._page = page; this.url = url; this._suggestedFilename = suggestedFilename; diff --git a/src/server/firefox/ffBrowser.ts b/src/server/firefox/ffBrowser.ts index 179acc4f1b..7d0bc7c015 100644 --- a/src/server/firefox/ffBrowser.ts +++ b/src/server/firefox/ffBrowser.ts @@ -330,6 +330,11 @@ export class FFBrowserContext extends BrowserContext { await this._browser._connection.send('Browser.removeBrowserContext', { browserContextId: this._browserContextId }); this._browser._contexts.delete(this._browserContextId); } + + async _doCancelDownload(uuid: string) { + // TODO: Have this implemented + throw new Error('Download cancellation not yet implemented in Firefox'); + } } function toJugglerProxyOptions(proxy: types.ProxySettings) { diff --git a/src/server/webkit/wkBrowser.ts b/src/server/webkit/wkBrowser.ts index 58ae10f471..0c3531094f 100644 --- a/src/server/webkit/wkBrowser.ts +++ b/src/server/webkit/wkBrowser.ts @@ -326,4 +326,9 @@ export class WKBrowserContext extends BrowserContext { await this._browser._browserSession.send('Playwright.deleteContext', { browserContextId: this._browserContextId }); this._browser._contexts.delete(this._browserContextId); } + + async _doCancelDownload(uuid: string) { + // TODO: Have this implemented + throw new Error('Download cancellation not yet implemented in WebKit'); + } } diff --git a/tests/config/checkCoverage.js b/tests/config/checkCoverage.js index 5b43f7b985..c13883d74c 100644 --- a/tests/config/checkCoverage.js +++ b/tests/config/checkCoverage.js @@ -45,6 +45,7 @@ if (browserName !== 'chromium') { api.delete('coverage.startCSSCoverage'); api.delete('coverage.stopCSSCoverage'); api.delete('page.pdf'); + api.delete('download._cancel'); } // Some permissions tests are disabled in webkit. See permissions.jest.js diff --git a/tests/download.spec.ts b/tests/download.spec.ts index 6997bddb34..e52fa10622 100644 --- a/tests/download.spec.ts +++ b/tests/download.spec.ts @@ -31,6 +31,14 @@ it.describe('download event', () => { res.setHeader('Content-Disposition', 'attachment; filename=file.txt'); res.end(`Hello world`); }); + server.setRoute('/downloadWithDelay', (req, res) => { + res.setHeader('Content-Type', 'application/octet-stream'); + res.setHeader('Content-Disposition', 'attachment; filename=file.txt'); + // Chromium requires a large enough payload to trigger the download event soon enough + res.write('a'.repeat(4096)); + res.write('foo'); + res.uncork(); + }); }); it('should report downloads with acceptDownloads: false', async ({browser, server}) => { @@ -461,6 +469,41 @@ it.describe('download event', () => { await page.close(); }); + it('should be able to cancel pending downloads', async ({browser, server, browserName, browserVersion}) => { + // The exact upstream change is in b449b5c, which still does not appear in the first few 91.* tags until 91.0.4437.0. + it.fixme(browserName === 'chromium' && Number(browserVersion.split('.')[0]) < 91, 'The upstream Browser.cancelDownload command is not available before Chrome 91'); + it.fixme(browserName !== 'chromium', 'Download cancellation currently implemented for only Chromium'); + const page = await browser.newPage({ acceptDownloads: true }); + await page.setContent(`download`); + const [ download ] = await Promise.all([ + page.waitForEvent('download'), + page.click('a') + ]); + await download._cancel(); + const failure = await download.failure(); + expect(failure).toBe('canceled'); + await page.close(); + }); + + it('should not fail explicitly to cancel a download even if that is already finished', async ({browser, server, browserName, browserVersion}) => { + // The exact upstream change is in b449b5c, which still does not appear in the first few 91.* tags until 91.0.4437.0. + it.fixme(browserName === 'chromium' && Number(browserVersion.split('.')[0]) < 91, 'The upstream Browser.cancelDownload command is not available before Chrome 91'); + it.fixme(browserName !== 'chromium', 'Download cancellation currently implemented for only Chromium'); + const page = await browser.newPage({ acceptDownloads: true }); + await page.setContent(`download`); + const [ download ] = await Promise.all([ + page.waitForEvent('download'), + page.click('a') + ]); + const path = await download.path(); + expect(fs.existsSync(path)).toBeTruthy(); + expect(fs.readFileSync(path).toString()).toBe('Hello world'); + await download._cancel(); + const failure = await download.failure(); + expect(failure).toBe(null); + await page.close(); + }); + it('should report downloads with interception', async ({browser, server}) => { const page = await browser.newPage({ acceptDownloads: true }); await page.route(/.*/, r => r.continue()); @@ -474,5 +517,4 @@ it.describe('download event', () => { expect(fs.readFileSync(path).toString()).toBe('Hello world'); await page.close(); }); - }); diff --git a/types/types.d.ts b/types/types.d.ts index b74ecccc11..dfb44bbc36 100644 --- a/types/types.d.ts +++ b/types/types.d.ts @@ -9495,6 +9495,14 @@ export interface Dialog { * performed and user has no access to the downloaded files. */ export interface Download { + /** + * **Chromium-only** Cancels a download. Will not fail if the download is already finished or canceled. Upon successful + * cancellations, `download.failure()` would resolve to `'canceled'`. + * + * Currently **experimental** and may subject to further changes. + */ + _cancel(): Promise; + /** * Returns readable stream for current download or `null` if download failed. */ diff --git a/utils/doclint/missingDocs.js b/utils/doclint/missingDocs.js index 7ed5e197f8..ca56e5bf2c 100644 --- a/utils/doclint/missingDocs.js +++ b/utils/doclint/missingDocs.js @@ -78,6 +78,9 @@ function paramsForMember(member) { return new Set(member.argsArray.map(a => a.alias)); } +// Including experimental method names (with a leading underscore) that would be otherwise skipped +const allowExperimentalMethods = new Set([ 'Download._cancel' ]); + /** * @param {string[]} rootNames */ @@ -109,6 +112,20 @@ function listMethods(rootNames, apiFileName) { return null; } + /** + * @param {string} className + * @param {string} methodName + */ + function shouldSkipMethodByName(className, methodName) { + if (allowExperimentalMethods.has(`${className}.${methodName}`)) + return false; + if (methodName.startsWith('_') || methodName === 'T' || methodName === 'toString') + return true; + if (/** @type {any} */(EventEmitter).prototype.hasOwnProperty(methodName)) + return true; + return false; + } + /** * @param {string} className * @param {!ts.Type} classType @@ -120,9 +137,7 @@ function listMethods(rootNames, apiFileName) { apiMethods.set(className, methods); } for (const [name, member] of /** @type {any[]} */(classType.symbol.members || [])) { - if (name.startsWith('_') || name === 'T' || name === 'toString') - continue; - if (/** @type {any} */(EventEmitter).prototype.hasOwnProperty(name)) + if (shouldSkipMethodByName(className, name)) continue; const memberType = checker.getTypeOfSymbolAtLocation(member, member.valueDeclaration); const signature = signatureForType(memberType);