fix: end WebDriver BiDi session on disconnect (#11470)

Co-authored-by: Maksim Sadym <sadym@google.com>

docs/api/puppeteer.browser.disconnect.md

@@ -10,10 +10,10 @@ Disconnects Puppeteer from this [browser](./puppeteer.browser.md), but leaves th
 
 ```typescript
 class Browser {
-  abstract disconnect(): void;
+  abstract disconnect(): Promise<void>;
 }
 ```
 
 **Returns:**
 
-void
+Promise&lt;void&gt;

docs/api/puppeteer.browser.md

@@ -46,7 +46,7 @@ const browser = await puppeteer.launch();
 // Store the endpoint to be able to reconnect to the browser.
 const browserWSEndpoint = browser.wsEndpoint();
 // Disconnect puppeteer from the browser.
-browser.disconnect();
+await browser.disconnect();
 
 // Use the endpoint to reestablish a connection
 const browser2 = await puppeteer.connect({browserWSEndpoint});

packages/puppeteer-core/src/api/Browser.ts

@@ -228,7 +228,7 @@ export interface BrowserEvents extends Record<EventType, unknown> {
  * // Store the endpoint to be able to reconnect to the browser.
  * const browserWSEndpoint = browser.wsEndpoint();
  * // Disconnect puppeteer from the browser.
- * browser.disconnect();
+ * await browser.disconnect();
  *
  * // Use the endpoint to reestablish a connection
  * const browser2 = await puppeteer.connect({browserWSEndpoint});
@@ -414,7 +414,7 @@ export abstract class Browser extends EventEmitter<BrowserEvents> {
    * Disconnects Puppeteer from this {@link Browser | browser}, but leaves the
    * process running.
    */
-  abstract disconnect(): void;
+  abstract disconnect(): Promise<void>;
 
   /**
    * Whether Puppeteer is connected to this {@link Browser | browser}.

packages/puppeteer-core/src/bidi/BidiOverCdp.ts

@@ -49,6 +49,7 @@ export async function connectBidiOverCdp(
     close(): void {
       bidiServer.close();
       cdpConnectionAdapter.close();
+      cdp.dispose();
     },
     onmessage(_message: string): void {
       // The method is overridden by the Connection.

packages/puppeteer-core/src/bidi/Browser.ts

@@ -254,8 +254,8 @@ export class BidiBrowser extends Browser {
       return;
     }
     await this.#connection.send('browser.close', {});
-    this.#connection.dispose();
     await this.#closeCallback?.call(null);
+    this.#connection.dispose();
   }
 
   override get connected(): boolean {
@@ -323,7 +323,13 @@ export class BidiBrowser extends Browser {
     return this.#browserTarget;
   }
 
-  override disconnect(): void {
-    this;
+  override async disconnect(): Promise<void> {
+    try {
+      // Fail silently if the session cannot be ended.
+      await this.#connection.send('session.end', {});
+    } catch (e) {
+      debugError(e);
+    }
+    this.#connection.dispose();
   }
 }

packages/puppeteer-core/src/bidi/Connection.ts

@@ -111,6 +111,10 @@ export interface Commands {
     returnType: Bidi.EmptyResult;
   };
 
+  'session.end': {
+    params: Bidi.EmptyParams;
+    returnType: Bidi.EmptyResult;
+  };
   'session.new': {
     params: Bidi.Session.NewParameters;
     returnType: Bidi.Session.NewResult;

packages/puppeteer-core/src/cdp/Browser.ts

@@ -410,13 +410,14 @@ export class CdpBrowser extends BrowserBase {
 
   override async close(): Promise<void> {
     await this.#closeCallback.call(null);
-    this.disconnect();
+    await this.disconnect();
   }
 
-  override disconnect(): void {
+  override disconnect(): Promise<void> {
     this.#targetManager.dispose();
     this.#connection.dispose();
     this._detach();
+    return Promise.resolve();
   }
 
   override get connected(): boolean {

test/TestExpectations.json

@@ -383,12 +383,6 @@
     "parameters": ["chrome", "headless"],
     "expectations": ["FAIL"]
   },
-  {
-    "testIdPattern": "[browser.spec] Browser specs Browser.isConnected should set the browser connected state",
-    "platforms": ["darwin", "linux", "win32"],
-    "parameters": ["webDriverBiDi"],
-    "expectations": ["FAIL"]
-  },
   {
     "testIdPattern": "[browser.spec] Browser specs Browser.process should return child_process instance",
     "platforms": ["darwin", "linux", "win32"],

test/src/browser.spec.ts

@@ -70,7 +70,7 @@ describe('Browser specs', function () {
         protocol: browser.protocol,
       });
       expect(remoteBrowser.process()).toBe(null);
-      remoteBrowser.disconnect();
+      await remoteBrowser.disconnect();
     });
   });
 
@@ -84,7 +84,7 @@ describe('Browser specs', function () {
         protocol: browser.protocol,
       });
       expect(newBrowser.isConnected()).toBe(true);
-      newBrowser.disconnect();
+      await newBrowser.disconnect();
       expect(newBrowser.isConnected()).toBe(false);
     });
   });

test/src/browsercontext.spec.ts

@@ -230,7 +230,7 @@ describe('BrowserContext', function () {
     });
     const contexts = remoteBrowser.browserContexts();
     expect(contexts).toHaveLength(2);
-    remoteBrowser.disconnect();
+    await remoteBrowser.disconnect();
     await context.close();
   });
 

test/src/chromiumonly.spec.ts

@@ -38,7 +38,7 @@ describe('Chromium-Specific Launcher tests', function () {
             return 7 * 8;
           })
         ).toBe(56);
-        browser1.disconnect();
+        await browser1.disconnect();
 
         const browser2 = await puppeteer.connect({
           browserURL: browserURL + '/',
@@ -49,7 +49,7 @@ describe('Chromium-Specific Launcher tests', function () {
             return 8 * 7;
           })
         ).toBe(56);
-        browser2.disconnect();
+        await browser2.disconnect();
       } finally {
         await close();
       }

test/src/launcher.spec.ts

@@ -52,7 +52,7 @@ describe('Launcher specs', function () {
               return error_;
             });
           await server.waitForRequest('/one-style.css');
-          remote.disconnect();
+          await remote.disconnect();
           const error = await navigationPromise;
           expect(
             [
@@ -78,7 +78,7 @@ describe('Launcher specs', function () {
             .catch(error_ => {
               return error_;
             });
-          remote.disconnect();
+          await remote.disconnect();
           const error = await watchdog;
           expect(error.message).toContain('Session closed.');
         } finally {
@@ -635,7 +635,7 @@ describe('Launcher specs', function () {
               return 7 * 8;
             })
           ).toBe(56);
-          otherBrowser.disconnect();
+          await otherBrowser.disconnect();
 
           const secondPage = await browser.newPage();
           expect(
@@ -776,7 +776,7 @@ describe('Launcher specs', function () {
 
           await page2.close();
           await page1.close();
-          remoteBrowser.disconnect();
+          await remoteBrowser.disconnect();
           await browser.close();
         } finally {
           await close();
@@ -788,7 +788,7 @@ describe('Launcher specs', function () {
           const browserWSEndpoint = browser.wsEndpoint();
           const page = await browser.newPage();
           await page.goto(server.PREFIX + '/frames/nested-frames.html');
-          browser.disconnect();
+          await browser.disconnect();
 
           const remoteBrowser = await puppeteer.connect({
             browserWSEndpoint,
@@ -858,7 +858,7 @@ describe('Launcher specs', function () {
           const browserWSEndpoint = browserOne.wsEndpoint();
           const pageOne = await browserOne.newPage();
           await pageOne.goto(server.EMPTY_PAGE);
-          browserOne.disconnect();
+          await browserOne.disconnect();
 
           const browserTwo = await puppeteer.connect({
             browserWSEndpoint,

test/src/oopif.spec.ts

@@ -418,7 +418,7 @@ describe('OOPIF', function () {
       return target.url().endsWith('dynamic-oopif.html');
     });
     await target.page();
-    browser1.disconnect();
+    await browser1.disconnect();
   });
 
   it('should support lazy OOP frames', async () => {