Refactored StringToIntEnumListPolicyHandler to be more general purpose

Created new StringMappingListPolicyHandler class to provide general
mapping between policy string lists and generic pref types (int lists,
string lists, etc).

BUG=none

Review URL: https://codereview.chromium.org/341723002

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

chrome/browser/policy/configuration_policy_handler_list_factory.cc

@@ -484,15 +484,29 @@ const PolicyToPreferenceMapEntry kSimplePolicyMap[] = {
 };
 
 #if !defined(OS_IOS)
-// Mapping from extension type names to Manifest::Type.
-StringToIntEnumListPolicyHandler::MappingEntry kExtensionAllowedTypesMap[] = {
-  { "extension", extensions::Manifest::TYPE_EXTENSION },
-  { "theme", extensions::Manifest::TYPE_THEME },
-  { "user_script", extensions::Manifest::TYPE_USER_SCRIPT },
-  { "hosted_app", extensions::Manifest::TYPE_HOSTED_APP },
-  { "legacy_packaged_app", extensions::Manifest::TYPE_LEGACY_PACKAGED_APP },
-  { "platform_app", extensions::Manifest::TYPE_PLATFORM_APP },
-};
+void GetExtensionAllowedTypesMap(
+    ScopedVector<StringMappingListPolicyHandler::MappingEntry>* result) {
+  // Mapping from extension type names to Manifest::Type.
+  result->push_back(new StringMappingListPolicyHandler::MappingEntry(
+      "extension", scoped_ptr<base::Value>(base::Value::CreateIntegerValue(
+          extensions::Manifest::TYPE_EXTENSION))));
+  result->push_back(new StringMappingListPolicyHandler::MappingEntry(
+      "theme", scoped_ptr<base::Value>(base::Value::CreateIntegerValue(
+          extensions::Manifest::TYPE_THEME))));
+  result->push_back(new StringMappingListPolicyHandler::MappingEntry(
+      "user_script", scoped_ptr<base::Value>(base::Value::CreateIntegerValue(
+          extensions::Manifest::TYPE_USER_SCRIPT))));
+  result->push_back(new StringMappingListPolicyHandler::MappingEntry(
+      "hosted_app", scoped_ptr<base::Value>(base::Value::CreateIntegerValue(
+          extensions::Manifest::TYPE_HOSTED_APP))));
+  result->push_back(new StringMappingListPolicyHandler::MappingEntry(
+      "legacy_packaged_app", scoped_ptr<base::Value>(
+          base::Value::CreateIntegerValue(
+              extensions::Manifest::TYPE_LEGACY_PACKAGED_APP))));
+  result->push_back(new StringMappingListPolicyHandler::MappingEntry(
+      "platform_app", scoped_ptr<base::Value>(base::Value::CreateIntegerValue(
+          extensions::Manifest::TYPE_PLATFORM_APP))));
+}
 #endif  // !defined(OS_IOS)
 
 }  // namespace
@@ -560,11 +574,10 @@ scoped_ptr<ConfigurationPolicyHandlerList> BuildHandlerList(
           key::kExtensionInstallSources,
           extensions::pref_names::kAllowedInstallSites)));
   handlers->AddHandler(make_scoped_ptr<ConfigurationPolicyHandler>(
-      new StringToIntEnumListPolicyHandler(
+      new StringMappingListPolicyHandler(
           key::kExtensionAllowedTypes,
           extensions::pref_names::kAllowedTypes,
-          kExtensionAllowedTypesMap,
-          kExtensionAllowedTypesMap + arraysize(kExtensionAllowedTypesMap))));
+          base::Bind(GetExtensionAllowedTypesMap))));
 #endif  // !defined(OS_IOS)
 
 #if !defined(OS_CHROMEOS) && !defined(OS_ANDROID) && !defined(OS_IOS)

components/policy/core/browser/configuration_policy_handler.cc

@@ -151,19 +151,25 @@ bool IntRangePolicyHandlerBase::EnsureInRange(const base::Value* input,
 }
 
 
-// StringToIntEnumListPolicyHandler implementation -----------------------------
+// StringMappingListPolicyHandler implementation -----------------------------
 
-StringToIntEnumListPolicyHandler::StringToIntEnumListPolicyHandler(
+StringMappingListPolicyHandler::MappingEntry::MappingEntry(
+    const char* policy_value, scoped_ptr<base::Value> map)
+    : enum_value(policy_value), mapped_value(map.Pass()) {}
+
+StringMappingListPolicyHandler::MappingEntry::~MappingEntry() {}
+
+StringMappingListPolicyHandler::StringMappingListPolicyHandler(
     const char* policy_name,
     const char* pref_path,
-    const MappingEntry* mapping_begin,
-    const MappingEntry* mapping_end)
+    const GenerateMapCallback& callback)
     : TypeCheckingPolicyHandler(policy_name, base::Value::TYPE_LIST),
       pref_path_(pref_path),
