Bug 64852 – [Chromium] Turn on new WebSocket protocol in worker shadow page

• Component: WebCore Misc.
• Status: RESOLVED FIXED

[Yuta Kitamura]

I'm trying to turn on the new WebSocket protocol by default.

This should be done in the following steps:

(1) Change the default preference value in webkit/glue/webpreferences.{h,cc} (in Chromium tree).
(2) Change the default preference value in worker shadow page initialized in WebWorkerBase::initializeLoader() (in WebKit tree).

This bug takes care of step (2). Step (2) is required to enable the new protocol in WebSocket objects initialized in worker context, because they live in worker loader context and look at the value of a Settings flag of the shadow page.

[Yuta Kitamura]

A patch for step (1) is under review at <http://codereview.chromium.org/7462009/>.

[Yuta Kitamura]

Created attachment 101454 [details]
Patch

[Yuta Kitamura]

Comment on attachment 101454 [details]
Patch

Setting CQ- since this patch should be committed after <http://codereview.chromium.org/7462009/>.

[Yuta Kitamura]

Dmitry, could you take a look at this patch?

I think it's better to copy the value of Settings::useHixie76WebSocketProtocol() of the main thread (rather than setting a constant "false"), but I could not figure out a good way to do this.

[Kent Tamura]

(In reply to comment #4)
> Dmitry, could you take a look at this patch?
> 
> I think it's better to copy the value of Settings::useHixie76WebSocketProtocol() of the main thread (rather than setting a constant "false"), but I could not figure out a good way to do this.

Though I don't know how to retrieve the value from the main thread, I think we need a FIXME comment.

[Yuta Kitamura]

Created attachment 101723 [details]
Patch v2 (add FIXME comment)

[Dmitry Titov]

Comment on attachment 101723 [details]
Patch v2 (add FIXME comment)

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

r=me with a note.

> Source/WebKit/chromium/src/WebWorkerBase.cpp:242
> +    // FIXME: Should copy the value of Settings::hixie76ProtocolEnabled() of a view in the main thread.

This code is running in the main WebKit thread, but it runs in a sandboxed Worker process. Settings information should be passed to the Worker process from Browser process when the worker is created (similar to RenderThread::OnCreateNewView).  Could you please change the comment to reflect this?

[Yuta Kitamura]

Created attachment 101809 [details]
Patch for landing

[Yuta Kitamura]

Comment on attachment 101723 [details]
Patch v2 (add FIXME comment)

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

Thank you for your review!

>> Source/WebKit/chromium/src/WebWorkerBase.cpp:242
>> +    // FIXME: Should copy the value of Settings::hixie76ProtocolEnabled() of a view in the main thread.
> 
> This code is running in the main WebKit thread, but it runs in a sandboxed Worker process. Settings information should be passed to the Worker process from Browser process when the worker is created (similar to RenderThread::OnCreateNewView).  Could you please change the comment to reflect this?

Updated the comment as per your suggestion.

[WebKit Review Bot]

Comment on attachment 101809 [details]
Patch for landing

Clearing flags on attachment: 101809

Committed r91634: <http://trac.webkit.org/changeset/91634>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.