Added boolean policy to enable deactivation of bluetooth for managed devices

Added boolean policy to enable deactivation of bluetooth for managed devices

This is cl #1 of 3 to tackle this issue:
#1 boolean policy
#2 actual changes in the Bluetooth code + policy handler
#3 UI part.

BUG=463578
Committed: https://crrev.com/376c69d2c253569a9404c750cdd2b0a93eebedac
Cr-Commit-Position: refs/heads/master@{#381538}

[Ivan Šandrk, 2016-02-26 17:52:13]

isandrk@chromium.org changed reviewers:
+ bartfab@chromium.org, cschuet@chromium.org

[Ivan Šandrk, 2016-02-26 17:52:13]

cschuet@chromium.org: Please review changes in 

bartfab@chromium.org: Please review changes in 

ptal

[cschuet (SLOW), 2016-02-27 21:02:16]

On 2016/02/26 17:52:13, Ivan Šandrk wrote:
> mailto:cschuet@chromium.org: Please review changes in 
> 
> mailto:bartfab@chromium.org: Please review changes in 
> 
> ptal

In this CL I would try to make all necessary code changes to the introduction of
this new boolean policy only. You are still missing some changes. Take another
look at cl/776093004.

Buildbots are failing because you haven't defined kDisableBluetooth anywhere.

I think you should modify the following files as well
chrome/test/data/policy/policy_test_cases.json
chromeos/settings/cros_settings_names.{h,cc} to give your policy a name by which
you can receive the value from CrosSetting interface
components/policy/resources/policy_templates.json from which we generate our
policy documentation

[bartfab (slow), 2016-03-01 16:48:05]

As Christoph says, you have not actually defined any policy. You just
implemented part of the policy parsing code.

[Ivan Šandrk, 2016-03-04 18:32:36]

Updated it, ptal guys :)

[bartfab (slow), 2016-03-07 11:28:24]

You wired up policy decoding now but the policy still does nothing. Is this
intentional? Do you want to put the actual enforcement in a separate CL?

https://codereview.chromium.org/1736293002/diff/20001/chrome/browser/chromeos...
File chrome/browser/chromeos/policy/device_policy_decoder_chromeos.cc (right):

https://codereview.chromium.org/1736293002/diff/20001/chrome/browser/chromeos...
chrome/browser/chromeos/policy/device_policy_decoder_chromeos.cc:310: NULL);
Nit: s/NULL/nullptr/

https://codereview.chromium.org/1736293002/diff/20001/chrome/browser/chromeos...
File chrome/browser/chromeos/policy/proto/chrome_device_policy.proto (right):

https://codereview.chromium.org/1736293002/diff/20001/chrome/browser/chromeos...
chrome/browser/chromeos/policy/proto/chrome_device_policy.proto:659: // Policy
which disables the bluetooth if the flag is set to false.
Nit: s/disables the bluetooth if the flag is set to false/controls whether
Bluetooth is available/

https://codereview.chromium.org/1736293002/diff/20001/components/policy/resou...
File components/policy/resources/policy_templates.json (right):

https://codereview.chromium.org/1736293002/diff/20001/components/policy/resou...
components/policy/resources/policy_templates.json:8285: 'desc': '''If this
policy is set to false, <ph name="PRODUCT_OS_NAME">$2<ex>Google Chrome
OS</ex></ph> will disable bluetooth (it will also be shown as disabled and
greyed out in the UI).''',
Nit 1: No need to specify that it will be greyed out in the UI. This is implied.
Nit 2: Specify what happens if the policy is set to true.
Nit 3: Specify what happens when the policy is unset (i.e. the default).

[Ivan Šandrk, 2016-03-07 13:07:23]

> You wired up policy decoding now but the policy still does nothing. Is this
intentional? Do you want to put the actual enforcement in a separate CL?

Yes, that was the idea Christoph had in mind. Sorry, maybe I should've specified
that. So the three parts are:
- boolean policy
- actual changes in the Bluetooth code + some way to connect it to previous part
- UI part.

