Merge pull request #228 from YusukeIwaki/porting/8348

fix: only check loading iframe in lifecycling

lib/puppeteer/frame.rb

@@ -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
@@ -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]
@@ -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'

lib/puppeteer/frame_manager.rb

@@ -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
@@ -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

lib/puppeteer/lifecycle_watcher.rb

@@ -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

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>

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>

spec/integration/frame_spec.rb

@@ -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

spec/integration/oopif_spec.rb

@@ -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")