Use source parameter to determine policy priority.

Use source parameter too to determine policy priority.

This fixes the problem of Enterprise default policies sometimes
overwriting Active Directory-set policies.

BUG=683150
Review-Url: https://codereview.chromium.org/2641343002
Cr-Commit-Position: refs/heads/master@{#445389}
Committed: https://chromium.googlesource.com/chromium/src/+/877b2113a4375f07ec1cd07a7ab4c417d957a37e

[Marton Hunyady, 2017-01-20 14:53:00]

Description was changed from

==========
Use source parameter to determine policy priority.

This fixes the problem of Enterprise default policies sometimes
overwriting Active Directory-set policies.

BUG=683150
==========

to

==========
Use source parameter too to determine policy priority.

This fixes the problem of Enterprise default policies sometimes
overwriting Active Directory-set policies.

BUG=683150
==========

[Marton Hunyady, 2017-01-20 14:53:53]

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

[Marton Hunyady, 2017-01-20 15:56:48]

The CQ bit was checked by hunyadym@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-20 15:57:02]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-20 16:55:11]

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

[commit-bot: I haz the power, 2017-01-20 16:55:11]

Dry run: This issue passed the CQ dry run.

[Thiemo Nagel, 2017-01-23 13:13:52]

I think it would be a nice idea to add a method-level comment which details the
priority of the properties (i.e. level > scope > source).

https://codereview.chromium.org/2641343002/diff/20001/components/policy/core/...
File components/policy/core/common/policy_map.cc (right):

https://codereview.chromium.org/2641343002/diff/20001/components/policy/core/...
components/policy/core/common/policy_map.cc:41: if (scope == other.scope)
This requires braces.

[Marton Hunyady, 2017-01-23 13:27:32]

The CQ bit was checked by hunyadym@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-23 13:27:41]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Marton Hunyady, 2017-01-23 13:27:50]

Done.

https://codereview.chromium.org/2641343002/diff/20001/components/policy/core/...
File components/policy/core/common/policy_map.cc (right):

https://codereview.chromium.org/2641343002/diff/20001/components/policy/core/...
components/policy/core/common/policy_map.cc:41: if (scope == other.scope)
On 2017/01/23 13:13:52, Thiemo Nagel (slow) wrote:
> This requires braces.

Done.

[Thiemo Nagel, 2017-01-23 13:32:51]

lgtm % nit

https://codereview.chromium.org/2641343002/diff/40001/components/policy/core/...
File components/policy/core/common/policy_map.h (right):

https://codereview.chromium.org/2641343002/diff/40001/components/policy/core/...
components/policy/core/common/policy_map.h:48: // the fields are level > scope >
source.
Nit: |level| > |scope| > |source|

[Marton Hunyady, 2017-01-23 13:36:17]

https://codereview.chromium.org/2641343002/diff/40001/components/policy/core/...
File components/policy/core/common/policy_map.h (right):

https://codereview.chromium.org/2641343002/diff/40001/components/policy/core/...
components/policy/core/common/policy_map.h:48: // the fields are level > scope >
source.
On 2017/01/23 13:32:51, Thiemo Nagel (slow) wrote:
> Nit: |level| > |scope| > |source|

Done.

[Marton Hunyady, 2017-01-23 13:36:38]

The CQ bit was checked by hunyadym@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-23 13:36:52]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-23 13:37:42]

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

[commit-bot: I haz the power, 2017-01-23 13:37:43]

Dry run: Try jobs failed on following builders:
  win_chromium_compile_dbg_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
  win_clang on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds/...)

[Thiemo Nagel, 2017-01-23 14:03:39]

> Done.

still lgtm

[Marton Hunyady, 2017-01-23 14:53:42]

The CQ bit was checked by hunyadym@chromium.org

[commit-bot: I haz the power, 2017-01-23 14:53:55]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-23 14:54:47]

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

[commit-bot: I haz the power, 2017-01-23 14:54:48]

Try jobs failed on following builders:
  win_chromium_compile_dbg_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)
  win_clang on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds/...)

[Marton Hunyady, 2017-01-23 15:15:36]

The CQ bit was checked by hunyadym@chromium.org

[commit-bot: I haz the power, 2017-01-23 15:15:43]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-23 15:16:37]

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

[commit-bot: I haz the power, 2017-01-23 15:16:38]

Try jobs failed on following builders:
  win_chromium_compile_dbg_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)

[Marton Hunyady, 2017-01-23 15:29:42]

The CQ bit was checked by hunyadym@chromium.org

[commit-bot: I haz the power, 2017-01-23 15:29:52]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-23 15:30:42]

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

[commit-bot: I haz the power, 2017-01-23 15:30:43]

Try jobs failed on following builders:
  win_clang on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds/...)

[Marton Hunyady, 2017-01-23 16:47:40]

The CQ bit was checked by hunyadym@chromium.org

[commit-bot: I haz the power, 2017-01-23 16:47:51]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-23 16:52:18]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1485190060338770,
"parent_rev": "c2c9e69eb418629cc7f2ee5a60d913690883145e", "commit_rev":
"877b2113a4375f07ec1cd07a7ab4c417d957a37e"}

[commit-bot: I haz the power, 2017-01-23 16:52:49]

Description was changed from

==========
Use source parameter too to determine policy priority.

This fixes the problem of Enterprise default policies sometimes
overwriting Active Directory-set policies.

BUG=683150
==========

to

==========
Use source parameter too to determine policy priority.

This fixes the problem of Enterprise default policies sometimes
overwriting Active Directory-set policies.

BUG=683150

Review-Url: https://codereview.chromium.org/2641343002
Cr-Commit-Position: refs/heads/master@{#445389}
Committed:
https://chromium.googlesource.com/chromium/src/+/877b2113a4375f07ec1cd07a7ab4...
==========

[commit-bot: I haz the power, 2017-01-23 16:52:51]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/877b2113a4375f07ec1cd07a7ab4...