Enterprise policy to prevent queries to the Quirks Server

Enterprise policy to prevent queries to the Quirks Server

BUG=567791
TEST=Set policy to disable queries, then run OOBE on device that would
otherwise download an icc file (see Issue 549349 for details).  Observe
that no icc file is downloaded.

Committed: https://crrev.com/5b821b5e52442ba95bcbecf788888c6435f5c6a5
Cr-Commit-Position: refs/heads/master@{#384927}

[Thiemo Nagel, 2016-03-08 20:07:58]

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

[Thiemo Nagel, 2016-03-08 20:07:58]

https://codereview.chromium.org/1775023002/diff/1/components/policy/resources...
File components/policy/resources/policy_templates.json (right):

https://codereview.chromium.org/1775023002/diff/1/components/policy/resources...
components/policy/resources/policy_templates.json:8225: 'name':
'QuirksClientEnabled',
This should be a device policy, thus please prefix the name with "Device".  Also
I'd suggest to include "Display" in the name to make it easier to understand. 
Lastly I'd suggest to replace Client with Download (also to make it easier to
understand).

What would you think about DeviceDisplayQuirksDownloadEnabled?

https://codereview.chromium.org/1775023002/diff/1/components/policy/resources...
components/policy/resources/policy_templates.json:8231: 'per_profile': False,
Please drop per_profile since it doesn't apply to device policy.

https://codereview.chromium.org/1775023002/diff/1/components/policy/resources...
components/policy/resources/policy_templates.json:8235:
'default_for_enterprise_users': True,
This is only for cases where the enterprise default differs from the general
default.  Please drop.

https://codereview.chromium.org/1775023002/diff/1/components/policy/resources...
components/policy/resources/policy_templates.json:8236: 'caption': '''Enable
queries to Quirks Server''',
Quirks --> Display Quirks

https://codereview.chromium.org/1775023002/diff/1/components/policy/resources...
components/policy/resources/policy_templates.json:8239: '''When this policy is
set to false, the device will not attempt to
I'd start out with one sentence explaining what display quirks is about and only
discuss the policy after that.

Is there any public documentation that you could refer to?  Contacting a Google
service always tends to raise eyebrows.  Please consider hosting (a copy of) the
design doc under chromium.org.

[Greg Levin, 2016-03-14 17:29:47]

Please have another look!

https://codereview.chromium.org/1775023002/diff/1/components/policy/resources...
File components/policy/resources/policy_templates.json (right):

https://codereview.chromium.org/1775023002/diff/1/components/policy/resources...
components/policy/resources/policy_templates.json:8225: 'name':
'QuirksClientEnabled',
On 2016/03/08 20:07:58, Thiemo Nagel wrote:
> This should be a device policy, thus please prefix the name with "Device". 
Also
> I'd suggest to include "Display" in the name to make it easier to understand. 
> Lastly I'd suggest to replace Client with Download (also to make it easier to
> understand).
> 
> What would you think about DeviceDisplayQuirksDownloadEnabled?

How about DeviceQuirksDownloadEnabled?  As noted in
https://bugs.chromium.org/p/chromium/issues/detail?id=567791#c15
while Quirks is currently only handling display profiles, we expect it could
eventually handle other hardware config files.

https://codereview.chromium.org/1775023002/diff/1/components/policy/resources...
components/policy/resources/policy_templates.json:8230: 'dynamic_refresh': True,
http://www.chromium.org/developers/how-tos/enterprise/adding-new-policies says,
"Wherever possible, we would like to support dynamic policy refresh [...]
This means that you should listen for preference change notifications for your
preference."
My code does not listen for pref change notifications, but does check the pref
right before each download, in effect responding "dynamically" to policy
changes.  Is this sufficient to label this as True?

https://codereview.chromium.org/1775023002/diff/1/components/policy/resources...
components/policy/resources/policy_templates.json:8231: 'per_profile': False,
On 2016/03/08 20:07:58, Thiemo Nagel wrote:
> Please drop per_profile since it doesn't apply to device policy.

Done, but...

https://codereview.chromium.org/1775023002/diff/1/components/policy/resources...
components/policy/resources/policy_templates.json:8235:
'default_for_enterprise_users': True,
On 2016/03/08 20:07:58, Thiemo Nagel wrote:
> This is only for cases where the enterprise default differs from the general
> default.  Please drop.

Done.

https://codereview.chromium.org/1775023002/diff/1/components/policy/resources...
components/policy/resources/policy_templates.json:8236: 'caption': '''Enable
queries to Quirks Server''',
On 2016/03/08 20:07:58, Thiemo Nagel wrote:
> Quirks --> Display Quirks

Avoiding "display", but added a little more info.

https://codereview.chromium.org/1775023002/diff/1/components/policy/resources...
components/policy/resources/policy_templates.json:8239: '''When this policy is
set to false, the device will not attempt to
On 2016/03/08 20:07:58, Thiemo Nagel wrote:
> I'd start out with one sentence explaining what display quirks is about and
only
> discuss the policy after that.

