Reduce duplication across Linux/Windows policy providers, and unify names used therein.

BUG=http://crbug.com/45344
TEST=no change

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

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

8 files changed, 108 insertions, 84 deletions

diff --git a/chrome/browser/config_dir_policy_provider.cc b/chrome/browser/config_dir_policy_provider.cc
index d791e66..f0ec6eb 100644
--- a/chrome/browser/config_dir_policy_provider.cc
+++ b/chrome/browser/config_dir_policy_provider.cc
@@ -9,6 +9,7 @@
 #include "base/file_util.h"
 #include "base/logging.h"
 #include "base/scoped_ptr.h"
+#include "base/utf_string_conversions.h"
 #include "base/values.h"
 #include "chrome/common/json_value_serializer.h"
 
@@ -54,3 +55,21 @@ DictionaryValue* ConfigDirPolicyProvider::ReadPolicies() {
 
   return policy;
 }
+
+void ConfigDirPolicyProvider::DecodePolicyValueTree(
+    DictionaryValue* policies,
+    ConfigurationPolicyStore* store) {
+  const PolicyValueMap* mapping = PolicyValueMapping();
+  for (PolicyValueMap::const_iterator i = mapping->begin();
+       i != mapping->end(); ++i) {
+    const PolicyValueMapEntry& entry(*i);
+    Value* value;
+    if (policies->Get(UTF8ToWide(entry.name), &value) &&
+        value->IsType(entry.value_type))
+      store->Apply(entry.policy_type, value->DeepCopy());
+  }
+
+  // TODO(mnissler): Handle preference overrides once |ConfigurationPolicyStore|
+  // supports it.
+}
+
diff --git a/chrome/browser/config_dir_policy_provider.h b/chrome/browser/config_dir_policy_provider.h
index 05bffd0..06c1363 100644
--- a/chrome/browser/config_dir_policy_provider.h
+++ b/chrome/browser/config_dir_policy_provider.h
@@ -29,6 +29,10 @@ class ConfigDirPolicyProvider : public ConfigurationPolicyProvider {
   // Read and merge the files from the configuration directory.
   DictionaryValue* ReadPolicies();
 
+  // Decodes the value tree and writes the configuration to the given |store|.
+  void DecodePolicyValueTree(DictionaryValue* policies,
+                             ConfigurationPolicyStore* store);
+
   // The directory in which we look for configuration files.
   const FilePath config_dir_;
 
diff --git a/chrome/browser/config_dir_policy_provider_unittest.cc b/chrome/browser/config_dir_policy_provider_unittest.cc
index b3d4a27..0d3c2a0 100644
--- a/chrome/browser/config_dir_policy_provider_unittest.cc
+++ b/chrome/browser/config_dir_policy_provider_unittest.cc
@@ -67,7 +67,7 @@ TEST_F(ConfigDirPolicyProviderTest, ReadPrefsNonExistentDirectory) {
 // Test reading back a single preference value.
 TEST_F(ConfigDirPolicyProviderTest, ReadPrefsSinglePref) {
   DictionaryValue test_dict;
-  test_dict.SetString(L"homepage", L"http://www.google.com");
+  test_dict.SetString(L"Homepage", L"http://www.google.com");
   WriteConfigFile(test_dict, "config_file");
   ConfigDirPolicyProvider provider(test_dir_);
 
@@ -91,11 +91,11 @@ TEST_F(ConfigDirPolicyProviderTest, ReadPrefsMergePrefs) {
   // filesystem may return files in arbitrary order, there is no way to be sure,
   // but this is better than nothing.
   DictionaryValue test_dict_bar;
-  test_dict_bar.SetString(L"homepage", L"http://bar.com");
+  test_dict_bar.SetString(L"Homepage", L"http://bar.com");
   for (unsigned int i = 1; i <= 4; ++i)
     WriteConfigFile(test_dict_bar, IntToString(i));
   DictionaryValue test_dict_foo;
-  test_dict_foo.SetString(L"homepage", L"http://foo.com");
+  test_dict_foo.SetString(L"Homepage", L"http://foo.com");
   WriteConfigFile(test_dict_foo, "9");
   for (unsigned int i = 5; i <= 8; ++i)
     WriteConfigFile(test_dict_bar, IntToString(i));
diff --git a/chrome/browser/configuration_policy_provider.cc b/chrome/browser/configuration_policy_provider.cc
index a47e8e2..ec49ec7 100644
--- a/chrome/browser/configuration_policy_provider.cc
+++ b/chrome/browser/configuration_policy_provider.cc
@@ -5,36 +5,43 @@
 #include "chrome/browser/configuration_policy_provider.h"
 
 #include "base/values.h"
-#include "chrome/browser/configuration_policy_store.h"
 
 namespace {
 
-struct PolicyValueTreeMappingEntry {
+// TODO(avi): Use this mapping to auto-generate MCX manifests and Windows
+// ADM/ADMX files. http://crbug.com/45334
+
+struct InternalPolicyValueMapEntry {
   ConfigurationPolicyStore::PolicyType policy_type;
   Value::ValueType value_type;
-  const wchar_t* const name;
+  const char* name;
 };
 
-const PolicyValueTreeMappingEntry kPolicyValueTreeMapping[] = {
+const InternalPolicyValueMapEntry kPolicyValueMap[] = {
   { ConfigurationPolicyStore::kPolicyHomePage,
-    Value::TYPE_STRING, L"homepage" },
+    Value::TYPE_STRING, "Homepage" },
   { ConfigurationPolicyStore::kPolicyHomepageIsNewTabPage,
-    Value::TYPE_BOOLEAN, L"homepage_is_newtabpage" },
+    Value::TYPE_BOOLEAN, "HomepageIsNewTabPage" },
   { ConfigurationPolicyStore::kPolicyCookiesMode,
-    Value::TYPE_INTEGER, L"cookies_enabled" }
+    Value::TYPE_INTEGER, "CookiesMode" }
 };
 
-}
-
-void DecodePolicyValueTree(DictionaryValue* policies,
-                           ConfigurationPolicyStore* store) {
-  for (size_t i = 0; i < arraysize(kPolicyValueTreeMapping); ++i) {
-    const PolicyValueTreeMappingEntry& entry(kPolicyValueTreeMapping[i]);
-    Value* value;
-    if (policies->Get(entry.name, &value) && value->IsType(entry.value_type))
-      store->Apply(entry.policy_type, value->DeepCopy());
+}  // namespace
+
+/* static */
+const ConfigurationPolicyProvider::PolicyValueMap*
+    ConfigurationPolicyProvider::PolicyValueMapping() {
+  static PolicyValueMap* mapping;
+  if (!mapping) {
+    mapping = new PolicyValueMap();
+    for (size_t i = 0; i < arraysize(kPolicyValueMap); ++i) {
+      const InternalPolicyValueMapEntry& internal_entry = kPolicyValueMap[i];
+      PolicyValueMapEntry entry;
+      entry.policy_type = internal_entry.policy_type;
+      entry.value_type = internal_entry.value_type;
+      entry.name = std::string(internal_entry.name);
+      mapping->push_back(entry);
+    }
   }
-
-  // TODO (mnissler) Handle preference overrides once |ConfigurationPolicyStore|
-  // supports it.
+  return mapping;
 }
diff --git a/chrome/browser/configuration_policy_provider.h b/chrome/browser/configuration_policy_provider.h
index aedc4e7..f0f6051 100644
--- a/chrome/browser/configuration_policy_provider.h
+++ b/chrome/browser/configuration_policy_provider.h
@@ -5,12 +5,14 @@
 #ifndef CHROME_BROWSER_CONFIGURATION_POLICY_PROVIDER_H_
 #define CHROME_BROWSER_CONFIGURATION_POLICY_PROVIDER_H_
 
+#include <vector>
+
 #include "base/basictypes.h"
+#include "chrome/browser/configuration_policy_store.h"
 
-class ConfigurationPolicyStore;
 class DictionaryValue;
 
-// An abstract super class for platform-specific policy providers.
+// A mostly-abstract super class for platform-specific policy providers.
 // Platform-specific policy providers (Windows Group Policy, gconf,
 // etc.) should implement a subclass of this class.
 class ConfigurationPolicyProvider {
@@ -27,15 +29,22 @@ class ConfigurationPolicyProvider {
   // Returns true if the policy could be provided, otherwise false.
   virtual bool Provide(ConfigurationPolicyStore* store) = 0;
 
+ protected:
+  // A structure mapping policies to their implementations by providers.
+  struct PolicyValueMapEntry {
+    ConfigurationPolicyStore::PolicyType policy_type;
+    Value::ValueType value_type;
+    std::string name;
+  };
+  typedef std::vector<PolicyValueMapEntry> PolicyValueMap;
+
+  // Returns the mapping from policy values to the actual names used by
+  // implementations.
+  static const PolicyValueMap* PolicyValueMapping();
+
  private:
   DISALLOW_COPY_AND_ASSIGN(ConfigurationPolicyProvider);
 };
 
-// Base functionality for all policy providers that use a DictionaryValue tree
-// structure holding key-value pairs for storing policy settings. Decodes the
-// value tree and writes the configuration to the given |store|.
-void DecodePolicyValueTree(DictionaryValue* policies,
-                           ConfigurationPolicyStore* store);
-
 #endif  // CHROME_BROWSER_CONFIGURATION_POLICY_PROVIDER_H_
 
diff --git a/chrome/browser/configuration_policy_provider_win.cc b/chrome/browser/configuration_policy_provider_win.cc
index f81758f6..420ff36 100755
--- a/chrome/browser/configuration_policy_provider_win.cc
+++ b/chrome/browser/configuration_policy_provider_win.cc
@@ -9,16 +9,11 @@
 #include "base/logging.h"
 #include "base/registry.h"
 #include "base/scoped_ptr.h"
+#include "base/string_piece.h"
 #include "base/sys_string_conversions.h"
+#include "base/utf_string_conversions.h"
 #include "base/values.h"
 
-const wchar_t ConfigurationPolicyProviderWin::kHomepageRegistryValueName[] =
-    L"Homepage";
-const wchar_t ConfigurationPolicyProviderWin::
-    kHomepageIsNewTabPageRegistryValueName[] = L"HomepageIsNewTabPage";
-const wchar_t ConfigurationPolicyProviderWin::kCookiesModeRegistryValueName[] =
-    L"CookiesMode";
-
 #if defined(GOOGLE_CHROME_BUILD)
 const wchar_t ConfigurationPolicyProviderWin::kPolicyRegistrySubKey[] =
     L"SOFTWARE\\Policies\\Google\\Google Chrome";
@@ -101,48 +96,32 @@ bool ConfigurationPolicyProviderWin::GetRegistryPolicyInteger(
   return false;
 }
 
-const ConfigurationPolicyProviderWin::RegistryPolicyMapEntry
-    ConfigurationPolicyProviderWin::registry_to_policy_map_[] = {
-    { Value::TYPE_STRING,
-      ConfigurationPolicyStore::kPolicyHomePage,
-      kHomepageRegistryValueName },
-    { Value::TYPE_BOOLEAN,
-      ConfigurationPolicyStore::kPolicyHomepageIsNewTabPage,
-      kHomepageIsNewTabPageRegistryValueName },
-    { Value::TYPE_INTEGER,
-      ConfigurationPolicyStore::kPolicyCookiesMode,
-      kCookiesModeRegistryValueName },
-};
-
 bool ConfigurationPolicyProviderWin::Provide(
     ConfigurationPolicyStore* store) {
-  const RegistryPolicyMapEntry* current;
-  const RegistryPolicyMapEntry* end = registry_to_policy_map_ +
-      arraysize(registry_to_policy_map_);
+  const PolicyValueMap* mapping = PolicyValueMapping();
 
-  for (current = registry_to_policy_map_; current != end; ++current) {
+  for (PolicyValueMap::const_iterator current = mapping->begin();
+       current != mapping->end(); ++current) {
+    std::wstring name = UTF8ToWide(current->name);
     std::wstring string_value;
     uint32 int_value;
     bool bool_value;
     switch (current->value_type) {
       case Value::TYPE_STRING:
-        if (GetRegistryPolicyString(current->registry_value_name,
-                                    &string_value)) {
+        if (GetRegistryPolicyString(name.c_str(), &string_value)) {
           store->Apply(
               current->policy_type,
               Value::CreateStringValueFromUTF16(string_value));
         }
         break;
       case Value::TYPE_BOOLEAN:
-        if (GetRegistryPolicyBoolean(current->registry_value_name,
-                                     &bool_value)) {
+        if (GetRegistryPolicyBoolean(name.c_str(), &bool_value)) {
           store->Apply(current->policy_type,
                        Value::CreateBooleanValue(bool_value));
         }
         break;
       case Value::TYPE_INTEGER:
-        if (GetRegistryPolicyInteger(current->registry_value_name,
-                                     &int_value)) {
+        if (GetRegistryPolicyInteger(name.c_str(), &int_value)) {
           store->Apply(current->policy_type,
                        Value::CreateIntegerValue(int_value));
         }
diff --git a/chrome/browser/configuration_policy_provider_win.h b/chrome/browser/configuration_policy_provider_win.h
index 235e2ee1..f5c5eee 100755
--- a/chrome/browser/configuration_policy_provider_win.h
+++ b/chrome/browser/configuration_policy_provider_win.h
@@ -27,25 +27,7 @@ class ConfigurationPolicyProviderWin : public ConfigurationPolicyProvider {
   // Windows registry.
   static const wchar_t kPolicyRegistrySubKey[];
 
-  // Constants specifying the names of policy-specifying registry values
-  // in |kChromiumPolicySubKey|.
-  static const wchar_t kHomepageRegistryValueName[];
-  static const wchar_t kHomepageIsNewTabPageRegistryValueName[];
-  static const wchar_t kCookiesModeRegistryValueName[];
-
  private:
-  // For each Group Policy registry value that maps to a browser policy, there
-  // is an entry in |registry_to_policy_map_| of type |RegistryPolicyMapEntry|
-  // that specifies the registry value name and the browser policy that it
-  // maps to.
-  struct RegistryPolicyMapEntry {
-    Value::ValueType value_type;
-    ConfigurationPolicyStore::PolicyType policy_type;
-    const wchar_t* registry_value_name;
-  };
-
-  static const RegistryPolicyMapEntry registry_to_policy_map_[];
-
   // Methods to perfrom type-specific policy lookups in the registry.
   // HKLM is checked first, then HKCU.
   bool GetRegistryPolicyString(const wchar_t* value_name, string16* result);
diff --git a/chrome/browser/configuration_policy_provider_win_unittest.cc b/chrome/browser/configuration_policy_provider_win_unittest.cc
index a63c6dc..b4fc671 100755
--- a/chrome/browser/configuration_policy_provider_win_unittest.cc
+++ b/chrome/browser/configuration_policy_provider_win_unittest.cc
@@ -9,17 +9,21 @@
 #include "base/logging.h"
 #include "base/registry.h"
 #include "base/scoped_ptr.h"
+#include "base/string_piece.h"
+#include "base/utf_string_conversions.h"
 #include "chrome/browser/configuration_policy_provider_win.h"
 #include "chrome/browser/mock_configuration_policy_store.h"
 #include "chrome/common/pref_names.h"
 
 namespace {
+
 const wchar_t kUnitTestRegistrySubKey[] = L"SOFTWARE\\Chromium Unit Tests";
 const wchar_t kUnitTestMachineOverrideSubKey[] =
     L"SOFTWARE\\Chromium Unit Tests\\HKLM Override";
 const wchar_t kUnitTestUserOverrideSubKey[] =
     L"SOFTWARE\\Chromium Unit Tests\\HKCU Override";
-}
+
+}  // namespace
 
 // A subclass of |ConfigurationPolicyProviderWin| providing access to
 // internal protected constants without an orgy of FRIEND_TESTS.
@@ -33,8 +37,28 @@ class TestConfigurationPolicyProviderWin
   void SetHomepageRegistryValueWrongType(HKEY hive);
   void SetHomepageIsNewTabPage(HKEY hive, bool value);
   void SetCookiesMode(HKEY hive, uint32 value);
+
+  typedef std::vector<PolicyValueMapEntry> PolicyValueMap;
+  static const PolicyValueMap* PolicyValueMapping() {
+    return ConfigurationPolicyProvider::PolicyValueMapping();
+  }
 };
 
+namespace {
+
+std::wstring NameForPolicy(ConfigurationPolicyStore::PolicyType policy) {
+  const TestConfigurationPolicyProviderWin::PolicyValueMap* mapping =
+      TestConfigurationPolicyProviderWin::PolicyValueMapping();
+  for (TestConfigurationPolicyProviderWin::PolicyValueMap::const_iterator
+       current = mapping->begin(); current != mapping->end(); ++current) {
+    if (current->policy_type == policy)
+      return UTF8ToWide(current->name);
+  }
+  return NULL;
+}
+
+}  // namespace
+
 void TestConfigurationPolicyProviderWin::SetHomepageRegistryValue(
     HKEY hive,
     const wchar_t* value) {
@@ -42,7 +66,7 @@ void TestConfigurationPolicyProviderWin::SetHomepageRegistryValue(
       ConfigurationPolicyProviderWin::kPolicyRegistrySubKey,
       KEY_ALL_ACCESS);
   EXPECT_TRUE(key.WriteValue(
-      ConfigurationPolicyProviderWin::kHomepageRegistryValueName,
+      NameForPolicy(ConfigurationPolicyStore::kPolicyHomePage).c_str(),
       value));
 }
 
@@ -52,7 +76,7 @@ void TestConfigurationPolicyProviderWin::SetHomepageRegistryValueWrongType(
       ConfigurationPolicyProviderWin::kPolicyRegistrySubKey,
       KEY_ALL_ACCESS);
   EXPECT_TRUE(key.WriteValue(
-      ConfigurationPolicyProviderWin::kHomepageRegistryValueName,
+      NameForPolicy(ConfigurationPolicyStore::kPolicyHomePage).c_str(),
       5));
 }
 
@@ -62,8 +86,8 @@ void TestConfigurationPolicyProviderWin::SetHomepageIsNewTabPage(
   RegKey key(hive,
       ConfigurationPolicyProviderWin::kPolicyRegistrySubKey,
       KEY_ALL_ACCESS);
-  EXPECT_TRUE(key.WriteValue(
-      ConfigurationPolicyProviderWin::kHomepageIsNewTabPageRegistryValueName,
+  EXPECT_TRUE(key.WriteValue(NameForPolicy(
+      ConfigurationPolicyStore::kPolicyHomepageIsNewTabPage).c_str(),
       value));
 }
 
@@ -74,7 +98,7 @@ void TestConfigurationPolicyProviderWin::SetCookiesMode(
       ConfigurationPolicyProviderWin::kPolicyRegistrySubKey,
       KEY_ALL_ACCESS);
   EXPECT_TRUE(key.WriteValue(
-      ConfigurationPolicyProviderWin::kCookiesModeRegistryValueName,
+      NameForPolicy(ConfigurationPolicyStore::kPolicyCookiesMode).c_str(),
       value));
 }