ChromeOS: Add p2p_enabled field to AutoUpdateSettingsProto

ChromeOS: Add p2p_enabled field to AutoUpdateSettingsProto

BUG=chromium:260442
TEST=None

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

[zeuthen, 2013-09-10 00:07:11]

Hey guys, please review this CL. sosa@ for functional review and mnissler@ for
OWNERS. Thanks in advance!

[sosa, 2013-09-10 00:31:10]

lgtm

[Mattias Nissler (ping if slow), 2013-09-10 14:07:46]

There are some more steps required to get the policy declared properly for it to
show up in chrome://policy. In particular, you need to declare the policy in
chrome/app/policy/policy_templates.json and add some decoding logic in
chrome/browser/chromeos/policy/device_policy_decoder_chromeos.cc.

This recent CL is a good starting point for adding a device policy (check the
changes to the two files I mentioned above):
https://chromiumcodereview.appspot.com/18348016

And for completeness, here's more background on adding policies (but mostly
targeted at policy enforced in the browser):
http://dev.chromium.org/developers/how-tos/enterprise/adding-new-policies

[zeuthen, 2013-09-10 17:47:20]

Thanks for the review - the reason my CL is this short is that I based it on
Jay's http_downloads_enabled CL here:

 https://chromiumcodereview.appspot.com/14715019

which doesn't have the additional bells + whistles you asked for. I'll go ahead
and implement the requested changes - thanks again for the review!

Just a quick question - why is the http_downloads_enabled CL so much shorter?
And if so, is this something we need to fix now? Thanks!

[zeuthen, 2013-09-10 23:00:09]

Hi, here's an updated patch. Hope I got everything right. Thanks in advance!

[Mattias Nissler (ping if slow), 2013-09-11 08:56:30]

LGTM

The fact that I haven't asked for the device policy decoding code on the earlier
http_downloads_enabled CL is an oversight on my side. I'll send a CL to fix
that.

[commit-bot: I haz the power, 2013-09-11 16:29:24]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/zeuthen@chromium.org/23672033/7001

[zeuthen, 2013-09-11 16:29:27]

Cool - thanks for the review! David

[commit-bot: I haz the power, 2013-09-11 16:38:39]

Retried try job too often on chromium_presubmit for step(s) presubmit
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=chromium_p...

[zeuthen, 2013-09-11 18:11:45]

As per chrome/app/policy/OWNERS it looks like I need LGTM from one of
dconnelly@chromium.org, joaodasilva@chromium.org. Thanks in advance for
considering this CL.

[Joao da Silva, 2013-09-11 18:47:59]

lgtm after adding the missing documentation. Thanks!

https://codereview.chromium.org/23672033/diff/7001/chrome/app/policy/policy_t...
File chrome/app/policy/policy_templates.json (right):

https://codereview.chromium.org/23672033/diff/7001/chrome/app/policy/policy_t...
chrome/app/policy/policy_templates.json:3681: 'desc': '''Specifies whether p2p
is to be used for OS update payloads. If set to True, devices will share and
consume update payloads on the LAN, potentially reducing Internet bandwidth
usage and congestion.''',
Please document the default behavior, and the behavior when this is set to
False. I assume that both disallow p2p updates.

[zeuthen, 2013-09-11 19:07:20]

Thanks for the review. New CL coming up.

https://codereview.chromium.org/23672033/diff/7001/chrome/app/policy/policy_t...
File chrome/app/policy/policy_templates.json (right):

https://codereview.chromium.org/23672033/diff/7001/chrome/app/policy/policy_t...
chrome/app/policy/policy_templates.json:3681: 'desc': '''Specifies whether p2p
is to be used for OS update payloads. If set to True, devices will share and
consume update payloads on the LAN, potentially reducing Internet bandwidth
usage and congestion.''',
On 2013/09/11 18:48:00, Joao da Silva wrote:
> Please document the default behavior, and the behavior when this is set to
> False. I assume that both disallow p2p updates.

Correct. I'm also adding "If the update payload is not available on the LAN, the
device will fall back to downloading from an update server." to be more precise.

[commit-bot: I haz the power, 2013-09-11 19:09:13]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/zeuthen@chromium.org/23672033/22001

[commit-bot: I haz the power, 2013-09-11 21:12:53]

Retried try job too often on win_rel for step(s) unit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[commit-bot: I haz the power, 2013-09-11 21:25:13]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/zeuthen@chromium.org/23672033/22001

[commit-bot: I haz the power, 2013-09-12 01:51:43]

Change committed as 222697