Updated text.

> Is there any public documentation that you could refer to?  Contacting a
Google
> service always tends to raise eyebrows.  Please consider hosting (a copy of)
the
> design doc under http://chromium.org.

I added a Question for this in the design doc (go/cros-quirks-client-dd), which
I'm sort of using as a TODO list.  We'll figure out what we want this to look
like, and I'll link to it from here when it's ready.

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

https://codereview.chromium.org/1775023002/diff/20001/chrome/browser/chromeos...
chrome/browser/chromeos/policy/device_policy_decoder_chromeos.cc:829: if
(container.has_quirks_download_enabled()) {
Is this second "if" statement needed?  Some of the above policy blocks have two
if's, and some have one.  I didn't dig deeply enough into the auto-generated
chrome_device_policy.pb.h file to figure out what's going on in there, or if my
code here is just right.

https://codereview.chromium.org/1775023002/diff/20001/chrome/browser/chromeos...
chrome/browser/chromeos/policy/device_policy_decoder_chromeos.cc:836: NULL);
git cl format complained about this block, requesting that it be formatted more
like the has_login_screen_domain_auto_complete() block above.  But most
policies->Set() calls have one arg per line.  I'm assuming this is preferred? 
Should I reformat the two blocks above to make them consistent?

https://codereview.chromium.org/1775023002/diff/20001/chrome/browser/chromeos...
File chrome/browser/chromeos/settings/device_settings_provider.cc (right):

https://codereview.chromium.org/1775023002/diff/20001/chrome/browser/chromeos...
chrome/browser/chromeos/settings/device_settings_provider.cc:451:
new_values_cache->SetBoolean(
As in device_policy_decoder_chromeos.cc, is the second "if" needed?  Some blocks
put both conditions in "if (X  && Y)", others do "if (X) if (Y)", and some just
have "if (X)".  Which is right in my case?

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

https://codereview.chromium.org/1775023002/diff/20001/components/policy/resou...
components/policy/resources/policy_templates.json:8231: },
... when I removed 'per_profile', the presubmit check complained:

In features DeviceQuirksDownloadEnabled: Error: Features must have a bool
"per_profile".
  Offending: {
  "dynamic_refresh": true
}
Finished checking
/ssd/chrome/src/components/policy/resources/policy_templates.json. 1 errors, 0
warnings.

How can I remove this error without simply bypassing hooks?

[Thiemo Nagel, 2016-03-23 14:53:24]

https://codereview.chromium.org/1775023002/diff/1/components/policy/resources...
File components/policy/resources/policy_templates.json (right):

https://codereview.chromium.org/1775023002/diff/1/components/policy/resources...
components/policy/resources/policy_templates.json:8225: 'name':
'QuirksClientEnabled',
> How about DeviceQuirksDownloadEnabled?

SGTM.

https://codereview.chromium.org/1775023002/diff/1/components/policy/resources...
components/policy/resources/policy_templates.json:8230: 'dynamic_refresh': True,
> My code does not listen for pref change notifications, but does check the pref
> right before each download, in effect responding "dynamically" to policy
> changes.  Is this sufficient to label this as True?

Absolutely.

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

https://codereview.chromium.org/1775023002/diff/20001/chrome/browser/chromeos...
chrome/browser/chromeos/policy/device_policy_decoder_chromeos.cc:829: if
(container.has_quirks_download_enabled()) {
On 2016/03/14 17:29:47, Greg Levin wrote:
> Is this second "if" statement needed?

It all depends on the policy proto that we receive from the server.  In the
spirit of Postel's law, I'd prefer to check the input as carefully as possible
and thus to include the if.

https://codereview.chromium.org/1775023002/diff/20001/chrome/browser/chromeos...
chrome/browser/chromeos/policy/device_policy_decoder_chromeos.cc:836: NULL);
On 2016/03/14 17:29:47, Greg Levin wrote:
> git cl format complained about this block, requesting that it be formatted
more
> like the has_login_screen_domain_auto_complete() block above.  But most
> policies->Set() calls have one arg per line.  I'm assuming this is preferred?

