fix(connectOverCDP): ensure cleanup when remote ws closes (#9873)

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<void>();
+    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() {

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({