Issue 130918 in chromium: Simplify policy providers

[chro...@googlecode.com]

Status: Started
Owner: joaod...@chromium.org
CC: mnis...@chromium.org
Labels: Type-Bug Pri-2 Feature-Enterprise Policy

New issue 130918 by joaod...@chromium.org: Simplify policy providers
http://code.google.com/p/chromium/issues/detail?id=130918

The platform policy providers are implemented using a rigid class hierarchy
and involve the provider, a loader and a delegate. This code should be
simplified and refactored to suport watching multiple files, and to merge
mandatory and recommended providers.

[chro...@googlecode.com]

Comment #1 on issue 130918 by bugdro...@chromium.org: Simplify policy
providers
http://code.google.com/p/chromium/issues/detail?id=130918#c1

The following revision refers to this bug:
http://src.chromium.org/viewvc/chrome?view=rev&revision=140518

------------------------------------------------------------------------
r140518 | joaod...@chromium.org | Tue Jun 05 05:49:49 PDT 2012

Changed paths:
A
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/policy/async_policy_loader.h?r1=140518&r2=140517&pathrev=140518
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/chrome_browser.gypi?r1=140518&r2=140517&pathrev=140518
A
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/policy/async_policy_loader.cc?r1=140518&r2=140517&pathrev=140518
A
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/policy/async_policy_provider.cc?r1=140518&r2=140517&pathrev=140518
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/chrome_tests.gypi?r1=140518&r2=140517&pathrev=140518
A
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/policy/async_policy_provider_unittest.cc?r1=140518&r2=140517&pathrev=140518
A
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/policy/async_policy_provider.h?r1=140518&r2=140517&pathrev=140518

Add the AsyncPolicyProvider and AsyncPolicyLoader, which provide the same
functionality as the AsychronousPolicyProvider/Loader and
FileBasedPolicyProvider/Loader.

Also has a simpler interface for implementations to override.

BUG=130918
TEST=unit_testsAsyncPolicyProviderTest.*

Review URL: https://chromiumcodereview.appspot.com/10448118
------------------------------------------------------------------------

[chro...@googlecode.com]

Comment #2 on issue 130918 by bugdro...@chromium.org: Simplify policy
providers
http://code.google.com/p/chromium/issues/detail?id=130918#c2

The following revision refers to this bug:

http://src.chromium.org/viewvc/chrome?view=rev&revision=140530

------------------------------------------------------------------------
r140530 | joaod...@chromium.org | Tue Jun 05 08:43:39 PDT 2012

Changed paths:
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/policy/browser_policy_connector.cc?r1=140530&r2=140529&pathrev=140530
A
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/policy/policy_loader_mac_unittest.cc?r1=140530&r2=140529&pathrev=140530
(from
/trunk/src/chrome/browser/policy/configuration_policy_provider_mac_unittest.cc
revision 140529)
A
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/policy/policy_loader_mac.cc?r1=140530&r2=140529&pathrev=140530
(from /trunk/src/chrome/browser/policy/configuration_policy_provider_mac.cc
revision 140529)
A
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/policy/policy_loader_mac.h?r1=140530&r2=140529&pathrev=140530
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/chrome_browser.gypi?r1=140530&r2=140529&pathrev=140530
D
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/policy/configuration_policy_provider_mac.h?r1=140530&r2=140529&pathrev=140530
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/policy/configuration_policy_provider_test.cc?r1=140530&r2=140529&pathrev=140530
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/policy/configuration_policy_provider_test.h?r1=140530&r2=140529&pathrev=140530
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/chrome_tests.gypi?r1=140530&r2=140529&pathrev=140530

Implement the mac policy provider based on the AsyncPolicyLoader.

BUG=130918
TEST=policies still work on mac, unit_tests green

Review URL: https://chromiumcodereview.appspot.com/10496013
------------------------------------------------------------------------

[chro...@googlecode.com]

Comment #3 on issue 130918 by bugdro...@chromium.org: Simplify policy
providers
http://code.google.com/p/chromium/issues/detail?id=130918#c3

The following revision refers to this bug:

http://src.chromium.org/viewvc/chrome?view=rev&revision=140549

------------------------------------------------------------------------
r140549 | joaod...@chromium.org | Tue Jun 05 10:35:45 PDT 2012