I'd agree.  Consistency trumps everything.

> Should I reformat the two blocks above to make them consistent?

That would be awesome. :)

https://codereview.chromium.org/1775023002/diff/20001/chrome/browser/chromeos...
File chrome/browser/chromeos/settings/device_settings_provider.cc (right):

https://codereview.chromium.org/1775023002/diff/20001/chrome/browser/chromeos...
chrome/browser/chromeos/settings/device_settings_provider.cc:451:
new_values_cache->SetBoolean(
On 2016/03/14 17:29:47, Greg Levin wrote:
> As in device_policy_decoder_chromeos.cc, is the second "if" needed?  Some
blocks
> put both conditions in "if (X  && Y)", others do "if (X) if (Y)", and some
just
> have "if (X)".  Which is right in my case?

Again, I'd say yes, use both conditions.  The "if (X && Y)" formulation seems to
be the most readable to me.

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

https://codereview.chromium.org/1775023002/diff/20001/components/policy/resou...
components/policy/resources/policy_templates.json:8231: },
On 2016/03/14 17:29:47, Greg Levin wrote:
> ... when I removed 'per_profile', the presubmit check complained:
> 
> In features DeviceQuirksDownloadEnabled: Error: Features must have a bool
> "per_profile".
>   Offending: {
>   "dynamic_refresh": true
> }
> Finished checking
> /ssd/chrome/src/components/policy/resources/policy_templates.json. 1 errors, 0
> warnings.
> 
> How can I remove this error without simply bypassing hooks?

I think once you add "'device_only': True" after supported_on... the presubmit
shouldn't complain about missing per_profile anymore.

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

https://codereview.chromium.org/1775023002/diff/40001/chrome/browser/chromeos...
chrome/browser/chromeos/policy/device_policy_decoder_chromeos.cc:886: NULL);
Please use nullptr.

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

https://codereview.chromium.org/1775023002/diff/40001/chrome/browser/chromeos...
chrome/browser/chromeos/policy/proto/chrome_device_policy.proto:688: optional
bool quirks_download_enabled = 1 [default = true];
I wouldn't specify a default because it is meaningless (we don't use it for
anything).

https://codereview.chromium.org/1775023002/diff/40001/chromeos/settings/cros_...
File chromeos/settings/cros_settings_names.cc (right):

https://codereview.chromium.org/1775023002/diff/40001/chromeos/settings/cros_...
chromeos/settings/cros_settings_names.cc:194: // A boolean pref to enable any
pings or requests to the Quirks Server.
"any pings or requests" seems a bit broad.  Are there any plans that transcend
download requests?

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

https://codereview.chromium.org/1775023002/diff/40001/components/policy/resou...
components/policy/resources/policy_templates.json:8356: icc display profiles to
adjust monitor calibration.
icc --> ICC

