diff --git a/browsers.json b/browsers.json index 1850c6d3cde40..0bdc09bcb6624 100644 --- a/browsers.json +++ b/browsers.json @@ -8,7 +8,7 @@ }, { "name": "firefox", - "revision": "1179", + "revision": "1180", "download": true }, { diff --git a/src/server/browserContext.ts b/src/server/browserContext.ts index a205a7b250586..b8a9a835efbcd 100644 --- a/src/server/browserContext.ts +++ b/src/server/browserContext.ts @@ -265,15 +265,10 @@ export abstract class BrowserContext extends EventEmitter { } async close() { - if (this._isPersistentContext) { - // Default context is only created in 'persistent' mode and closing it should close - // the browser. - await this._browser.close(); - return; - } if (this._closedStatus === 'open') { this._closedStatus = 'closing'; - await this._doClose(); + + // Collect videos/downloads that we will await. const promises: Promise[] = []; for (const download of this._downloads) promises.push(download.delete()); @@ -281,7 +276,24 @@ export abstract class BrowserContext extends EventEmitter { if (video._context === this) promises.push(video._finishedPromise); } + + if (this._isPersistentContext) { + // Close all the pages instead of the context, + // because we cannot close the default context. + await Promise.all(this.pages().map(page => page.close())); + } else { + // Close the context. + await this._doClose(); + } + + // Wait for the videos/downloads to finish. await Promise.all(promises); + + // Persistent context should also close the browser. + if (this._isPersistentContext) + await this._browser.close(); + + // Bookkeeping. for (const listener of contextListeners) await listener.onContextDestroyed(this); this._didCloseInternal(); diff --git a/test/downloads-path.spec.ts b/test/downloads-path.spec.ts index 1439135e73940..77f55e2e833ed 100644 --- a/test/downloads-path.spec.ts +++ b/test/downloads-path.spec.ts @@ -83,7 +83,6 @@ it('should delete downloads when context closes', async ({downloadsBrowser, serv expect(fs.existsSync(path)).toBeTruthy(); await page.close(); expect(fs.existsSync(path)).toBeFalsy(); - }); it('should report downloads in downloadsPath folder', async ({downloadsBrowser, testOutputPath, server}) => { @@ -98,7 +97,7 @@ it('should report downloads in downloadsPath folder', async ({downloadsBrowser, await page.close(); }); -it('should accept downloads', async ({persistentDownloadsContext, testOutputPath, server}) => { +it('should accept downloads in persistent context', async ({persistentDownloadsContext, testOutputPath, server}) => { const page = persistentDownloadsContext.pages()[0]; const [ download ] = await Promise.all([ page.waitForEvent('download'), @@ -110,13 +109,14 @@ it('should accept downloads', async ({persistentDownloadsContext, testOutputPath expect(path.startsWith(testOutputPath(''))).toBeTruthy(); }); -it('should not delete downloads when the context closes', async ({persistentDownloadsContext}) => { +it('should delete downloads when persistent context closes', async ({persistentDownloadsContext}) => { const page = persistentDownloadsContext.pages()[0]; const [ download ] = await Promise.all([ page.waitForEvent('download'), page.click('a') ]); const path = await download.path(); - await persistentDownloadsContext.close(); expect(fs.existsSync(path)).toBeTruthy(); + await persistentDownloadsContext.close(); + expect(fs.existsSync(path)).toBeFalsy(); }); diff --git a/test/screencast.spec.ts b/test/screencast.spec.ts index b84402d2f2533..c499c09aac1e8 100644 --- a/test/screencast.spec.ts +++ b/test/screencast.spec.ts @@ -341,9 +341,7 @@ describe('screencast', suite => { expect(await videoPlayer.videoHeight).toBe(720); }); - it('should capture static page in persistent context', test => { - test.fixme('We do not wait for the video finish when closing persistent context.'); - }, async ({launchPersistent, testOutputPath}) => { + it('should capture static page in persistent context', async ({launchPersistent, testOutputPath}) => { const videosPath = testOutputPath(''); const size = { width: 320, height: 240 }; const { context, page } = await launchPersistent({