From 06c1588016e1ebef5ed8f079dc34507f6d781e07 Mon Sep 17 00:00:00 2001
From: Nikolay Vitkov <34244704+Lightning00Blade@users.noreply.github.com>
Date: Mon, 4 Sep 2023 16:16:37 +0200
Subject: [PATCH] fix: LifecycleWatcher sub frames handling (#10841)

---
 .../puppeteer-core/src/common/LifecycleWatcher.ts    | 10 ++++++++--
 test/TestExpectations.json                           | 12 ++++++++++++
 test/src/navigation.spec.ts                          | 11 +++++++++++
 3 files changed, 31 insertions(+), 2 deletions(-)

diff --git a/packages/puppeteer-core/src/common/LifecycleWatcher.ts b/packages/puppeteer-core/src/common/LifecycleWatcher.ts
index 7a1356b299e..375f574352f 100644
--- a/packages/puppeteer-core/src/common/LifecycleWatcher.ts
+++ b/packages/puppeteer-core/src/common/LifecycleWatcher.ts
@@ -22,6 +22,7 @@ import {Deferred} from '../util/Deferred.js';
 
 import {TimeoutError} from './Errors.js';
 import {CDPFrame, FrameEmittedEvents} from './Frame.js';
+import {FrameManagerEmittedEvents} from './FrameManager.js';
 import {HTTPRequest} from './HTTPRequest.js';
 import {NetworkManager, NetworkManagerEmittedEvents} from './NetworkManager.js';
 import {
@@ -100,8 +101,9 @@ export class LifecycleWatcher {
     this.#timeout = timeout;
     this.#eventListeners = [
       addEventListener(
-        frame,
-        FrameEmittedEvents.LifecycleEvent,
+        // Revert if TODO #1 is done
+        frame._frameManager,
+        FrameManagerEmittedEvents.LifecycleEvent,
         this.#checkLifecycleComplete.bind(this)
       ),
       addEventListener(
@@ -254,6 +256,10 @@ export class LifecycleWatcher {
           return false;
         }
       }
+      // TODO(#1): Its possible we don't need this check
+      // CDP provided the correct order for Loading Events
+      // And NetworkIdle is a global state
+      // Consider removing
       for (const child of frame.childFrames()) {
         if (
           child._hasStartedLoading &&
diff --git a/test/TestExpectations.json b/test/TestExpectations.json
index c657ca8115c..2577962cac6 100644
--- a/test/TestExpectations.json
+++ b/test/TestExpectations.json
@@ -1181,6 +1181,12 @@
     "parameters": ["webDriverBiDi"],
     "expectations": ["FAIL"]
   },
+  {
+    "testIdPattern": "[navigation.spec] navigation Page.goto should navigate to page with iframe and networkidle0",
+    "platforms": ["darwin", "linux", "win32"],
+    "parameters": ["webDriverBiDi"],
+    "expectations": ["FAIL"]
+  },
   {
     "testIdPattern": "[navigation.spec] navigation Page.goto should send referer",
     "platforms": ["darwin", "linux", "win32"],
@@ -2939,6 +2945,12 @@
     "parameters": ["cdp", "firefox"],
     "expectations": ["SKIP"]
   },
+  {
+    "testIdPattern": "[navigation.spec] navigation Page.goto should navigate to page with iframe and networkidle0",
+    "platforms": ["darwin", "linux", "win32"],
+    "parameters": ["cdp", "firefox"],
+    "expectations": ["TIMEOUT"]
+  },
   {
     "testIdPattern": "[navigation.spec] navigation Page.goto should navigate to URL with hash and fire requests without hash",
     "platforms": ["darwin", "linux", "win32"],
diff --git a/test/src/navigation.spec.ts b/test/src/navigation.spec.ts
index afdc3bdc590..89f761fc5b5 100644
--- a/test/src/navigation.spec.ts
+++ b/test/src/navigation.spec.ts
@@ -129,6 +129,17 @@ describe('navigation', function () {
       });
       expect(response!.status()).toBe(200);
     });
+    it('should navigate to page with iframe and networkidle0', async () => {
+      const {page, server} = await getTestState();
+
+      const response = await page.goto(
+        server.PREFIX + '/frames/one-frame.html',
+        {
+          waitUntil: 'networkidle0',
+        }
+      );
+      expect(response!.status()).toBe(200);
+    });
     it('should navigate to empty page with networkidle2', async () => {
       const {page, server} = await getTestState();