Setup Chromad user policy plumbing

chrome/browser/ui/webui/policy_ui_handler.cc

Index: chrome/browser/ui/webui/policy_ui_handler.cc
diff --git a/chrome/browser/ui/webui/policy_ui_handler.cc b/chrome/browser/ui/webui/policy_ui_handler.cc
index d9bb3ccd01874c90768d230acfea3fcf460cabf9..78e641b39f089de47ec458a3fb5b6af2b1618830 100644
--- a/chrome/browser/ui/webui/policy_ui_handler.cc
+++ b/chrome/browser/ui/webui/policy_ui_handler.cc
@@ -59,6 +59,7 @@
 #include "chrome/browser/chromeos/policy/device_active_directory_policy_manager.h"
 #include "chrome/browser/chromeos/policy/device_cloud_policy_store_chromeos.h"
 #include "chrome/browser/chromeos/policy/device_local_account_policy_service.h"
+#include "chrome/browser/chromeos/policy/user_active_directory_policy_manager.h"
 #include "chrome/browser/chromeos/policy/user_cloud_policy_manager_chromeos.h"
 #include "chrome/browser/chromeos/policy/user_cloud_policy_manager_factory_chromeos.h"
 #include "chrome/browser/chromeos/settings/install_attributes.h"
@@ -250,16 +251,16 @@ class CloudPolicyCoreStatusProvider
 };
 
 // A cloud policy status provider for user policy.
-class UserPolicyStatusProvider : public CloudPolicyCoreStatusProvider {
+class UserCloudPolicyStatusProvider : public CloudPolicyCoreStatusProvider {
  public:
-  explicit UserPolicyStatusProvider(policy::CloudPolicyCore* core);
-  ~UserPolicyStatusProvider() override;
+  explicit UserCloudPolicyStatusProvider(policy::CloudPolicyCore* core);
+  ~UserCloudPolicyStatusProvider() override;
 
   // CloudPolicyCoreStatusProvider implementation.
   void GetStatus(base::DictionaryValue* dict) override;
 
  private:
-  DISALLOW_COPY_AND_ASSIGN(UserPolicyStatusProvider);
+  DISALLOW_COPY_AND_ASSIGN(UserCloudPolicyStatusProvider);
 };
 
 #if defined(OS_CHROMEOS)
@@ -308,14 +309,13 @@ class DeviceLocalAccountPolicyStatusProvider
   DISALLOW_COPY_AND_ASSIGN(DeviceLocalAccountPolicyStatusProvider);
 };
 
-// Provides status for DeviceActiveDirectoryPolicyManager.
-class DeviceActiveDirectoryPolicyStatusProvider
+// Provides status for any Active Directory policy manager.

[emaxx, 2016/12/28 19:09:20]

nit: Add comment to explain explicitly that it's both for device and user
policy? Unless I misunderstood this.

[Thiemo Nagel, 2016/12/29 15:08:28]

