Support cloud policy for extensions on the desktop platforms.

Support cloud policy for extensions on the desktop platforms.

The ComponentCloudPolicyService manages the storage and updates of cloud policy
for chrome components, such as extensions. This component is now managed by
the CloudPolicyManager, so that the UserCloudPolicyManagers can share its
lifetime logic.

Later it may be used by the DeviceCloudPolicyManager and the local accounts
policy manager.

This feature is still disabled by default behind a flag.

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

[Joao da Silva, 2013-11-20 18:29:49]

Wrapping up weeks of work, we're finally able to enable this stuff on the
desktop :-) I'm also tentatively enabling it by default.

@Bartosz: please review, since you have all the mental context by now :-)

@Mattias: FYI

[Joao da Silva, 2013-11-25 09:23:46]

PTAL

@Bartosz: ping

@Jochen: gypi and profile changes

Thanks!

[bartfab (slow), 2013-11-25 15:05:49]

lgtm

https://codereview.chromium.org/79023002/diff/560006/chrome/browser/chromeos/...
File chrome/browser/chromeos/policy/user_cloud_policy_manager_chromeos.cc
(right):

https://codereview.chromium.org/79023002/diff/560006/chrome/browser/chromeos/...
chrome/browser/chromeos/policy/user_cloud_policy_manager_chromeos.cc:19:
#include "chrome/browser/policy/cloud/resource_cache.h"
Nit: No longeer used.

https://codereview.chromium.org/79023002/diff/560006/chrome/browser/chromeos/...
chrome/browser/chromeos/policy/user_cloud_policy_manager_chromeos.cc:20:
#include "components/policy/core/common/policy_bundle.h"
Nit: No longer used.

https://codereview.chromium.org/79023002/diff/560006/chrome/browser/chromeos/...
File chrome/browser/chromeos/policy/user_cloud_policy_manager_chromeos.h
(right):

https://codereview.chromium.org/79023002/diff/560006/chrome/browser/chromeos/...
chrome/browser/chromeos/policy/user_cloud_policy_manager_chromeos.h:53: // Note:
currently these must be the FILE and the IO BrowserThreads.
Nit: "these" is ambiguous (there are three task runners, which one(s) must be
FILE, which one(s) must be IO?)

https://codereview.chromium.org/79023002/diff/560006/chrome/browser/policy/cl...
File chrome/browser/policy/cloud/cloud_policy_manager.cc (right):

https://codereview.chromium.org/79023002/diff/560006/chrome/browser/policy/cl...
chrome/browser/policy/cloud/cloud_policy_manager.cc:95: scoped_ptr<PolicyBundle>
bundle(new PolicyBundle);
Nit: #include "base/memory/scoped_ptr.h"

https://codereview.chromium.org/79023002/diff/560006/chrome/browser/policy/cl...
File chrome/browser/policy/cloud/cloud_policy_manager.h (right):

https://codereview.chromium.org/79023002/diff/560006/chrome/browser/policy/cl...
chrome/browser/policy/cloud/cloud_policy_manager.h:31: class PolicyBundle;
Nit: No longer used.

https://codereview.chromium.org/79023002/diff/560006/chrome/browser/policy/cl...
chrome/browser/policy/cloud/cloud_policy_manager.h:47: // Note: currently these
must be the FILE and the IO BrowserThreads.
Nit: "these" is ambiguous (there are three task runners, which one(s) must be
FILE, which one(s) must be IO?)

https://codereview.chromium.org/79023002/diff/560006/chrome/browser/policy/cl...
File chrome/browser/policy/cloud/component_cloud_policy_service.h (right):

https://codereview.chromium.org/79023002/diff/560006/chrome/browser/policy/cl...
chrome/browser/policy/cloud/component_cloud_policy_service.h:115: #if
!defined(OS_ANDROID) && !defined(OS_IOS)
Nit: We do not tend to have blank lines around #ifs.

