refactor: use generic implementation for waitForNetworkIdle (#11757)

docs/api/index.md

@@ -135,6 +135,7 @@ sidebar_label: API
 | [SnapshotOptions](./puppeteer.snapshotoptions.md)                           |                                                                                                                                                                                     |
 | [TracingOptions](./puppeteer.tracingoptions.md)                             |                                                                                                                                                                                     |
 | [Viewport](./puppeteer.viewport.md)                                         |                                                                                                                                                                                     |
+| [WaitForNetworkIdleOptions](./puppeteer.waitfornetworkidleoptions.md)       |                                                                                                                                                                                     |
 | [WaitForOptions](./puppeteer.waitforoptions.md)                             |                                                                                                                                                                                     |
 | [WaitForSelectorOptions](./puppeteer.waitforselectoroptions.md)             |                                                                                                                                                                                     |
 | [WaitForTargetOptions](./puppeteer.waitfortargetoptions.md)                 |                                                                                                                                                                                     |

docs/api/puppeteer.page.md

@@ -157,7 +157,7 @@ page.off('request', logRequest);
 | [waitForFrame](./puppeteer.page.waitforframe.md)                                               |           | Waits for a frame matching the given conditions to appear.                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                              |
 | [waitForFunction](./puppeteer.page.waitforfunction.md)                                         |           | Waits for the provided function, <code>pageFunction</code>, to return a truthy value when evaluated in the page's context.                                                                                                                                                                                                                                                                                                                                                                                                                                                                              |
 | [waitForNavigation](./puppeteer.page.waitfornavigation.md)                                     |           | Waits for the page to navigate to a new URL or to reload. It is useful when you run code that will indirectly cause the page to navigate.                                                                                                                                                                                                                                                                                                                                                                                                                                                               |
-| [waitForNetworkIdle](./puppeteer.page.waitfornetworkidle.md)                                   |           |                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                         |
+| [waitForNetworkIdle](./puppeteer.page.waitfornetworkidle.md)                                   |           | Waits for the network to be idle.                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                       |
 | [waitForRequest](./puppeteer.page.waitforrequest.md)                                           |           |                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                         |
 | [waitForResponse](./puppeteer.page.waitforresponse.md)                                         |           |                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                         |
 | [waitForSelector](./puppeteer.page.waitforselector.md)                                         |           | Wait for the <code>selector</code> to appear in page. If at the moment of calling the method the <code>selector</code> already exists, the method will return immediately. If the <code>selector</code> doesn't appear after the <code>timeout</code> milliseconds of waiting, the function will throw.                                                                                                                                                                                                                                                                                                 |

docs/api/puppeteer.page.waitfornetworkidle.md

@@ -4,25 +4,24 @@ sidebar_label: Page.waitForNetworkIdle
 
 # Page.waitForNetworkIdle() method
 
+Waits for the network to be idle.
+
 #### Signature:
 
 ```typescript
 class Page {
-  abstract waitForNetworkIdle(options?: {
+  waitForNetworkIdle(options?: WaitForNetworkIdleOptions): Promise<void>;
-    idleTime?: number;
-    timeout?: number;
-  }): Promise<void>;
 }
 ```
 
 ## Parameters
 
 | Parameter | Type                                                                  | Description                                         |
-| --------- | -------------------------------------------------- | ---------------------------------------- |
+| --------- | --------------------------------------------------------------------- | --------------------------------------------------- |
-| options   | &#123; idleTime?: number; timeout?: number; &#125; | _(Optional)_ Optional waiting parameters |
+| options   | [WaitForNetworkIdleOptions](./puppeteer.waitfornetworkidleoptions.md) | _(Optional)_ Options to configure waiting behavior. |
 
 **Returns:**
 
 Promise&lt;void&gt;
 
-Promise which resolves when network is idle
+A promise which resolves once the network is idle.

docs/api/puppeteer.waitfornetworkidleoptions.md

@@ -0,0 +1,20 @@
+---
+sidebar_label: WaitForNetworkIdleOptions
+---
+
+# WaitForNetworkIdleOptions interface
+
+#### Signature:
+
+```typescript
+export interface WaitForNetworkIdleOptions extends WaitTimeoutOptions
+```
+
+**Extends:** [WaitTimeoutOptions](./puppeteer.waittimeoutoptions.md)
+
+## Properties
+
+| Property    | Modifiers             | Type   | Description                                                                 | Default          |
+| ----------- | --------------------- | ------ | --------------------------------------------------------------------------- | ---------------- |
+| concurrency | <code>optional</code> | number | Maximum number concurrent of network connections to be considered inactive. | <code>0</code>   |
+| idleTime    | <code>optional</code> | number | Time (in milliseconds) the network should be idle.                          | <code>500</code> |

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

@@ -9,7 +9,8 @@ import type {Readable} from 'stream';
 import type {Protocol} from 'devtools-protocol';
 
 import {
-  delay,
+  concat,
+  EMPTY,
   filter,
   filterAsync,
   first,
@@ -17,23 +18,22 @@ import {
   from,
   map,
   merge,
+  mergeMap,
   of,
+  race,
   raceWith,
   startWith,
   switchMap,
+  takeUntil,
+  timer,
   type Observable,
 } from '../../third_party/rxjs/rxjs.js';
 import type {HTTPRequest} from '../api/HTTPRequest.js';
 import type {HTTPResponse} from '../api/HTTPResponse.js';
-import type {BidiNetworkManager} from '../bidi/NetworkManager.js';
 import type {Accessibility} from '../cdp/Accessibility.js';
 import type {Coverage} from '../cdp/Coverage.js';
 import type {DeviceRequestPrompt} from '../cdp/DeviceRequestPrompt.js';
-import type {
+import type {Credentials, NetworkConditions} from '../cdp/NetworkManager.js';
-  NetworkManager as CdpNetworkManager,
-  Credentials,
-  NetworkConditions,
-} from '../cdp/NetworkManager.js';
 import type {Tracing} from '../cdp/Tracing.js';
 import type {ConsoleMessage} from '../common/ConsoleMessage.js';
 import type {Device} from '../common/Device.js';
@@ -45,7 +45,6 @@ import {
   type Handler,
 } from '../common/EventEmitter.js';
 import type {FileChooser} from '../common/FileChooser.js';
-import {NetworkManagerEvent} from '../common/NetworkManagerEvents.js';
 import type {PDFOptions} from '../common/PDFOptions.js';
 import {TimeoutSettings} from '../common/TimeoutSettings.js';
 import type {
@@ -61,6 +60,7 @@ import {
   fromEmitterEvent,
   importFSPromises,
   isString,
+  NETWORK_IDLE_TIME,
   timeout,
   withSourcePuppeteerURLIfNone,
 } from '../common/util.js';
@@ -126,6 +126,24 @@ export interface Metrics {
   JSHeapTotalSize?: number;
 }
 
+/**
+ * @public
+ */
+export interface WaitForNetworkIdleOptions extends WaitTimeoutOptions {
+  /**
+   * Time (in milliseconds) the network should be idle.
+   *
+   * @defaultValue `500`
+   */
+  idleTime?: number;
+  /**
+   * Maximum number concurrent of network connections to be considered inactive.
+   *
+   * @defaultValue `0`
+   */
+  concurrency?: number;
+}
+
 /**
  * @public
  */
@@ -586,11 +604,48 @@ export abstract class Page extends EventEmitter<PageEvents> {
 
   #requestHandlers = new WeakMap<Handler<HTTPRequest>, Handler<HTTPRequest>>();
 
+  #requestsInFlight = 0;
+  #inflight$: Observable<number>;
+
   /**
    * @internal
    */
   constructor() {
     super();
+
+    this.#inflight$ = fromEmitterEvent(this, PageEvent.Request).pipe(
+      takeUntil(fromEmitterEvent(this, PageEvent.Close)),
+      mergeMap(request => {
+        return concat(
+          of(1),
+          race(
+            fromEmitterEvent(this, PageEvent.Response).pipe(
+              filter(response => {
+                return response.request()._requestId === request._requestId;
+              })
+            ),
+            fromEmitterEvent(this, PageEvent.RequestFailed).pipe(
+              filter(failure => {
+                return failure._requestId === request._requestId;
+              })
+            ),
+            fromEmitterEvent(this, PageEvent.RequestFinished).pipe(
+              filter(success => {
+                return success._requestId === request._requestId;
+              })
+            )
+          ).pipe(
+            map(() => {
+              return -1;
+            })
+          )
+        );
+      })
+    );
+
+    this.#inflight$.subscribe(count => {
+      this.#requestsInFlight += count;
+    });
   }
 
   /**
@@ -1699,34 +1754,45 @@ export abstract class Page extends EventEmitter<PageEvents> {
   }
 
   /**
-   * @param options - Optional waiting parameters
+   * Waits for the network to be idle.
-   * @returns Promise which resolves when network is idle
+   *
+   * @param options - Options to configure waiting behavior.
+   * @returns A promise which resolves once the network is idle.
    */
-  abstract waitForNetworkIdle(options?: {
+  waitForNetworkIdle(options: WaitForNetworkIdleOptions = {}): Promise<void> {
-    idleTime?: number;
+    return firstValueFrom(this.waitForNetworkIdle$(options));
-    timeout?: number;
+  }
-  }): Promise<void>;
 
   /**
    * @internal
    */
-  _waitForNetworkIdle(
+  waitForNetworkIdle$(
-    networkManager: BidiNetworkManager | CdpNetworkManager,
+    options: WaitForNetworkIdleOptions = {}
-    idleTime: number,
-    requestsInFlight = 0
   ): Observable<void> {
-    return merge(
+    const {
-      fromEmitterEvent(networkManager, NetworkManagerEvent.Request),
+      timeout: ms = this._timeoutSettings.timeout(),
-      fromEmitterEvent(networkManager, NetworkManagerEvent.Response),
+      idleTime = NETWORK_IDLE_TIME,
-      fromEmitterEvent(networkManager, NetworkManagerEvent.RequestFailed)
+      concurrency = 0,
-    ).pipe(
+    } = options;
-      startWith(undefined),
+
-      filter(() => {
+    return this.#inflight$.pipe(
-        return networkManager.inFlightRequestsCount() <= requestsInFlight;
+      startWith(this.#requestsInFlight),
-      }),
       switchMap(() => {
-        return of(undefined).pipe(delay(idleTime));
+        if (this.#requestsInFlight > concurrency) {
+          return EMPTY;
+        } else {
+          return timer(idleTime);
+        }
+      }),
+      map(() => {}),
+      raceWith(
+        timeout(ms),
+        fromEmitterEvent(this, PageEvent.Close).pipe(
+          map(() => {
+            throw new TargetCloseError('Page closed!');
           })
+        )
+      )
     );
   }
 

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

@@ -14,6 +14,7 @@ import {
   map,
   merge,
   raceWith,
+  zip,
 } from '../../third_party/rxjs/rxjs.js';
 import type {CDPSession} from '../api/CDPSession.js';
 import type {ElementHandle} from '../api/ElementHandle.js';
@@ -27,7 +28,12 @@ import type {WaitForSelectorOptions} from '../api/Page.js';
 import {UnsupportedOperation} from '../common/Errors.js';
 import type {TimeoutSettings} from '../common/TimeoutSettings.js';
 import type {Awaitable, NodeFor} from '../common/types.js';
-import {fromEmitterEvent, timeout, UTILITY_WORLD_NAME} from '../common/util.js';
+import {
+  fromEmitterEvent,
+  NETWORK_IDLE_TIME,
+  timeout,
+  UTILITY_WORLD_NAME,
+} from '../common/util.js';
 import {Deferred} from '../util/Deferred.js';
 import {disposeSymbol} from '../util/disposable.js';
 
@@ -128,21 +134,33 @@ export class BidiFrame extends Frame {
 
     const [readiness, networkIdle] = getBiDiReadinessState(waitUntil);
 
-    const response = await firstValueFrom(
+    const result$ = zip(
-      this.#page
+      from(
-        ._waitWithNetworkIdle(
         this.#context.connection.send('browsingContext.navigate', {
           context: this.#context.id,
           url,
           wait: readiness,
+        })
+      ),
+      ...(networkIdle !== null
+        ? [
+            this.#page.waitForNetworkIdle$({
+              timeout: ms,
+              concurrency: networkIdle === 'networkidle2' ? 2 : 0,
+              idleTime: NETWORK_IDLE_TIME,
             }),
-          networkIdle
+          ]
-        )
+        : [])
-        .pipe(raceWith(timeout(ms), from(this.#abortDeferred.valueOrThrow())))
+    ).pipe(
-        .pipe(rewriteNavigationError(url, ms))
+      map(([{result}]) => {
+        return result;
+      }),
+      raceWith(timeout(ms), from(this.#abortDeferred.valueOrThrow())),
+      rewriteNavigationError(url, ms)
     );
 
-    return this.#page.getNavigationResponse(response?.result.navigation);
+    const result = await firstValueFrom(result$);
+    return this.#page.getNavigationResponse(result.navigation);
   }
 
   @throwIfDetached
@@ -157,9 +175,7 @@ export class BidiFrame extends Frame {
 
     const [waitEvent, networkIdle] = getBiDiLifecycleEvent(waitUntil);
 
-    await firstValueFrom(
+    const result$ = zip(
-      this.#page
-        ._waitWithNetworkIdle(
       forkJoin([
         fromEmitterEvent(this.#context, waitEvent).pipe(first()),
         from(this.setFrameContent(html)),
@@ -168,11 +184,21 @@ export class BidiFrame extends Frame {
           return null;
         })
       ),
-          networkIdle
+      ...(networkIdle !== null
-        )
+        ? [
-        .pipe(raceWith(timeout(ms), from(this.#abortDeferred.valueOrThrow())))
+            this.#page.waitForNetworkIdle$({
-        .pipe(rewriteNavigationError('setContent', ms))
+              timeout: ms,
+              concurrency: networkIdle === 'networkidle2' ? 2 : 0,
+              idleTime: NETWORK_IDLE_TIME,
+            }),
+          ]
+        : [])
+    ).pipe(
+      raceWith(timeout(ms), from(this.#abortDeferred.valueOrThrow())),
+      rewriteNavigationError('setContent', ms)
     );
+
+    await firstValueFrom(result$);
   }
 
   context(): BrowsingContext {
@@ -190,7 +216,7 @@ export class BidiFrame extends Frame {
 
     const [waitUntilEvent, networkIdle] = getBiDiLifecycleEvent(waitUntil);
 
-    const navigatedObservable = merge(
+    const navigation$ = merge(
       forkJoin([
         fromEmitterEvent(
           this.#context,
@@ -211,13 +237,26 @@ export class BidiFrame extends Frame {
       })
     );
 
-    const response = await firstValueFrom(
+    const result$ = zip(
-      this.#page
+      navigation$,
-        ._waitWithNetworkIdle(navigatedObservable, networkIdle)
+      ...(networkIdle !== null
-        .pipe(raceWith(timeout(ms), from(this.#abortDeferred.valueOrThrow())))
+        ? [
+            this.#page.waitForNetworkIdle$({
+              timeout: ms,
+              concurrency: networkIdle === 'networkidle2' ? 2 : 0,
+              idleTime: NETWORK_IDLE_TIME,
+            }),
+          ]
+        : [])
+    ).pipe(
+      map(([{result}]) => {
+        return result;
+      }),
+      raceWith(timeout(ms), from(this.#abortDeferred.valueOrThrow()))
     );
 
-    return this.#page.getNavigationResponse(response?.result.navigation);
+    const result = await firstValueFrom(result$);
+    return this.#page.getNavigationResponse(result.navigation);
   }
 
   override waitForDevicePrompt(): never {

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

@@ -9,14 +9,12 @@ import type {Readable} from 'stream';
 import type * as Bidi from 'chromium-bidi/lib/cjs/protocol/protocol.js';
 import type Protocol from 'devtools-protocol';
 
-import type {Observable, ObservableInput} from '../../third_party/rxjs/rxjs.js';
 import {
-  first,
   firstValueFrom,
-  forkJoin,
   from,
   map,
   raceWith,
+  zip,
 } from '../../third_party/rxjs/rxjs.js';
 import type {CDPSession} from '../api/CDPSession.js';
 import type {BoundingBox} from '../api/ElementHandle.js';
@@ -75,7 +73,6 @@ import type {BidiHTTPRequest} from './HTTPRequest.js';
 import type {BidiHTTPResponse} from './HTTPResponse.js';
 import {BidiKeyboard, BidiMouse, BidiTouchscreen} from './Input.js';
 import type {BidiJSHandle} from './JSHandle.js';
-import type {BiDiNetworkIdle} from './lifecycle.js';
 import {getBiDiReadinessState, rewriteNavigationError} from './lifecycle.js';
 import {BidiNetworkManager} from './NetworkManager.js';
 import {createBidiHandle} from './Realm.js';
@@ -497,19 +494,32 @@ export class BidiPage extends Page {
 
     const [readiness, networkIdle] = getBiDiReadinessState(waitUntil);
 
-    const response = await firstValueFrom(
+    const result$ = zip(
-      this._waitWithNetworkIdle(
+      from(
         this.#connection.send('browsingContext.reload', {
           context: this.mainFrame()._id,
           wait: readiness,
+        })
+      ),
+      ...(networkIdle !== null
+        ? [
+            this.waitForNetworkIdle$({
+              timeout: ms,
+              concurrency: networkIdle === 'networkidle2' ? 2 : 0,
+              idleTime: NETWORK_IDLE_TIME,
             }),
-        networkIdle
+          ]
-      )
+        : [])
-        .pipe(raceWith(timeout(ms), from(this.#closedDeferred.valueOrThrow())))
+    ).pipe(
-        .pipe(rewriteNavigationError(this.url(), ms))
+      map(([{result}]) => {
+        return result;
+      }),
+      raceWith(timeout(ms), from(this.#closedDeferred.valueOrThrow())),
+      rewriteNavigationError(this.url(), ms)
     );
 
-    return this.getNavigationResponse(response?.result.navigation);
+    const result = await firstValueFrom(result$);
+    return this.getNavigationResponse(result.navigation);
   }
 
   override setDefaultNavigationTimeout(timeout: number): void {
@@ -701,48 +711,6 @@ export class BidiPage extends Page {
     return data;
   }
 
-  override async waitForNetworkIdle(
-    options: {idleTime?: number; timeout?: number} = {}
-  ): Promise<void> {
-    const {
-      idleTime = NETWORK_IDLE_TIME,
-      timeout: ms = this._timeoutSettings.timeout(),
-    } = options;
-
-    await firstValueFrom(
-      this._waitForNetworkIdle(this.#networkManager, idleTime).pipe(
-        raceWith(timeout(ms), from(this.#closedDeferred.valueOrThrow()))
-      )
-    );
-  }
-
-  /** @internal */
-  _waitWithNetworkIdle(
-    observableInput: ObservableInput<{
-      result: Bidi.BrowsingContext.NavigateResult;
-    } | null>,
-    networkIdle: BiDiNetworkIdle
-  ): Observable<{
-    result: Bidi.BrowsingContext.NavigateResult;
-  } | null> {
-    const delay = networkIdle
-      ? this._waitForNetworkIdle(
-          this.#networkManager,
-          NETWORK_IDLE_TIME,
-          networkIdle === 'networkidle0' ? 0 : 2
-        )
-      : from(Promise.resolve());
-
-    return forkJoin([
-      from(observableInput).pipe(first()),
-      delay.pipe(first()),
-    ]).pipe(
-      map(([response]) => {
-        return response;
-      })
-    );
-  }
-
   override async createCDPSession(): Promise<CDPSession> {
     const {sessionId} = await this.mainFrame()
       .context()

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

@@ -42,7 +42,6 @@ import {
   evaluationString,
   getReadableAsBuffer,
   getReadableFromProtocolStream,
-  NETWORK_IDLE_TIME,
   parsePDFOptions,
   timeout,
   validateDialogType,
@@ -909,24 +908,6 @@ export class CdpPage extends Page {
     return await this.target().createCDPSession();
   }
 
-  override async waitForNetworkIdle(
-    options: {idleTime?: number; timeout?: number} = {}
-  ): Promise<void> {
-    const {
-      idleTime = NETWORK_IDLE_TIME,
-      timeout: ms = this._timeoutSettings.timeout(),
-    } = options;
-
-    await firstValueFrom(
-      this._waitForNetworkIdle(
-        this.#frameManager.networkManager,
-        idleTime
-      ).pipe(
-        raceWith(timeout(ms), from(this.#sessionCloseDeferred.valueOrThrow()))
-      )
-    );
-  }
-
   override async goBack(
     options: WaitForOptions = {}
   ): Promise<HTTPResponse | null> {

packages/puppeteer-core/third_party/rxjs/rxjs.ts

@@ -6,6 +6,7 @@
 export {
   bufferCount,
   catchError,
+  concat,
   concatMap,
   defaultIfEmpty,
   defer,
@@ -37,6 +38,7 @@ export {
   tap,
   throwIfEmpty,
   timer,
+  zip,
 } from 'rxjs';
 
 export type * from 'rxjs';

test/src/page.spec.ts

@@ -851,12 +851,8 @@ describe('Page', function () {
           return Date.now();
         }),
         page
-          .evaluate(() => {
+          .evaluate(async () => {
-            return (async () => {
+            await Promise.all([fetch('/digits/1.png'), fetch('/digits/2.png')]);
-              await Promise.all([
-                fetch('/digits/1.png'),
-                fetch('/digits/2.png'),
-              ]);
             await new Promise(resolve => {
               return setTimeout(resolve, 200);
             });
@@ -865,7 +861,6 @@ describe('Page', function () {
               return setTimeout(resolve, 200);
             });
             await fetch('/digits/4.png');
-            })();
           })
           .then(() => {
             return Date.now();
@@ -938,6 +933,34 @@ describe('Page', function () {
 
       expect(error).toBe(false);
     });
+    it('should work with delayed response', async () => {
+      const {page, server} = await getTestState();
+      await page.goto(server.EMPTY_PAGE);
+      let response!: ServerResponse;
+      server.setRoute('/fetch-request-b.js', (_req, res) => {
+        response = res;
+      });
+      const t0 = Date.now();
+      const [t1, t2] = await Promise.all([
+        page.waitForNetworkIdle({idleTime: 100}).then(() => {
+          return Date.now();
+        }),
+        new Promise<number>(res => {
+          setTimeout(() => {
+            response.end();
+            res(Date.now());
+          }, 300);
+        }),
+        page.evaluate(async () => {
+          await fetch('/fetch-request-b.js');
+        }),
+      ]);
+      expect(t1).toBeGreaterThan(t2);
+      // request finished + idle time.
+      expect(t1 - t0).toBeGreaterThan(400);
+      // request finished + idle time - request finished.
+      expect(t1 - t2).toBeGreaterThanOrEqual(100);
+    });
   });
 
   describe('Page.exposeFunction', function () {