fix: Ferrum::NoExecutionContextError can be raised when we call frame_by and lock on execution_id (#307)

Author: route

CHANGELOG.md

@@ -8,6 +8,8 @@
   - `#create_page(proxy: { host: "x.x.x.x", port: "8800", user: "user", password: "pa$$" })`
   proxy option, supports creating a page in a new context that uses proxy settings.
 - `Ferrum::Page#timeout = n` page supports its own timeout
+- `Ferrum::Frame#execution_id` returns execution context id and doesn't raises error
+- `Ferrum::Frame#execution_id!` returns execution context id and raises error when times out on borrowing
 
 ### Changed
 
@@ -16,6 +18,10 @@
 - `Ferrum::Keyboard`
   - `#up, #down` accept only one key.
 - `Ferrum::Page#goto` fixed undefined method url for nil:NilClass when page times out and there are pending requests.
+- `Runtime.consoleAPICalled` didn't show log messages
+- `Ferrum::Page#subscribe_frame_detached` added to clean up old frames
+- `Ferrum::Proxy` was hanging at the exit due to issue in Webrick
+- `Ferrum::NoExecutionContextError` is raised sometimes when we block on `Ferrum::Page#frame_by`
 
 ### Removed
 

lib/ferrum/frame.rb

@@ -118,19 +118,34 @@ def content=(html)
 
     #
     # Execution context id which is used by JS, each frame has it's own
-    # context in which JS evaluates.
+    # context in which JS evaluates. Locks for a page timeout and raises
+    # an error if an execution id hasn't been set yet, if id is set
+    # returns immediately.
     #
     # @return [Integer]
     #
     # @raise [NoExecutionContextError]
     #
-    def execution_id
+    def execution_id!
       value = @execution_id.borrow(@page.timeout, &:itself)
       raise NoExecutionContextError if value.instance_of?(Object)
 
       value
     end
 
+    #
+    # Execution context id which is used by JS, each frame has it's own
+    # context in which JS evaluates.
+    #
+    # @return [Integer, nil]
+    #
+    def execution_id
+      value = @execution_id.value
+      return if value.instance_of?(Object)
+
+      value
+    end
+
     def execution_id=(value)
       if value.nil?
         @execution_id.try_take!

lib/ferrum/frame/runtime.rb

@@ -112,7 +112,7 @@ def call(expression:, arguments: [], on: nil, wait: 0, handle: true, **options)
           end
 
           if params[:executionContextId].nil? && params[:objectId].nil?
-            params = params.merge(executionContextId: execution_id)
+            params = params.merge(executionContextId: execution_id!)
           end
 
           response = @page.command("Runtime.callFunctionOn",

lib/ferrum/page.rb

@@ -35,7 +35,7 @@ def reset
     extend Forwardable
     delegate %i[at_css at_xpath css xpath
                 current_url current_title url title body doctype content=
-                execution_id evaluate evaluate_on evaluate_async execute evaluate_func
+                execution_id execution_id! evaluate evaluate_on evaluate_async execute evaluate_func
                 add_script_tag add_style_tag] => :main_frame
 
     include Animation
@@ -72,7 +72,7 @@ def reset
     attr_reader :cookies
 
     def initialize(target_id, browser, proxy: nil)
-      @frames = {}
+      @frames = Concurrent::Map.new
       @main_frame = Frame.new(nil, self)
       @browser = browser
       @target_id = target_id
@@ -411,7 +411,7 @@ def inject_extensions
         # We also evaluate script just in case because
         # `Page.addScriptToEvaluateOnNewDocument` doesn't work in popups.
         command("Runtime.evaluate", expression: extension,
-                                    contextId: execution_id,
+                                    executionContextId: execution_id!,
                                     returnByValue: true)
       end
     end

lib/ferrum/page/frames.rb

@@ -44,6 +44,9 @@ def frames
       # @param [String] name
       #   Frame's name if there's one.
       #
+      # @param [String] execution_id
+      #   Frame's context execution id.
+      #
       # @return [Frame, nil]
       #   The matching frame.
       #
@@ -83,26 +86,26 @@ def frames_subscribe
       def subscribe_frame_attached
         on("Page.frameAttached") do |params|
           parent_frame_id, frame_id = params.values_at("parentFrameId", "frameId")
-          @frames[frame_id] = Frame.new(frame_id, self, parent_frame_id)
+          @frames.put_if_absent(frame_id, Frame.new(frame_id, self, parent_frame_id))
         end
       end
 
       def subscribe_frame_detached
         on("Page.frameDetached") do |params|
           frame = @frames[params["frameId"]]
 
-          if frame.main?
+          if frame&.main?
             frame.execution_id = nil
           else
-            @frames.delete(frame.id)
+            @frames.delete(params["frameId"])
           end
         end
       end
 
       def subscribe_frame_started_loading
         on("Page.frameStartedLoading") do |params|
           frame = @frames[params["frameId"]]
-          frame.state = :started_loading
+          frame.state = :started_loading if frame
           @event.reset
         end
       end
@@ -111,8 +114,11 @@ def subscribe_frame_navigated
         on("Page.frameNavigated") do |params|
           frame_id, name = params["frame"]&.values_at("id", "name")
           frame = @frames[frame_id]
-          frame.state = :navigated
-          frame.name = name unless name.to_s.empty?
+
+          if frame
+            frame.state = :navigated
+            frame.name = name
+          end
         end
       end
 
@@ -159,14 +165,12 @@ def subscribe_execution_context_created
             root_frame = command("Page.getFrameTree").dig("frameTree", "frame", "id")
             if frame_id == root_frame
               @main_frame.id = frame_id
-              @frames[frame_id] = @main_frame
+              @frames.put_if_absent(frame_id, @main_frame)
             end
           end
 
-          frame = @frames[frame_id] || Frame.new(frame_id, self)
+          frame = @frames.fetch_or_store(frame_id, Frame.new(frame_id, self))
           frame.execution_id = context_id
-
-          @frames[frame_id] ||= frame
         end
       end
 
@@ -180,13 +184,12 @@ def subscribe_execution_context_destroyed
 
       def subscribe_execution_contexts_cleared
         on("Runtime.executionContextsCleared") do
-          @frames.delete_if { |_, f| !f.main? }
-          @main_frame.execution_id = nil
+          @frames.each_value { |f| f.execution_id = nil }
         end
       end
 
       def idling?
-        @frames.all? { |_, f| f.state == :stopped_loading }
+        @frames.values.all? { |f| f.state == :stopped_loading }
       end
     end
   end