https://codereview.chromium.org/79023002/diff/560006/chrome/browser/policy/cl...
chrome/browser/policy/cloud/component_cloud_policy_service.h:150: #endif  //
!defined(OS_ANDROID) && !defined(OS_IOS)
Nit: We do not tend to have blank lines before #endifs.

https://codereview.chromium.org/79023002/diff/560006/chrome/browser/policy/cl...
File chrome/browser/policy/cloud/user_cloud_policy_manager.h (right):

https://codereview.chromium.org/79023002/diff/560006/chrome/browser/policy/cl...
chrome/browser/policy/cloud/user_cloud_policy_manager.h:46: // Note: currently
these must be the FILE and the IO BrowserThreads.
Nit: "these" is ambiguous (there are three task runners, which one(s) must be
FILE, which one(s) must be IO?)

https://codereview.chromium.org/79023002/diff/560006/chrome/browser/policy/cl...
File chrome/browser/policy/cloud/user_cloud_policy_manager_factory.cc (right):

https://codereview.chromium.org/79023002/diff/560006/chrome/browser/policy/cl...
chrome/browser/policy/cloud/user_cloud_policy_manager_factory.cc:23: //
Directory inside the profile directory where other resources are stored.
Nit: "Other resources" is extremely generic.

https://codereview.chromium.org/79023002/diff/560006/chrome/browser/policy/cl...
File chrome/browser/policy/cloud/user_cloud_policy_manager_factory.h (right):

https://codereview.chromium.org/79023002/diff/560006/chrome/browser/policy/cl...
chrome/browser/policy/cloud/user_cloud_policy_manager_factory.h:61: // Note:
currently these must be the FILE and the IO BrowserThreads.
Nit: "these" is ambiguous (there are three task runners, which one(s) must be
FILE, which one(s) must be IO?)

https://codereview.chromium.org/79023002/diff/560006/chrome/browser/policy/cl...
File chrome/browser/policy/cloud/user_cloud_policy_manager_unittest.cc (right):

https://codereview.chromium.org/79023002/diff/560006/chrome/browser/policy/cl...
chrome/browser/policy/cloud/user_cloud_policy_manager_unittest.cc:8: #include
"base/files/file_path.h"
Nit: Already included by header file.

https://codereview.chromium.org/79023002/diff/560006/chrome/browser/policy/cl...
File chrome/browser/policy/cloud/user_policy_signin_service_unittest.cc (right):

https://codereview.chromium.org/79023002/diff/560006/chrome/browser/policy/cl...
chrome/browser/policy/cloud/user_policy_signin_service_unittest.cc:167:
base::FilePath(),
Nit: #include "base/files/file_path.h"

https://codereview.chromium.org/79023002/diff/560006/components/policy/core/c...
File components/policy/core/common/policy_switches.cc (right):

https://codereview.chromium.org/79023002/diff/560006/components/policy/core/c...
components/policy/core/common/policy_switches.cc:20: const char
kDisableComponentCloudPolicy[]    = "enable-component-cloud-policy";
s/enable/disable/

[jochen (gone - plz use gerrit), 2013-11-25 15:51:21]

can you please split up the CL such that enabling this is a very small CL that
can easily be reverted?

Also, the associated bug doesn't have a milestone. Please set the target
milestone to bring the bug on the radar of the resp. stakeholders.

[Joao da Silva, 2013-11-25 16:44:11]

Addressed Bartosz' comments; also updated the bug (and its privacy review bug)
with a milestone and more details, and left the feature disabled by default.

@Jochen: thanks for the suggestions. PTAL

https://codereview.chromium.org/79023002/diff/560006/chrome/browser/chromeos/...
File chrome/browser/chromeos/policy/user_cloud_policy_manager_chromeos.cc
(right):