[Ivan Šandrk, 2016-03-07 13:07:44]

https://codereview.chromium.org/1736293002/diff/20001/chrome/browser/chromeos...
File chrome/browser/chromeos/policy/device_policy_decoder_chromeos.cc (right):

https://codereview.chromium.org/1736293002/diff/20001/chrome/browser/chromeos...
chrome/browser/chromeos/policy/device_policy_decoder_chromeos.cc:310: NULL);
On 2016/03/07 11:28:24, bartfab (slow) wrote:
> Nit: s/NULL/nullptr/

Done.

https://codereview.chromium.org/1736293002/diff/20001/chrome/browser/chromeos...
File chrome/browser/chromeos/policy/proto/chrome_device_policy.proto (right):

https://codereview.chromium.org/1736293002/diff/20001/chrome/browser/chromeos...
chrome/browser/chromeos/policy/proto/chrome_device_policy.proto:659: // Policy
which disables the bluetooth if the flag is set to false.
On 2016/03/07 11:28:24, bartfab (slow) wrote:
> Nit: s/disables the bluetooth if the flag is set to false/controls whether
> Bluetooth is available/

Done.

https://codereview.chromium.org/1736293002/diff/20001/components/policy/resou...
File components/policy/resources/policy_templates.json (right):

https://codereview.chromium.org/1736293002/diff/20001/components/policy/resou...
components/policy/resources/policy_templates.json:8285: 'desc': '''If this
policy is set to false, <ph name="PRODUCT_OS_NAME">$2<ex>Google Chrome
OS</ex></ph> will disable bluetooth (it will also be shown as disabled and
greyed out in the UI).''',
On 2016/03/07 11:28:24, bartfab (slow) wrote:
> Nit 1: No need to specify that it will be greyed out in the UI. This is
implied.
> Nit 2: Specify what happens if the policy is set to true.
> Nit 3: Specify what happens when the policy is unset (i.e. the default).

Done.

[bartfab (slow), 2016-03-07 13:48:42]

https://codereview.chromium.org/1736293002/diff/40001/components/policy/resou...
File components/policy/resources/policy_templates.json (right):

https://codereview.chromium.org/1736293002/diff/40001/components/policy/resou...
components/policy/resources/policy_templates.json:8285: 'desc': '''If this
policy is set to false, <ph name="PRODUCT_OS_NAME">$2<ex>Google Chrome
OS</ex></ph> will disable bluetooth. If it's set to true (or is unset/default),
this policy does nothing.''',
Nit 1: s:unset/default:left unset:
Nit 2: I think your description is incorrect. If the policy is left unset,
Bluetooth will be on by default (please specify this) and the user will be able
to turn it on/off. But if the policy is explicitly set to true, the user should
actually be *unable* to turn off Bluetooth. See some other policy descriptions
for common wording we tend to use.

[bartfab (slow), 2016-03-07 13:49:45]

On 2016/03/07 13:07:23, Ivan Šandrk wrote:
> > You wired up policy decoding now but the policy still does nothing. Is this
> intentional? Do you want to put the actual enforcement in a separate CL?
> 
> Yes, that was the idea Christoph had in mind. Sorry, maybe I should've
specified
> that. So the three parts are:
> - boolean policy
> - actual changes in the Bluetooth code + some way to connect it to previous
part
> - UI part.

No problem, but please make this clear in the CL description.

[Ivan Šandrk, 2016-03-07 15:37:48]

https://codereview.chromium.org/1736293002/diff/40001/components/policy/resou...
File components/policy/resources/policy_templates.json (right):

https://codereview.chromium.org/1736293002/diff/40001/components/policy/resou...
components/policy/resources/policy_templates.json:8285: 'desc': '''If this
policy is set to false, <ph name="PRODUCT_OS_NAME">$2<ex>Google Chrome
OS</ex></ph> will disable bluetooth. If it's set to true (or is unset/default),
this policy does nothing.''',
On 2016/03/07 13:48:42, bartfab (slow) wrote:
> Nit 1: s:unset/default:left unset:
> Nit 2: I think your description is incorrect. If the policy is left unset,
> Bluetooth will be on by default (please specify this) and the user will be
able
> to turn it on/off. But if the policy is explicitly set to true, the user
should
> actually be *unable* to turn off Bluetooth. See some other policy descriptions
> for common wording we tend to use.

