refactor: use RxJS for waitForTarget (#11029)

docs/api/puppeteer.browsercontext.waitfortarget.md

@@ -14,9 +14,7 @@ This will look all open [browser contexts](./puppeteer.browsercontext.md).
 class BrowserContext {
   abstract waitForTarget(
     predicate: (x: Target) => boolean | Promise<boolean>,
-    options?: {
-      timeout?: number;
-    }
+    options?: WaitForTargetOptions
   ): Promise<Target>;
 }
 ```
@@ -26,7 +24,7 @@ class BrowserContext {
 | Parameter | Type                                                                         | Description  |
 | --------- | ---------------------------------------------------------------------------- | ------------ |
 | predicate | (x: [Target](./puppeteer.target.md)) =&gt; boolean \| Promise&lt;boolean&gt; |              |
-| options   | { timeout?: number; }                                                        | _(Optional)_ |
+| options   | [WaitForTargetOptions](./puppeteer.waitfortargetoptions.md)                  | _(Optional)_ |
 
 **Returns:**
 

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

@@ -18,15 +18,23 @@ import type {ChildProcess} from 'child_process';
 
 import type {Protocol} from 'devtools-protocol';
 
+import {
+  firstValueFrom,
+  from,
+  merge,
+  raceWith,
+  filterAsync,
+  fromEvent,
+  type Observable,
+} from '../../third_party/rxjs/rxjs.js';
 import {EventEmitter, type EventType} from '../common/EventEmitter.js';
-import {debugError, waitWithTimeout} from '../common/util.js';
-import {Deferred} from '../util/Deferred.js';
+import {debugError} from '../common/util.js';
+import {timeout} from '../common/util.js';
 import {asyncDisposeSymbol, disposeSymbol} from '../util/disposable.js';
 
 import type {BrowserContext} from './BrowserContext.js';
 import type {Page} from './Page.js';
 import type {Target} from './Target.js';
-
 /**
  * @public
  */
@@ -387,31 +395,14 @@ export abstract class Browser extends EventEmitter<BrowserEvents> {
     predicate: (x: Target) => boolean | Promise<boolean>,
     options: WaitForTargetOptions = {}
   ): Promise<Target> {
-    const {timeout = 30000} = options;
-    const targetDeferred = Deferred.create<Target | PromiseLike<Target>>();
-
-    this.on(BrowserEvent.TargetCreated, check);
-    this.on(BrowserEvent.TargetChanged, check);
-    try {
-      this.targets().forEach(check);
-      if (!timeout) {
-        return await targetDeferred.valueOrThrow();
-      }
-      return await waitWithTimeout(
-        targetDeferred.valueOrThrow(),
-        'target',
-        timeout
-      );
-    } finally {
-      this.off(BrowserEvent.TargetCreated, check);
-      this.off(BrowserEvent.TargetChanged, check);
-    }
-
-    async function check(target: Target): Promise<void> {
-      if ((await predicate(target)) && !targetDeferred.resolved()) {
-        targetDeferred.resolve(target);
-      }
-    }
+    const {timeout: ms = 30000} = options;
+    return await firstValueFrom(
+      merge(
+        fromEvent(this, BrowserEvent.TargetCreated) as Observable<Target>,
+        fromEvent(this, BrowserEvent.TargetChanged) as Observable<Target>,
+        from(this.targets())
+      ).pipe(filterAsync(predicate), raceWith(timeout(ms)))
+    );
   }
 
   /**

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

@@ -18,7 +18,7 @@ import {EventEmitter, type EventType} from '../common/EventEmitter.js';
 import {debugError} from '../common/util.js';
 import {asyncDisposeSymbol, disposeSymbol} from '../util/disposable.js';
 
-import type {Browser, Permission} from './Browser.js';
+import type {Browser, Permission, WaitForTargetOptions} from './Browser.js';
 import type {Page} from './Page.js';
 import type {Target} from './Target.js';
 
@@ -128,7 +128,7 @@ export abstract class BrowserContext extends EventEmitter<BrowserContextEvents>
    */
   abstract waitForTarget(
     predicate: (x: Target) => boolean | Promise<boolean>,
-    options?: {timeout?: number}
+    options?: WaitForTargetOptions
   ): Promise<Target>;
 
   /**

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

@@ -16,6 +16,7 @@
 
 import * as Bidi from 'chromium-bidi/lib/cjs/protocol/protocol.js';
 
+import type {WaitForTargetOptions} from '../api/Browser.js';
 import {BrowserContext} from '../api/BrowserContext.js';
 import type {Page} from '../api/Page.js';
 import type {Target} from '../api/Target.js';
@@ -58,7 +59,7 @@ export class BidiBrowserContext extends BrowserContext {
 
   override waitForTarget(
     predicate: (x: Target) => boolean | Promise<boolean>,
-    options: {timeout?: number} = {}
+    options: WaitForTargetOptions = {}
   ): Promise<Target> {
     return this.#browser.waitForTarget(target => {
       return target.browserContext() === this && predicate(target);

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

@@ -288,8 +288,9 @@ export class BidiConnection extends EventEmitter<BidiEvents> {
       return;
     }
     this.#closed = true;
-    this.#transport.onmessage = undefined;
-    this.#transport.onclose = undefined;
+    // Both may still be invoked and produce errors
+    this.#transport.onmessage = () => {};
+    this.#transport.onclose = () => {};
 
     this.#callbacks.clear();
   }

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

@@ -27,6 +27,7 @@ import {
   type IsPageTargetCallback,
   type Permission,
   type TargetFilterCallback,
+  type WaitForTargetOptions,
 } from '../api/Browser.js';
 import {BrowserContext, BrowserContextEvent} from '../api/BrowserContext.js';
 import {CDPSessionEvent, type CDPSession} from '../api/CDPSession.js';
@@ -453,7 +454,7 @@ export class CdpBrowserContext extends BrowserContext {
 
   override waitForTarget(
     predicate: (x: Target) => boolean | Promise<boolean>,
-    options: {timeout?: number} = {}
+    options: WaitForTargetOptions = {}
   ): Promise<Target> {
     return this.#browser.waitForTarget(target => {
       return target.browserContext() === this && predicate(target);

test/src/navigation.spec.ts

@@ -19,6 +19,7 @@ import type {ServerResponse} from 'http';
 import expect from 'expect';
 import {type Frame, TimeoutError} from 'puppeteer';
 import type {HTTPRequest} from 'puppeteer-core/internal/api/HTTPRequest.js';
+import type {HTTPResponse} from 'puppeteer-core/internal/api/HTTPResponse.js';
 import {Deferred} from 'puppeteer-core/internal/util/Deferred.js';
 
 import {getTestState, setupTestBrowserHooks} from './mocha-utils.js';
@@ -837,18 +838,27 @@ describe('navigation', function () {
       server.setRoute('/one-style.html', (_req, res) => {
         return serverResponses.push(res);
       });
-      const navigations = [];
+      const navigations: Array<Promise<HTTPResponse | null>> = [];
       for (let i = 0; i < 3; ++i) {
         navigations.push(frames[i]!.goto(server.PREFIX + '/one-style.html'));
         await server.waitForRequest('/one-style.html');
       }
       // Respond from server out-of-order.
       const serverResponseTexts = ['AAA', 'BBB', 'CCC'];
-      for (const i of [1, 2, 0]) {
-        serverResponses[i]!.end(serverResponseTexts[i]);
-        const response = (await navigations[i])!;
-        expect(response.frame()).toBe(frames[i]);
-        expect(await response.text()).toBe(serverResponseTexts[i]);
+      try {
+        for (const i of [1, 2, 0]) {
+          const response = await getResponse(i);
+          expect(response.frame()).toBe(frames[i]);
+          expect(await response.text()).toBe(serverResponseTexts[i]);
+        }
+      } catch (error) {
+        await Promise.all([getResponse(0), getResponse(1), getResponse(2)]);
+        throw error;
+      }
+
+      async function getResponse(index: number) {
+        serverResponses[index]!.end(serverResponseTexts[index]);
+        return (await navigations[index])!;
       }
     });
   });