bluetooth: Add a WebBluetooth command line flag.

bluetooth: Add a WebBluetooth command line flag.

Adds WebBluetooth command line flag for enabling WebBluetooth. Also adds
it to about:flags and to bad_flags_prompt to make sure users know using
the flag is dangerous.

This is the browser side patch to introduce the WebBluetooth Flag:
 [1] https://codereview.chromium.org/1136913002
 [2] This patch.
 [3] https://codereview.chromium.org/1127313004

BUG=486456
Committed: https://crrev.com/89be0ad96e7968a977db955441ddab26685b7cc8
Cr-Commit-Position: refs/heads/master@{#331185}

[ortuno, 2015-05-10 02:25:19]

Patchset #1 (id:1) has been deleted

[felt, 2015-05-11 13:53:03]

felt@chromium.org changed reviewers:
+ felt@chromium.org

[felt, 2015-05-11 13:53:04]

https://codereview.chromium.org/1134473003/diff/20001/chrome/browser/ui/start...
File chrome/browser/ui/startup/bad_flags_prompt.cc (right):

https://codereview.chromium.org/1134473003/diff/20001/chrome/browser/ui/start...
chrome/browser/ui/startup/bad_flags_prompt.cc:91: switches::kEnableWebBluetooth,
you might want to keep the extra blank line between the last switch and NULL

https://codereview.chromium.org/1134473003/diff/20001/content/shell/app/shell...
File content/shell/app/shell_main_delegate.cc (right):

https://codereview.chromium.org/1134473003/diff/20001/content/shell/app/shell...
content/shell/app/shell_main_delegate.cc:174:
command_line.AppendSwitch(switches::kEnableWebBluetooth);
what's kStableReleaseMode? why do we want to automatically enable this flag
without the user flipping it?

[ortuno, 2015-05-11 17:46:43]

Thanks Adrienne! Ready for review again.

https://codereview.chromium.org/1134473003/diff/20001/chrome/browser/ui/start...
File chrome/browser/ui/startup/bad_flags_prompt.cc (right):

https://codereview.chromium.org/1134473003/diff/20001/chrome/browser/ui/start...
chrome/browser/ui/startup/bad_flags_prompt.cc:91: switches::kEnableWebBluetooth,
On 2015/05/11 at 13:53:03, felt wrote:
> you might want to keep the extra blank line between the last switch and NULL
Done.

https://codereview.chromium.org/1134473003/diff/20001/content/shell/app/shell...
File content/shell/app/shell_main_delegate.cc (right):

https://codereview.chromium.org/1134473003/diff/20001/content/shell/app/shell...
content/shell/app/shell_main_delegate.cc:174:
command_line.AppendSwitch(switches::kEnableWebBluetooth);
On 2015/05/11 at 13:53:03, felt wrote:
> what's kStableReleaseMode? why do we want to automatically enable this flag
without the user flipping it?

It's a flag to disable some web-platform runtime features so that we test
content_shell as if it were a stable release.
We are adding the flag automatically only during layout tests.

[ortuno, 2015-05-11 17:46:46]

Thanks Adrienne! Ready for review again.

[felt, 2015-05-11 18:29:20]

lgtm

https://codereview.chromium.org/1134473003/diff/20001/content/shell/app/shell...
File content/shell/app/shell_main_delegate.cc (right):

https://codereview.chromium.org/1134473003/diff/20001/content/shell/app/shell...
content/shell/app/shell_main_delegate.cc:174:
command_line.AppendSwitch(switches::kEnableWebBluetooth);
On 2015/05/11 17:46:43, ortuno wrote:
> On 2015/05/11 at 13:53:03, felt wrote:
> > what's kStableReleaseMode? why do we want to automatically enable this flag
> without the user flipping it?
> 
> It's a flag to disable some web-platform runtime features so that we test
> content_shell as if it were a stable release.
> We are adding the flag automatically only during layout tests.

ah, ok

[scheib, 2015-05-12 17:51:25]

scheib@chromium.org changed reviewers:
+ scheib@chromium.org

[scheib, 2015-05-12 17:51:26]

LGTM, but please:

https://codereview.chromium.org/1134473003/diff/40001/content/shell/app/shell...
File content/shell/app/shell_main_delegate.cc (right):

https://codereview.chromium.org/1134473003/diff/40001/content/shell/app/shell...
content/shell/app/shell_main_delegate.cc:174:
command_line.AppendSwitch(switches::kEnableWebBluetooth);
Add the explanation you gave to felt@ as a comment here.

[ortuno, 2015-05-12 20:06:06]

Thanks!

https://codereview.chromium.org/1134473003/diff/40001/content/shell/app/shell...
File content/shell/app/shell_main_delegate.cc (right):

https://codereview.chromium.org/1134473003/diff/40001/content/shell/app/shell...
content/shell/app/shell_main_delegate.cc:174:
command_line.AppendSwitch(switches::kEnableWebBluetooth);
On 2015/05/12 at 17:51:26, scheib wrote:
> Add the explanation you gave to felt@ as a comment here.

Done.

[ortuno, 2015-05-12 20:35:44]

ortuno@chromium.org changed reviewers:
+ cpu@chromium.org

[ortuno, 2015-05-12 20:35:44]

@cpu: We are adding a new Web Bluetooth flag. Please owners review for
content_switches.

[cpu_(ooo_6.6-7.5), 2015-05-12 22:27:14]

content_switches lgtm

[ortuno, 2015-05-12 23:54:10]

ortuno@chromium.org changed reviewers:
+ jochen@chromium.org

[ortuno, 2015-05-12 23:54:11]

@jochen: We are adding a new Web Bluetooth flag. PTAL

[jochen (gone - plz use gerrit), 2015-05-15 06:11:33]

https://codereview.chromium.org/1134473003/diff/60001/content/child/runtime_f...
File content/child/runtime_features.cc (right):

https://codereview.chromium.org/1134473003/diff/60001/content/child/runtime_f...
content/child/runtime_features.cc:95: if
(command_line.HasSwitch(switches::kEnableWebBluetooth))
this should be in chrome/renderer/chrome_render_process_observer.cc

https://codereview.chromium.org/1134473003/diff/60001/content/public/common/c...
File content/public/common/content_switches.h (right):

https://codereview.chromium.org/1134473003/diff/60001/content/public/common/c...
content/public/common/content_switches.h:107: CONTENT_EXPORT extern const char
kEnableWebBluetooth[];
this should be a chrome switch

https://codereview.chromium.org/1134473003/diff/60001/content/shell/app/shell...
File content/shell/app/shell_main_delegate.cc (right):

https://codereview.chromium.org/1134473003/diff/60001/content/shell/app/shell...
content/shell/app/shell_main_delegate.cc:175:
command_line.AppendSwitch(switches::kEnableWebBluetooth);
layout test related flags should go into the layout_test subdir. however, since
this is an experimental web platform feature, it's enabled for layout tests
anyways

[ortuno, 2015-05-15 16:31:14]

@jochen: Thanks! Questions inline. For more context please see:

https://codereview.chromium.org/1136913002
https://codereview.chromium.org/1127313004

https://codereview.chromium.org/1134473003/diff/60001/content/child/runtime_f...
File content/child/runtime_features.cc (right):

https://codereview.chromium.org/1134473003/diff/60001/content/child/runtime_f...
content/child/runtime_features.cc:95: if
(command_line.HasSwitch(switches::kEnableWebBluetooth))
On 2015/05/15 at 06:11:32, jochen (traveling) wrote:
> this should be in chrome/renderer/chrome_render_process_observer.cc

Why? If we are extracting the feature from Experimental Web Platform Features I
would think it would be close to it.

https://codereview.chromium.org/1134473003/diff/60001/content/public/common/c...
File content/public/common/content_switches.h (right):

https://codereview.chromium.org/1134473003/diff/60001/content/public/common/c...
content/public/common/content_switches.h:107: CONTENT_EXPORT extern const char
kEnableWebBluetooth[];
On 2015/05/15 at 06:11:33, jochen (traveling) wrote:
> this should be a chrome switch

Why should it be a chrome switch? We are extracting WebBluetooth from the
Experimental Web Platform Features flag. So it made sense to keep
kEnableWebBluetooth in the same place as kEnableExperimentalWebPlatformFeatures.

https://codereview.chromium.org/1134473003/diff/60001/content/shell/app/shell...
File content/shell/app/shell_main_delegate.cc (right):

https://codereview.chromium.org/1134473003/diff/60001/content/shell/app/shell...
content/shell/app/shell_main_delegate.cc:175:
command_line.AppendSwitch(switches::kEnableWebBluetooth);
On 2015/05/15 at 06:11:33, jochen (traveling) wrote:
> layout test related flags should go into the layout_test subdir. however,
since this is an experimental web platform feature, it's enabled for layout
tests anyways

The purpose of this set of patches is to extract the WebBluetooth feature from
Experimental Web Platform Features so we need to enable the flag manually. Which
layout_test subdir?

[jochen (gone - plz use gerrit), 2015-05-19 11:01:40]

On 2015/05/15 at 16:31:14, ortuno wrote:
> @jochen: Thanks! Questions inline. For more context please see:
> 
> https://codereview.chromium.org/1136913002
> https://codereview.chromium.org/1127313004
> 
>
https://codereview.chromium.org/1134473003/diff/60001/content/child/runtime_f...
> File content/child/runtime_features.cc (right):
> 
>
https://codereview.chromium.org/1134473003/diff/60001/content/child/runtime_f...
> content/child/runtime_features.cc:95: if
(command_line.HasSwitch(switches::kEnableWebBluetooth))
> On 2015/05/15 at 06:11:32, jochen (traveling) wrote:
> > this should be in chrome/renderer/chrome_render_process_observer.cc
> 
> Why? If we are extracting the feature from Experimental Web Platform Features
I would think it would be close to it.

content switches should have some effect on content (and not on chrome or
blink).

> 
>
https://codereview.chromium.org/1134473003/diff/60001/content/public/common/c...
> File content/public/common/content_switches.h (right):
> 
>
https://codereview.chromium.org/1134473003/diff/60001/content/public/common/c...
> content/public/common/content_switches.h:107: CONTENT_EXPORT extern const char
kEnableWebBluetooth[];
> On 2015/05/15 at 06:11:33, jochen (traveling) wrote:
> > this should be a chrome switch
> 
> Why should it be a chrome switch? We are extracting WebBluetooth from the
Experimental Web Platform Features flag. So it made sense to keep
kEnableWebBluetooth in the same place as kEnableExperimentalWebPlatformFeatures.
> 
>
https://codereview.chromium.org/1134473003/diff/60001/content/shell/app/shell...
> File content/shell/app/shell_main_delegate.cc (right):
> 
>
https://codereview.chromium.org/1134473003/diff/60001/content/shell/app/shell...
> content/shell/app/shell_main_delegate.cc:175:
command_line.AppendSwitch(switches::kEnableWebBluetooth);
> On 2015/05/15 at 06:11:33, jochen (traveling) wrote:
> > layout test related flags should go into the layout_test subdir. however,
since this is an experimental web platform feature, it's enabled for layout
tests anyways
> 
> The purpose of this set of patches is to extract the WebBluetooth feature from
Experimental Web Platform Features so we need to enable the flag manually. Which
layout_test subdir?

content/shell/*/layout_test/

Looking at the patches, I don't see a reason somehow special case it and move it
out of experimental.

If you want to avoid --enable-experimental-blink-features to turn it on, just
move it to status=test.

[ortuno, 2015-05-19 16:18:03]

On 2015/05/19 at 11:01:40, jochen wrote:
> On 2015/05/15 at 16:31:14, ortuno wrote:
> > @jochen: Thanks! Questions inline. For more context please see:
> > 
> > https://codereview.chromium.org/1136913002
> > https://codereview.chromium.org/1127313004
> > 
> >
https://codereview.chromium.org/1134473003/diff/60001/content/child/runtime_f...
> > File content/child/runtime_features.cc (right):
> > 
> >
https://codereview.chromium.org/1134473003/diff/60001/content/child/runtime_f...
> > content/child/runtime_features.cc:95: if
(command_line.HasSwitch(switches::kEnableWebBluetooth))
> > On 2015/05/15 at 06:11:32, jochen (traveling) wrote:
> > > this should be in chrome/renderer/chrome_render_process_observer.cc
> > 
> > Why? If we are extracting the feature from Experimental Web Platform
Features I would think it would be close to it.
> 
> content switches should have some effect on content (and not on chrome or
blink).

Ok. That makes sense.

> > 
> >
https://codereview.chromium.org/1134473003/diff/60001/content/public/common/c...
> > File content/public/common/content_switches.h (right):
> > 
> >
https://codereview.chromium.org/1134473003/diff/60001/content/public/common/c...
> > content/public/common/content_switches.h:107: CONTENT_EXPORT extern const
char kEnableWebBluetooth[];
> > On 2015/05/15 at 06:11:33, jochen (traveling) wrote:
> > > this should be a chrome switch
> > 
> > Why should it be a chrome switch? We are extracting WebBluetooth from the
Experimental Web Platform Features flag. So it made sense to keep
kEnableWebBluetooth in the same place as kEnableExperimentalWebPlatformFeatures.
> > 
> >
https://codereview.chromium.org/1134473003/diff/60001/content/shell/app/shell...
> > File content/shell/app/shell_main_delegate.cc (right):
> > 
> >
https://codereview.chromium.org/1134473003/diff/60001/content/shell/app/shell...
> > content/shell/app/shell_main_delegate.cc:175:
command_line.AppendSwitch(switches::kEnableWebBluetooth);
> > On 2015/05/15 at 06:11:33, jochen (traveling) wrote:
> > > layout test related flags should go into the layout_test subdir. however,
since this is an experimental web platform feature, it's enabled for layout
tests anyways
> > 
> > The purpose of this set of patches is to extract the WebBluetooth feature
from Experimental Web Platform Features so we need to enable the flag manually.
Which layout_test subdir?
> 
> content/shell/*/layout_test/
> 
> Looking at the patches, I don't see a reason somehow special case it and move
it out of experimental.
> 
> If you want to avoid --enable-experimental-blink-features to turn it on, just
move it to status=test.

You can find the reason in this bug: http://crbug.com/486456

We can't move it to status=test because we want to let developers enable Web
Bluetooth.

[jochen (gone - plz use gerrit), 2015-05-19 16:21:50]

status=test means it's just enabled for layout tests.

you can still add an extra command line flag to turn it on for dev uses

[ortuno, 2015-05-19 16:22:44]

On 2015/05/19 at 16:21:50, jochen wrote:
> status=test means it's just enabled for layout tests.
> 
> you can still add an extra command line flag to turn it on for dev uses

Great! Will do then.

[ortuno, 2015-05-19 19:15:07]

On 2015/05/19 at 11:01:40, jochen wrote:
> On 2015/05/15 at 16:31:14, ortuno wrote:
> > @jochen: Thanks! Questions inline. For more context please see:
> > 
> > https://codereview.chromium.org/1136913002
> > https://codereview.chromium.org/1127313004
> > 
> >
https://codereview.chromium.org/1134473003/diff/60001/content/child/runtime_f...
> > File content/child/runtime_features.cc (right):
> > 
> >
https://codereview.chromium.org/1134473003/diff/60001/content/child/runtime_f...
> > content/child/runtime_features.cc:95: if
(command_line.HasSwitch(switches::kEnableWebBluetooth))
> > On 2015/05/15 at 06:11:32, jochen (traveling) wrote:
> > > this should be in chrome/renderer/chrome_render_process_observer.cc
> > 
> > Why? If we are extracting the feature from Experimental Web Platform
Features I would think it would be close to it.
> 
> content switches should have some effect on content (and not on chrome or
blink).

We can't move it to chrome_render_process_observer because the flag is needed in
render_process_host_impl.cc
 
> > 
> >
https://codereview.chromium.org/1134473003/diff/60001/content/public/common/c...
> > File content/public/common/content_switches.h (right):
> > 
> >
https://codereview.chromium.org/1134473003/diff/60001/content/public/common/c...
> > content/public/common/content_switches.h:107: CONTENT_EXPORT extern const
char kEnableWebBluetooth[];
> > On 2015/05/15 at 06:11:33, jochen (traveling) wrote:
> > > this should be a chrome switch
> > 
> > Why should it be a chrome switch? We are extracting WebBluetooth from the
Experimental Web Platform Features flag. So it made sense to keep
kEnableWebBluetooth in the same place as kEnableExperimentalWebPlatformFeatures.
> > 
> >
https://codereview.chromium.org/1134473003/diff/60001/content/shell/app/shell...
> > File content/shell/app/shell_main_delegate.cc (right):
> > 
> >
https://codereview.chromium.org/1134473003/diff/60001/content/shell/app/shell...
> > content/shell/app/shell_main_delegate.cc:175:
command_line.AppendSwitch(switches::kEnableWebBluetooth);
> > On 2015/05/15 at 06:11:33, jochen (traveling) wrote:
> > > layout test related flags should go into the layout_test subdir. however,
since this is an experimental web platform feature, it's enabled for layout
tests anyways
> > 
> > The purpose of this set of patches is to extract the WebBluetooth feature
from Experimental Web Platform Features so we need to enable the flag manually.
Which layout_test subdir?
> 
> content/shell/*/layout_test/
> 
> Looking at the patches, I don't see a reason somehow special case it and move
it out of experimental.
> 
> If you want to avoid --enable-experimental-blink-features to turn it on, just
move it to status=test.

This doesn't work. The blink side gets enabled but not the browser side i.e.
render_process_host_impl doesn't see the flag.

[jochen (gone - plz use gerrit), 2015-05-21 07:44:57]

lgtm

[ortuno, 2015-05-22 17:08:56]

The CQ bit was checked by ortuno@chromium.org

[ortuno, 2015-05-22 17:08:56]

The patchset sent to the CQ was uploaded after l-g-t-m from scheib@chromium.org,
felt@chromium.org
Link to the patchset: https://codereview.chromium.org/1134473003/#ps60001
(title: "Added comment to explain why we add the flag.")

[commit-bot: I haz the power, 2015-05-22 17:09:30]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1134473003/60001

[commit-bot: I haz the power, 2015-05-22 17:13:43]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-05-22 17:13:45]

Try jobs failed on following builders:
  ios_dbg_simulator_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)
  ios_rel_device_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...)
  mac_chromium_compile_dbg_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[ortuno, 2015-05-22 18:16:18]

ortuno@chromium.org changed reviewers:
+ asvitkine@chromium.org

[ortuno, 2015-05-22 18:16:19]

@asvitkine: OWNERS review for histograms.xml please.

[Alexei Svitkine (slow), 2015-05-22 18:18:46]

lgtm

[ortuno, 2015-05-22 19:39:37]

The CQ bit was checked by ortuno@chromium.org

[ortuno, 2015-05-22 19:39:38]

The patchset sent to the CQ was uploaded after l-g-t-m from jochen@chromium.org,
cpu@chromium.org, felt@chromium.org, scheib@chromium.org
Link to the patchset: https://codereview.chromium.org/1134473003/#ps100001
(title: "Add to flag to histogram")

[commit-bot: I haz the power, 2015-05-22 19:40:03]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1134473003/100001

[commit-bot: I haz the power, 2015-05-22 21:54:27]

Committed patchset #5 (id:100001)

[commit-bot: I haz the power, 2015-05-22 21:57:04]

Patchset 5 (id:??) landed as
https://crrev.com/89be0ad96e7968a977db955441ddab26685b7cc8
Cr-Commit-Position: refs/heads/master@{#331185}