diff --git a/packages/playwright-core/src/server/chromium/chromium.ts b/packages/playwright-core/src/server/chromium/chromium.ts index bf0ec447c4..1c50df84fc 100644 --- a/packages/playwright-core/src/server/chromium/chromium.ts +++ b/packages/playwright-core/src/server/chromium/chromium.ts @@ -25,7 +25,7 @@ import { rewriteErrorMessage } from '../../utils/stackTrace'; import { BrowserType } from '../browserType'; import { ConnectionTransport, ProtocolRequest, WebSocketTransport } from '../transport'; import { CRDevTools } from './crDevTools'; -import { BrowserOptions, BrowserProcess, PlaywrightOptions } from '../browser'; +import { Browser, BrowserOptions, BrowserProcess, PlaywrightOptions } from '../browser'; import * as types from '../types'; import { debugMode, fetchData, headersArrayToObject, removeFolders, streamToString } from '../../utils/utils'; import { RecentLogsCollector } from '../../utils/debugLogger'; @@ -36,6 +36,7 @@ import { CallMetadata } from '../instrumentation'; import http from 'http'; import https from 'https'; import { registry } from '../../utils/registry'; +import { ManualPromise } from 'playwright-core/lib/utils/async'; const ARTIFACTS_FOLDER = path.join(os.tmpdir(), 'playwright-artifacts-'); @@ -68,10 +69,15 @@ export class Chromium extends BrowserType { progress.throwIfAborted(); const chromeTransport = await WebSocketTransport.connect(progress, wsEndpoint, headersMap); - const doClose = async () => { + const cleanedUp = new ManualPromise(); + const doCleanup = async () => { await removeFolders([ artifactsDir ]); - await chromeTransport.closeAndWait(); await onClose?.(); + cleanedUp.resolve(); + }; + const doClose = async () => { + await chromeTransport.closeAndWait(); + await cleanedUp; }; const browserProcess: BrowserProcess = { close: doClose, kill: doClose }; const browserOptions: BrowserOptions = { @@ -88,7 +94,9 @@ export class Chromium extends BrowserType { tracesDir: artifactsDir }; progress.throwIfAborted(); - return await CRBrowser.connect(chromeTransport, browserOptions); + const browser = await CRBrowser.connect(chromeTransport, browserOptions); + browser.on(Browser.Events.Disconnected, doCleanup); + return browser; } private _createDevTools() { diff --git a/tests/chromium/chromium.spec.ts b/tests/chromium/chromium.spec.ts index 21a547ecb4..2df27927bc 100644 --- a/tests/chromium/chromium.spec.ts +++ b/tests/chromium/chromium.spec.ts @@ -18,6 +18,7 @@ import { contextTest as test, expect } from '../config/browserTest'; import { playwrightTest } from '../config/browserTest'; import http from 'http'; +import fs from 'fs'; import { getUserAgent } from 'playwright-core/lib/utils/utils'; import { suppressCertificateWarning } from '../config/utils'; @@ -111,6 +112,27 @@ playwrightTest('should connect to an existing cdp session', async ({ browserType } }); +playwrightTest('should cleanup artifacts dir after connectOverCDP disconnects due to ws close', async ({ browserType, toImpl, mode }, testInfo) => { + playwrightTest.skip(mode !== 'default'); + + const port = 9339 + testInfo.workerIndex; + const browserServer = await browserType.launch({ + args: ['--remote-debugging-port=' + port] + }); + const cdpBrowser = await browserType.connectOverCDP({ + endpointURL: `http://localhost:${port}/`, + }); + const dir = toImpl(cdpBrowser).options.artifactsDir; + const exists1 = fs.existsSync(dir); + await Promise.all([ + new Promise(f => cdpBrowser.on('disconnected', f)), + browserServer.close() + ]); + const exists2 = fs.existsSync(dir); + expect(exists1).toBe(true); + expect(exists2).toBe(false); +}); + playwrightTest('should connect to an existing cdp session twice', async ({ browserType, server }, testInfo) => { const port = 9339 + testInfo.workerIndex; const browserServer = await browserType.launch({