https://codereview.chromium.org/79023002/diff/560006/chrome/browser/chromeos/...
chrome/browser/chromeos/policy/user_cloud_policy_manager_chromeos.cc:19:
#include "chrome/browser/policy/cloud/resource_cache.h"
On 2013/11/25 15:05:50, bartfab wrote:
> Nit: No longeer used.

Done.

https://codereview.chromium.org/79023002/diff/560006/chrome/browser/chromeos/...
chrome/browser/chromeos/policy/user_cloud_policy_manager_chromeos.cc:20:
#include "components/policy/core/common/policy_bundle.h"
On 2013/11/25 15:05:50, bartfab wrote:
> Nit: No longer used.

Done.

https://codereview.chromium.org/79023002/diff/560006/chrome/browser/chromeos/...
File chrome/browser/chromeos/policy/user_cloud_policy_manager_chromeos.h
(right):

https://codereview.chromium.org/79023002/diff/560006/chrome/browser/chromeos/...
chrome/browser/chromeos/policy/user_cloud_policy_manager_chromeos.h:53: // Note:
currently these must be the FILE and the IO BrowserThreads.
On 2013/11/25 15:05:50, bartfab wrote:
> Nit: "these" is ambiguous (there are three task runners, which one(s) must be
> FILE, which one(s) must be IO?)

Done.

https://codereview.chromium.org/79023002/diff/560006/chrome/browser/policy/cl...
File chrome/browser/policy/cloud/cloud_policy_manager.cc (right):

https://codereview.chromium.org/79023002/diff/560006/chrome/browser/policy/cl...
chrome/browser/policy/cloud/cloud_policy_manager.cc:95: scoped_ptr<PolicyBundle>
bundle(new PolicyBundle);
On 2013/11/25 15:05:50, bartfab wrote:
> Nit: #include "base/memory/scoped_ptr.h"

Done.

https://codereview.chromium.org/79023002/diff/560006/chrome/browser/policy/cl...
File chrome/browser/policy/cloud/cloud_policy_manager.h (right):

https://codereview.chromium.org/79023002/diff/560006/chrome/browser/policy/cl...
chrome/browser/policy/cloud/cloud_policy_manager.h:31: class PolicyBundle;
On 2013/11/25 15:05:50, bartfab wrote:
> Nit: No longer used.

Done.

https://codereview.chromium.org/79023002/diff/560006/chrome/browser/policy/cl...
chrome/browser/policy/cloud/cloud_policy_manager.h:47: // Note: currently these
must be the FILE and the IO BrowserThreads.
On 2013/11/25 15:05:50, bartfab wrote:
> Nit: "these" is ambiguous (there are three task runners, which one(s) must be
> FILE, which one(s) must be IO?)

Done.

https://codereview.chromium.org/79023002/diff/560006/chrome/browser/policy/cl...
File chrome/browser/policy/cloud/component_cloud_policy_service.h (right):

https://codereview.chromium.org/79023002/diff/560006/chrome/browser/policy/cl...
chrome/browser/policy/cloud/component_cloud_policy_service.h:115: #if
!defined(OS_ANDROID) && !defined(OS_IOS)
On 2013/11/25 15:05:50, bartfab wrote:
> Nit: We do not tend to have blank lines around #ifs.

Done.

https://codereview.chromium.org/79023002/diff/560006/chrome/browser/policy/cl...
chrome/browser/policy/cloud/component_cloud_policy_service.h:150: #endif  //
!defined(OS_ANDROID) && !defined(OS_IOS)
On 2013/11/25 15:05:50, bartfab wrote:
> Nit: We do not tend to have blank lines before #endifs.

Done.

https://codereview.chromium.org/79023002/diff/560006/chrome/browser/policy/cl...
File chrome/browser/policy/cloud/user_cloud_policy_manager.h (right):

https://codereview.chromium.org/79023002/diff/560006/chrome/browser/policy/cl...
chrome/browser/policy/cloud/user_cloud_policy_manager.h:46: // Note: currently
these must be the FILE and the IO BrowserThreads.
On 2013/11/25 15:05:50, bartfab wrote:
> Nit: "these" is ambiguous (there are three task runners, which one(s) must be
> FILE, which one(s) must be IO?)

