Split UserCloudPolicyManager implementation.

chrome/browser/policy/browser_policy_connector.h

Index: chrome/browser/policy/browser_policy_connector.h
diff --git a/chrome/browser/policy/browser_policy_connector.h b/chrome/browser/policy/browser_policy_connector.h
index c8b5834d0960f42099940c9ab3f458f574d24dfa..a6d98ee79dad3c74e491823bf065f989c8ec1529 100644
--- a/chrome/browser/policy/browser_policy_connector.h
+++ b/chrome/browser/policy/browser_policy_connector.h
@@ -33,6 +33,7 @@ class NetworkConfigurationUpdater;
 class PolicyService;
 class PolicyStatisticsCollector;
 class UserCloudPolicyManager;

[Joao da Silva, 2012/11/21 17:06:34]

nit: not needed

[Mattias Nissler (ping if slow), 2012/11/22 20:51:59]

Done.

+class UserCloudPolicyManagerChromeOS;
 class UserPolicyTokenCache;
 
 // Manages the lifecycle of browser-global policy infrastructure, such as the
@@ -62,16 +63,6 @@ class BrowserPolicyConnector : public content::NotificationObserver {
   // Returns true if Init() has been called but Shutdown() hasn't been yet.
   bool is_initialized() const { return is_initialized_; }
 
-  // Creates a UserCloudPolicyManager for the given profile, or returns NULL if
-  // it is not supported on this platform. If |force_immediate_policy_load| is
-  // true, then any underlying policy files will be loaded before this routine
-  // returns - this is used when the caller (such as
-  // CreateProfile(CREATE_MODE_SYNCHRONOUS)) needs to access the policy values
-  // immediately without waiting for tasks to complete.
-  scoped_ptr<UserCloudPolicyManager> CreateCloudPolicyManager(
-      Profile* profile,
-      bool force_immediate_policy_load);
-
   // Creates a new policy service for the given profile.
   scoped_ptr<PolicyService> CreatePolicyService(Profile* profile);
 
@@ -174,6 +165,9 @@ class BrowserPolicyConnector : public content::NotificationObserver {
   DeviceCloudPolicyManagerChromeOS* GetDeviceCloudPolicyManager() {
     return device_cloud_policy_manager_.get();
   }
+  UserCloudPolicyManagerChromeOS* GetUserCloudPolicyManager() {
+    return user_cloud_policy_manager_.get();
+  }
 #endif
 
   // Sets a |provider| that will be included in PolicyServices returned by
@@ -232,8 +226,14 @@ class BrowserPolicyConnector : public content::NotificationObserver {
 #if defined(OS_CHROMEOS)
   scoped_ptr<EnterpriseInstallAttributes> install_attributes_;
   scoped_ptr<DeviceCloudPolicyManagerChromeOS> device_cloud_policy_manager_;
+  scoped_ptr<UserCloudPolicyManagerChromeOS> user_cloud_policy_manager_;
+
+  // This policy provider is used on Chrome OS to feed user policy into the
+  // global PolicyService instance. This works by installing
+  // |user_cloud_policy_manager_| as the delegate once the former is
+  // initialized.
+  ProxyPolicyProvider global_user_cloud_policy_provider_;

[Joao da Silva, 2012/11/21 17:06:34]

This works but I'd prefer to always create the user_cloud_policy_manager_ early
and plug it into the PolicyService, and initialize it (and its store) later, at
InitializeUserPolicy().

IMO that'd be one less class to understand, and all the providers are obvious at
the creation time of PolicyService. It also follows the pattern that providers
become ready when well, they're ready :-) This mostly means loading caches, and
for this provider it means "user signed in, cryptohome is mounted and caches
have been loaded".

That said, it's a matter of personal preference and that's just my opinion. The
proxy may also come in handy for user cloud policy on the desktop later.

[Mattias Nissler (ping if slow), 2012/11/22 20:51:59]

I disagree strongly on the becoming ready part. If I did what you suggest, the
global PolicyService would only flag itself ready after user policy is
available, which is obviously wrong (e.g. if somebody decides to init the
local_state PrefService asynchronously).

This is also further complicated by the fact that public accounts might have a
different UserCloudPolicyManager implementation, at which point we need to
decide at login time which of the two we're actually going to wire to the global
PolicyService.

 #endif
-  ProxyPolicyProvider user_cloud_policy_provider_;
 
   // Must be deleted before all the policy providers.
   scoped_ptr<PolicyService> policy_service_;