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: custom sessions should not emit targetcreated events #259

Merged
merged 1 commit into from Sep 18, 2022
Merged
Show file tree
Hide file tree
Changes from all 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/browser.rb
Expand Up @@ -196,7 +196,7 @@ def create_target(target_info, session)
session: session,
browser_context: context,
target_manager: @target_manager,
session_factory: -> { @connection.create_session(target_info) },
session_factory: -> (auto_attach_emulated) { @connection.create_session(target_info, auto_attach_emulated: auto_attach_emulated) },
ignore_https_errors: @ignore_https_errors,
default_viewport: @default_viewport,
is_page_target_callback: @is_page_target_callback,
Expand Down
10 changes: 8 additions & 2 deletions lib/puppeteer/chrome_target_manager.rb
Expand Up @@ -129,7 +129,7 @@ def remove_target_interceptor(client, interceptor)
# Special case (https://crbug.com/1338156): currently, shared_workers
# don't get auto-attached. This should be removed once the auto-attach
# works.
@connection.create_session(target_info)
@connection.create_session(target_info, auto_attach_emulated: true)
end
end

Expand Down Expand Up @@ -185,6 +185,8 @@ class SessionNotCreatedError < StandardError ; end
end
}

return unless @connection.auto_attached?(target_info.target_id)

# Special case for service workers: being attached to service workers will
# prevent them from ever being destroyed. Therefore, we silently detach
# from service workers unless the connection was manually created via
Expand Down Expand Up @@ -212,6 +214,8 @@ class SessionNotCreatedError < StandardError ; end
return
end

is_existing_target = @attached_targets_by_target_id.has_key?(target_info.target_id)

target = @attached_targets_by_target_id[target_info.target_id] || @target_factory.call(target_info, session)
setup_attachment_listeners(session)

Expand All @@ -233,7 +237,9 @@ class SessionNotCreatedError < StandardError ; end
end

@target_ids_for_init.delete(target.target_id)
future { emit_event(TargetManagerEmittedEvents::TargetAvailable, target) }
unless is_existing_target
future { emit_event(TargetManagerEmittedEvents::TargetAvailable, target) }
end
finish_initialization_if_ready

future do
Expand Down
9 changes: 6 additions & 3 deletions lib/puppeteer/connection.rb
Expand Up @@ -319,15 +319,18 @@ def dispose
end

def auto_attached?(target_id)
@manually_attached.include?(target_id)
!@manually_attached.include?(target_id)
end

# @param {Protocol.Target.TargetInfo} targetInfo
# @return [CDPSession]
def create_session(target_info)
@manually_attached << target_info.target_id
def create_session(target_info, auto_attach_emulated: false)
unless auto_attach_emulated
@manually_attached << target_info.target_id
end
result = send_message('Target.attachToTarget', targetId: target_info.target_id, flatten: true)
session_id = result['sessionId']
@manually_attached.delete(target_info.target_id)
@sessions[session_id]
end
end
4 changes: 2 additions & 2 deletions lib/puppeteer/target.rb
Expand Up @@ -93,7 +93,7 @@ def session
end

def create_cdp_session
@session_factory.call
@session_factory.call(false)
end

def target_manager
Expand All @@ -102,7 +102,7 @@ def target_manager

def page
if @is_page_target_callback.call(@target_info) && @page.nil?
client = @session || @session_factory.call
client = @session || @session_factory.call(true)
@page = Puppeteer::Page.create(client, self, @ignore_https_errors, @default_viewport)
end
@page
Expand Down
37 changes: 21 additions & 16 deletions spec/integration/cdp_session_spec.rb
Expand Up @@ -11,23 +11,28 @@
expect(foo).to eq('bar')
end

context 'with empty page', sinatra: true do
before {
sinatra.get('/') { 'Hello' }
}

it 'should send events' do
client = page.target.create_cdp_session

client.send_message('Network.enable')

events = []
client.on('Network.requestWillBeSent') do |event|
events << event
end
page.goto("#{server_prefix}/")
expect(events.size).to eq(1)
it 'should not report created targets for custom CDP sessions', puppeteer: :browser do
called = false
browser.browser_contexts.first.on('targetcreated') do |target|
raise 'Too many targets created' if called
called = true

target.create_cdp_session
end
browser.new_page
end

it 'should send events', sinatra: true do
client = page.target.create_cdp_session

client.send_message('Network.enable')

events = []
client.on('Network.requestWillBeSent') do |event|
events << event
end
page.goto(server_empty_page)
expect(events.size).to eq(1)
end

# it('should enable and disable domains independently', async () => {
Expand Down