fix: do not use loaderId for Lifecycle events (#8353)

puppeteer · May 17, 2022 · 7107d2d · 7107d2d
1 parent 7438030
commit 7107d2d
Show file tree

Hide file tree

Showing 2 changed files with 15 additions and 18 deletions.
diff --git a/src/common/FrameManager.ts b/src/common/FrameManager.ts
@@ -194,17 +194,15 @@ export class FrameManager extends EventEmitter {
     } = options;
 
     const watcher = new LifecycleWatcher(this, frame, waitUntil, timeout);
-    let ensureNewDocumentNavigation = false;
     let error = await Promise.race([
       navigate(this._client, url, referer, frame._id),
       watcher.timeoutOrTerminationPromise(),
     ]);
     if (!error) {
       error = await Promise.race([
         watcher.timeoutOrTerminationPromise(),
-        ensureNewDocumentNavigation
-          ? watcher.newDocumentNavigationPromise()
-          : watcher.sameDocumentNavigationPromise(),
+        watcher.newDocumentNavigationPromise(),
+        watcher.sameDocumentNavigationPromise(),
       ]);
     }
     watcher.dispose();
@@ -223,7 +221,6 @@ export class FrameManager extends EventEmitter {
           referrer,
           frameId,
         });
-        ensureNewDocumentNavigation = !!response.loaderId;
         return response.errorText
           ? new Error(`${response.errorText} at ${url}`)
           : null;

diff --git a/src/common/LifecycleWatcher.ts b/src/common/LifecycleWatcher.ts
@@ -64,7 +64,6 @@ export class LifecycleWatcher {
   _timeout: number;
   _navigationRequest?: HTTPRequest;
   _eventListeners: PuppeteerEventListener[];
-  _initialLoaderId: string;
 
   _sameDocumentNavigationPromise: Promise<Error | null>;
   _sameDocumentNavigationCompleteCallback: (x?: Error) => void;
@@ -82,6 +81,7 @@ export class LifecycleWatcher {
 
   _maximumTimer?: NodeJS.Timeout;
   _hasSameDocumentNavigation?: boolean;
+  _newDocumentNavigation?: boolean;
   _swapped?: boolean;
 
   constructor(
@@ -100,7 +100,6 @@ export class LifecycleWatcher {
 
     this._frameManager = frameManager;
     this._frame = frame;
-    this._initialLoaderId = frame._loaderId;
     this._timeout = timeout;
     this._navigationRequest = null;
     this._eventListeners = [
@@ -122,6 +121,11 @@ export class LifecycleWatcher {
         FrameManagerEmittedEvents.FrameNavigatedWithinDocument,
         this._navigatedWithinDocument.bind(this)
       ),
+      helper.addEventListener(
+        this._frameManager,
+        FrameManagerEmittedEvents.FrameNavigated,
+        this._navigated.bind(this)
+      ),
       helper.addEventListener(
         this._frameManager,
         FrameManagerEmittedEvents.FrameSwapped,
@@ -217,6 +221,12 @@ export class LifecycleWatcher {
     this._checkLifecycleComplete();
   }
 
+  _navigated(frame: Frame): void {
+    if (frame !== this._frame) return;
+    this._newDocumentNavigation = true;
+    this._checkLifecycleComplete();
+  }
+
   _frameSwapped(frame: Frame): void {
     if (frame !== this._frame) return;
     this._swapped = true;
@@ -227,19 +237,9 @@ export class LifecycleWatcher {
     // We expect navigation to commit.
     if (!checkLifecycle(this._frame, this._expectedLifecycle)) return;
     this._lifecycleCallback();
-    if (
-      this._frame._loaderId === this._initialLoaderId &&
-      !this._hasSameDocumentNavigation
-    ) {
-      if (this._swapped) {
-        this._swapped = false;
-        this._newDocumentNavigationCompleteCallback();
-      }
-      return;
-    }
     if (this._hasSameDocumentNavigation)
       this._sameDocumentNavigationCompleteCallback();
-    if (this._frame._loaderId !== this._initialLoaderId)
+    if (this._swapped || this._newDocumentNavigation)
       this._newDocumentNavigationCompleteCallback();
 
     /**