From 9bf0d7b433fb8f8c7e98b0f0f9642e47effc73fd Mon Sep 17 00:00:00 2001 From: YusukeIwaki Date: Wed, 21 Sep 2022 02:40:06 +0900 Subject: [PATCH] fix!: remove root from WaitForSelectorOptions chore: make execution context frame-independent --- lib/puppeteer.rb | 2 +- lib/puppeteer/aria_query_handler.rb | 62 ++++++++++++++---- lib/puppeteer/custom_query_handler.rb | 31 ++++++++- lib/puppeteer/element_handle.rb | 25 +++----- lib/puppeteer/execution_context.rb | 63 +------------------ lib/puppeteer/frame.rb | 34 +++++----- lib/puppeteer/frame_manager.rb | 4 +- .../{dom_world.rb => isolated_world.rb} | 56 +++++++++++------ lib/puppeteer/js_handle.rb | 8 +-- lib/puppeteer/page.rb | 3 +- lib/puppeteer/query_handler_manager.rb | 4 +- lib/puppeteer/remote_object.rb | 4 +- spec/integration/wait_task_spec.rb | 4 +- spec/puppeteer/element_handle_spec.rb | 7 ++- 14 files changed, 156 insertions(+), 151 deletions(-) rename lib/puppeteer/{dom_world.rb => isolated_world.rb} (93%) diff --git a/lib/puppeteer.rb b/lib/puppeteer.rb index 7602eb74..20977efc 100644 --- a/lib/puppeteer.rb +++ b/lib/puppeteer.rb @@ -32,7 +32,6 @@ module Puppeteer; end require 'puppeteer/custom_query_handler' require 'puppeteer/devices' require 'puppeteer/dialog' -require 'puppeteer/dom_world' require 'puppeteer/emulation_manager' require 'puppeteer/exception_details' require 'puppeteer/executable_path_finder' @@ -43,6 +42,7 @@ module Puppeteer; end require 'puppeteer/frame_manager' require 'puppeteer/http_request' require 'puppeteer/http_response' +require 'puppeteer/isolated_world' require 'puppeteer/js_coverage' require 'puppeteer/js_handle' require 'puppeteer/keyboard' diff --git a/lib/puppeteer/aria_query_handler.rb b/lib/puppeteer/aria_query_handler.rb index bde3c1bb..78578c29 100644 --- a/lib/puppeteer/aria_query_handler.rb +++ b/lib/puppeteer/aria_query_handler.rb @@ -25,43 +25,83 @@ class Puppeteer::AriaQueryHandler query_options end - def query_one(element, selector) - context = element.execution_context + # @param element [Puppeteer::ElementHandle] + # @param selector [String] + private def query_one_id(element, selector) parse_result = parse_aria_selector(selector) res = element.query_ax_tree(accessible_name: parse_result[:name], role: parse_result[:role]) - if res.empty? + + if res.first.is_a?(Hash) + res.first['backendDOMNodeId'] + else nil + end + end + + def query_one(element, selector) + id = query_one_id(element, selector) + + if id + element.frame.main_world.adopt_backend_node(id) else - context.adopt_backend_node_id(res.first['backendDOMNodeId']) + nil end end - def wait_for(dom_world, selector, visible: nil, hidden: nil, timeout: nil, root: nil) + def wait_for(element_or_frame, selector, visible: nil, hidden: nil, timeout: nil) + case element_or_frame + when Puppeteer::Frame + frame = element_or_frame + element = nil + when Puppeteer::ElementHandle + frame = element_or_frame.frame + element = frame.puppeteer_world.adopt_handle(element_or_frame) + else + raise ArgumentError.new("element_or_frame must be a Frame or ElementHandle. #{element_or_frame.inspect}") + end + # addHandlerToWorld - binding_function = Puppeteer::DOMWorld::BindingFunction.new( + binding_function = Puppeteer::IsolaatedWorld::BindingFunction.new( name: 'ariaQuerySelector', - proc: -> (sel) { query_one(root || dom_world.send(:document), sel) }, + proc: -> (sel) { + id = query_one_id(element || frame.puppeteer_world.document, sel) + + if id + frame.puppeteer_world.adopt_backend_node(id) + else + nil + end + }, ) - dom_world.send(:wait_for_selector_in_page, + result = frame.puppeteer_world.send(:wait_for_selector_in_page, '(_, selector) => globalThis.ariaQuerySelector(selector)', + element, selector, visible: visible, hidden: hidden, timeout: timeout, binding_function: binding_function, - root: root, ) + + element&.dispose + + if result.is_a?(Puppeteer::ElementHandle) + result.frame.main_world.transfer_handle(result) + else + result&.dispose + nil + end end def query_all(element, selector) - context = element.execution_context + world = element.frame.main_world parse_result = parse_aria_selector(selector) res = element.query_ax_tree(accessible_name: parse_result[:name], role: parse_result[:role]) if res.empty? nil else promises = res.map do |ax_node| - context.send(:async_adopt_backend_node_id, ax_node['backendDOMNodeId']) + world.send(:async_adopt_backend_node, ax_node['backendDOMNodeId']) end await_all(*promises) end diff --git a/lib/puppeteer/custom_query_handler.rb b/lib/puppeteer/custom_query_handler.rb index d0c8879f..b1bdc5e5 100644 --- a/lib/puppeteer/custom_query_handler.rb +++ b/lib/puppeteer/custom_query_handler.rb @@ -21,12 +21,39 @@ def query_one(element, selector) nil end - def wait_for(dom_world, selector, visible: nil, hidden: nil, timeout: nil, root: nil) + def wait_for(element_or_frame, selector, visible: nil, hidden: nil, timeout: nil) + case element_or_frame + when Puppeteer::Frame + frame = element_or_frame + element = nil + when Puppeteer::ElementHandle + frame = element_or_frame.frame + element = frame.puppeteer_world.adopt_handle(element_or_frame) + else + raise ArgumentError.new("element_or_frame must be a Frame or ElementHandle. #{element_or_frame.inspect}") + end + unless @query_one raise NotImplementedError.new("#{self.class}##{__method__} is not implemented.") end - dom_world.send(:wait_for_selector_in_page, @query_one, selector, visible: visible, hidden: hidden, timeout: timeout, root: root) + result = frame.puppeteer_world.send(:wait_for_selector_in_page, + @query_one, + element, + selector, + visible: visible, + hidden: hidden, + timeout: timeout, + ) + + element&.dispose + + if result.is_a?(Puppeteer::ElementHandle) + result.frame.main_world.transfer_handle(result) + else + result&.dispose + nil + end end def query_all(element, selector) diff --git a/lib/puppeteer/element_handle.rb b/lib/puppeteer/element_handle.rb index 9e38bd89..61d9872b 100644 --- a/lib/puppeteer/element_handle.rb +++ b/lib/puppeteer/element_handle.rb @@ -12,16 +12,16 @@ class Puppeteer::ElementHandle < Puppeteer::JSHandle # @param client [Puppeteer::CDPSession] # @param remote_object [Puppeteer::RemoteObject] # @param frame [Puppeteer::Frame] - # @param page [Puppeteer::Page] - # @param frame_manager [Puppeteer::FrameManager] - def initialize(context:, client:, remote_object:, frame:, page:, frame_manager:) + def initialize(context:, client:, remote_object:, frame:) super(context: context, client: client, remote_object: remote_object) @frame = frame - @page = page - @frame_manager = frame_manager + @page = frame.page + @frame_manager = frame.frame_manager @disposed = false end + attr_reader :page, :frame, :frame_manager + def inspect values = %i[context remote_object page disposed].map do |sym| value = instance_variable_get(:"@#{sym}") @@ -60,18 +60,7 @@ def inspect # (30 seconds). Pass `0` to disable timeout. The default value can be changed # by using the {@link Page.setDefaultTimeout} method. def wait_for_selector(selector, visible: nil, hidden: nil, timeout: nil) - frame = @context.frame - - secondary_world = frame.secondary_world - adopted_root = secondary_world.execution_context.adopt_element_handle(self) - handle = secondary_world.wait_for_selector(selector, visible: visible, hidden: hidden, timeout: timeout, root: adopted_root) - adopted_root.dispose - return nil unless handle - - main_world = frame.main_world - result = main_world.execution_context.adopt_element_handle(handle) - handle.dispose - result + query_handler_manager.detect_query_handler(selector).wait_for(self, visible: visible, hidden: hidden, timeout: timeout) end define_async_method :async_wait_for_selector @@ -653,6 +642,6 @@ def intersecting_viewport?(threshold: nil) # used in AriaQueryHandler def query_ax_tree(accessible_name: nil, role: nil) @remote_object.query_ax_tree(@client, - accessible_name: accessible_name, role: role) + accessible_name: accessible_name, role: role) end end diff --git a/lib/puppeteer/execution_context.rb b/lib/puppeteer/execution_context.rb index 9cdee1cf..c9fed481 100644 --- a/lib/puppeteer/execution_context.rb +++ b/lib/puppeteer/execution_context.rb @@ -7,7 +7,7 @@ class Puppeteer::ExecutionContext # @param client [Puppeteer::CDPSession] # @param context_payload [Hash] - # @param world [Puppeteer::DOMWorld?] + # @param world [Puppeteer::IsolaatedWorld?] def initialize(client, context_payload, world) @client = client @world = world @@ -17,23 +17,16 @@ def initialize(client, context_payload, world) attr_reader :client, :world - # only used in DOMWorld + # only used in IsolaatedWorld private def _context_id @context_id end - # only used in DOMWorld::BindingFunction#add_binding_to_context + # only used in IsolaatedWorld::BindingFunction#add_binding_to_context private def _context_name @context_name end - # @return [Puppeteer::Frame] - def frame - if_present(@world) do |world| - world.frame - end - end - # @param page_function [String] # @return [Object] def evaluate(page_function, *args) @@ -208,54 +201,4 @@ class EvaluationError < StandardError; end context_id: @context_id, ) end - - # /** - # * @param {!JSHandle} prototypeHandle - # * @return {!Promise} - # */ - # async queryObjects(prototypeHandle) { - # assert(!prototypeHandle._disposed, 'Prototype JSHandle is disposed!'); - # assert(prototypeHandle._remoteObject.objectId, 'Prototype JSHandle must not be referencing primitive value'); - # const response = await this._client.send('Runtime.queryObjects', { - # prototypeObjectId: prototypeHandle._remoteObject.objectId - # }); - # return createJSHandle(this, response.objects); - # } - - # @param backend_node_id [Integer] - # @return [Puppeteer::ElementHandle] - def adopt_backend_node_id(backend_node_id) - response = @client.send_message('DOM.resolveNode', - backendNodeId: backend_node_id, - executionContextId: @context_id, - ) - Puppeteer::JSHandle.create( - context: self, - remote_object: Puppeteer::RemoteObject.new(response["object"]), - ) - end - private define_async_method :async_adopt_backend_node_id - - # @param element_handle [Puppeteer::ElementHandle] - # @return [Puppeteer::ElementHandle] - def adopt_element_handle(element_handle) - if element_handle.execution_context == self - raise ArgumentError.new('Cannot adopt handle that already belongs to this execution context') - end - - unless @world - raise 'Cannot adopt handle without DOMWorld' - end - - node_info = element_handle.remote_object.node_info(@client) - response = @client.send_message('DOM.resolveNode', - backendNodeId: node_info["node"]["backendNodeId"], - executionContextId: @context_id, - ) - - Puppeteer::JSHandle.create( - context: self, - remote_object: Puppeteer::RemoteObject.new(response["object"]), - ) - end end diff --git a/lib/puppeteer/frame.rb b/lib/puppeteer/frame.rb index 5c1a9c48..bae05a16 100644 --- a/lib/puppeteer/frame.rb +++ b/lib/puppeteer/frame.rb @@ -37,8 +37,8 @@ def _client # @param client [Puppeteer::CDPSession] private def update_client(client) @client = client - @main_world = Puppeteer::DOMWorld.new(@client, @frame_manager, self, @frame_manager.timeout_settings) - @secondary_world = Puppeteer::DOMWorld.new(@client, @frame_manager, self, @frame_manager.timeout_settings) + @main_world = Puppeteer::IsolaatedWorld.new(@client, @frame_manager, self, @frame_manager.timeout_settings) + @puppeteer_world = Puppeteer::IsolaatedWorld.new(@client, @frame_manager, self, @frame_manager.timeout_settings) end def page @@ -49,7 +49,7 @@ def oop_frame? @client != @frame_manager.client end - attr_accessor :frame_manager, :id, :loader_id, :lifecycle_events, :main_world, :secondary_world + attr_accessor :frame_manager, :id, :loader_id, :lifecycle_events, :main_world, :puppeteer_world def has_started_loading? @has_started_loading @@ -154,14 +154,14 @@ def query_selector_all(selector) # @return [String] def content - @secondary_world.content + @puppeteer_world.content end # @param html [String] # @param timeout [Integer] # @param wait_until [String|Array] def set_content(html, timeout: nil, wait_until: nil) - @secondary_world.set_content(html, timeout: timeout, wait_until: wait_until) + @puppeteer_world.set_content(html, timeout: timeout, wait_until: wait_until) end # @return [String] @@ -212,35 +212,35 @@ def add_style_tag(url: nil, path: nil, content: nil) # @param button [String] "left"|"right"|"middle" # @param click_count [Number] def click(selector, delay: nil, button: nil, click_count: nil) - @secondary_world.click(selector, delay: delay, button: button, click_count: click_count) + @puppeteer_world.click(selector, delay: delay, button: button, click_count: click_count) end define_async_method :async_click # @param {string} selector def focus(selector) - @secondary_world.focus(selector) + @puppeteer_world.focus(selector) end define_async_method :async_focus # @param {string} selector def hover(selector) - @secondary_world.hover(selector) + @puppeteer_world.hover(selector) end # @param {string} selector # @param {!Array} values # @return {!Promise>} def select(selector, *values) - @secondary_world.select(selector, *values) + @puppeteer_world.select(selector, *values) end define_async_method :async_select # @param {string} selector def tap(selector) - @secondary_world.tap(selector) + @puppeteer_world.tap(selector) end define_async_method :async_tap @@ -259,14 +259,8 @@ def type_text(selector, text, delay: nil) # @param hidden [Boolean] Wait for element invisible ('display: none' nor 'visibility: hidden') on true. default to false. # @param timeout [Integer] def wait_for_selector(selector, visible: nil, hidden: nil, timeout: nil) - handle = @secondary_world.wait_for_selector(selector, visible: visible, hidden: hidden, timeout: timeout) - if !handle - return nil - end - main_execution_context = @main_world.execution_context - result = main_execution_context.adopt_element_handle(handle) - handle.dispose - result + query_handler_manager = Puppeteer::QueryHandlerManager.instance + query_handler_manager.detect_query_handler(selector).wait_for(self, visible: visible, hidden: hidden, timeout: timeout) end define_async_method :async_wait_for_selector @@ -306,7 +300,7 @@ def wait_for_function(page_function, args: [], polling: nil, timeout: nil) # @return [String] def title - @secondary_world.title + @puppeteer_world.title end # @param frame_payload [Hash] @@ -344,7 +338,7 @@ def handle_loading_stopped def detach @detached = true @main_world.detach - @secondary_world.detach + @puppeteer_world.detach if @parent_frame @parent_frame._child_frames.delete(self) end diff --git a/lib/puppeteer/frame_manager.rb b/lib/puppeteer/frame_manager.rb index 63d6c664..55262153 100644 --- a/lib/puppeteer/frame_manager.rb +++ b/lib/puppeteer/frame_manager.rb @@ -413,11 +413,11 @@ def handle_execution_context_created(context_payload, session) if context_payload.dig('auxData', 'isDefault') world = frame.main_world - elsif context_payload['name'] == UTILITY_WORLD_NAME && !frame.secondary_world.has_context? + elsif context_payload['name'] == UTILITY_WORLD_NAME && !frame.puppeteer_world.has_context? # In case of multiple sessions to the same target, there's a race between # connections so we might end up creating multiple isolated worlds. # We can use either. - world = frame.secondary_world + world = frame.puppeteer_world end end diff --git a/lib/puppeteer/dom_world.rb b/lib/puppeteer/isolated_world.rb similarity index 93% rename from lib/puppeteer/dom_world.rb rename to lib/puppeteer/isolated_world.rb index bcfd8d0a..6912a5e4 100644 --- a/lib/puppeteer/dom_world.rb +++ b/lib/puppeteer/isolated_world.rb @@ -1,7 +1,7 @@ require 'thread' -# https://github.com/puppeteer/puppeteer/blob/master/src/DOMWorld.js -class Puppeteer::DOMWorld +# https://github.com/puppeteer/puppeteer/blob/master/src/IsolaatedWorld.js +class Puppeteer::IsolaatedWorld using Puppeteer::DefineAsyncMethod class BindingFunction @@ -152,7 +152,7 @@ def query_selector(selector) raise 'Bug of puppeteer-ruby...' end - private def document + def document @document ||= evaluate_document.as_element end @@ -416,16 +416,6 @@ def type_text(selector, text, delay: nil) handle.dispose end - # @param selector [String] - # @param visible [Boolean] Wait for element visible (not 'display: none' nor 'visibility: hidden') on true. default to false. - # @param hidden [Boolean] Wait for element invisible ('display: none' nor 'visibility: hidden') on true. default to false. - # @param timeout [Integer] - def wait_for_selector(selector, visible: nil, hidden: nil, timeout: nil, root: nil) - # call wait_for_selector_in_page with custom query selector. - query_selector_manager = Puppeteer::QueryHandlerManager.instance - query_selector_manager.detect_query_handler(selector).wait_for(self, visible: visible, hidden: hidden, timeout: timeout, root: root) - end - private def binding_identifier(name, context) "#{name}_#{context.send(:_context_id)}" end @@ -497,7 +487,7 @@ def add_binding_to_context(context, binding_function) # @param visible [Boolean] Wait for element visible (not 'display: none' nor 'visibility: hidden') on true. default to false. # @param hidden [Boolean] Wait for element invisible ('display: none' nor 'visibility: hidden') on true. default to false. # @param timeout [Integer] - private def wait_for_selector_in_page(query_one, selector, visible: nil, hidden: nil, timeout: nil, root: nil, binding_function: nil) + private def wait_for_selector_in_page(query_one, root, selector, visible: nil, hidden: nil, timeout: nil, binding_function: nil) option_wait_for_visible = visible || false option_wait_for_hidden = hidden || false option_timeout = timeout || @timeout_settings.timeout @@ -531,12 +521,7 @@ def add_binding_to_context(context, binding_function) root: option_root, binding_function: binding_function, ) - handle = wait_task.await_promise - unless handle.as_element - handle.dispose - return nil - end - handle.as_element + wait_task.await_promise end # @param page_function [String] @@ -601,4 +586,35 @@ def title } JAVASCRIPT end + + # @param backend_node_id [Integer] + # @return [Puppeteer::ElementHandle] + def adopt_backend_node(backend_node_id) + response = @client.send_message('DOM.resolveNode', + backendNodeId: backend_node_id, + executionContextId: execution_context.send(:_context_id), + ) + Puppeteer::JSHandle.create( + context: execution_context, + remote_object: Puppeteer::RemoteObject.new(response["object"]), + ) + end + private define_async_method :async_adopt_backend_node + + # @param element_handle [Puppeteer::ElementHandle] + # @return [Puppeteer::ElementHandle] + def adopt_handle(element_handle) + if element_handle.execution_context == execution_context + raise ArgumentError.new('Cannot adopt handle that already belongs to this execution context') + end + + node_info = element_handle.remote_object.node_info(@client) + adopt_backend_node(node_info["node"]["backendNodeId"]) + end + + def transfer_handle(element_handle) + result = adopt_handle(element_handle) + element_handle.dispose + result + end end diff --git a/lib/puppeteer/js_handle.rb b/lib/puppeteer/js_handle.rb index 444a0c8c..273cb38f 100644 --- a/lib/puppeteer/js_handle.rb +++ b/lib/puppeteer/js_handle.rb @@ -5,16 +5,12 @@ class Puppeteer::JSHandle # @param context [Puppeteer::ExecutionContext] # @param remote_object [Puppeteer::RemoteObject] def self.create(context:, remote_object:) - frame = context.frame - if remote_object.sub_type == 'node' && frame - frame_manager = frame.frame_manager + if remote_object.sub_type == 'node' && context.world Puppeteer::ElementHandle.new( context: context, client: context.client, remote_object: remote_object, - frame: frame, - page: frame_manager.page, - frame_manager: frame_manager, + frame: context.world.frame, ) else Puppeteer::JSHandle.new( diff --git a/lib/puppeteer/page.rb b/lib/puppeteer/page.rb index 58dd7d84..a75836c7 100644 --- a/lib/puppeteer/page.rb +++ b/lib/puppeteer/page.rb @@ -191,8 +191,7 @@ def handle_file_chooser(event) return if @file_chooser_interceptors.empty? frame = @frame_manager.frame(event['frameId']) - context = frame.execution_context - element = context.adopt_backend_node_id(event['backendNodeId']) + element = frame.main_world.adopt_backend_node(event['backendNodeId']) interceptors = @file_chooser_interceptors.to_a @file_chooser_interceptors.clear file_chooser = Puppeteer::FileChooser.new(element, event) diff --git a/lib/puppeteer/query_handler_manager.rb b/lib/puppeteer/query_handler_manager.rb index 32989fed..89fac7bd 100644 --- a/lib/puppeteer/query_handler_manager.rb +++ b/lib/puppeteer/query_handler_manager.rb @@ -61,8 +61,8 @@ def query_one(element_handle) @query_handler.query_one(element_handle, @selector) end - def wait_for(dom_world, visible:, hidden:, timeout:, root:) - @query_handler.wait_for(dom_world, @selector, visible: visible, hidden: hidden, timeout: timeout, root: root) + def wait_for(element_or_frame, visible:, hidden:, timeout:) + @query_handler.wait_for(element_or_frame, @selector, visible: visible, hidden: hidden, timeout: timeout) end def query_all(element_handle) diff --git a/lib/puppeteer/remote_object.rb b/lib/puppeteer/remote_object.rb index b7babffb..9da9a28f 100644 --- a/lib/puppeteer/remote_object.rb +++ b/lib/puppeteer/remote_object.rb @@ -105,8 +105,8 @@ def query_ax_tree(client, accessible_name: nil, role: nil) role: role, }.compact) - result['nodes'].reject do |node| - node['role']['value'] == 'text' + result['nodes'].select do |node| + node['role'] && node['role']['value'] != 'StaticText' end end diff --git a/spec/integration/wait_task_spec.rb b/spec/integration/wait_task_spec.rb index f0be030c..d75bc500 100644 --- a/spec/integration/wait_task_spec.rb +++ b/spec/integration/wait_task_spec.rb @@ -109,7 +109,7 @@ other_frame.evaluate(add_element, 'div') page.evaluate(add_element, 'div') handle = await watchdog - expect(handle.execution_context.frame).to eq(page.main_frame) + expect(handle.frame).to eq(page.main_frame) end it 'should run in specified frame', sinatra: true do @@ -122,7 +122,7 @@ frame1.evaluate(add_element, 'div') frame2.evaluate(add_element, 'div') handle = await promise - expect(handle.execution_context.frame).to eq(frame2) + expect(handle.frame).to eq(frame2) end it_fails_firefox 'should throw when frame is detached', sinatra: true do diff --git a/spec/puppeteer/element_handle_spec.rb b/spec/puppeteer/element_handle_spec.rb index 22228d81..2647b5f0 100644 --- a/spec/puppeteer/element_handle_spec.rb +++ b/spec/puppeteer/element_handle_spec.rb @@ -44,9 +44,10 @@ context: double(Puppeteer::ExecutionContext), client: double(Puppeteer::CDPSession), remote_object: double(Puppeteer::RemoteObject), - frame: double(Puppeteer::Frame), - page: double(Puppeteer::Page), - frame_manager: double(Puppeteer::FrameManager), + frame: double(Puppeteer::Frame, + page: double(Puppeteer::Page), + frame_manager: double(Puppeteer::FrameManager), + ), ) }