Done.

[bartfab (slow), 2016-03-10 12:28:43]

https://codereview.chromium.org/1736293002/diff/60001/components/policy/resou...
File components/policy/resources/policy_templates.json (right):

https://codereview.chromium.org/1736293002/diff/60001/components/policy/resou...
components/policy/resources/policy_templates.json:8285: 'desc': '''If this
policy is set to false, <ph name="PRODUCT_OS_NAME">$2<ex>Google Chrome
OS</ex></ph> will disable bluetooth. If it's set to true (or is left unset), it
won't affect Bluetooth (the user can enable or disable it as he wishes).''',
Please align this with the style used to describe other policies, e.g.:

If you set this policy to false, BT will be always be on.

If you set this policy to true, BT will be always be off.

If you leave this policy unset, BT will be on by default.

If you set this policy, the user cannot change or override it.

[bartfab (slow), 2016-03-10 12:28:47]

On 2016/03/07 13:49:45, bartfab (slow) wrote:
> On 2016/03/07 13:07:23, Ivan Šandrk wrote:
> > > You wired up policy decoding now but the policy still does nothing. Is
this
> > intentional? Do you want to put the actual enforcement in a separate CL?
> > 
> > Yes, that was the idea Christoph had in mind. Sorry, maybe I should've
> specified
> > that. So the three parts are:
> > - boolean policy
> > - actual changes in the Bluetooth code + some way to connect it to previous
> part
> > - UI part.
> 
> No problem, but please make this clear in the CL description.

This still stands - please expand the CL description.

[Ivan Šandrk, 2016-03-10 13:31:36]

Description was changed from

==========
Added boolean policy to enable deactivation of bluetooth for managed devices

BUG=463578
==========

to

==========
Added boolean policy to enable deactivation of bluetooth for managed devices

This is cl #1 of 3 to tackle this issue:
#1 boolean policy
#2 actual changes in the Bluetooth code + policy handler
#3 UI part.

BUG=463578
==========

[Ivan Šandrk, 2016-03-10 15:59:27]

https://codereview.chromium.org/1736293002/diff/60001/components/policy/resou...
File components/policy/resources/policy_templates.json (right):

https://codereview.chromium.org/1736293002/diff/60001/components/policy/resou...
components/policy/resources/policy_templates.json:8285: 'desc': '''If this
policy is set to false, <ph name="PRODUCT_OS_NAME">$2<ex>Google Chrome
OS</ex></ph> will disable bluetooth. If it's set to true (or is left unset), it
won't affect Bluetooth (the user can enable or disable it as he wishes).''',
On 2016/03/10 12:28:43, bartfab (slow) wrote:
> Please align this with the style used to describe other policies, e.g.:
> 
> If you set this policy to false, BT will be always be on.
> 
> If you set this policy to true, BT will be always be off.
> 
> If you leave this policy unset, BT will be on by default.
> 
> If you set this policy, the user cannot change or override it.

Done.

[bartfab (slow), 2016-03-10 17:57:34]

lgtm

https://codereview.chromium.org/1736293002/diff/80001/components/policy/resou...
File components/policy/resources/policy_templates.json (right):

https://codereview.chromium.org/1736293002/diff/80001/components/policy/resou...
components/policy/resources/policy_templates.json:8287: If this policy is set to
true, the user will be able to enable or disable Bluetooth as he wishes.
Final nit: It is OK to join these two into "If this policy is set to true or
left unset..."

[Ivan Šandrk, 2016-03-11 13:22:42]

The CQ bit was checked by isandrk@chromium.org

[Ivan Šandrk, 2016-03-11 13:22:43]

