From ab924b94f377706ab99de7df6a66d6eee602a20f Mon Sep 17 00:00:00 2001 From: Nikolay Vitkov Date: Fri, 19 Apr 2024 11:12:47 +0200 Subject: [PATCH] refactor: simplify the Lifecycle Watcher --- .../puppeteer-core/src/cdp/FrameManager.ts | 2 -- .../src/cdp/FrameManagerEvents.ts | 1 - .../src/cdp/LifecycleWatcher.ts | 28 +++++-------------- 3 files changed, 7 insertions(+), 24 deletions(-) diff --git a/packages/puppeteer-core/src/cdp/FrameManager.ts b/packages/puppeteer-core/src/cdp/FrameManager.ts index 48ed9ac2f5e7d..3341dd02cb1f0 100644 --- 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 { 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 { 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 index 645dd86d71753..984f16ec8a12d 100644 --- 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 { [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 index 052d5481f8f49..a3f9dc2284203 100644 --- 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; } }