From 74380303ac6cc6e2d84948a10920d56e665ccebe Mon Sep 17 00:00:00 2001
From: jrandolf <101637635+jrandolf@users.noreply.github.com>
Date: Tue, 17 May 2022 14:15:44 +0200
Subject: [PATCH] fix: only check loading iframe in lifecycling (#8348)

---
 src/common/FrameManager.ts         | 20 ++++++++++++++++++++
 src/common/LifecycleWatcher.ts     |  7 ++++++-
 test/assets/frames/lazy-frame.html |  3 +++
 test/assets/lazy-oopif-frame.html  |  3 +++
 test/frame.spec.ts                 | 12 ++++++++++++
 test/oopif.spec.ts                 | 18 +++++++++++++++++-
 6 files changed, 61 insertions(+), 2 deletions(-)
 create mode 100644 test/assets/frames/lazy-frame.html
 create mode 100644 test/assets/lazy-oopif-frame.html

diff --git a/src/common/FrameManager.ts b/src/common/FrameManager.ts
index 52b8cc77d2a92..c9e3960e6f942 100644
--- a/src/common/FrameManager.ts
+++ b/src/common/FrameManager.ts
@@ -108,6 +108,9 @@ export class FrameManager extends EventEmitter {
         );
       }
     );
+    session.on('Page.frameStartedLoading', (event) => {
+      this._onFrameStartedLoading(event.frameId);
+    });
     session.on('Page.frameStoppedLoading', (event) => {
       this._onFrameStoppedLoading(event.frameId);
     });
@@ -287,6 +290,12 @@ export class FrameManager extends EventEmitter {
     this.emit(FrameManagerEmittedEvents.LifecycleEvent, frame);
   }
 
+  _onFrameStartedLoading(frameId: string): void {
+    const frame = this._frames.get(frameId);
+    if (!frame) return;
+    frame._onLoadingStarted();
+  }
+
   _onFrameStoppedLoading(frameId: string): void {
     const frame = this._frames.get(frameId);
     if (!frame) return;
@@ -650,6 +659,10 @@ export class Frame {
    * @internal
    */
   _name?: string;
+  /**
+   * @internal
+   */
+  _hasStartedLoading = false;
 
   /**
    * @internal
@@ -1416,6 +1429,13 @@ export class Frame {
     this._lifecycleEvents.add('load');
   }
 
+  /**
+   * @internal
+   */
+  _onLoadingStarted(): void {
+    this._hasStartedLoading = true;
+  }
+
   /**
    * @internal
    */
diff --git a/src/common/LifecycleWatcher.ts b/src/common/LifecycleWatcher.ts
index 0407a9cc9b9fb..276ddde42ed8b 100644
--- a/src/common/LifecycleWatcher.ts
+++ b/src/common/LifecycleWatcher.ts
@@ -255,7 +255,12 @@ export class LifecycleWatcher {
         if (!frame._lifecycleEvents.has(event)) return false;
       }
       for (const child of frame.childFrames()) {
-        if (!checkLifecycle(child, expectedLifecycle)) return false;
+        if (
+          child._hasStartedLoading &&
+          !checkLifecycle(child, expectedLifecycle)
+        ) {
+          return false;
+        }
       }
       return true;
     }
diff --git a/test/assets/frames/lazy-frame.html b/test/assets/frames/lazy-frame.html
new file mode 100644
index 0000000000000..4821cd76cd2ab
--- /dev/null
+++ b/test/assets/frames/lazy-frame.html
@@ -0,0 +1,3 @@
+<iframe width="100%" height="300" src="about:blank"></iframe>
+<div style="height: 800vh"></div>
+<iframe width="100%" height="300" src='./frame.html' loading="lazy"></iframe>
\ No newline at end of file
diff --git a/test/assets/lazy-oopif-frame.html b/test/assets/lazy-oopif-frame.html
new file mode 100644
index 0000000000000..7c259b6673ac1
--- /dev/null
+++ b/test/assets/lazy-oopif-frame.html
@@ -0,0 +1,3 @@
+<iframe width="100%" height="300" src="about:blank"></iframe>
+<div style="height: 800vh"></div>
+<iframe width="100%" height="300" src='www.example.com' loading="lazy"></iframe>
\ No newline at end of file
diff --git a/test/frame.spec.ts b/test/frame.spec.ts
index 45d19279b2c0e..40efc7989708c 100644
--- a/test/frame.spec.ts
+++ b/test/frame.spec.ts
@@ -278,6 +278,18 @@ describe('Frame specs', function () {
         server.PREFIX + '/frames/frame.html?param=value#fragment'
       );
     });
+    itFailsFirefox('should support lazy frames', async () => {
+      const { page, server } = getTestState();
+
+      await page.setViewport({ width: 1000, height: 1000 });
+      await page.goto(server.PREFIX + '/frames/lazy-frame.html');
+
+      expect(page.frames().map((frame) => frame._hasStartedLoading)).toEqual([
+        true,
+        true,
+        false,
+      ]);
+    });
   });
 
   describe('Frame.client', function () {
diff --git a/test/oopif.spec.ts b/test/oopif.spec.ts
index fbd1cdf5fccd3..2c7d9baca92e3 100644
--- a/test/oopif.spec.ts
+++ b/test/oopif.spec.ts
@@ -16,7 +16,11 @@
 
 import utils from './utils.js';
 import expect from 'expect';
-import { getTestState, describeChromeOnly } from './mocha-utils'; // eslint-disable-line import/extensions
+import {
+  getTestState,
+  describeChromeOnly,
+  itFailsFirefox,
+} from './mocha-utils'; // eslint-disable-line import/extensions
 
 describeChromeOnly('OOPIF', function () {
   /* We use a special browser for this test as we need the --site-per-process flag */
@@ -386,6 +390,18 @@ describeChromeOnly('OOPIF', function () {
     await target.page();
     browser1.disconnect();
   });
+  itFailsFirefox('should support lazy OOP frames', async () => {
+    const { server } = getTestState();
+
+    await page.goto(server.PREFIX + '/lazy-oopif-frame.html');
+    await page.setViewport({ width: 1000, height: 1000 });
+
+    expect(page.frames().map((frame) => frame._hasStartedLoading)).toEqual([
+      true,
+      true,
+      false,
+    ]);
+  });
 
   describe('waitForFrame', () => {
     it('should resolve immediately if the frame already exists', async () => {