refactor: simplify the Lifecycle Watcher

puppeteer · Apr 23, 2024 · ab924b9 · ab924b9
1 parent 02cf1c7
commit ab924b9
Show file tree

Hide file tree

Showing 3 changed files with 7 additions and 24 deletions.
diff --git a/packages/puppeteer-core/src/cdp/FrameManager.ts b/packages/puppeteer-core/src/cdp/FrameManager.ts
@@ -298,7 +298,6 @@ export class FrameManager extends EventEmitter<FrameManagerEvents> {
       return;
     }
     frame._onLifecycleEvent(event.loaderId, event.name);
-    this.emit(FrameManagerEvent.LifecycleEvent, frame);
     frame.emit(FrameEvent.LifecycleEvent, undefined);
   }
 
@@ -316,7 +315,6 @@ export class FrameManager extends EventEmitter<FrameManagerEvents> {
       return;
     }
     frame._onLoadingStopped();
-    this.emit(FrameManagerEvent.LifecycleEvent, frame);
     frame.emit(FrameEvent.LifecycleEvent, undefined);
   }
 

diff --git a/packages/puppeteer-core/src/cdp/FrameManagerEvents.ts b/packages/puppeteer-core/src/cdp/FrameManagerEvents.ts
@@ -34,6 +34,5 @@ export interface FrameManagerEvents extends Record<EventType, unknown> {
   [FrameManagerEvent.FrameNavigated]: CdpFrame;
   [FrameManagerEvent.FrameDetached]: CdpFrame;
   [FrameManagerEvent.FrameSwapped]: CdpFrame;
-  [FrameManagerEvent.LifecycleEvent]: CdpFrame;
   [FrameManagerEvent.FrameNavigatedWithinDocument]: CdpFrame;
 }
diff --git a/packages/puppeteer-core/src/cdp/LifecycleWatcher.ts b/packages/puppeteer-core/src/cdp/LifecycleWatcher.ts
@@ -17,7 +17,6 @@ import {Deferred} from '../util/Deferred.js';
 import {DisposableStack} from '../util/disposable.js';
 
 import type {CdpFrame} from './Frame.js';
-import {FrameManagerEvent} from './FrameManagerEvents.js';
 import type {NetworkManager} from './NetworkManager.js';
 
 /**
@@ -103,15 +102,12 @@ export class LifecycleWatcher {
 
     this.#frame = frame;
     this.#timeout = timeout;
-    const frameManagerEmitter = this.#subscriptions.use(
-      new EventEmitter(frame._frameManager)
-    );
-    frameManagerEmitter.on(
-      FrameManagerEvent.LifecycleEvent,
-      this.#checkLifecycleComplete.bind(this)
-    );
 
     const frameEmitter = this.#subscriptions.use(new EventEmitter(frame));
+    frameEmitter.on(
+      FrameEvent.LifecycleEvent,
+      this.#checkLifecycleComplete.bind(this)
+    );
     frameEmitter.on(
       FrameEvent.FrameNavigatedWithinDocument,
       this.#navigatedWithinDocument.bind(this)
@@ -227,10 +223,11 @@ export class LifecycleWatcher {
   }
 
   #checkLifecycleComplete(): void {
-    // We expect navigation to commit.
+    console.log('Check Lifecycle for', this.#frame._id);
     if (!checkLifecycle(this.#frame, this.#expectedLifecycle)) {
       return;
     }
+
     this.#lifecycleDeferred.resolve();
     if (this.#hasSameDocumentNavigation) {
       this.#sameDocumentNavigationDeferred.resolve(undefined);
@@ -248,18 +245,7 @@ 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 &&
-          !checkLifecycle(child, expectedLifecycle)
-        ) {
-          return false;
-        }
-      }
+
       return true;
     }
   }