fix: [#583] Don't raise timeout on lazy iframes

CHANGELOG.md

@@ -1,14 +1,19 @@
 ## [Unreleased](https://github.com/rubycdp/ferrum/compare/v0.17.2...main) ##
 
 ### Added
+- `Ferrum::Frame#loader_id` provides a loader id when frame navigates [#583]
+- `Ferrum::Frame#lifecycle_events` provides a list of frame's events like init, networkIdle, firstPaint, etc. [#583]
+- `Ferrum::Frame#idle?` whether frame was loaded [#583]
 
 ### Changed
+- `Ferrum::PendingConnectionsError` and `Ferrum::TimeoutError` were swallowed even though happening when traffic iterator results in empty array. [#583]
 
 ### Fixed
 - Full-page screenshots no longer resize the window, preventing focus steal on macOS [#580]
 - DOM.enable is now has `includeWhitespace: "all"` to keep track of new line nodes which previously were resolved to 0, and errored with NodeNotFoundError [#596]
 - `DOM.requestNode` call is moved to the node class and being done lazily, this reduces number of intermediate node ids sent by backend to frontend [#596]
 - Fix `context` can be nilable in ensure Browser#create_page [#582]
+- `Ferrum::Page#idling?` no longer blocks on `loading="lazy"` iframes that Chrome never starts loading [#583]
 
 ### Removed
 

lib/ferrum/errors.rb

@@ -20,8 +20,8 @@ class PendingConnectionsError < StatusError
 
     def initialize(url, pendings = [])
       @pendings = pendings
-
-      message = "Request to #{url} reached server, but there are still pending connections: #{pendings.join(', ')}"
+      message = "Request to #{url} reached server, but there are still pending connections"
+      message += ": #{pendings.join(', ')}" unless @pendings.empty?
 
       super(url, message)
     end

lib/ferrum/frame.rb

@@ -12,6 +12,7 @@ class Frame
       started_loading
       navigated
       stopped_loading
+      canceled
     ].freeze
 
     # The Frame's unique id.
@@ -36,13 +37,24 @@ class Frame
 
     # One of the states frame's in.
     #
-    # @return [:started_loading, :navigated, :stopped_loading, nil]
+    # @return [:started_loading, :navigated, :stopped_loading, :canceled, nil]
     attr_reader :state
 
+    # Frame loader id.
+    #
+    # @return [String, nil]
+    attr_accessor :loader_id
+
+    # Frame's lifecycle events (navigation, load, paint, etc.).
+    #
+    # @return [Array<Hash{String => (String|Float)}>]
+    attr_reader :lifecycle_events
+
     def initialize(id, page, parent_id = nil)
       @id = id
       @page = page
       @parent_id = parent_id
+      @lifecycle_events = []
       @execution_id = Concurrent::MVar.new
     end
 
@@ -94,6 +106,20 @@ def main?
       @parent_id.nil?
     end
 
+    #
+    # Returns whether the frame has finished loading (+:stopped_loading+ state).
+    # Frames in +:canceled+ state (execution context torn down mid-navigation)
+    # are not considered idle.
+    #
+    # @return [Boolean]
+    #
+    # @example
+    #   browser.go_to("https://example.com")
+    #   browser.main_frame.idle? # => true
+    def idle?
+      state == :stopped_loading
+    end
+
     #
     # Returns the parent frame if this frame is nested in another one.
     #
@@ -177,6 +203,8 @@ def inspect
         "@id=#{@id.inspect} " \
         "@parent_id=#{@parent_id.inspect} " \
         "@name=#{@name.inspect} " \
+        "@loader_id=#{@loader_id.inspect} " \
+        "@lifecycle_events=#{@lifecycle_events.inspect} " \
         "@state=#{@state.inspect} " \
         "@execution_id=#{@execution_id.inspect}>"
     end

lib/ferrum/page.rb

@@ -118,7 +118,7 @@ def go_to(url = nil)
     rescue TimeoutError
       if @options.pending_connection_errors
         pendings = network.traffic.select(&:pending?).map(&:url).compact
-        raise PendingConnectionsError.new(options[:url], pendings) unless pendings.empty?
+        raise PendingConnectionsError.new(options[:url], Array(pendings))
       end
     end
     alias goto go_to
@@ -361,7 +361,7 @@ def command(method, wait: 0, slowmoable: false, **params)
       if wait.positive?
         # Wait a bit after command and check if iteration has
         # changed which means there was some network event for
-        # the main frame and it started to load new content.
+        # the main frame, and it started to load new content.
         @event.wait(wait)
         if iteration != @event.iteration
           set = @event.wait(timeout)
@@ -458,6 +458,7 @@ def subscribe
 
     def prepare_page
       command("Page.enable")
+      command("Page.setLifecycleEventsEnabled", enabled: true)
       command("Runtime.enable")
       command("DOM.enable", includeWhitespace: "all")
       command("CSS.enable")

lib/ferrum/page/frames.rb

@@ -63,12 +63,16 @@ def frame_by(id: nil, name: nil, execution_id: nil)
         end
       end
 
+      private
+
       def frames_subscribe
         subscribe_frame_attached
         subscribe_frame_detached
+        subscribe_frame_started_navigating
         subscribe_frame_started_loading
         subscribe_frame_navigated
         subscribe_frame_stopped_loading
+        subscribe_frame_lifecycle_events
 
         subscribe_navigated_within_document
 
@@ -79,8 +83,6 @@ def frames_subscribe
         subscribe_execution_contexts_cleared
       end
 
-      private
-
       def subscribe_frame_attached
         on("Page.frameAttached") do |params|
           parent_frame_id, frame_id = params.values_at("parentFrameId", "frameId")
@@ -100,6 +102,18 @@ def subscribe_frame_detached
         end
       end
 
+      # Tracks the +loaderId+ for each navigating frame and clears stale
+      # lifecycle events from the previous navigation.
+      def subscribe_frame_started_navigating
+        on("Page.frameStartedNavigating") do |params|
+          frame = @frames[params["frameId"]]
+          if frame
+            frame.loader_id = params["loaderId"]
+            frame.lifecycle_events.clear
+          end
+        end
+      end
+
       def subscribe_frame_started_loading
         on("Page.frameStartedLoading") do |params|
           frame = @frames[params["frameId"]]
@@ -138,6 +152,33 @@ def subscribe_frame_stopped_loading
         end
       end
 
+      # Appends +Page.lifecycleEvent+ events to {Frame#lifecycle_events}.
+      # Events from a superseded navigation (+loaderId+ mismatch) are dropped.
+      # Transitions +loading="lazy"+ iframes that Chrome never starts loading
+      # to +:stopped_loading+ on +networkIdle+, preventing a {Page#go_to} timeout.
+      def subscribe_frame_lifecycle_events
+        on("Page.lifecycleEvent") do |params|
+          frame = @frames[params["frameId"]]
+          next unless frame
+
+          frame.loader_id = params["loaderId"] unless frame.loader_id
+          # Reject stale events from a superseded navigation, iframes are not destroyed by Chrome, instead it creates
+          # new ones. Main frame stays with the same id, but new events start to flow in.
+          next if frame.loader_id != params["loaderId"]
+
+          event = params.slice("name", "timestamp")
+          frame.lifecycle_events << event
+
+          # This handles iframes with loading="lazy", those that Chrome attaches but parks outside the viewport.
+          # They do not trigger any events except `Page.frameAttached` and lifecycle events like:
+          # `init` and `networkIdle`. Without it `go_to` would wait for such iframe until it raises timeout.
+          if event["name"] == "networkIdle" && !frame.main?
+            frame.state = :stopped_loading
+            @event.set if idling?
+          end
+        end
+      end
+
       def subscribe_navigated_within_document
         on("Page.navigatedWithinDocument") do
           @event.set if idling?
@@ -177,21 +218,36 @@ def subscribe_execution_context_destroyed
           execution_id = params["executionContextId"]
           frame = frame_by(execution_id: execution_id)
           frame&.execution_id = nil
-          frame&.state = :stopped_loading
+          frame&.state = :canceled
         end
       end
 
+      # On full navigations/reloads Chrome fires +Page.frameAttached+ with new
+      # frame IDs but skips +Page.frameDetached+ for old ones. Removing stale
+      # child frames here prevents them from blocking {#idling?} indefinitely.
+      # The main frame is reset in-place; child frames are re-added via
+      # +Page.frameAttached+.
       def subscribe_execution_contexts_cleared
         on("Runtime.executionContextsCleared") do
-          @frames.each_value do |f|
-            f.execution_id = nil
-            f.state = :stopped_loading
+          children = []
+
+          @frames.each do |frame_id, f|
+            if f.main?
+              f.execution_id = nil
+              f.loader_id = nil
+              f.lifecycle_events.clear
+              f.state = :canceled
+            else
+              children << frame_id
+            end
           end
+
+          children.each { |id| @frames.delete(id) }
         end
       end
 
       def idling?
-        @frames.values.all? { |f| f.state == :stopped_loading }
+        @frames.values.all?(&:idle?)
       end
     end
   end

sig/ferrum/frame.rbs

@@ -4,17 +4,21 @@ module Ferrum
 
     include Runtime
 
-    STATE_VALUES: ::Array[:started_loading | :navigated | :stopped_loading]
+    STATE_VALUES: ::Array[:started_loading | :navigated | :stopped_loading | :canceled]
 
-    attr_accessor id: untyped
+    attr_accessor id: String
 
-    attr_accessor name: untyped
+    attr_accessor name: String?
 
     attr_reader page: untyped
 
-    attr_reader parent_id: untyped
+    attr_reader parent_id: String?
 
-    attr_reader state: untyped
+    attr_reader state: (:started_loading | :navigated | :stopped_loading | :canceled)?
+
+    attr_accessor loader_id: String?
+
+    attr_reader lifecycle_events: ::Array[::Hash[::String, ::String | ::Float]]
 
     def initialize: (untyped id, untyped page, ?untyped? parent_id) -> void
 
@@ -24,7 +28,9 @@ module Ferrum
 
     def title: () -> untyped
 
-    def main?: () -> untyped
+    def main?: () -> bool
+
+    def idle?: () -> bool
 
     def content=: (untyped html) -> untyped
 

sig/ferrum/page/frames.rbs

@@ -7,20 +7,24 @@ module Ferrum
 
       def frame_by: (?id: untyped?, ?name: untyped?, ?execution_id: untyped?) -> untyped
 
-      def frames_subscribe: () -> untyped
-
       private
 
+      def frames_subscribe: () -> untyped
+
       def subscribe_frame_attached: () -> untyped
 
       def subscribe_frame_detached: () -> untyped
 
+      def subscribe_frame_started_navigating: () -> untyped
+
       def subscribe_frame_started_loading: () -> untyped
 
       def subscribe_frame_navigated: () -> untyped
 
       def subscribe_frame_stopped_loading: () -> untyped
 
+      def subscribe_frame_lifecycle_events: () -> untyped
+
       def subscribe_navigated_within_document: () -> untyped
 
       def subscribe_request_will_be_sent: () -> untyped

spec/frame_spec.rb

@@ -125,6 +125,33 @@
     end
   end
 
+  context "with lazy loading iframes" do
+    it "does not block visit on lazy iframe" do
+      expect { page.go_to("/lazy_iframe") }.not_to raise_error
+
+      states = page.frames.reject(&:main?).map(&:state)
+      expect(states).to eq(%i[stopped_loading stopped_loading])
+    end
+
+    it "does not block click whose handler attaches a lazy iframe" do
+      page.go_to("/lazy_iframe")
+
+      expect { page.at_css("#add_iframe").click }.not_to raise_error
+
+      states = page.frames.reject(&:main?).map(&:state)
+      expect(states).to eq(%i[stopped_loading stopped_loading stopped_loading])
+    end
+
+    it "loads in-viewport lazy iframes" do
+      page.go_to("/lazy_iframe")
+      frame = page.frame_by(name: "lazy_in_viewport")
+
+      states = page.frames.reject(&:main?).map(&:state)
+      expect(frame.body).to include("slow page")
+      expect(states).to eq(%i[stopped_loading stopped_loading])
+    end
+  end
+
   it "supports clicking in a frame", skip: true do
     page.go_to
     page.execute <<-JS

spec/node_spec.rb

@@ -872,7 +872,7 @@
       expect { shallow_node.click }.to raise_error(Ferrum::NodeNotFoundError)
 
       node = browser.at_xpath(".//a")
-      expect { node.click }.not_to raise_error(Ferrum::NodeNotFoundError)
+      expect { node.click }.not_to raise_error
     end
   end
 end

spec/support/views/lazy_iframe.erb

@@ -0,0 +1,36 @@
+<!DOCTYPE html>
+<html>
+  <head>
+    <title>Lazy iframe</title>
+    <link rel="icon" href="data:,">
+  </head>
+  <body>
+    <h1>Lazy iframe in viewport</h1>
+    <iframe loading="lazy" src="/slow" name="lazy_in_viewport"></iframe>
+
+    <h1>Lazy iframe</h1>
+    <details>
+      <summary>Hidden until expanded</summary>
+      <iframe loading="lazy" src="/slow" name="lazy_frame"></iframe>
+    </details>
+
+    <h1>Lazy iframe via click</h1>
+    <button id="add_iframe" onclick="addIframe()">Add iframe</button>
+    <script>
+      function addIframe() {
+        var iframe = document.createElement("iframe");
+        iframe.name = "lazy_added";
+        iframe.loading = "lazy";
+        iframe.src = "/slow";
+        // Positioned far off-screen so Chrome never starts loading it.
+        iframe.style = "position: absolute; top: 10000px;";
+        document.body.appendChild(iframe);
+        // Trigger Page.navigatedWithinDocument so the idle check is
+        // re-evaluated with the nil-state lazy frame in @frames. A full
+        // navigation would fire Runtime.executionContextsCleared and
+        // overwrite the frame's state, masking the bug.
+        location.hash = "added-" + Date.now();
+      }
+    </script>
+  </body>
+</html>