Changed paths:
A
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/policy/config_dir_policy_loader_unittest.cc?r1=140549&r2=140548&pathrev=140549
(from
/trunk/src/chrome/browser/policy/config_dir_policy_provider_unittest.cc
revision 140548)
A
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/policy/config_dir_policy_loader.h?r1=140549&r2=140548&pathrev=140549
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/policy/browser_policy_connector.cc?r1=140549&r2=140548&pathrev=140549
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/chrome_browser.gypi?r1=140549&r2=140548&pathrev=140549
A
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/policy/config_dir_policy_loader.cc?r1=140549&r2=140548&pathrev=140549
(from /trunk/src/chrome/browser/policy/config_dir_policy_provider.cc
revision 140548)
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/policy/browser_policy_connector.h?r1=140549&r2=140548&pathrev=140549
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/chrome_tests.gypi?r1=140549&r2=140548&pathrev=140549
D
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/policy/config_dir_policy_provider.h?r1=140549&r2=140548&pathrev=140549

Implement the ConfigDirPolicyProvider based on the AsyncPolicyLoader.

Also makes the ConfigDirPolicyLoader load both mandatory and recommended
policies, and removes the recommended_platform_provider from the connector;
there's only one platform provider now.

BUG=108996,130918
TEST=Nothing breaks

Review URL: https://chromiumcodereview.appspot.com/10443108
------------------------------------------------------------------------

[chro...@googlecode.com]

Comment #4 on issue 130918 by bugdro...@chromium.org: Simplify policy
providers
http://code.google.com/p/chromium/issues/detail?id=130918#c4

The following revision refers to this bug:

http://src.chromium.org/viewvc/chrome?view=rev&revision=140770

------------------------------------------------------------------------
r140770 | joaod...@chromium.org | Wed Jun 06 08:51:50 PDT 2012

Changed paths:
D
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/policy/configuration_policy_loader_win.cc?r1=140770&r2=140769&pathrev=140770
A
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/policy/policy_loader_win_unittest.cc?r1=140770&r2=140769&pathrev=140770
(from
/trunk/src/chrome/browser/policy/configuration_policy_provider_win_unittest.cc
revision 140769)
A
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/policy/policy_loader_win.h?r1=140770&r2=140769&pathrev=140770
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/policy/browser_policy_connector.cc?r1=140770&r2=140769&pathrev=140770
D
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/policy/configuration_policy_provider_win.cc?r1=140770&r2=140769&pathrev=140770
D
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/policy/configuration_policy_provider_win.h?r1=140770&r2=140769&pathrev=140770
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/chrome_browser.gypi?r1=140770&r2=140769&pathrev=140770
A
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/policy/policy_loader_win.cc?r1=140770&r2=140769&pathrev=140770
(from
/trunk/src/chrome/browser/policy/configuration_policy_provider_delegate_win.cc
revision 140769)
D
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/policy/configuration_policy_loader_win.h?r1=140770&r2=140769&pathrev=140770
D
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/policy/configuration_policy_provider_delegate_win.h?r1=140770&r2=140769&pathrev=140770
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/chrome_tests.gypi?r1=140770&r2=140769&pathrev=140770

Implement the windows policy provider based on the AsyncPolicyLoader.

BUG=130918
TEST=windows policy still works, unit_tests green

Review URL: https://chromiumcodereview.appspot.com/10491013
------------------------------------------------------------------------

[chro...@googlecode.com]

Comment #5 on issue 130918 by bugdro...@chromium.org: Simplify policy
providers
http://code.google.com/p/chromium/issues/detail?id=130918#c5

The following revision refers to this bug:

http://src.chromium.org/viewvc/chrome?view=rev&revision=140784

------------------------------------------------------------------------
r140784 | joaod...@chromium.org | Wed Jun 06 10:30:29 PDT 2012

Changed paths:
D
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/policy/asynchronous_policy_provider.cc?r1=140784&r2=140783&pathrev=140784
D
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/policy/file_based_policy_loader.h?r1=140784&r2=140783&pathrev=140784
D
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/policy/asynchronous_policy_provider_unittest.cc?r1=140784&r2=140783&pathrev=140784
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/chrome_browser.gypi?r1=140784&r2=140783&pathrev=140784
D
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/policy/asynchronous_policy_provider.h?r1=140784&r2=140783&pathrev=140784
D
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/policy/file_based_policy_loader.cc?r1=140784&r2=140783&pathrev=140784
D
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/policy/asynchronous_policy_loader_unittest.cc?r1=140784&r2=140783&pathrev=140784
D
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/policy/file_based_policy_provider.cc?r1=140784&r2=140783&pathrev=140784
D
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/policy/asynchronous_policy_loader.h?r1=140784&r2=140783&pathrev=140784
D
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/policy/asynchronous_policy_test_base.cc?r1=140784&r2=140783&pathrev=140784
D
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/policy/asynchronous_policy_test_base.h?r1=140784&r2=140783&pathrev=140784
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/chrome_tests.gypi?r1=140784&r2=140783&pathrev=140784
D
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/policy/file_based_policy_provider_unittest.cc?r1=140784&r2=140783&pathrev=140784
D
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/policy/file_based_policy_provider.h?r1=140784&r2=140783&pathrev=140784
D
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/policy/asynchronous_policy_loader.cc?r1=140784&r2=140783&pathrev=140784

