Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: only check loading iframe in lifecycling #228

Merged
merged 1 commit into from Jun 28, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 9 additions & 0 deletions lib/puppeteer/frame.rb
Expand Up @@ -10,6 +10,7 @@ def initialize(frame_manager, parent_frame, frame_id, client)
@parent_frame = parent_frame
@id = frame_id
@detached = false
@has_started_loading = false

@loader_id = ''
@lifecycle_events = Set.new
Expand Down Expand Up @@ -46,6 +47,10 @@ def oop_frame?

attr_accessor :frame_manager, :id, :loader_id, :lifecycle_events, :main_world, :secondary_world

def has_started_loading?
@has_started_loading
end

# @param url [String]
# @param rederer [String]
# @param timeout [number|nil]
Expand Down Expand Up @@ -316,6 +321,10 @@ def handle_lifecycle_event(loader_id, name)
@lifecycle_events << name
end

def handle_loading_started
@has_started_loading = true
end

def handle_loading_stopped
@lifecycle_events << 'DOMContentLoaded'
@lifecycle_events << 'load'
Expand Down
12 changes: 11 additions & 1 deletion lib/puppeteer/frame_manager.rb
Expand Up @@ -43,6 +43,9 @@ def initialize(client, page, ignore_https_errors, timeout_settings)
client.on_event('Page.frameDetached') do |event|
handle_frame_detached(event['frameId'], event['reason'])
end
client.on_event('Page.frameStartedLoading') do |event|
handle_frame_started_loading(event['frameId'])
end
client.on_event('Page.frameStoppedLoading') do |event|
handle_frame_stopped_loading(event['frameId'])
end
Expand Down Expand Up @@ -207,7 +210,14 @@ def handle_lifecycle_event(event)
emit_event(FrameManagerEmittedEvents::LifecycleEvent, frame)
end

# @param {string} frameId
# @param frame_id [String]
def handle_frame_started_loading(frame_id)
frame = @frames[frame_id]
return if !frame
frame.handle_loading_started
end

# @param frame_id [String]
def handle_frame_stopped_loading(frame_id)
frame = @frames[frame_id]
return if !frame
Expand Down
2 changes: 1 addition & 1 deletion lib/puppeteer/lifecycle_watcher.rb
Expand Up @@ -43,7 +43,7 @@ def completed?(frame)
if expected_lifecycle.any? { |event| !frame.lifecycle_events.include?(event) }
return false
end
if frame.child_frames.any? { |child| !completed?(child) }
if frame.child_frames.any? { |child| child.has_started_loading? && !completed?(child) }
return false
end
true
Expand Down
3 changes: 3 additions & 0 deletions spec/assets/frames/lazy-frame.html
@@ -0,0 +1,3 @@
<iframe width="100%" height="300" src="about:blank"></iframe>
<div style="height: 800vh"></div>
<iframe width="100%" height="300" src='./frame.html' loading="lazy"></iframe>
3 changes: 3 additions & 0 deletions spec/assets/lazy-oopif-frame.html
@@ -0,0 +1,3 @@
<iframe width="100%" height="300" src="about:blank"></iframe>
<div style="height: 800vh"></div>
<iframe width="100%" height="300" src='www.example.com' loading="lazy"></iframe>
7 changes: 7 additions & 0 deletions spec/integration/frame_spec.rb
Expand Up @@ -231,5 +231,12 @@
expect(page.frames.size).to eq(2)
expect(page.frames.last.url).to eq("#{server_prefix}/frames/frame.html?param=value#fragment")
end

it_fails_firefox 'should support lazy frames' do
page.viewport = Puppeteer::Viewport.new(width: 1000, height: 1000)
page.goto("#{server_prefix}/frames/lazy-frame.html")

expect(page.frames.map { |frame| frame.has_started_loading? }).to eq([true, true, false])
end
end
end
6 changes: 6 additions & 0 deletions spec/integration/oopif_spec.rb
Expand Up @@ -222,6 +222,12 @@ def oopifs(context)
expect(result_bounding_box.y).to be > 150 # padding + margin + border top
end

it 'should support lazy OOP frames' do
page.goto("#{server_prefix}/lazy-oopif-frame.html")
page.viewport = Puppeteer::Viewport.new(width: 1000, height: 1000)
expect(page.frames.map { |frame| frame.has_started_loading? }).to eq([true, true, false])
end

it 'should resolve immediately if the frame already exists' do
page.goto(server_empty_page)
attach_frame(page, 'frame2', "#{server_cross_process_prefix}/empty.html")
Expand Down