Split UserCloudPolicyManager implementation.

chrome/browser/policy/browser_policy_connector.h

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef CHROME_BROWSER_POLICY_BROWSER_POLICY_CONNECTOR_H_
 #define CHROME_BROWSER_POLICY_BROWSER_POLICY_CONNECTOR_H_
 
 #include <string>
 
 #include "base/basictypes.h"
@@ skipping 14 matching lines @@
 class AppPackUpdater;
 class CloudPolicyDataStore;
 class CloudPolicyProvider;
 class CloudPolicySubsystem;
 class ConfigurationPolicyProvider;
 class DeviceCloudPolicyManagerChromeOS;
 class DeviceManagementService;
 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
 // platform policy providers, device- and the user-cloud policy infrastructure.
 // TODO(gfeher,mnissler): Factor out device and user specific methods into their
 // respective classes.
 class BrowserPolicyConnector : public content::NotificationObserver {
  public:
   // Builds an uninitialized BrowserPolicyConnector, suitable for testing.
   // Init() should be called to create and start the policy machinery.
   BrowserPolicyConnector();
 
   // Invoke Shutdown() before deleting, see below.
   virtual ~BrowserPolicyConnector();
 
   // Creates the policy providers and finalizes the initialization of the
   // connector. This call can be skipped on tests that don't require the full
   // policy system running.
   void Init();
 
   // Stops the policy providers and cleans up the connector before it can be
   // safely deleted. This must be invoked before the destructor and while the
   // threads are still running. The policy providers are still valid but won't
   // update anymore after this call.
   void Shutdown();
 
   // 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);
 
   // Returns the browser-global PolicyService, that contains policies for the
   // whole browser.
   PolicyService* GetPolicyService();
 
   // Returns a weak pointer to the CloudPolicySubsystem corresponding to the
   // device policy managed by this policy connector, or NULL if no such
   // subsystem exists (i.e. when running outside ChromeOS).
@@ skipping 82 matching lines @@
   NetworkConfigurationUpdater* GetNetworkConfigurationUpdater();
 
   DeviceManagementService* device_management_service() {
     return device_management_service_.get();
   }
 
 #if defined(OS_CHROMEOS)
   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
   // CreatePolicyService. This is a static method because local state is
   // created immediately after the connector, and tests don't have a chance to
   // inject the provider otherwise. |provider| must outlive the connector, and
   // its ownership is not taken though the connector will initialize and shut it
   // down.
   static void SetPolicyProviderForTesting(
       ConfigurationPolicyProvider* provider);
@@ skipping 38 matching lines @@
 
   scoped_ptr<ConfigurationPolicyProvider> platform_provider_;
   scoped_ptr<CloudPolicyProvider> cloud_provider_;
 
   // Components of the new-style cloud policy implementation.
   // TODO(mnissler): Remove the old-style components below once we have
   // completed the switch to the new cloud policy implementation.
 #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_;
 
 #if defined(OS_CHROMEOS)
   scoped_ptr<CloudPolicyDataStore> device_data_store_;
   scoped_ptr<CloudPolicySubsystem> device_cloud_policy_subsystem_;
 #endif
 
   scoped_ptr<UserPolicyTokenCache> user_policy_token_cache_;
@@ skipping 19 matching lines @@
   scoped_ptr<AppPackUpdater> app_pack_updater_;
   scoped_ptr<NetworkConfigurationUpdater> network_configuration_updater_;
 #endif
 
   DISALLOW_COPY_AND_ASSIGN(BrowserPolicyConnector);
 };
 
 }  // namespace policy
 
 #endif  // CHROME_BROWSER_POLICY_BROWSER_POLICY_CONNECTOR_H_