fix(network): invalidate stale network idle when requests start

packages/playwright-core/src/server/frames.ts

@@ -397,8 +397,10 @@ export class FrameManager {
     if (this._isExcludedFromNetworkIdle(request) || !frame)
       return;
     frame._inflightRequests.add(request);
-    if (frame._inflightRequests.size === 1)
+    if (frame._inflightRequests.size === 1) {
       frame._stopNetworkIdleTimer();
+      this._page.mainFrame()._recalculateNetworkIdle();
+    }
   }
 
   private _isExcludedFromNetworkIdle(request: network.Request): boolean {
@@ -578,7 +580,7 @@ export class Frame extends SdkObject<FrameEventMap> {
     this._stopNetworkIdleTimer();
     if (this._inflightRequests.size === 0)
       this._startNetworkIdleTimer();
-    this._page.mainFrame()._recalculateNetworkIdle(this);
+    this._page.mainFrame()._recalculateNetworkIdle();
     this.onLifecycleEvent('commit');
   }
 
@@ -635,10 +637,10 @@ export class Frame extends SdkObject<FrameEventMap> {
     });
   }
 
-  _recalculateNetworkIdle(frameThatAllowsRemovingNetworkIdle?: Frame) {
+  _recalculateNetworkIdle() {
     let isNetworkIdle = this._firedNetworkIdleSelf;
     for (const child of this._childFrames) {
-      child._recalculateNetworkIdle(frameThatAllowsRemovingNetworkIdle);
+      child._recalculateNetworkIdle();
       // We require networkidle event to be fired in the whole frame subtree, and then consider it done.
       if (!child._firedLifecycleEvents.has('networkidle'))
         isNetworkIdle = false;
@@ -649,9 +651,8 @@ export class Frame extends SdkObject<FrameEventMap> {
       if (this === this._page.mainFrame() && this._url !== 'about:blank')
         this.apiLog(`  "networkidle" event fired`);
     }
-    if (frameThatAllowsRemovingNetworkIdle !== this && this._firedLifecycleEvents.has('networkidle') && !isNetworkIdle) {
-      // Usually, networkidle is fired once and not removed after that.
-      // However, when we clear them right before a new commit, this is allowed for a particular frame.
+    if (this._firedLifecycleEvents.has('networkidle') && !isNetworkIdle) {
+      // A new busy period invalidates networkidle for the frame and its ancestors.
       this._firedLifecycleEvents.delete('networkidle');
       this.emit(Frame.Events.RemoveLifecycle, 'networkidle');
     }

tests/library/chromium/network-idle.spec.ts

@@ -0,0 +1,80 @@
+/**
+ * Copyright (c) Microsoft Corporation.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import { browserTest as test, expect } from '../../config/browserTest';
+import type { ServerResponse } from 'http';
+
+for (const finishBeforeDCL of [true, false]) {
+  test(`should reach networkidle when requests finish ${finishBeforeDCL ? 'before' : 'after'} DOMContentLoaded following an initial idle`, async ({ page, server }) => {
+    test.info().annotations.push({ type: 'issue', description: 'https://github.com/microsoft/playwright/issues/42598' });
+
+    let schemaResponse: ServerResponse;
+    const schemaFinished = page.waitForEvent('requestfinished', request => request.url().endsWith('/schema.json'));
+    const scriptFinished = page.waitForEvent('requestfinished', request => request.url().endsWith('/finish.js'));
+    server.setRoute('/network-idle.html', (req, res) => {
+      res.setHeader('Content-Type', 'text/html');
+      res.end('<script src="/app.js"></script>');
+    });
+    server.setRoute('/app.js', (req, res) => {
+      res.setHeader('Content-Type', 'text/javascript');
+      res.end(`
+        debugger;
+        window.result = fetch('/schema.json').then(r => r.json());
+        document.write('<script src="/finish.js"></' + 'script>');
+      `);
+    });
+    server.setRoute('/schema.json', (req, res) => {
+      schemaResponse = res;
+      if (finishBeforeDCL)
+        res.end('{"name":"done"}');
+    });
+    server.setRoute('/finish.js', async (req, res) => {
+      if (finishBeforeDCL)
+        await schemaFinished;
+      res.setHeader('Content-Type', 'text/javascript');
+      res.end('debugger;');
+    });
+    let domContentLoaded = false;
+    page.on('domcontentloaded', () => domContentLoaded = true);
+    const session = await page.context().newCDPSession(page);
+    await session.send('Debugger.enable');
+
+    // Hold the parser, but not the network or Playwright's real idle timer.
+    const firstPause = new Promise<void>(resolve => session.once('Debugger.paused', () => resolve()));
+    await page.goto(server.PREFIX + '/network-idle.html', { waitUntil: 'commit' });
+    await firstPause;
+    await page.waitForLoadState('networkidle');
+    expect(domContentLoaded).toBe(false);
+
+    const secondPause = new Promise<void>(resolve => session.once('Debugger.paused', () => resolve()));
+    await session.send('Debugger.resume');
+    await secondPause;
+    await scriptFinished;
+    if (finishBeforeDCL)
+      await schemaFinished;
+    expect(domContentLoaded).toBe(false);
+
+    await session.send('Debugger.resume');
+    await page.waitForLoadState('domcontentloaded');
+    if (!finishBeforeDCL)
+      schemaResponse.end('{"name":"done"}');
+    await schemaFinished;
+    await page.waitForLoadState('load');
+    await page.waitForLoadState('networkidle', { timeout: 3000 });
+    expect(await page.evaluate(() => window['result'])).toEqual({ name: 'done' });
+    await session.detach();
+  });
+}

tests/page/page-network-idle.spec.ts

@@ -18,12 +18,126 @@
 import { test as it, expect } from './pageTest';
 import type { Frame } from 'playwright-core';
 import type { TestServer } from '../config/testserver';
+import type { ServerResponse } from 'http';
+import type { server as coreServer } from '../../packages/playwright-core/lib/coreBundle';
 
 it('should navigate to empty page with networkidle', async ({ page, server }) => {
   const response = await page.goto(server.EMPTY_PAGE, { waitUntil: 'networkidle' });
   expect(response.status()).toBe(200);
 });
 
+for (const inChildFrame of [false, true]) {
+  it(`should wait for repeated networkidle in the ${inChildFrame ? 'child' : 'main'} frame`, async ({ page, server }) => {
+    it.info().annotations.push({ type: 'issue', description: 'https://github.com/microsoft/playwright/issues/42598' });
+
+    await page.goto(server.EMPTY_PAGE);
+    if (inChildFrame)
+      await page.setContent(`<iframe src="${server.EMPTY_PAGE}"></iframe><iframe src="${server.EMPTY_PAGE}"></iframe>`);
+    await page.waitForLoadState('networkidle');
+    const frame = inChildFrame ? page.frames()[1] : page.mainFrame();
+    let responseA: ServerResponse;
+    let responseB: ServerResponse;
+    server.setRoute('/fetch-a', (req, res) => responseA = res);
+    server.setRoute('/fetch-b', (req, res) => responseB = res);
+
+    for (let i = 0; i < 3; ++i) {
+      await Promise.all([
+        server.waitForRequest('/fetch-a'),
+        server.waitForRequest('/fetch-b'),
+        page.waitForRequest(server.PREFIX + '/fetch-a'),
+        page.waitForRequest(server.PREFIX + '/fetch-b'),
+        frame.evaluate(() => {
+          void fetch('/fetch-a');
+          void fetch('/fetch-b');
+        }),
+      ]);
+      let frameIdle = false;
+      let pageIdle = false;
+      const idlePromise = Promise.all([
+        frame.waitForLoadState('networkidle').then(() => frameIdle = true),
+        page.waitForLoadState('networkidle').then(() => pageIdle = true),
+      ]);
+      // Round trips let an incorrectly resolved wait settle while requests are held.
+      await page.evaluate(() => 1);
+      expect(frameIdle).toBe(false);
+      expect(pageIdle).toBe(false);
+
+      const requestFinished = page.waitForEvent('requestfinished', request => request.url().endsWith('/fetch-a'));
+      responseA.end('a');
+      await requestFinished;
+      await page.evaluate(() => 1);
+      expect(frameIdle).toBe(false);
+      expect(pageIdle).toBe(false);
+
+      let timerTriggered = false;
+      const timer = setTimeout(() => timerTriggered = true, 500);
+      try {
+        responseB.end('b');
+        await idlePromise;
+        expect(timerTriggered).toBe(true);
+      } finally {
+        clearTimeout(timer);
+      }
+    }
+  });
+}
+
+it('should notify networkidle transitions once and recover when a busy frame detaches', async ({ page, server, toImpl }) => {
+  await page.goto(server.EMPTY_PAGE);
+  await page.setContent(`<iframe src="${server.EMPTY_PAGE}"></iframe>`, { waitUntil: 'networkidle' });
+  const frame = page.frames()[1];
+  const events: string[] = [];
+  for (const [name, target] of [['main', page.mainFrame()], ['child', frame]] as const) {
+    const impl: coreServer.Frame = toImpl(target);
+    impl.on('addlifecycle', event => {
+      if (event === 'networkidle')
+        events.push(`${name}:idle`);
+    });
+    impl.on('removelifecycle', event => {
+      if (event === 'networkidle')
+        events.push(`${name}:busy`);
+    });
+  }
+
+  let responseA: ServerResponse;
+  let responseB: ServerResponse;
+  server.setRoute('/fetch-a', (req, res) => responseA = res);
+  server.setRoute('/fetch-b', (req, res) => responseB = res);
+  await Promise.all([
+    server.waitForRequest('/fetch-a'),
+    server.waitForRequest('/fetch-b'),
+    page.waitForRequest(server.PREFIX + '/fetch-a'),
+    page.waitForRequest(server.PREFIX + '/fetch-b'),
+    frame.evaluate(() => {
+      void fetch('/fetch-a');
+      void fetch('/fetch-b');
+    }),
+  ]);
+  expect(events).toEqual(['child:busy', 'main:busy']);
+
+  const requestFinished = page.waitForEvent('requestfinished', request => request.url().endsWith('/fetch-a'));
+  responseA.end('a');
+  await requestFinished;
+  expect(events).toEqual(['child:busy', 'main:busy']);
+  responseB.end('b');
+  await page.waitForLoadState('networkidle');
+  expect(events).toEqual(['child:busy', 'main:busy', 'child:idle', 'main:idle']);
+  events.length = 0;
+
+  const [request] = await Promise.all([
+    page.waitForRequest(server.PREFIX + '/fetch-a'),
+    frame.evaluate(() => { void fetch('/fetch-a'); }),
+  ]);
+  expect(events).toEqual(['child:busy', 'main:busy']);
+  await Promise.all([
+    page.waitForEvent('requestfailed', failed => failed === request),
+    page.waitForEvent('framedetached', detached => detached === frame),
+    page.evaluate(() => document.querySelector('iframe').remove()),
+  ]);
+  await page.waitForLoadState('networkidle');
+  expect(events).toEqual(['child:busy', 'main:busy', 'main:idle']);
+});
+
 async function networkIdleTest(frame: Frame, server: TestServer, action: () => Promise<any>, isSetContent?: boolean) {
   const waitForRequest = (suffix: string) => {
     return Promise.all([