https://codereview.chromium.org/1775023002/diff/40001/components/policy/resou...
components/policy/resources/policy_templates.json:8361: If this policy is true
or not configured then enterprise-enrolled devices
I'd suggest to "enterprise-enrolled devices" --> "<ph
name="PRODUCT_OS_NAME">$2<ex>Google Chrome OS</ex></ph>" because as it stands
right now, it seems to imply that the default on enrolled devices works
differently than on non-enrolled devices, which is not correct (at least I hope
that it isn't).

[Greg Levin, 2016-03-24 23:21:38]

glevin@chromium.org changed reviewers:
+ holte@chromium.org, oshima@chromium.org

[Greg Levin, 2016-03-24 23:21:40]

+oshima@ :
  chrome/browser/chromeos/display/*
  chrome/browser/chromeos/settings/device_settings_provider.cc
  chromeos/settings/*
  components/quirks/*

+holte@ :
  histograms.xml

Please have a look!

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

https://codereview.chromium.org/1775023002/diff/20001/chrome/browser/chromeos...
chrome/browser/chromeos/policy/device_policy_decoder_chromeos.cc:829: if
(container.has_quirks_download_enabled()) {
On 2016/03/23 14:53:24, Thiemo Nagel wrote:
> On 2016/03/14 17:29:47, Greg Levin wrote:
> > Is this second "if" statement needed?
> 
> It all depends on the policy proto that we receive from the server.  In the
> spirit of Postel's law, I'd prefer to check the input as carefully as possible
> and thus to include the if.

Acknowledged.

https://codereview.chromium.org/1775023002/diff/20001/chrome/browser/chromeos...
chrome/browser/chromeos/policy/device_policy_decoder_chromeos.cc:836: NULL);
On 2016/03/23 14:53:24, Thiemo Nagel wrote:
> On 2016/03/14 17:29:47, Greg Levin wrote:
> > git cl format complained about this block, requesting that it be formatted
> more
> > like the has_login_screen_domain_auto_complete() block above.  But most
> > policies->Set() calls have one arg per line.  I'm assuming this is
preferred?
> 
> I'd agree.  Consistency trumps everything.
> 
> > Should I reformat the two blocks above to make them consistent?
> 
> That would be awesome. :)

Done.

https://codereview.chromium.org/1775023002/diff/20001/chrome/browser/chromeos...
File chrome/browser/chromeos/settings/device_settings_provider.cc (right):

https://codereview.chromium.org/1775023002/diff/20001/chrome/browser/chromeos...
chrome/browser/chromeos/settings/device_settings_provider.cc:451:
new_values_cache->SetBoolean(
On 2016/03/23 14:53:24, Thiemo Nagel wrote:
> On 2016/03/14 17:29:47, Greg Levin wrote:
> > As in device_policy_decoder_chromeos.cc, is the second "if" needed?  Some
> blocks
> > put both conditions in "if (X  && Y)", others do "if (X) if (Y)", and some
> just
> > have "if (X)".  Which is right in my case?
> 
> Again, I'd say yes, use both conditions.  The "if (X && Y)" formulation seems
to
> be the most readable to me.

Done.

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

https://codereview.chromium.org/1775023002/diff/20001/components/policy/resou...
components/policy/resources/policy_templates.json:8231: },
On 2016/03/23 14:53:24, Thiemo Nagel wrote:
> On 2016/03/14 17:29:47, Greg Levin wrote:
> > ... when I removed 'per_profile', the presubmit check complained:
> > 
> > In features DeviceQuirksDownloadEnabled: Error: Features must have a bool
> > "per_profile".
> >   Offending: {
> >   "dynamic_refresh": true
> > }
> > Finished checking
> > /ssd/chrome/src/components/policy/resources/policy_templates.json. 1 errors,
0
> > warnings.
> > 
> > How can I remove this error without simply bypassing hooks?
> 
> I think once you add "'device_only': True" after supported_on... the presubmit
> shouldn't complain about missing per_profile anymore.

Done.

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

https://codereview.chromium.org/1775023002/diff/40001/chrome/browser/chromeos...
chrome/browser/chromeos/policy/device_policy_decoder_chromeos.cc:886: NULL);
On 2016/03/23 14:53:24, Thiemo Nagel wrote:
> Please use nullptr.

Done.

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

https://codereview.chromium.org/1775023002/diff/40001/chrome/browser/chromeos...
chrome/browser/chromeos/policy/proto/chrome_device_policy.proto:688: optional
bool quirks_download_enabled = 1 [default = true];
On 2016/03/23 14:53:24, Thiemo Nagel wrote:
> I wouldn't specify a default because it is meaningless (we don't use it for
> anything).

Done.

https://codereview.chromium.org/1775023002/diff/40001/chromeos/settings/cros_...
File chromeos/settings/cros_settings_names.cc (right):

https://codereview.chromium.org/1775023002/diff/40001/chromeos/settings/cros_...
chromeos/settings/cros_settings_names.cc:194: // A boolean pref to enable any
pings or requests to the Quirks Server.
On 2016/03/23 14:53:24, Thiemo Nagel wrote:
> "any pings or requests" seems a bit broad.  Are there any plans that transcend
> download requests?

Potentially.  At some point we might query the server for "last modified" dates
for files.  See 5/5a in go/cros-quirks-client-dd.

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

https://codereview.chromium.org/1775023002/diff/40001/components/policy/resou...
components/policy/resources/policy_templates.json:8356: icc display profiles to
adjust monitor calibration.
On 2016/03/23 14:53:24, Thiemo Nagel wrote:
> icc --> ICC

Done.

https://codereview.chromium.org/1775023002/diff/40001/components/policy/resou...
components/policy/resources/policy_templates.json:8361: If this policy is true
or not configured then enterprise-enrolled devices
On 2016/03/23 14:53:24, Thiemo Nagel wrote:
> I'd suggest to "enterprise-enrolled devices" --> "<ph
> name="PRODUCT_OS_NAME">$2<ex>Google Chrome OS</ex></ph>" because as it stands
> right now, it seems to imply that the default on enrolled devices works
> differently than on non-enrolled devices, which is not correct (at least I
hope
> that it isn't).

Done.

[Steven Holte, 2016-03-25 02:00:01]

histograms.xml lgtm

[Greg Levin, 2016-03-25 18:06:57]

Note: I copied policy browser test code from

https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/chr...

and Quirks browser test code from

https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/chr...

https://codereview.chromium.org/1775023002/diff/80001/chrome/browser/chromeos...
File chrome/browser/chromeos/display/quirks_browsertest.cc (right):

https://codereview.chromium.org/1775023002/diff/80001/chrome/browser/chromeos...
chrome/browser/chromeos/display/quirks_browsertest.cc:103: }
Just fixing some dumb code.  Not related to Quirks policy.

https://codereview.chromium.org/1775023002/diff/80001/chrome/browser/chromeos...
File chrome/browser/chromeos/policy/device_quirks_policy_browsertest.cc (right):

https://codereview.chromium.org/1775023002/diff/80001/chrome/browser/chromeos...
chrome/browser/chromeos/policy/device_quirks_policy_browsertest.cc:32:
InstallOwnerKey();
device_system_use_24hour_clock_browsertest.cc also had
    MarkAsEnterpriseOwned();
My tests run fine without this line, so I assume it's not needed here?

[oshima, 2016-03-31 20:34:37]

lgtm with nits & q

https://codereview.chromium.org/1775023002/diff/80001/chrome/browser/chromeos...
File chrome/browser/chromeos/policy/device_quirks_policy_browsertest.cc (right):

https://codereview.chromium.org/1775023002/diff/80001/chrome/browser/chromeos...
chrome/browser/chromeos/policy/device_quirks_policy_browsertest.cc:46:
ASSERT_TRUE(created);
nit: << error;

https://codereview.chromium.org/1775023002/diff/80001/chrome/browser/chromeos...
chrome/browser/chromeos/policy/device_quirks_policy_browsertest.cc:52:
ASSERT_TRUE(bytes_written == sizeof(kFakeIccData));
ASSERT_EQ

https://codereview.chromium.org/1775023002/diff/80001/chrome/browser/chromeos...
chrome/browser/chromeos/policy/device_quirks_policy_browsertest.cc:63:
base::MessageLoop::current()->Run();
What's the logic to wait for the settings to be updated?

[Andrew T Wilson (Slow), 2016-04-01 13:58:14]

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

[Andrew T Wilson (Slow), 2016-04-01 13:58:15]

lgtm

https://codereview.chromium.org/1775023002/diff/80001/chrome/browser/chromeos...
File chrome/browser/chromeos/settings/device_settings_provider.cc (right):

https://codereview.chromium.org/1775023002/diff/80001/chrome/browser/chromeos...
chrome/browser/chromeos/settings/device_settings_provider.cc:59:
kAllowBluetooth,
thanks for moving this to the right place :)

[Greg Levin, 2016-04-01 18:34:00]

https://codereview.chromium.org/1775023002/diff/80001/chrome/browser/chromeos...
File chrome/browser/chromeos/policy/device_quirks_policy_browsertest.cc (right):

https://codereview.chromium.org/1775023002/diff/80001/chrome/browser/chromeos...
chrome/browser/chromeos/policy/device_quirks_policy_browsertest.cc:46:
ASSERT_TRUE(created);
On 2016/03/31 20:34:37, oshima wrote:
> nit: << error;

Done.

https://codereview.chromium.org/1775023002/diff/80001/chrome/browser/chromeos...
chrome/browser/chromeos/policy/device_quirks_policy_browsertest.cc:52:
ASSERT_TRUE(bytes_written == sizeof(kFakeIccData));
On 2016/03/31 20:34:37, oshima wrote:
> ASSERT_EQ

Done.

https://codereview.chromium.org/1775023002/diff/80001/chrome/browser/chromeos...
chrome/browser/chromeos/policy/device_quirks_policy_browsertest.cc:63:
base::MessageLoop::current()->Run();
On 2016/03/31 20:34:37, oshima wrote:
> What's the logic to wait for the settings to be updated?

I'm not sure I understand the question.  This code is copied from
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/chr...

Without this line, the code in RefreshDevicePolicy() doesn't complete, and the
new policy settings don't take effect, before the tests are run.

[Greg Levin, 2016-04-01 19:38:32]

The CQ bit was checked by glevin@chromium.org

[Greg Levin, 2016-04-01 19:38:33]

The patchset sent to the CQ was uploaded after l-g-t-m from holte@chromium.org,
oshima@chromium.org, atwilson@chromium.org
Link to the patchset: https://codereview.chromium.org/1775023002/#ps100001
(title: "Logic fix, small review fixes, merge")

[commit-bot: I haz the power, 2016-04-01 19:39:07]

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

[commit-bot: I haz the power, 2016-04-01 20:03:38]

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

[commit-bot: I haz the power, 2016-04-01 20:03:39]

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

[Greg Levin, 2016-04-01 20:27:31]

The CQ bit was checked by glevin@chromium.org

[Greg Levin, 2016-04-01 20:27:31]

The patchset sent to the CQ was uploaded after l-g-t-m from oshima@chromium.org,
atwilson@chromium.org, holte@chromium.org
Link to the patchset: https://codereview.chromium.org/1775023002/#ps120001
(title: "Fix display unit test")

[commit-bot: I haz the power, 2016-04-01 20:28:01]

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

[commit-bot: I haz the power, 2016-04-01 23:07:52]

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

[commit-bot: I haz the power, 2016-04-01 23:07:53]

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

[Andrew T Wilson (Slow), 2016-04-04 08:23:34]

https://codereview.chromium.org/1775023002/diff/120001/components/policy/reso...
File components/policy/resources/policy_templates.json (right):

https://codereview.chromium.org/1775023002/diff/120001/components/policy/reso...
components/policy/resources/policy_templates.json:8398: If this policy is true
or not configured then
I don't think this statement is true - when you set
"default_for_enterprise_users = false" this means that when the policy is not
configured it should act as if the policy is set to false, not true.

[Greg Levin, 2016-04-04 15:27:18]

The CQ bit was checked by glevin@chromium.org

[Greg Levin, 2016-04-04 15:27:18]

The patchset sent to the CQ was uploaded after l-g-t-m from oshima@chromium.org,
atwilson@chromium.org, holte@chromium.org
Link to the patchset: https://codereview.chromium.org/1775023002/#ps140001
(title: "Remove default_for_enterprise_users")

[commit-bot: I haz the power, 2016-04-04 15:27:38]

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

[commit-bot: I haz the power, 2016-04-04 16:42:57]

Committed patchset #8 (id:140001)

[commit-bot: I haz the power, 2016-04-04 16:44:00]

Description was changed from

==========
Enterprise policy to prevent queries to the Quirks Server

BUG=567791
TEST=Set policy to disable queries, then run OOBE on device that would
otherwise download an icc file (see Issue 549349 for details).  Observe
that no icc file is downloaded.
==========

to

==========
Enterprise policy to prevent queries to the Quirks Server

BUG=567791
TEST=Set policy to disable queries, then run OOBE on device that would
otherwise download an icc file (see Issue 549349 for details).  Observe
that no icc file is downloaded.

Committed: https://crrev.com/5b821b5e52442ba95bcbecf788888c6435f5c6a5
Cr-Commit-Position: refs/heads/master@{#384927}
==========

[commit-bot: I haz the power, 2016-04-04 16:44:01]

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

[Greg Levin, 2016-04-04 16:46:32]

https://codereview.chromium.org/1775023002/diff/120001/components/policy/reso...
File components/policy/resources/policy_templates.json (right):

https://codereview.chromium.org/1775023002/diff/120001/components/policy/reso...
components/policy/resources/policy_templates.json:8398: If this policy is true
or not configured then
On 2016/04/04 08:23:33, Andrew T Wilson (Slow) wrote:
> I don't think this statement is true - when you set
> "default_for_enterprise_users = false" this means that when the policy is not
> configured it should act as if the policy is set to false, not true.

That did the trick, thanks!!  The line was removed awhile ago, and seems to have
reappeared unnoticed due to a merge error.  Removing it fixed the tests.