Delete old policy code after AsyncPolicyLoader refactor.

BUG=130918
TEST=nothing breaks

Review URL: https://chromiumcodereview.appspot.com/10500013
------------------------------------------------------------------------

[chro...@googlecode.com]

Comment #6 on issue 130918 by bugdro...@chromium.org: Simplify policy
providers
http://code.google.com/p/chromium/issues/detail?id=130918#c6

The following revision refers to this bug:

http://src.chromium.org/viewvc/chrome?view=rev&revision=141008

------------------------------------------------------------------------
r141008 | joaod...@chromium.org | Thu Jun 07 07:11:01 PDT 2012

Changed paths:
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/policy/policy_loader_win_unittest.cc?r1=141008&r2=141007&pathrev=141008
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/policy/config_dir_policy_loader_unittest.cc?r1=141008&r2=141007&pathrev=141008
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/policy/config_dir_policy_loader.h?r1=141008&r2=141007&pathrev=141008
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/policy/browser_policy_connector.cc?r1=141008&r2=141007&pathrev=141008
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/policy/user_cloud_policy_manager_unittest.cc?r1=141008&r2=141007&pathrev=141008
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/policy/user_cloud_policy_manager.h?r1=141008&r2=141007&pathrev=141008
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/policy/user_cloud_policy_manager.cc?r1=141008&r2=141007&pathrev=141008
D
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/policy/configuration_policy_provider_mac_unittest.cc?r1=141008&r2=141007&pathrev=141008
D
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/policy/config_dir_policy_provider.cc?r1=141008&r2=141007&pathrev=141008
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/policy/async_policy_provider.cc?r1=141008&r2=141007&pathrev=141008
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/policy/configuration_policy_provider.cc?r1=141008&r2=141007&pathrev=141008
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/policy/cloud_policy_provider.cc?r1=141008&r2=141007&pathrev=141008
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/policy/policy_loader_mac_unittest.cc?r1=141008&r2=141007&pathrev=141008
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/policy/cloud_policy_provider_unittest.cc?r1=141008&r2=141007&pathrev=141008
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/policy/proxy_policy_provider.cc?r1=141008&r2=141007&pathrev=141008
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/policy/cloud_policy_provider.h?r1=141008&r2=141007&pathrev=141008
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/policy/mock_configuration_policy_provider.cc?r1=141008&r2=141007&pathrev=141008
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/policy/configuration_policy_provider.h?r1=141008&r2=141007&pathrev=141008
D
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/policy/config_dir_policy_provider_unittest.cc?r1=141008&r2=141007&pathrev=141008
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/policy/async_policy_provider_unittest.cc?r1=141008&r2=141007&pathrev=141008
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/policy/async_policy_provider.h?r1=141008&r2=141007&pathrev=141008
D
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/policy/configuration_policy_provider_mac.cc?r1=141008&r2=141007&pathrev=141008

Removed the PolicyDefinitionList from the ConfigurationPolicyProvider
interface.

Also deleted some files left behind when the linux and mac providers were
refactored.

BUG=130918
TEST=nothing breaks


Review URL: https://chromiumcodereview.appspot.com/10545033
------------------------------------------------------------------------

[chro...@googlecode.com]

Updates:
Status: Fixed

Comment #7 on issue 130918 by joaod...@chromium.org: Simplify policy
providers
http://code.google.com/p/chromium/issues/detail?id=130918

(No comment was entered for this change.)

[chro...@googlecode.com]

Comment #8 on issue 130918 by bugdro...@chromium.org: Simplify policy
providers
http://code.google.com/p/chromium/issues/detail?id=130918#c8

The following revision refers to this bug:

http://src.chromium.org/viewvc/chrome?view=rev&revision=143165

------------------------------------------------------------------------
r143165 | joaod...@chromium.org | Wed Jun 20 06:14:11 PDT 2012

Changed paths:
D
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/policy/configuration_policy_provider_win_unittest.cc?r1=143165&r2=143164&pathrev=143165
D
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/policy/configuration_policy_provider_delegate_win.cc?r1=143165&r2=143164&pathrev=143165

Remove old files that aren't used anymore.

These somehow weren't included in
https://chromiumcodereview.appspot.com/10491013.

TBR=mnis...@chromium.org
BUG=130918
TEST=Nothing breaks


Review URL: https://chromiumcodereview.appspot.com/10578037
------------------------------------------------------------------------