Actually it's "either/or" not "both".  I've updated the comment to (hopefully)
make that more clear.  (Not visible here because I've moved the webui changes
out to a dependent CL: https://codereview.chromium.org/2608573002/.)

+class ActiveDirectoryPolicyStatusProvider
     : public PolicyStatusProvider,
       public policy::CloudPolicyStore::Observer {
  public:
-  explicit DeviceActiveDirectoryPolicyStatusProvider(
-      policy::DeviceActiveDirectoryPolicyManager* manager);
-  ~DeviceActiveDirectoryPolicyStatusProvider() override;
+  explicit ActiveDirectoryPolicyStatusProvider(policy::CloudPolicyStore* store);
+  ~ActiveDirectoryPolicyStatusProvider() override;
 
   // PolicyStatusProvider implementation.
   void GetStatus(base::DictionaryValue* dict) override;
@@ -327,7 +327,7 @@ class DeviceActiveDirectoryPolicyStatusProvider
  private:
   policy::CloudPolicyStore* store_;
 
-  DISALLOW_COPY_AND_ASSIGN(DeviceActiveDirectoryPolicyStatusProvider);
+  DISALLOW_COPY_AND_ASSIGN(ActiveDirectoryPolicyStatusProvider);
 };
 #endif
 
@@ -369,14 +369,13 @@ void CloudPolicyCoreStatusProvider::OnStoreError(
   NotifyStatusChange();
 }
 
-UserPolicyStatusProvider::UserPolicyStatusProvider(
-    policy::CloudPolicyCore* core) : CloudPolicyCoreStatusProvider(core) {
-}
+UserCloudPolicyStatusProvider::UserCloudPolicyStatusProvider(
+    policy::CloudPolicyCore* core)
+    : CloudPolicyCoreStatusProvider(core) {}
 
-UserPolicyStatusProvider::~UserPolicyStatusProvider() {
-}
+UserCloudPolicyStatusProvider::~UserCloudPolicyStatusProvider() {}
 
-void UserPolicyStatusProvider::GetStatus(base::DictionaryValue* dict) {
+void UserCloudPolicyStatusProvider::GetStatus(base::DictionaryValue* dict) {
   if (!core_->store()->is_managed())
     return;
   GetStatusFromCore(core_, dict);
@@ -440,33 +439,31 @@ void DeviceLocalAccountPolicyStatusProvider::OnDeviceLocalAccountsChanged() {
   NotifyStatusChange();
 }
 
-DeviceActiveDirectoryPolicyStatusProvider::
-    DeviceActiveDirectoryPolicyStatusProvider(
-        policy::DeviceActiveDirectoryPolicyManager* manager)
-    : store_(manager->store()) {
+ActiveDirectoryPolicyStatusProvider::ActiveDirectoryPolicyStatusProvider(
+    policy::CloudPolicyStore* store)
+    : store_(store) {
   store_->AddObserver(this);
 }
 
-DeviceActiveDirectoryPolicyStatusProvider::
-    ~DeviceActiveDirectoryPolicyStatusProvider() {
+ActiveDirectoryPolicyStatusProvider::~ActiveDirectoryPolicyStatusProvider() {
   store_->RemoveObserver(this);
 }
 
 // TODO(tnagel): Provide more details and/or remove unused fields from UI.  See
 // https://crbug.com/664747.
-void DeviceActiveDirectoryPolicyStatusProvider::GetStatus(
+void ActiveDirectoryPolicyStatusProvider::GetStatus(
     base::DictionaryValue* dict) {
   base::string16 status =
       policy::FormatStoreStatus(store_->status(), store_->validation_status());
   dict->SetString("status", status);
 }
 
-void DeviceActiveDirectoryPolicyStatusProvider::OnStoreLoaded(
+void ActiveDirectoryPolicyStatusProvider::OnStoreLoaded(
     policy::CloudPolicyStore* store) {
   NotifyStatusChange();
 }
 
-void DeviceActiveDirectoryPolicyStatusProvider::OnStoreError(
+void ActiveDirectoryPolicyStatusProvider::OnStoreError(
     policy::CloudPolicyStore* store) {
   NotifyStatusChange();
 }
@@ -528,8 +525,8 @@ void PolicyUIHandler::RegisterMessages() {
   if (connector->IsEnterpriseManaged()) {
     if (connector->GetDeviceActiveDirectoryPolicyManager()) {
       device_status_provider_ =
-          base::MakeUnique<DeviceActiveDirectoryPolicyStatusProvider>(
-              connector->GetDeviceActiveDirectoryPolicyManager());
+          base::MakeUnique<ActiveDirectoryPolicyStatusProvider>(
+              connector->GetDeviceActiveDirectoryPolicyManager()->store());
     } else {
       device_status_provider_ =
           base::MakeUnique<DevicePolicyStatusProvider>(connector);
@@ -548,13 +545,22 @@ void PolicyUIHandler::RegisterMessages() {
               local_account_service);
     }
   } else {
-    policy::UserCloudPolicyManagerChromeOS* user_cloud_policy_manager =
-        policy::UserCloudPolicyManagerFactoryChromeOS::GetForProfile(
-            Profile::FromWebUI(web_ui()));
-    if (user_cloud_policy_manager) {
-      user_status_provider_ =
-          base::MakeUnique<UserPolicyStatusProvider>(
-              user_cloud_policy_manager->core());
+    Profile* profile = Profile::FromWebUI(web_ui());
+    policy::UserCloudPolicyManagerChromeOS* user_cloud_policy =
+        policy::UserCloudPolicyManagerFactoryChromeOS::
+            GetCloudPolicyManagerForProfile(profile);
+    if (user_cloud_policy) {
+      user_status_provider_ = base::MakeUnique<UserCloudPolicyStatusProvider>(
+          user_cloud_policy->core());
+    } else {
+      policy::UserActiveDirectoryPolicyManager* active_directory_policy =
+          policy::UserCloudPolicyManagerFactoryChromeOS::
+              GetActiveDirectoryPolicyManagerForProfile(profile);
+      if (active_directory_policy) {

[emaxx, 2016/12/28 19:09:20]

Could we transform this into a more readable sequence of conditions like
if ...
else if ...
else if ...
?

[Thiemo Nagel, 2016/12/29 15:08:28]

Done.  (Also moved to separate CL.)

+        user_status_provider_ =
+            base::MakeUnique<ActiveDirectoryPolicyStatusProvider>(
+                active_directory_policy->store());
+      }
     }
   }
 #else
@@ -562,7 +568,7 @@ void PolicyUIHandler::RegisterMessages() {
       policy::UserCloudPolicyManagerFactory::GetForBrowserContext(
           web_ui()->GetWebContents()->GetBrowserContext());
   if (user_cloud_policy_manager) {
-    user_status_provider_ = base::MakeUnique<UserPolicyStatusProvider>(
+    user_status_provider_ = base::MakeUnique<UserCloudPolicyStatusProvider>(
         user_cloud_policy_manager->core());
   }
 #endif