Display policy parsing errors in about:policy.

BUG=99178
TEST=Set policy values with the wrong types (e.g. ShowHomeButton = 2) or with conflicting values. The "status" column in about:policy shows a description of the error.


Review URL: http://codereview.chromium.org/8413037

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@107749 0039d316-1c4b-4281-b951-d872f2087c98

6 files changed, 46 insertions, 36 deletions

diff --git a/chrome/browser/policy/configuration_policy_handler.cc b/chrome/browser/policy/configuration_policy_handler.cc
index 5efda16..5d4b252 100644
--- a/chrome/browser/policy/configuration_policy_handler.cc
+++ b/chrome/browser/policy/configuration_policy_handler.cc
@@ -1029,9 +1029,9 @@ bool ProxyPolicyHandler::CheckPolicySettings(const PolicyMap* policies,
     if (!entry.pac_url_allowed && pac_url)
       errors->AddError(kPolicyProxyPacUrl, entry.error_message_id);
     if (!entry.bypass_list_allowed && bypass_list)
-      errors->AddError(kPolicyProxyPacUrl, entry.error_message_id);
+      errors->AddError(kPolicyProxyBypassList, entry.error_message_id);
     if (!entry.server_allowed && server)
-      errors->AddError(kPolicyProxyPacUrl, entry.error_message_id);
+      errors->AddError(kPolicyProxyServer, entry.error_message_id);
 
     if ((!entry.pac_url_allowed && pac_url) ||
         (!entry.bypass_list_allowed && bypass_list) ||
diff --git a/chrome/browser/policy/configuration_policy_reader.cc b/chrome/browser/policy/configuration_policy_reader.cc
index 1854969..20dea0f9 100644
--- a/chrome/browser/policy/configuration_policy_reader.cc
+++ b/chrome/browser/policy/configuration_policy_reader.cc
@@ -13,7 +13,9 @@
 #include "base/utf_string_conversions.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/policy/browser_policy_connector.h"
+#include "chrome/browser/policy/configuration_policy_handler.h"
 #include "chrome/browser/policy/configuration_policy_pref_store.h"
+#include "chrome/browser/policy/policy_error_map.h"
 #include "chrome/browser/policy/policy_map.h"
 #include "policy/policy_constants.h"
 
@@ -70,23 +72,34 @@ DictionaryValue* ConfigurationPolicyStatusKeeper::GetPolicyStatus(
 
 void ConfigurationPolicyStatusKeeper::GetPoliciesFromProvider(
     ConfigurationPolicyProvider* provider) {
-  scoped_ptr<PolicyMap> policies(new PolicyMap());
-  if (!provider->Provide(policies.get()))
-    LOG(WARNING) << "Failed to get policy from provider.";
+  PolicyMap policies;
+  if (!provider->Provide(&policies))
+    DLOG(WARNING) << "Failed to get policy from provider.";
 
-  PolicyMap::const_iterator policy = policies->begin();
-  for ( ; policy != policies->end(); ++policy) {
-    string16 name = ASCIIToUTF16(GetPolicyName(policy->first));
+  const ConfigurationPolicyHandler::HandlerList* handlers =
+      g_browser_process->browser_policy_connector()->
+          GetConfigurationPolicyHandlerList();
+
+  PolicyErrorMap errors;
+  ConfigurationPolicyHandler::HandlerList::const_iterator handler;
+  for (handler = handlers->begin(); handler != handlers->end(); ++handler)
+    (*handler)->CheckPolicySettings(&policies, &errors);
 
-    // TODO(simo) actually determine whether the policy is a user or a device
-    // one and whether the policy could be enforced or not once this information
-    // is available.
+  PolicyMap::const_iterator policy = policies.begin();
+  for ( ; policy != policies.end(); ++policy) {
+    string16 name = ASCIIToUTF16(GetPolicyName(policy->first));
+    string16 error_message = errors.GetErrors(policy->first);
+    PolicyStatusInfo::PolicyStatus status =
+        error_message.empty() ? PolicyStatusInfo::ENFORCED
+                              : PolicyStatusInfo::FAILED;
+    // TODO(joaodasilva): determine whether this is a user or device policy.
+    // http://crbug.com/102114
     PolicyStatusInfo* info = new PolicyStatusInfo(name,
                                                   PolicyStatusInfo::USER,
                                                   policy_level_,
                                                   policy->second->DeepCopy(),
-                                                  PolicyStatusInfo::ENFORCED,
-                                                  string16());
+                                                  status,
+                                                  error_message);
     policy_map_[policy->first] = info;
   }
 }
@@ -214,7 +227,7 @@ void PolicyStatus::RemoveObserver(Observer* observer) const {
 
 ListValue* PolicyStatus::GetPolicyStatusList(bool* any_policies_set) const {
   ListValue* result = new ListValue();
-  std::vector<DictionaryValue*> unsent_policies;
+  std::vector<DictionaryValue*> unset_policies;
 
   *any_policies_set = false;
   const PolicyDefinitionList* supported_policies =
@@ -228,15 +241,15 @@ ListValue* PolicyStatus::GetPolicyStatusList(bool* any_policies_set) const {
                             Value::CreateNullValue(),
                             PolicyStatusInfo::STATUS_UNDEFINED,
                             string16());
-      unsent_policies.push_back(info.GetDictionaryValue());
+      unset_policies.push_back(info.GetDictionaryValue());
     } else {
       *any_policies_set = true;
     }
   }
 
   // Add policies that weren't actually sent from providers to list.
-  std::vector<DictionaryValue*>::const_iterator info = unsent_policies.begin();
-  for ( ; info != unsent_policies.end(); ++info)
+  std::vector<DictionaryValue*>::const_iterator info = unset_policies.begin();
+  for ( ; info != unset_policies.end(); ++info)
     result->Append(*info);
 
   return result;
diff --git a/chrome/browser/policy/configuration_policy_reader_unittest.cc b/chrome/browser/policy/configuration_policy_reader_unittest.cc
index 670c0b5..8c5b275 100644
--- a/chrome/browser/policy/configuration_policy_reader_unittest.cc
+++ b/chrome/browser/policy/configuration_policy_reader_unittest.cc
@@ -27,7 +27,7 @@ class ConfigurationPolicyReaderTest : public testing::Test {
         new ConfigurationPolicyReader(&provider_, PolicyStatusInfo::MANDATORY));
     recommended_reader_.reset(new ConfigurationPolicyReader(&provider_,
                                   PolicyStatusInfo::RECOMMENDED));
-    status_ok_ = ASCIIToUTF16("ok");
+    status_ok_ = ASCIIToUTF16("OK");
   }
 
   DictionaryValue* CreateDictionary(const char* policy_name,
@@ -158,11 +158,11 @@ class PolicyStatusTest : public testing::Test {
                                           managed_cloud_,
                                           recommended_platform_,
                                           recommended_cloud_));
-    status_ok_ = ASCIIToUTF16("ok");
+    status_ok_ = ASCIIToUTF16("OK");
 
     policy_list_ = GetChromePolicyDefinitionList();
     policy_list_size_ =
-      static_cast<size_t>(policy_list_->end - policy_list_->begin);
+        static_cast<size_t>(policy_list_->end - policy_list_->begin);
   }
 
   void DontSendPolicies() {
diff --git a/chrome/browser/policy/policy_error_map.cc b/chrome/browser/policy/policy_error_map.cc
index bbe6e1c..956392a 100644
--- a/chrome/browser/policy/policy_error_map.cc
+++ b/chrome/browser/policy/policy_error_map.cc
@@ -6,6 +6,7 @@
 
 #include <utility>
 
+#include "base/string_util.h"
 #include "base/utf_string_conversions.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/resource/resource_bundle.h"
@@ -50,18 +51,13 @@ void PolicyErrorMap::AddError(ConfigurationPolicyType policy,
   AddError(PendingError(policy, message_id, ASCIIToUTF16(replacement)));
 }
 
-ListValue* PolicyErrorMap::GetErrors(ConfigurationPolicyType policy) {
+string16 PolicyErrorMap::GetErrors(ConfigurationPolicyType policy) {
   CheckReadyAndConvert();
   std::pair<const_iterator, const_iterator> range = map_.equal_range(policy);
-
-  if (range.first == range.second)
-    return NULL;
-
-  ListValue* list = new ListValue();
+  std::vector<string16> list;
   for (const_iterator it = range.first; it != range.second; ++it)
-    list->Append(Value::CreateStringValue(it->second));
-
-  return list;
+    list.push_back(it->second);
+  return JoinString(list, ' ');
 }
 
 bool PolicyErrorMap::empty() {
diff --git a/chrome/browser/policy/policy_error_map.h b/chrome/browser/policy/policy_error_map.h
index b031b22..3c06193 100644
--- a/chrome/browser/policy/policy_error_map.h
+++ b/chrome/browser/policy/policy_error_map.h
@@ -8,10 +8,10 @@
 
 #include <map>
 #include <string>
+#include <vector>
 
 #include "base/basictypes.h"
 #include "base/string16.h"
-#include "base/values.h"
 #include "policy/configuration_policy_type.h"
 
 namespace policy {
@@ -41,10 +41,9 @@ class PolicyErrorMap {
                 int message_id,
                 const std::string& replacement_string);
 
-  // Returns a list of all the error messages stored for |policy|. Returns NULL
-  // if there are no error messages for |policy. The caller acquires ownership
-  // of the returned ListValue pointer.
-  ListValue* GetErrors(ConfigurationPolicyType policy);
+  // Returns all the error messages stored for |policy|, separated by a white
+  // space. Returns an empty string if there are no errors for |policy|.
+  string16 GetErrors(ConfigurationPolicyType policy);
 
   bool empty();
   size_t size();
diff --git a/chrome/browser/policy/policy_status_info.cc b/chrome/browser/policy/policy_status_info.cc
index 2b740df..395bf62 100644
--- a/chrome/browser/policy/policy_status_info.cc
+++ b/chrome/browser/policy/policy_status_info.cc
@@ -8,6 +8,8 @@
 
 #include "base/logging.h"
 #include "base/utf_string_conversions.h"
+#include "grit/generated_resources.h"
+#include "ui/base/l10n/l10n_util.h"
 
 namespace policy {
 
@@ -46,8 +48,8 @@ PolicyStatusInfo::~PolicyStatusInfo() {
 DictionaryValue* PolicyStatusInfo::GetDictionaryValue() const {
   string16 level_string = GetPolicyLevelString(level);
   string16 source_type_string = GetSourceTypeString(source_type);
-  string16 status_message = status == ENFORCED ? ASCIIToUTF16("ok")
-                                               : error_message;
+  string16 status_message =
+      status == ENFORCED ? l10n_util::GetStringUTF16(IDS_OK) : error_message;
   DictionaryValue* result = new DictionaryValue();
   result->SetString(std::string(kNameDictPath), name);
   result->SetString(std::string(kLevelDictPath), level_string);