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: make sure inner OOPIFs can be attached to #223

Merged
merged 5 commits into from Jun 25, 2022
Merged
Show file tree
Hide file tree
Changes from 4 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
2 changes: 1 addition & 1 deletion lib/puppeteer/execution_context.rb
Expand Up @@ -2,7 +2,7 @@ class Puppeteer::ExecutionContext
include Puppeteer::IfPresent
using Puppeteer::DefineAsyncMethod

EVALUATION_SCRIPT_URL = '__puppeteer_evaluation_script__'
EVALUATION_SCRIPT_URL = 'pprt://__puppeteer_evaluation_script__'
SOURCE_URL_REGEX = /^[\040\t]*\/\/[@#] sourceURL=\s*(\S*?)\s*$/m

# @param client [Puppeteer::CDPSession]
Expand Down
12 changes: 9 additions & 3 deletions lib/puppeteer/frame_manager.rb
Expand Up @@ -71,11 +71,17 @@ def initialize(client, page, ignore_https_errors, timeout_settings)
private def init(cdp_session = nil)
client = cdp_session || @client

results = await_all(
promises = [
client.async_send_message('Page.enable'),
client.async_send_message('Page.getFrameTree'),
)
frame_tree = results.last['frameTree']
cdp_session&.async_send_message('Target.setAutoAttach', {
autoAttach: true,
waitForDebuggerOnStart: false,
flatten: true,
})

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[rubocop] reported by reviewdog 🐶
[Correctable] Style/TrailingCommaInArrayLiteral: Put a comma after the last item of a multiline array.

].compact
results = await_all(*promises)
frame_tree = results[1]['frameTree']
handle_frame_tree(client, frame_tree)
await_all(
client.async_send_message('Page.setLifecycleEventsEnabled', enabled: true),
Expand Down
4 changes: 4 additions & 0 deletions lib/puppeteer/page.rb
Expand Up @@ -814,6 +814,10 @@ def wait_for_frame(url: nil, predicate: nil, timeout: nil)
predicate
end

frames.each do |frame|
return frame if frame_predicate.call(frame)
end

wait_for_frame_manager_event(
FrameManagerEmittedEvents::FrameAttached,
FrameManagerEmittedEvents::FrameNavigated,
Expand Down
10 changes: 10 additions & 0 deletions spec/assets/inner-frame1.html
@@ -0,0 +1,10 @@
<script>
window.addEventListener('DOMContentLoaded', () => {
const iframe = document.createElement('iframe');
const url = new URL(location.href);
url.hostname = 'inner-frame2.test';
url.pathname = '/inner-frame2.html';
iframe.src = url.toString();
document.body.appendChild(iframe);
}, false);
</script>
1 change: 1 addition & 0 deletions spec/assets/inner-frame2.html
@@ -0,0 +1 @@
<button>click</button>
10 changes: 10 additions & 0 deletions spec/assets/main-frame.html
@@ -0,0 +1,10 @@
<script>
window.addEventListener('DOMContentLoaded', () => {
const iframe = document.createElement('iframe');
const url = new URL(location.href);
url.hostname = 'inner-frame1.test';
url.pathname = '/inner-frame1.html';
iframe.src = url.toString();
document.body.appendChild(iframe);
}, false);
</script>
18 changes: 18 additions & 0 deletions spec/integration/oopif_spec.rb
Expand Up @@ -145,6 +145,16 @@ def oopifs(context)
expect(page.frames.size).to eq(2)
end

it 'should wait for inner OOPIFs' do
predicate = -> (frame) { frame.url&.end_with?('/inner-frame2.html') }
frame2 = page.wait_for_frame(predicate: predicate) do
page.goto("http://mainframe:#{server_port}/main-frame.html")
end
expect(oopifs(browser_context).size).to eq(2)
expect(page.frames.count { |frame| frame.oop_frame? }).to eq(2)
expect(frame2.evaluate('() => document.querySelectorAll("button").length')).to eq(1)
end

it 'should load oopif iframes with subresources and request interception' do
predicate = -> (frame) { frame.url&.end_with?('/oopif.html') }
frame_promise = page.async_wait_for_frame(predicate: predicate)
Expand Down Expand Up @@ -211,4 +221,12 @@ def oopifs(context)
expect(result_bounding_box.x).to be > 150 # padding + margin + border left
expect(result_bounding_box.y).to be > 150 # padding + margin + border top
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")
predicate = ->(frame) { frame.url&.end_with?('/empty.html') }
frame = page.wait_for_frame(predicate: predicate)
expect(frame.url).to end_with("/empty.html")
end
end
8 changes: 5 additions & 3 deletions spec/spec_helper.rb
Expand Up @@ -78,6 +78,7 @@ def firefox?
if example.metadata[:enable_site_per_process_flag]
args = launch_options[:args] || []
args << '--site-per-process'
args << '--host-rules=MAP * 127.0.0.1'
launch_options[:args] = args
end

Expand Down Expand Up @@ -150,7 +151,7 @@ def default_launch_options
config.include PuppeteerMethods, type: :puppeteer

test_with_sinatra = Module.new do
attr_reader :server_prefix, :server_cross_process_prefix, :server_empty_page, :sinatra
attr_reader :server_port, :server_prefix, :server_cross_process_prefix, :server_empty_page, :sinatra
end
config.include(test_with_sinatra, sinatra: true)
config.around(sinatra: true) do |example|
Expand All @@ -163,8 +164,9 @@ def default_launch_options
sinatra_app.set(:quiet, true)
sinatra_app.set(:public_folder, File.join(__dir__, 'assets'))
sinatra_app.set(:logging, false)
@server_prefix = "http://localhost:4567"
@server_cross_process_prefix = "http://127.0.0.1:4567"
@server_port = 4567
@server_prefix = "http://localhost:#{@server_port}"
@server_cross_process_prefix = "http://127.0.0.1:#{@server_port}"
@server_empty_page = "#{@server_prefix}/empty.html"

sinatra_app.get('/_ping') { '_pong' }
Expand Down