Merge pull request #229 from YusukeIwaki/porting/8395

fix: do not use loaderId for lifecycle events
YusukeIwaki · Jun 28, 2022 · b567197 · b567197
2 parents 5749088 + 6347235
commit b567197
Show file tree

Hide file tree

Showing 2 changed files with 11 additions and 18 deletions.
diff --git a/lib/puppeteer/frame_manager.rb b/lib/puppeteer/frame_manager.rb
@@ -121,13 +121,12 @@ def navigate_frame(frame, url, referer: nil, timeout: nil, wait_until: nil)
     option_timeout = timeout || @timeout_settings.navigation_timeout
 
     watcher = Puppeteer::LifecycleWatcher.new(self, frame, option_wait_until, option_timeout)
-    ensure_new_document_navigation = false
 
     begin
       navigate = future do
         result = @client.send_message('Page.navigate', navigate_params)
         loader_id = result['loaderId']
-        ensure_new_document_navigation = !!loader_id
+
         if result['errorText']
           raise NavigationError.new("#{result['errorText']} at #{url}")
         end
@@ -137,14 +136,9 @@ def navigate_frame(frame, url, referer: nil, timeout: nil, wait_until: nil)
         watcher.timeout_or_termination_promise,
       )
 
-      document_navigation_promise =
-        if ensure_new_document_navigation
-          watcher.new_document_navigation_promise
-        else
-          watcher.same_document_navigation_promise
-        end
       await_any(
-        document_navigation_promise,
+        watcher.new_document_navigation_promise,
+        watcher.same_document_navigation_promise,
         watcher.timeout_or_termination_promise,
       )
     rescue Puppeteer::TimeoutError => err

diff --git a/lib/puppeteer/lifecycle_watcher.rb b/lib/puppeteer/lifecycle_watcher.rb
@@ -65,7 +65,6 @@ def initialize(frame_manager, frame, wait_until, timeout)
     @expected_lifecycle = ExpectedLifecycle.new(wait_until)
     @frame_manager = frame_manager
     @frame = frame
-    @initial_loader_id = frame.loader_id
     @timeout = timeout
 
     @listener_ids = {}
@@ -77,6 +76,7 @@ def initialize(frame_manager, frame, wait_until, timeout)
         check_lifecycle_complete
       end,
       @frame_manager.add_event_listener(FrameManagerEmittedEvents::FrameNavigatedWithinDocument, &method(:navigated_within_document)),
+      @frame_manager.add_event_listener(FrameManagerEmittedEvents::FrameNavigated, &method(:navigated)),
       @frame_manager.add_event_listener(FrameManagerEmittedEvents::FrameSwapped, &method(:handle_frame_swapped)),
       @frame_manager.add_event_listener(FrameManagerEmittedEvents::FrameDetached, &method(:handle_frame_detached)),
     ]
@@ -143,6 +143,12 @@ def timeout_or_termination_promise
     check_lifecycle_complete
   end
 
+  private def navigated(frame)
+    return if frame != @frame
+    @new_document_navigation = true
+    check_lifecycle_complete
+  end
+
   private def handle_frame_swapped(frame)
     return if frame != @frame
     @swapped = true
@@ -153,17 +159,10 @@ def timeout_or_termination_promise
     # We expect navigation to commit.
     return unless @expected_lifecycle.completed?(@frame)
     @lifecycle_promise.fulfill(true) if @lifecycle_promise.pending?
-    if @frame.loader_id == @initial_loader_id && !@has_same_document_navigation
-      if @swapped
-        @swapped = false
-        @new_document_navigation_promise.fulfill(true)
-      end
-      return
-    end
     if @has_same_document_navigation && @same_document_navigation_promise.pending?
       @same_document_navigation_promise.fulfill(true)
     end
-    if @frame.loader_id != @initial_loader_id && @new_document_navigation_promise.pending?
+    if (@swapped || @new_document_navigation) && @new_document_navigation_promise.pending?
       @new_document_navigation_promise.fulfill(true)
     end
   end