Done.

https://codereview.chromium.org/79023002/diff/560006/chrome/browser/policy/cl...
File chrome/browser/policy/cloud/user_cloud_policy_manager_factory.cc (right):

https://codereview.chromium.org/79023002/diff/560006/chrome/browser/policy/cl...
chrome/browser/policy/cloud/user_cloud_policy_manager_factory.cc:23: //
Directory inside the profile directory where other resources are stored.
On 2013/11/25 15:05:50, bartfab wrote:
> Nit: "Other resources" is extremely generic.

Done.

https://codereview.chromium.org/79023002/diff/560006/chrome/browser/policy/cl...
File chrome/browser/policy/cloud/user_cloud_policy_manager_factory.h (right):

https://codereview.chromium.org/79023002/diff/560006/chrome/browser/policy/cl...
chrome/browser/policy/cloud/user_cloud_policy_manager_factory.h:61: // Note:
currently these must be the FILE and the IO BrowserThreads.
On 2013/11/25 15:05:50, bartfab wrote:
> Nit: "these" is ambiguous (there are three task runners, which one(s) must be
> FILE, which one(s) must be IO?)

Done.

https://codereview.chromium.org/79023002/diff/560006/chrome/browser/policy/cl...
File chrome/browser/policy/cloud/user_cloud_policy_manager_unittest.cc (right):

https://codereview.chromium.org/79023002/diff/560006/chrome/browser/policy/cl...
chrome/browser/policy/cloud/user_cloud_policy_manager_unittest.cc:8: #include
"base/files/file_path.h"
On 2013/11/25 15:05:50, bartfab wrote:
> Nit: Already included by header file.

Done.

https://codereview.chromium.org/79023002/diff/560006/chrome/browser/policy/cl...
File chrome/browser/policy/cloud/user_policy_signin_service_unittest.cc (right):

https://codereview.chromium.org/79023002/diff/560006/chrome/browser/policy/cl...
chrome/browser/policy/cloud/user_policy_signin_service_unittest.cc:167:
base::FilePath(),
On 2013/11/25 15:05:50, bartfab wrote:
> Nit: #include "base/files/file_path.h"

Done.

https://codereview.chromium.org/79023002/diff/560006/components/policy/core/c...
File components/policy/core/common/policy_switches.cc (right):

https://codereview.chromium.org/79023002/diff/560006/components/policy/core/c...
components/policy/core/common/policy_switches.cc:20: const char
kDisableComponentCloudPolicy[]    = "enable-component-cloud-policy";
On 2013/11/25 15:05:50, bartfab wrote:
> s/enable/disable/

lolwat done

[bartfab (slow), 2013-11-25 16:46:31]

https://codereview.chromium.org/79023002/diff/560006/components/policy/core/c...
File components/policy/core/common/policy_switches.cc (right):

https://codereview.chromium.org/79023002/diff/560006/components/policy/core/c...
components/policy/core/common/policy_switches.cc:20: const char
kDisableComponentCloudPolicy[]    = "enable-component-cloud-policy";
On 2013/11/25 16:44:11, Joao da Silva wrote:
> On 2013/11/25 15:05:50, bartfab wrote:
> > s/enable/disable/
> 
> lolwat done

Nit: s/lolwat/lolwut/

[jochen (gone - plz use gerrit), 2013-11-26 08:31:30]

please update the CL description, since you don't turn it on by default anymore

lgtm

[Joao da Silva, 2013-11-26 08:36:58]

Updated the description.

Thanks for the reviews!

[commit-bot: I haz the power, 2013-11-26 08:37:17]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/joaodasilva@chromium.org/79023002/1130001

[commit-bot: I haz the power, 2013-11-26 10:38:49]

Change committed as 237300