The patchset sent to the CQ was uploaded after l-g-t-m from bartfab@chromium.org
Link to the patchset: https://codereview.chromium.org/1736293002/#ps120001
(title: "rebase")

[commit-bot: I haz the power, 2016-03-11 13:22:54]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1736293002/120001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1736293002/120001

[commit-bot: I haz the power, 2016-03-11 13:32:21]

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

[commit-bot: I haz the power, 2016-03-11 13:32:21]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[Ivan Šandrk, 2016-03-11 14:17:59]

isandrk@chromium.org changed reviewers:
+ holte@chromium.org, stevenjb@chromium.org

[Ivan Šandrk, 2016-03-11 14:18:00]

Stevenjb, ptal at chromeos/settings/cros_settings_names.{h,cc}, need LGTM.

Holte, ptal at tools/metrics/histograms/histograms.xml, need LGTM.

https://codereview.chromium.org/1736293002/diff/80001/components/policy/resou...
File components/policy/resources/policy_templates.json (right):

https://codereview.chromium.org/1736293002/diff/80001/components/policy/resou...
components/policy/resources/policy_templates.json:8287: If this policy is set to
true, the user will be able to enable or disable Bluetooth as he wishes.
On 2016/03/10 17:57:34, bartfab (slow) wrote:
> Final nit: It is OK to join these two into "If this policy is set to true or
> left unset..."

Done.

[stevenjb, 2016-03-11 18:46:20]

cros_settings_names lgtm

[Ivan Šandrk, 2016-03-16 13:26:11]

isandrk@chromium.org changed reviewers:
+ isherman@chromium.org
- holte@chromium.org

[Ivan Šandrk, 2016-03-16 13:26:12]

isherman, need LGTM on tools/metrics/histograms/histograms.xml

[Ilya Sherman, 2016-03-16 19:10:56]

histograms.xml lgtm

[Ivan Šandrk, 2016-03-16 19:24:36]

The CQ bit was checked by isandrk@chromium.org

[Ivan Šandrk, 2016-03-16 19:24:37]

The patchset sent to the CQ was uploaded after l-g-t-m from
bartfab@chromium.org, stevenjb@chromium.org, isherman@chromium.org
Link to the patchset: https://codereview.chromium.org/1736293002/#ps140001
(title: "rebase")

[commit-bot: I haz the power, 2016-03-16 19:25:02]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1736293002/140001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1736293002/140001

[commit-bot: I haz the power, 2016-03-16 20:58:28]

Description was changed from

==========
Added boolean policy to enable deactivation of bluetooth for managed devices

This is cl #1 of 3 to tackle this issue:
#1 boolean policy
#2 actual changes in the Bluetooth code + policy handler
#3 UI part.

BUG=463578
==========

to

==========
Added boolean policy to enable deactivation of bluetooth for managed devices

This is cl #1 of 3 to tackle this issue:
#1 boolean policy
#2 actual changes in the Bluetooth code + policy handler
#3 UI part.

BUG=463578
==========

[commit-bot: I haz the power, 2016-03-16 20:58:30]

Committed patchset #8 (id:140001)

[commit-bot: I haz the power, 2016-03-16 21:01:16]

Description was changed from

==========
Added boolean policy to enable deactivation of bluetooth for managed devices

This is cl #1 of 3 to tackle this issue:
#1 boolean policy
#2 actual changes in the Bluetooth code + policy handler
#3 UI part.

BUG=463578
==========

to

==========
Added boolean policy to enable deactivation of bluetooth for managed devices

This is cl #1 of 3 to tackle this issue:
#1 boolean policy
#2 actual changes in the Bluetooth code + policy handler
#3 UI part.

BUG=463578

Committed: https://crrev.com/376c69d2c253569a9404c750cdd2b0a93eebedac
Cr-Commit-Position: refs/heads/master@{#381538}
==========

[commit-bot: I haz the power, 2016-03-16 21:01:17]

Patchset 8 (id:??) landed as
https://crrev.com/376c69d2c253569a9404c750cdd2b0a93eebedac
Cr-Commit-Position: refs/heads/master@{#381538}