Add a runtime flag to enable an experimental WebSocket implementation.

Add a runtime flag to enable an experimental WebSocket implementation.


BUG=226933
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=194781

[jamesr, 2013-04-09 06:31:12]

cc/ is the place where the chromium compositor lives.  why would a websocket
flag live there?

[yhirano, 2013-04-09 07:01:19]

jamesr, thank you, you are right.
I have moved the switch from cc to content.
joi, can you take a look at it?

[Jói, 2013-04-09 09:29:22]

LGTM for content/public

https://codereview.chromium.org/13936002/diff/4001/content/public/common/cont...
File content/public/common/content_switches.cc (right):

https://codereview.chromium.org/13936002/diff/4001/content/public/common/cont...
content/public/common/content_switches.cc:224: // Enable an experimental
WebSocket implementation.
Nit: Document what this does if --disable-web-sockets is also passed.

[Adam Rice, 2013-04-09 12:21:45]

https://codereview.chromium.org/13936002/diff/4001/chrome/app/generated_resou...
File chrome/app/generated_resources.grd (right):

https://codereview.chromium.org/13936002/diff/4001/chrome/app/generated_resou...
chrome/app/generated_resources.grd:6529: +      <message
name="IDS_FLAGS_ENABLE_EXPERIMENTAL_WEBSOCKET_NAME" desc="Name of the flag that
enables experimental WebSocket impelementation in chrome/net">
s/impelementation/implementation/

https://codereview.chromium.org/13936002/diff/4001/chrome/app/generated_resou...
chrome/app/generated_resources.grd:6532: +      <message
name="IDS_FLAGS_ENABLE_EXPERIMENTAL_WEBSOCKET_DESCRIPTION" desc="Description for
the flag to enable experimental WebSocket impelementation in chrome/net">
s/impelementation/implementation/

https://codereview.chromium.org/13936002/diff/4001/chrome/app/generated_resou...
chrome/app/generated_resources.grd:6533: +        Use an explemental WebSocket
impelementation.
I think "Use the experimental WebSocket implementation." would be better.

https://codereview.chromium.org/13936002/diff/4001/chrome/browser/about_flags.cc
File chrome/browser/about_flags.cc (right):

https://codereview.chromium.org/13936002/diff/4001/chrome/browser/about_flags...
chrome/browser/about_flags.cc:1264: kOsLinux,
I think this can be kOsDesktop or maybe even kOsAll

[yhirano, 2013-04-10 00:52:44]

https://codereview.chromium.org/13936002/diff/4001/chrome/app/generated_resou...
File chrome/app/generated_resources.grd (right):

https://codereview.chromium.org/13936002/diff/4001/chrome/app/generated_resou...
chrome/app/generated_resources.grd:6529: +      <message
name="IDS_FLAGS_ENABLE_EXPERIMENTAL_WEBSOCKET_NAME" desc="Name of the flag that
enables experimental WebSocket impelementation in chrome/net">
On 2013/04/09 12:21:45, Adam Rice wrote:
> s/impelementation/implementation/

Done.

https://codereview.chromium.org/13936002/diff/4001/chrome/app/generated_resou...
chrome/app/generated_resources.grd:6532: +      <message
name="IDS_FLAGS_ENABLE_EXPERIMENTAL_WEBSOCKET_DESCRIPTION" desc="Description for
the flag to enable experimental WebSocket impelementation in chrome/net">
On 2013/04/09 12:21:45, Adam Rice wrote:
> s/impelementation/implementation/

Done.

https://codereview.chromium.org/13936002/diff/4001/chrome/app/generated_resou...
chrome/app/generated_resources.grd:6533: +        Use an explemental WebSocket
impelementation.
On 2013/04/09 12:21:45, Adam Rice wrote:
> I think "Use the experimental WebSocket implementation." would be better.

Done.

https://codereview.chromium.org/13936002/diff/4001/chrome/browser/about_flags.cc
File chrome/browser/about_flags.cc (right):

https://codereview.chromium.org/13936002/diff/4001/chrome/browser/about_flags...
chrome/browser/about_flags.cc:1264: kOsLinux,
On 2013/04/09 12:21:45, Adam Rice wrote:
> I think this can be kOsDesktop or maybe even kOsAll

Done.