-      mapping_begin_(mapping_begin),
-      mapping_end_(mapping_end) {}
+      map_getter_(callback) {}
+
+StringMappingListPolicyHandler::~StringMappingListPolicyHandler() {}
 
-bool StringToIntEnumListPolicyHandler::CheckPolicySettings(
+bool StringMappingListPolicyHandler::CheckPolicySettings(
     const PolicyMap& policies,
     PolicyErrorMap* errors) {
   const base::Value* value;
@@ -171,7 +177,7 @@ bool StringToIntEnumListPolicyHandler::CheckPolicySettings(
       Convert(value, NULL, errors);
 }
 
-void StringToIntEnumListPolicyHandler::ApplyPolicySettings(
+void StringMappingListPolicyHandler::ApplyPolicySettings(
     const PolicyMap& policies,
     PrefValueMap* prefs) {
   if (!pref_path_)
@@ -182,9 +188,9 @@ void StringToIntEnumListPolicyHandler::ApplyPolicySettings(
     prefs->SetValue(pref_path_, list.release());
 }
 
-bool StringToIntEnumListPolicyHandler::Convert(const base::Value* input,
-                                               base::ListValue* output,
-                                               PolicyErrorMap* errors) {
+bool StringMappingListPolicyHandler::Convert(const base::Value* input,
+                                             base::ListValue* output,
+                                             PolicyErrorMap* errors) {
   if (!input)
     return true;
 
@@ -206,17 +212,12 @@ bool StringToIntEnumListPolicyHandler::Convert(const base::Value* input,
       }
       continue;
     }
-    bool found = false;
-    for (const MappingEntry* mapping_entry(mapping_begin_);
-         mapping_entry != mapping_end_; ++mapping_entry) {
-      if (mapping_entry->enum_value == entry_value) {
-        found = true;
-        if (output)
-          output->AppendInteger(mapping_entry->int_value);
-        break;
-      }
-    }
-    if (!found) {
+
+    scoped_ptr<base::Value> mapped_value = Map(entry_value);
+    if (mapped_value) {
+      if (output)
+        output->Append(mapped_value.release());
+    } else {
       if (errors) {
         errors->AddError(policy_name(),
                          entry - list_value->begin(),
@@ -228,6 +229,23 @@ bool StringToIntEnumListPolicyHandler::Convert(const base::Value* input,
   return true;
 }
 
+scoped_ptr<base::Value> StringMappingListPolicyHandler::Map(
+    const std::string& entry_value) {
+  // Lazily generate the map of policy strings to mapped values.
+  if (map_.empty())
+    map_getter_.Run(&map_);
+
+  scoped_ptr<base::Value> return_value;
+  for (ScopedVector<MappingEntry>::const_iterator it = map_.begin();
+       it != map_.end(); ++it) {
+    const MappingEntry* mapping_entry = *it;
+    if (mapping_entry->enum_value == entry_value) {
+      return_value = make_scoped_ptr(mapping_entry->mapped_value->DeepCopy());
+      break;
+    }
+  }
+  return return_value.Pass();
+}
 
 // IntRangePolicyHandler implementation ----------------------------------------
 
@@ -251,7 +269,7 @@ void IntRangePolicyHandler::ApplyPolicySettings(const PolicyMap& policies,
   int value_in_range;
   if (value && EnsureInRange(value, &value_in_range, NULL)) {
     prefs->SetValue(pref_path_,
-                    base::Value::CreateIntegerValue(value_in_range));
+                    new base::FundamentalValue(value_in_range));
   }
 }
 

components/policy/core/browser/configuration_policy_handler.h

@@ -9,6 +9,7 @@
 #include <vector>
 
 #include "base/basictypes.h"
+#include "base/callback.h"
 #include "base/compiler_specific.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/scoped_vector.h"
@@ -159,20 +160,29 @@ class POLICY_EXPORT SimplePolicyHandler : public TypeCheckingPolicyHandler {
   DISALLOW_COPY_AND_ASSIGN(SimplePolicyHandler);
 };
 
-// A policy handler implementation that maps a string enum list to an int enum
-// list as specified by a mapping table.
-class POLICY_EXPORT StringToIntEnumListPolicyHandler
+// Base class that encapsulates logic for mapping from a string enum list
+// to a separate matching type value.
+class POLICY_EXPORT StringMappingListPolicyHandler
     : public TypeCheckingPolicyHandler {
  public:
-  struct POLICY_EXPORT MappingEntry {
+  // Data structure representing the map between policy strings and
+  // matching pref values.
+  class POLICY_EXPORT MappingEntry {
+   public:
+    MappingEntry(const char* policy_value, scoped_ptr<base::Value> map);
+    ~MappingEntry();
+
     const char* enum_value;
-    int int_value;
+    scoped_ptr<base::Value> mapped_value;
   };
 
-  StringToIntEnumListPolicyHandler(const char* policy_name,
-                                   const char* pref_path,
-                                   const MappingEntry* mapping_begin,
-                                   const MappingEntry* mapping_end);
+  // Callback that generates the map for this instance.
+  typedef base::Callback<void(ScopedVector<MappingEntry>*)> GenerateMapCallback;
+
+  StringMappingListPolicyHandler(const char* policy_name,
+                                 const char* pref_path,
+                                 const GenerateMapCallback& map_generator);
+  virtual ~StringMappingListPolicyHandler();
 
   // ConfigurationPolicyHandler methods:
   virtual bool CheckPolicySettings(const PolicyMap& policies,
@@ -187,14 +197,21 @@ class POLICY_EXPORT StringToIntEnumListPolicyHandler
                base::ListValue* output,
                PolicyErrorMap* errors);
 
+  // Helper method that converts from a policy value string to the associated
+  // pref value.
+  scoped_ptr<base::Value> Map(const std::string& entry_value);
+
   // Name of the pref to write.
   const char* pref_path_;
 
-  // The mapping table.
-  const MappingEntry* mapping_begin_;
-  const MappingEntry* mapping_end_;
+  // The callback invoked to generate the map for this instance.
+  GenerateMapCallback map_getter_;
+
+  // Map of string policy values to local pref values. This is generated lazily
+  // so the generation does not have to happen if no policy is present.
+  ScopedVector<MappingEntry> map_;
 
-  DISALLOW_COPY_AND_ASSIGN(StringToIntEnumListPolicyHandler);
+  DISALLOW_COPY_AND_ASSIGN(StringMappingListPolicyHandler);
 };
 
 // A policy handler implementation that ensures an int policy's value lies in an

components/policy/core/browser/configuration_policy_handler_unittest.cc

@@ -2,6 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "base/bind.h"
 #include "base/callback.h"
 #include "base/json/json_reader.h"
 #include "base/memory/scoped_ptr.h"
@@ -17,10 +18,13 @@ namespace policy {
 
 namespace {
 
-StringToIntEnumListPolicyHandler::MappingEntry kTestTypeMap[] = {
-  { "one", 1 },
-  { "two", 2 },
-};
+void GetIntegerTypeMap(
+    ScopedVector<StringMappingListPolicyHandler::MappingEntry>* result) {
+  result->push_back(new StringMappingListPolicyHandler::MappingEntry(
+      "one", scoped_ptr<base::Value>(new base::FundamentalValue(1))));
+  result->push_back(new StringMappingListPolicyHandler::MappingEntry(
+      "two", scoped_ptr<base::Value>(new base::FundamentalValue(2))));
+}
 
 const char kTestPolicy[] = "unit_test.test_policy";
 const char kTestPref[] = "unit_test.test_pref";
@@ -49,11 +53,10 @@ TEST(StringToIntEnumListPolicyHandlerTest, CheckPolicySettings) {
   base::ListValue list;
   PolicyMap policy_map;
   PolicyErrorMap errors;
-  StringToIntEnumListPolicyHandler handler(
+  StringMappingListPolicyHandler handler(
       kTestPolicy,
       kTestPref,
-      kTestTypeMap,
-      kTestTypeMap + arraysize(kTestTypeMap));
+      base::Bind(GetIntegerTypeMap));
 
   policy_map.Set(kTestPolicy, POLICY_LEVEL_MANDATORY,
                  POLICY_SCOPE_USER, list.DeepCopy(), NULL);
@@ -85,17 +88,16 @@ TEST(StringToIntEnumListPolicyHandlerTest, CheckPolicySettings) {
   EXPECT_FALSE(errors.GetErrors(kTestPolicy).empty());
 }
 
-TEST(StringToIntEnumListPolicyHandlerTest, ApplyPolicySettings) {
+TEST(StringMappingListPolicyHandlerTest, ApplyPolicySettings) {
   base::ListValue list;
   base::ListValue expected;
   PolicyMap policy_map;
   PrefValueMap prefs;
   base::Value* value;
-  StringToIntEnumListPolicyHandler handler(
+  StringMappingListPolicyHandler handler(
       kTestPolicy,
       kTestPref,
-      kTestTypeMap,
-      kTestTypeMap + arraysize(kTestTypeMap));
+      base::Bind(GetIntegerTypeMap));
 
   policy_map.Set(kTestPolicy, POLICY_LEVEL_MANDATORY,
                  POLICY_SCOPE_USER, list.DeepCopy(), NULL);