From 63472359c0db30a3e5aea5f0922b4e4c5dd49046 Mon Sep 17 00:00:00 2001 From: YusukeIwaki Date: Tue, 28 Jun 2022 17:17:53 +0900 Subject: [PATCH] fix: do not use loaderId for lifecycle events --- lib/puppeteer/frame_manager.rb | 12 +++--------- lib/puppeteer/lifecycle_watcher.rb | 17 ++++++++--------- 2 files changed, 11 insertions(+), 18 deletions(-) diff --git a/lib/puppeteer/frame_manager.rb b/lib/puppeteer/frame_manager.rb index 93ca0c0..a516126 100644 --- 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 index 388cff8..e2c3d45 100644 --- 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