diff --git a/src/server/chromium/crBrowser.ts b/src/server/chromium/crBrowser.ts index b4accdc4ad..4566c1c369 100644 --- a/src/server/chromium/crBrowser.ts +++ b/src/server/chromium/crBrowser.ts @@ -81,6 +81,8 @@ export class CRBrowser extends Browser { this._connection.on(ConnectionEvents.Disconnected, () => this._didClose()); this._session.on('Target.attachedToTarget', this._onAttachedToTarget.bind(this)); this._session.on('Target.detachedFromTarget', this._onDetachedFromTarget.bind(this)); + this._session.on('Browser.downloadWillBegin', this._onDownloadWillBegin.bind(this)); + this._session.on('Browser.downloadProgress', this._onDownloadProgress.bind(this)); } async newContext(options: types.BrowserContextOptions): Promise { @@ -188,6 +190,36 @@ export class CRBrowser extends Browser { } } + private _findOwningPage(frameId: string) { + for (const crPage of this._crPages.values()) { + const frame = crPage._page._frameManager.frame(frameId); + if (frame) + return crPage; + } + return null; + } + + _onDownloadWillBegin(payload: Protocol.Browser.downloadWillBeginPayload) { + const page = this._findOwningPage(payload.frameId); + assert(page, 'Download started in unknown page: ' + JSON.stringify(payload)); + page.willBeginDownload(); + + let originPage = page._initializedPage; + // If it's a new window download, report it on the opener page. + if (!originPage && page._opener) + originPage = page._opener._initializedPage; + if (!originPage) + return; + this._downloadCreated(originPage, payload.guid, payload.url, payload.suggestedFilename); + } + + _onDownloadProgress(payload: any) { + if (payload.state === 'completed') + this._downloadFinished(payload.guid, ''); + if (payload.state === 'canceled') + this._downloadFinished(payload.guid, 'canceled'); + } + async _closePage(crPage: CRPage) { await this._session.send('Target.closeTarget', { targetId: crPage._targetId }); } @@ -283,7 +315,8 @@ export class CRBrowserContext extends BrowserContext { promises.push(this._browser._session.send('Browser.setDownloadBehavior', { behavior: this._options.acceptDownloads ? 'allowAndName' : 'deny', browserContextId: this._browserContextId, - downloadPath: this._browser.options.downloadsPath + downloadPath: this._browser.options.downloadsPath, + eventsEnabled: true, })); } if (this._options.permissions) diff --git a/src/server/chromium/crPage.ts b/src/server/chromium/crPage.ts index 9f357b00c5..652c77c0c0 100644 --- a/src/server/chromium/crPage.ts +++ b/src/server/chromium/crPage.ts @@ -149,6 +149,10 @@ export class CRPage implements PageDelegate { return this._sessionForFrame(frame); } + willBeginDownload() { + this._mainFrameSession._willBeginDownload(); + } + async pageOrError(): Promise { return this._pagePromise; } @@ -415,8 +419,6 @@ class FrameSession { private _addBrowserListeners() { this._eventListeners.push(...[ helper.addEventListener(this._client, 'Inspector.targetCrashed', event => this._onTargetCrashed()), - helper.addEventListener(this._client, 'Page.downloadWillBegin', event => this._onDownloadWillBegin(event)), - helper.addEventListener(this._client, 'Page.downloadProgress', event => this._onDownloadProgress(event)), helper.addEventListener(this._client, 'Page.screencastFrame', event => this._onScreencastFrame(event)), helper.addEventListener(this._client, 'Page.windowOpen', event => this._onWindowOpen(event)), ]); @@ -818,26 +820,13 @@ class FrameSession { await this._page._onFileChooserOpened(handle); } - _onDownloadWillBegin(payload: Protocol.Page.downloadWillBeginPayload) { - let originPage = this._crPage._initializedPage; - // If it's a new window download, report it on the opener page. + _willBeginDownload() { + const originPage = this._crPage._initializedPage; if (!originPage) { // Resume the page creation with an error. The page will automatically close right // after the download begins. this._firstNonInitialNavigationCommittedReject(new Error('Starting new page download')); - if (this._crPage._opener) - originPage = this._crPage._opener._initializedPage; } - if (!originPage) - return; - this._crPage._browserContext._browser._downloadCreated(originPage, payload.guid, payload.url, payload.suggestedFilename); - } - - _onDownloadProgress(payload: Protocol.Page.downloadProgressPayload) { - if (payload.state === 'completed') - this._crPage._browserContext._browser._downloadFinished(payload.guid, ''); - if (payload.state === 'canceled') - this._crPage._browserContext._browser._downloadFinished(payload.guid, 'canceled'); } _onScreencastFrame(payload: Protocol.Page.screencastFramePayload) { diff --git a/tests/download.spec.ts b/tests/download.spec.ts index 87306fe1c4..6997bddb34 100644 --- a/tests/download.spec.ts +++ b/tests/download.spec.ts @@ -18,7 +18,6 @@ import { browserTest as it, expect } from './config/browserTest'; import fs from 'fs'; import path from 'path'; import crypto from 'crypto'; -import { chromiumVersionLessThan } from './config/utils'; it.describe('download event', () => { it.beforeEach(async ({server}) => { @@ -272,12 +271,7 @@ it.describe('download event', () => { await page.close(); }); - it('should report new window downloads', async ({browser, server, browserName, headless}) => { - it.fixme(browserName === 'chromium' && !headless); - - // TODO: - the test fails in headed Chromium as the popup page gets closed along - // with the session before download completed event arrives. - // - WebKit doesn't close the popup page + it('should report new window downloads', async ({browser, server}) => { const page = await browser.newPage({ acceptDownloads: true }); await page.setContent(`download`); const [ download ] = await Promise.all([ @@ -360,7 +354,7 @@ it.describe('download event', () => { expect(fs.existsSync(path.join(path1, '..'))).toBeFalsy(); }); - it('should close the context without awaiting the failed download', async ({browser, server, httpsServer, browserName, browserVersion}, testInfo) => { + it('should close the context without awaiting the failed download', async ({browser, server, httpsServer, browserName, headless}, testInfo) => { it.skip(browserName !== 'chromium', 'Only Chromium downloads on alt-click'); const page = await browser.newPage({ acceptDownloads: true }); @@ -378,13 +372,10 @@ it.describe('download event', () => { page.context().close(), ]); expect(downloadPath).toBe(null); - if (chromiumVersionLessThan(browserVersion, '91.0.4472.0')) - expect(saveError.message).toContain('File deleted upon browser context closure.'); - else - expect(saveError.message).toContain('File not found on disk. Check download.failure() for details.'); + expect(saveError.message).toContain('File not found on disk. Check download.failure() for details.'); }); - it('should close the context without awaiting the download', async ({browser, server, browserName, platform}, testInfo) => { + it('should close the context without awaiting the download', async ({browser, server, browserName, platform, headless}, testInfo) => { it.skip(browserName === 'webkit' && platform === 'linux', 'WebKit on linux does not convert to the download immediately upon receiving headers'); server.setRoute('/downloadStall', (req, res) => { @@ -408,7 +399,10 @@ it.describe('download event', () => { page.context().close(), ]); expect(downloadPath).toBe(null); - expect(saveError.message).toContain('File deleted upon browser context closure.'); + if (browserName === 'chromium' && headless) + expect(saveError.message).toContain('.saveAs: canceled'); + else + expect(saveError.message).toContain('File deleted upon browser context closure.'); }); it('should throw if browser dies', async ({ server, browserType, browserName, browserOptions, platform}, testInfo) => {