diff --git a/packages/playwright-core/src/server/browserContext.ts b/packages/playwright-core/src/server/browserContext.ts index 49fb561b75..7c556a9980 100644 --- a/packages/playwright-core/src/server/browserContext.ts +++ b/packages/playwright-core/src/server/browserContext.ts @@ -404,10 +404,6 @@ export abstract class BrowserContext extends SdkObject { if (this._customCloseHandler) { await this._customCloseHandler(); - } else 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(metadata))); } else { // Close the context. await this.doClose(); @@ -420,15 +416,8 @@ export abstract class BrowserContext extends SdkObject { await Promise.all(promises); // Custom handler should trigger didCloseInternal itself. - if (this._customCloseHandler) - return; - - // Persistent context should also close the browser. - if (this._isPersistentContext) - await this._browser.close(); - - // Bookkeeping. - this._didCloseInternal(); + if (!this._customCloseHandler) + this._didCloseInternal(); } await this._closePromise; } diff --git a/packages/playwright-core/src/server/chromium/crBrowser.ts b/packages/playwright-core/src/server/chromium/crBrowser.ts index ee95110832..5f0a07a7f1 100644 --- a/packages/playwright-core/src/server/chromium/crBrowser.ts +++ b/packages/playwright-core/src/server/chromium/crBrowser.ts @@ -334,13 +334,12 @@ export class CRBrowserContext extends BrowserContext { await Promise.all(promises); } + private _crPages() { + return [...this._browser._crPages.values()].filter(crPage => crPage._browserContext === this); + } + pages(): Page[] { - const result: Page[] = []; - for (const crPage of this._browser._crPages.values()) { - if (crPage._browserContext === this && crPage._initializedPage) - result.push(crPage._initializedPage); - } - return result; + return this._crPages().map(crPage => crPage._initializedPage).filter(Boolean) as Page[]; } async newPageDelegate(): Promise { @@ -489,19 +488,24 @@ export class CRBrowserContext extends BrowserContext { } async doClose() { - assert(this._browserContextId); // Headful chrome cannot dispose browser context with opened 'beforeunload' // dialogs, so we should close all that are currently opened. // We also won't get new ones since `Target.disposeBrowserContext` does not trigger // beforeunload. const openedBeforeUnloadDialogs: Dialog[] = []; - for (const crPage of this._browser._crPages.values()) { - if (crPage._browserContext !== this) - continue; + for (const crPage of this._crPages()) { const dialogs = [...crPage._page._frameManager._openedDialogs].filter(dialog => dialog.type() === 'beforeunload'); openedBeforeUnloadDialogs.push(...dialogs); } await Promise.all(openedBeforeUnloadDialogs.map(dialog => dialog.dismiss())); + + if (!this._browserContextId) { + await Promise.all(this._crPages().map(crPage => crPage._mainFrameSession._stopVideoRecording())); + // Closing persistent context should close the browser. + await this._browser.close(); + return; + } + await this._browser._session.send('Target.disposeBrowserContext', { browserContextId: this._browserContextId }); this._browser._contexts.delete(this._browserContextId); for (const [targetId, serviceWorker] of this._browser._serviceWorkers) { diff --git a/packages/playwright-core/src/server/firefox/ffBrowser.ts b/packages/playwright-core/src/server/firefox/ffBrowser.ts index 5a65732d8d..7df8d98b0a 100644 --- a/packages/playwright-core/src/server/firefox/ffBrowser.ts +++ b/packages/playwright-core/src/server/firefox/ffBrowser.ts @@ -359,9 +359,19 @@ export class FFBrowserContext extends BrowserContext { onClosePersistent() {} async doClose() { - assert(this._browserContextId); - await this._browser._connection.send('Browser.removeBrowserContext', { browserContextId: this._browserContextId }); - this._browser._contexts.delete(this._browserContextId); + if (!this._browserContextId) { + if (this._options.recordVideo) { + await this._browser._connection.send('Browser.setVideoRecordingOptions', { + options: undefined, + browserContextId: this._browserContextId + }); + } + // Closing persistent context should close the browser. + await this._browser.close(); + } else { + await this._browser._connection.send('Browser.removeBrowserContext', { browserContextId: this._browserContextId }); + this._browser._contexts.delete(this._browserContextId); + } } async cancelDownload(uuid: string) { diff --git a/packages/playwright-core/src/server/webkit/wkBrowser.ts b/packages/playwright-core/src/server/webkit/wkBrowser.ts index ee7dae1150..179c76dd45 100644 --- a/packages/playwright-core/src/server/webkit/wkBrowser.ts +++ b/packages/playwright-core/src/server/webkit/wkBrowser.ts @@ -338,9 +338,14 @@ export class WKBrowserContext extends BrowserContext { onClosePersistent() {} async doClose() { - assert(this._browserContextId); - await this._browser._browserSession.send('Playwright.deleteContext', { browserContextId: this._browserContextId }); - this._browser._contexts.delete(this._browserContextId); + if (!this._browserContextId) { + await Promise.all(this._wkPages().map(wkPage => wkPage._stopVideo())); + // Closing persistent context should close the browser. + await this._browser.close(); + } else { + await this._browser._browserSession.send('Playwright.deleteContext', { browserContextId: this._browserContextId }); + this._browser._contexts.delete(this._browserContextId); + } } async cancelDownload(uuid: string) { diff --git a/packages/playwright-core/src/server/webkit/wkPage.ts b/packages/playwright-core/src/server/webkit/wkPage.ts index e367a5dc69..edbee1d87c 100644 --- a/packages/playwright-core/src/server/webkit/wkPage.ts +++ b/packages/playwright-core/src/server/webkit/wkPage.ts @@ -825,7 +825,7 @@ export class WKPage implements PageDelegate { this._browserContext._browser._videoStarted(this._browserContext, screencastId, options.outputFile, this.pageOrError()); } - private async _stopVideo(): Promise { + async _stopVideo(): Promise { if (!this._recordingVideoFile) return; await this._pageProxySession.sendMayFail('Screencast.stopVideo');