Bug 183306 – Notify the NetworkProcess when a session is servicing an automation client

• Component: WebKit2
• Status: RESOLVED FIXED

[Brent Fulgham]

Network loads servicing WebDriver are done through an ephemeral session. While this is great for protecting a developer's machine from sharing state with test runs, it has the unintended effect of blocking certain logging operations.

We do not log content in ephemeral sessions to protect user privacy. However, ephemeral sessions generated by WebDriver should participate in logging so that proper testing (with logging) can be done.

This patch signals the NetworkProcess when an ephemeral session (created for automation purposes) is created, so that it can allow logging.

[Brent Fulgham]

Created attachment 334940 [details]
Patch

[Brent Fulgham]

Created attachment 334943 [details]
Patch

[Blaze Burg]

Comment on attachment 334940 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=334940&action=review

> Source/WebKit/UIProcess/WebPageProxy.cpp:427
> +    if (m_controlledByAutomation)

This won't work because the member isn't set in the constructor. You should put this message send in the setter.

[Blaze Burg]

Comment on attachment 334943 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=334943&action=review

> Source/WebKit/UIProcess/WebPageProxy.cpp:427
> +    if (m_controlledByAutomation)

Ditto to previous comment.

[Brent Fulgham]

Comment on attachment 334943 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=334943&action=review

>> Source/WebKit/UIProcess/WebPageProxy.cpp:427
>> +    if (m_controlledByAutomation)
> 
> Ditto to previous comment.

Doesn't it get assigned on Line 373 above?

[Brent Fulgham]

Created attachment 334961 [details]
Patch

[Brent Fulgham]

(In reply to Brent Fulgham from comment #5)
> Comment on attachment 334943 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=334943&action=review
> 
> >> Source/WebKit/UIProcess/WebPageProxy.cpp:427
> >> +    if (m_controlledByAutomation)
> > 
> > Ditto to previous comment.
> 
> Doesn't it get assigned on Line 373 above?

I guess you meant that it's only ever set by the API; we never set automation through the page configuration directly.

[Brent Fulgham]

Created attachment 334962 [details]
Patch

[Blaze Burg]

Comment on attachment 334962 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=334962&action=review

r=me

> Source/WebKit/UIProcess/WebPageProxy.cpp:1288
> +    m_process->processPool().sendToNetworkingProcess(Messages::NetworkProcess::SetSessionIsControlledByAutomation(m_websiteDataStore->sessionID(), m_controlledByAutomation));

This looks good!

[WebKit Commit Bot]

Comment on attachment 334962 [details]
Patch

Clearing flags on attachment: 334962

Committed r229201: <https://trac.webkit.org/changeset/229201>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/38107474>

[Brent Fulgham]

This is actually:

<rdar://problem/37835783>