https://codereview.chromium.org/13936002/diff/4001/content/public/common/cont...
File content/public/common/content_switches.cc (right):

https://codereview.chromium.org/13936002/diff/4001/content/public/common/cont...
content/public/common/content_switches.cc:224: // Enable an experimental
WebSocket implementation.
On 2013/04/09 09:29:22, Jói wrote:
> Nit: Document what this does if --disable-web-sockets is also passed.

Done.

[Adam Rice, 2013-04-10 03:15:38]

lgtm

lgtm

[commit-bot: I haz the power, 2013-04-10 03:17:47]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/yhirano@chromium.org/13936002/10001

[commit-bot: I haz the power, 2013-04-10 03:17:48]

Failed to apply patch for
content/browser/renderer_host/render_process_host_impl.cc:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file content/browser/renderer_host/render_process_host_impl.cc
  Hunk #1 FAILED at 812.
  1 out of 1 hunk FAILED -- saving rejects to file
content/browser/renderer_host/render_process_host_impl.cc.rej

Patch:       content/browser/renderer_host/render_process_host_impl.cc
Index: content/browser/renderer_host/render_process_host_impl.cc
diff --git a/content/browser/renderer_host/render_process_host_impl.cc
b/content/browser/renderer_host/render_process_host_impl.cc
index
f41db9c024051dc307f4da6fc99121ac14bf9dd8..2acd994c43f4a4e4990b61b323eef930de0e8040
100644
--- a/content/browser/renderer_host/render_process_host_impl.cc
+++ b/content/browser/renderer_host/render_process_host_impl.cc
@@ -812,6 +812,7 @@ void
RenderProcessHostImpl::PropagateBrowserCommandLineToRenderer(
     switches::kDisableWebAudio,
 #endif
     switches::kDisableWebSockets,
+    switches::kEnableExperimentalWebSocket,
     switches::kDomAutomationController,
     switches::kEnableAccessibilityLogging,
     switches::kEnableBrowserPluginForAllViewTypes,

[commit-bot: I haz the power, 2013-04-10 03:43:30]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/yhirano@chromium.org/13936002/21001

[commit-bot: I haz the power, 2013-04-10 03:43:36]

Presubmit check for 13936002-21001 failed and returned exit status 1.

INFO:root:Found 5 file(s).

Running presubmit commit checks ...
Running /b/commit-queue/workdir/chromium/PRESUBMIT.py
Running /b/commit-queue/workdir/chromium/chrome/PRESUBMIT.py

** Presubmit ERRORS **
Missing LGTM from an OWNER for these files:
    content/browser/renderer_host/render_process_host_impl.cc

Presubmit checks took 1.2s to calculate.

[yhirano, 2013-04-10 04:00:46]

aelias, I'm sorry for bothering you again. Can you review the patch?

[yhirano, 2013-04-17 04:40:21]

sadrul, can you review the patch for content/browser/render_host/ ?

[sadrul, 2013-04-17 16:42:06]

I am owner for the touch/gesture related code. But this is simple enough. LGTM.

https://codereview.chromium.org/13936002/diff/30001/content/public/common/con...
File content/public/common/content_switches.cc (right):

https://codereview.chromium.org/13936002/diff/30001/content/public/common/con...
content/public/common/content_switches.cc:223:
"enable-websocket-experimental-implementation";
The k... variable names usually mirror the actual flag. So the variable would be
kEnableWebSocketExperimentalImplementation, or the flag would be
'enable-experimental-websocket'

[yhirano, 2013-04-18 00:52:37]

Thank you very much, all reviewers.

https://codereview.chromium.org/13936002/diff/30001/content/public/common/con...
File content/public/common/content_switches.cc (right):

https://codereview.chromium.org/13936002/diff/30001/content/public/common/con...
content/public/common/content_switches.cc:223:
"enable-websocket-experimental-implementation";
On 2013/04/17 16:42:06, sadrul wrote:
> The k... variable names usually mirror the actual flag. So the variable would
be
> kEnableWebSocketExperimentalImplementation, or the flag would be
> 'enable-experimental-websocket'

Done.

[commit-bot: I haz the power, 2013-04-18 00:53:24]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/yhirano@chromium.org/13936002/35002

[commit-bot: I haz the power, 2013-04-18 05:31:43]

Change committed as 194781