Preference provider implementation backed by JSON files in a directory.

BUG=42412
TEST=Unit tests in chrome/browser/value_tree_policy_decoder.cc and base/values_unittest.cc

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

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

11 files changed, 372 insertions, 5 deletions

diff --git a/base/values.cc b/base/values.cc
index feff299..507646c 100644
--- a/base/values.cc
+++ b/base/values.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2009 The Chromium Authors. All rights reserved.
+// Copyright (c) 2010 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.
 
@@ -674,6 +674,26 @@ DictionaryValue* DictionaryValue::DeepCopyWithoutEmptyChildren() {
   return copy ? static_cast<DictionaryValue*>(copy) : new DictionaryValue;
 }
 
+void DictionaryValue::MergeDictionary(const DictionaryValue* dictionary) {
+  for (DictionaryValue::key_iterator key(dictionary->begin_keys());
+       key != dictionary->end_keys(); ++key) {
+    Value* merge_value;
+    if (dictionary->GetWithoutPathExpansion(*key, &merge_value)) {
+      // Check whether we have to merge dictionaries.
+      if (merge_value->IsType(Value::TYPE_DICTIONARY)) {
+        DictionaryValue* sub_dict;
+        if (GetDictionaryWithoutPathExpansion(*key, &sub_dict)) {
+          sub_dict->MergeDictionary(
+              static_cast<const DictionaryValue*>(merge_value));
+          continue;
+        }
+      }
+      // All other cases: Make a copy and hook it up.
+      SetWithoutPathExpansion(*key, merge_value->DeepCopy());
+    }
+  }
+}
+
 ///////////////////// ListValue ////////////////////
 
 ListValue::~ListValue() {
diff --git a/base/values.h b/base/values.h
index 248fc1d..5c0ea33 100644
--- a/base/values.h
+++ b/base/values.h
@@ -312,6 +312,12 @@ class DictionaryValue : public Value {
   // the copy.  This never returns NULL, even if |this| itself is empty.
   DictionaryValue* DeepCopyWithoutEmptyChildren();
 
+  // Merge a given dictionary into this dictionary. This is done recursively,
+  // i.e. any subdictionaries will be merged as well. In case of key collisions,
+  // the passed in dictionary takes precedence and data already present will be
+  // replaced.
+  void MergeDictionary(const DictionaryValue* dictionary);
+
   // This class provides an iterator for the keys in the dictionary.
   // It can't be used to modify the dictionary.
   //
diff --git a/base/values_unittest.cc b/base/values_unittest.cc
index a0ab47f..8aa8359 100644
--- a/base/values_unittest.cc
+++ b/base/values_unittest.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2009 The Chromium Authors. All rights reserved.
+// Copyright (c) 2010 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.
 
@@ -592,3 +592,48 @@ TEST(ValuesTest, RemoveEmptyChildren) {
     EXPECT_EQ(1U, inner2->GetSize());
   }
 }
+
+TEST(ValuesTest, MergeDictionary) {
+  scoped_ptr<DictionaryValue> base(new DictionaryValue);
+  base->SetString(L"base_key", "base_key_value_base");
+  base->SetString(L"collide_key", "collide_key_value_base");
+  DictionaryValue* base_sub_dict = new DictionaryValue;
+  base_sub_dict->SetString(L"sub_base_key", "sub_base_key_value_base");
+  base_sub_dict->SetString(L"sub_collide_key", "sub_collide_key_value_base");
+  base->Set(L"sub_dict_key", base_sub_dict);
+
+  scoped_ptr<DictionaryValue> merge(new DictionaryValue);
+  merge->SetString(L"merge_key", "merge_key_value_merge");
+  merge->SetString(L"collide_key", "collide_key_value_merge");
+  DictionaryValue* merge_sub_dict = new DictionaryValue;
+  merge_sub_dict->SetString(L"sub_merge_key", "sub_merge_key_value_merge");
+  merge_sub_dict->SetString(L"sub_collide_key", "sub_collide_key_value_merge");
+  merge->Set(L"sub_dict_key", merge_sub_dict);
+
+  base->MergeDictionary(merge.get());
+
+  EXPECT_EQ(4U, base->size());
+  std::string base_key_value;
+  EXPECT_TRUE(base->GetString(L"base_key", &base_key_value));
+  EXPECT_EQ("base_key_value_base", base_key_value); // Base value preserved.
+  std::string collide_key_value;
+  EXPECT_TRUE(base->GetString(L"collide_key", &collide_key_value));
+  EXPECT_EQ("collide_key_value_merge", collide_key_value); // Replaced.
+  std::string merge_key_value;
+  EXPECT_TRUE(base->GetString(L"merge_key", &merge_key_value));
+  EXPECT_EQ("merge_key_value_merge", merge_key_value); // Merged in.
+
+  DictionaryValue* res_sub_dict;
+  EXPECT_TRUE(base->GetDictionary(L"sub_dict_key", &res_sub_dict));
+  EXPECT_EQ(3U, res_sub_dict->size());
+  std::string sub_base_key_value;
+  EXPECT_TRUE(res_sub_dict->GetString(L"sub_base_key", &sub_base_key_value));
+  EXPECT_EQ("sub_base_key_value_base", sub_base_key_value); // Preserved.
+  std::string sub_collide_key_value;
+  EXPECT_TRUE(res_sub_dict->GetString(L"sub_collide_key",
+                                      &sub_collide_key_value));
+  EXPECT_EQ("sub_collide_key_value_merge", sub_collide_key_value); // Replaced.
+  std::string sub_merge_key_value;
+  EXPECT_TRUE(res_sub_dict->GetString(L"sub_merge_key", &sub_merge_key_value));
+  EXPECT_EQ("sub_merge_key_value_merge", sub_merge_key_value); // Merged in.
+}
diff --git a/chrome/browser/config_dir_policy_provider.cc b/chrome/browser/config_dir_policy_provider.cc
new file mode 100644
index 0000000..d791e66
--- /dev/null
+++ b/chrome/browser/config_dir_policy_provider.cc
@@ -0,0 +1,56 @@
+// Copyright (c) 2010 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.
+
+#include "chrome/browser/config_dir_policy_provider.h"
+
+#include <set>
+
+#include "base/file_util.h"
+#include "base/logging.h"
+#include "base/scoped_ptr.h"
+#include "base/values.h"
+#include "chrome/common/json_value_serializer.h"
+
+ConfigDirPolicyProvider::ConfigDirPolicyProvider(const FilePath& config_dir)
+    : config_dir_(config_dir) {
+}
+
+bool ConfigDirPolicyProvider::Provide(ConfigurationPolicyStore* store) {
+  scoped_ptr<DictionaryValue> policy(ReadPolicies());
+  DecodePolicyValueTree(policy.get(), store);
+  return true;
+}
+
+DictionaryValue* ConfigDirPolicyProvider::ReadPolicies() {
+  // Enumerate the files and sort them lexicographically.
+  std::set<FilePath> files;
+  file_util::FileEnumerator file_enumerator(config_dir_, false,
+                                            file_util::FileEnumerator::FILES);
+  for (FilePath config_file_path = file_enumerator.Next();
+       !config_file_path.empty(); config_file_path = file_enumerator.Next())
+    files.insert(config_file_path);
+
+  // Start with an empty dictionary and merge the files' contents.
+  DictionaryValue* policy = new DictionaryValue;
+  for (std::set<FilePath>::iterator config_file_iter = files.begin();
+       config_file_iter != files.end(); ++config_file_iter) {
+    JSONFileValueSerializer deserializer(*config_file_iter);
+    int error_code = 0;
+    std::string error_msg;
+    scoped_ptr<Value> value(deserializer.Deserialize(&error_code, &error_msg));
+    if (!value.get()) {
+      LOG(WARNING) << "Failed to read configuration file "
+                   << config_file_iter->value() << ": " << error_msg;
+      continue;
+    }
+    if (!value->IsType(Value::TYPE_DICTIONARY)) {
+      LOG(WARNING) << "Expected JSON dictionary in configuration file "
+                   << config_file_iter->value();
+      continue;
+    }
+    policy->MergeDictionary(static_cast<DictionaryValue*>(value.get()));
+  }
+
+  return policy;
+}
diff --git a/chrome/browser/config_dir_policy_provider.h b/chrome/browser/config_dir_policy_provider.h
new file mode 100644
index 0000000..05bffd0
--- /dev/null
+++ b/chrome/browser/config_dir_policy_provider.h
@@ -0,0 +1,38 @@
+// Copyright (c) 2010 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_CONFIG_DIR_POLICY_PROVIDER_H_
+#define CHROME_BROWSER_CONFIG_DIR_POLICY_PROVIDER_H_
+
+#include "base/basictypes.h"
+#include "base/file_path.h"
+#include "chrome/browser/configuration_policy_provider.h"
+
+class DictionaryValue;
+
+// A policy provider implementation backed by a set of files in a given
+// directory. The files should contain JSON-formatted policy settings. They are
+// merged together and the result is returned via the
+// ConfigurationPolicyProvider interface. The files are consulted in
+// lexicographic file name order, so the last value read takes precedence in
+// case of preference key collisions.
+class ConfigDirPolicyProvider : public ConfigurationPolicyProvider {
+ public:
+  explicit ConfigDirPolicyProvider(const FilePath& config_dir);
+  virtual ~ConfigDirPolicyProvider() { }
+
+  // ConfigurationPolicyProvider implementation.
+  virtual bool Provide(ConfigurationPolicyStore* store);
+
+ private:
+  // Read and merge the files from the configuration directory.
+  DictionaryValue* ReadPolicies();
+
+  // The directory in which we look for configuration files.
+  const FilePath config_dir_;
+
+  DISALLOW_COPY_AND_ASSIGN(ConfigDirPolicyProvider);
+};
+
+#endif  // CHROME_BROWSER_CONFIG_DIR_POLICY_PROVIDER_H_
diff --git a/chrome/browser/config_dir_policy_provider_unittest.cc b/chrome/browser/config_dir_policy_provider_unittest.cc
new file mode 100644
index 0000000..b3d4a27
--- /dev/null
+++ b/chrome/browser/config_dir_policy_provider_unittest.cc
@@ -0,0 +1,115 @@
+// Copyright (c) 2010 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.
+
+#include "base/file_util.h"
+#include "base/path_service.h"
+#include "base/string_util.h"
+#include "chrome/browser/config_dir_policy_provider.h"
+#include "chrome/browser/mock_configuration_policy_store.h"
+#include "chrome/common/json_value_serializer.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+class ConfigDirPolicyProviderTest : public testing::Test {
+ protected:
+  virtual void SetUp() {
+    // Determine the directory to use for testing.
+    ASSERT_TRUE(PathService::Get(base::DIR_TEMP, &test_dir_));
+    test_dir_ =
+        test_dir_.Append(FILE_PATH_LITERAL("ConfigDirPolicyProviderTest"));
+
+    // Make sure the directory is fresh.
+    file_util::Delete(test_dir_, true);
+    file_util::CreateDirectory(test_dir_);
+    ASSERT_TRUE(file_util::DirectoryExists(test_dir_));
+
+    // Create a fresh policy store mock.
+    policy_store_.reset(new MockConfigurationPolicyStore());
+  }
+
+  virtual void TearDown() {
+    // Clean up test directory.
+    ASSERT_TRUE(file_util::Delete(test_dir_, true));
+    ASSERT_FALSE(file_util::PathExists(test_dir_));
+  }
+
+  // JSON-encode a dictionary and write it to a file.
+  void WriteConfigFile(const DictionaryValue& dict,
+                       const std::string& file_name) {
+    std::string data;
+    JSONStringValueSerializer serializer(&data);
+    serializer.Serialize(dict);
+    FilePath file_path(test_dir_.AppendASCII(file_name));
+    file_util::WriteFile(file_path, data.c_str(), data.size());
+  }
+
+  FilePath test_dir_;
+  scoped_ptr<MockConfigurationPolicyStore> policy_store_;
+};
+
+// The preferences dictionary is expected to be empty when there are no files to
+// load.
+TEST_F(ConfigDirPolicyProviderTest, ReadPrefsEmpty) {
+  ConfigDirPolicyProvider provider(test_dir_);
+  EXPECT_TRUE(provider.Provide(policy_store_.get()));
+  EXPECT_TRUE(policy_store_->policy_map().empty());
+}
+
+// Reading from a non-existent directory should result in an empty preferences
+// dictionary.
+TEST_F(ConfigDirPolicyProviderTest, ReadPrefsNonExistentDirectory) {
+  FilePath non_existent_dir(test_dir_.Append(FILE_PATH_LITERAL("not_there")));
+  ConfigDirPolicyProvider provider(non_existent_dir);
+  EXPECT_TRUE(provider.Provide(policy_store_.get()));
+  EXPECT_TRUE(policy_store_->policy_map().empty());
+}
+
+// Test reading back a single preference value.
+TEST_F(ConfigDirPolicyProviderTest, ReadPrefsSinglePref) {
+  DictionaryValue test_dict;
+  test_dict.SetString(L"homepage", L"http://www.google.com");
+  WriteConfigFile(test_dict, "config_file");
+  ConfigDirPolicyProvider provider(test_dir_);
+
+  EXPECT_TRUE(provider.Provide(policy_store_.get()));
+  const MockConfigurationPolicyStore::PolicyMap& policy_map(
+      policy_store_->policy_map());
+  EXPECT_EQ(1U, policy_map.size());
+  MockConfigurationPolicyStore::PolicyMap::const_iterator entry =
+      policy_map.find(ConfigurationPolicyStore::kPolicyHomePage);
+  ASSERT_TRUE(entry != policy_map.end());
+
+  std::wstring str_value;
+  EXPECT_TRUE(entry->second->GetAsString(&str_value));
+  EXPECT_EQ(L"http://www.google.com", str_value);
+}
+
+// Test merging values from different files.
+TEST_F(ConfigDirPolicyProviderTest, ReadPrefsMergePrefs) {
+  // Write a bunch of data files in order to increase the chance to detect the
+  // provider not respecting lexicographic ordering when reading them. Since the
+  // 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");
+  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");
+  WriteConfigFile(test_dict_foo, "9");
+  for (unsigned int i = 5; i <= 8; ++i)
+    WriteConfigFile(test_dict_bar, IntToString(i));
+  ConfigDirPolicyProvider provider(test_dir_);
+
+  EXPECT_TRUE(provider.Provide(policy_store_.get()));
+  const MockConfigurationPolicyStore::PolicyMap& policy_map(
+      policy_store_->policy_map());
+  EXPECT_EQ(1U, policy_map.size());
+  MockConfigurationPolicyStore::PolicyMap::const_iterator entry =
+      policy_map.find(ConfigurationPolicyStore::kPolicyHomePage);
+  ASSERT_TRUE(entry != policy_map.end());
+
+  std::wstring str_value;
+  EXPECT_TRUE(entry->second->GetAsString(&str_value));
+  EXPECT_EQ(L"http://foo.com", str_value);
+}
diff --git a/chrome/browser/configuration_policy_provider.cc b/chrome/browser/configuration_policy_provider.cc
new file mode 100644
index 0000000..1b9a5a8
--- /dev/null
+++ b/chrome/browser/configuration_policy_provider.cc
@@ -0,0 +1,40 @@
+// Copyright (c) 2010 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.
+
+#include "chrome/browser/configuration_policy_provider.h"
+
+#include "base/values.h"
+#include "chrome/browser/configuration_policy_store.h"
+
+namespace {
+
+struct PolicyValueTreeMappingEntry {
+  ConfigurationPolicyStore::PolicyType policy_type;
+  Value::ValueType value_type;
+  const wchar_t* const name;
+};
+
+const PolicyValueTreeMappingEntry kPolicyValueTreeMapping[] = {
+  { ConfigurationPolicyStore::kPolicyHomePage,
+    Value::TYPE_STRING, L"homepage" },
+  { ConfigurationPolicyStore::kPolicyHomepageIsNewTabPage,
+    Value::TYPE_BOOLEAN, L"homepage_is_newtabpage" },
+  { ConfigurationPolicyStore::kPolicyCookiesEnabled,
+    Value::TYPE_BOOLEAN, L"cookies_enabled" }
+};
+
+}
+
+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());
+  }
+
+  // TODO (mnissler) Handle preference overrides once |ConfigurationPolicyStore|
+  // supports it.
+}
diff --git a/chrome/browser/configuration_policy_provider.h b/chrome/browser/configuration_policy_provider.h
index cae73f6..daf5cb4 100644
--- a/chrome/browser/configuration_policy_provider.h
+++ b/chrome/browser/configuration_policy_provider.h
@@ -5,21 +5,24 @@
 #ifndef CHROME_BROWSER_CONFIGURATION_POLICY_PROVIDER_H_
 #define CHROME_BROWSER_CONFIGURATION_POLICY_PROVIDER_H_
 
+#include "base/basictypes.h"
+
 class ConfigurationPolicyStore;
+class DictionaryValue;
 
 // An 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 {
  public:
-  ConfigurationPolicyProvider();
+  ConfigurationPolicyProvider() {}
   virtual ~ConfigurationPolicyProvider() {}
 
   // Must be implemented by provider subclasses to specify the
   // provider-specific policy decisions. The preference service
   // invokes this |Provide| method when it needs a policy
   // provider to specify its policy choices. In |Provide|,
-  // the |ConfigurationPolicyProvider| must make calls to the
+  // the ConfigurationPolicyProvider must make calls to the
   // |Apply| method of |store| to apply specific policies.
   // Returns true if the policy could be provided, otherwise false.
   virtual bool Provide(ConfigurationPolicyStore* store) = 0;
@@ -28,5 +31,10 @@ class ConfigurationPolicyProvider {
   DISALLOW_COPY_AND_ASSIGN(ConfigurationPolicyProvider);
 };
 
-#endif  // CHROME_BROWSER_CONFIGURATION_POLICY_PROVIDER_H_
+// 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/mock_configuration_policy_store.h b/chrome/browser/mock_configuration_policy_store.h
new file mode 100644
index 0000000..b1f3696
--- /dev/null
+++ b/chrome/browser/mock_configuration_policy_store.h
@@ -0,0 +1,34 @@
+// Copyright (c) 2010 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_MOCK_CONFIGURATION_POLICY_STORE_H_
+#define CHROME_BROWSER_MOCK_CONFIGURATION_POLICY_STORE_H_
+
+#include <map>
+
+#include "base/stl_util-inl.h"
+#include "chrome/browser/configuration_policy_store.h"
+
+// Mock ConfigurationPolicyStore implementation that records values for policy
+// settings as they get set.
+class MockConfigurationPolicyStore : public ConfigurationPolicyStore {
+ public:
+  MockConfigurationPolicyStore() {}
+  ~MockConfigurationPolicyStore() {
+    STLDeleteValues(&policy_map_);
+  }
+
+  typedef std::map<ConfigurationPolicyStore::PolicyType, Value*> PolicyMap;
+  const PolicyMap& policy_map() { return policy_map_; }
+
+  // ConfigurationPolicyStore implementation.
+  virtual void Apply(PolicyType policy, Value* value) {
+    policy_map_[policy] = value;
+  }
+
+ private:
+  PolicyMap policy_map_;
+};
+
+#endif  // CHROME_BROWSER_MOCK_CONFIGURATION_POLICY_STORE_H_
diff --git a/chrome/chrome_browser.gypi b/chrome/chrome_browser.gypi
index 19e1655..b36ab4e 100644
--- a/chrome/chrome_browser.gypi
+++ b/chrome/chrome_browser.gypi
@@ -855,6 +855,8 @@
         'browser/cocoa/window_size_autosaver.mm',
         'browser/command_updater.cc',
         'browser/command_updater.h',
+        'browser/config_dir_policy_provider.cc',
+        'browser/config_dir_policy_provider.h',
         'browser/content_exceptions_table_model.cc',
         'browser/content_exceptions_table_model.h',
         'browser/content_setting_bubble_model.cc',
@@ -875,6 +877,7 @@
         'browser/configuration_policy_pref_store.cc',
         'browser/configuration_policy_pref_store.h',
         'browser/configuration_policy_provider.h',
+        'browser/configuration_policy_provider.cc',
         'browser/cross_site_request_manager.cc',
         'browser/cross_site_request_manager.h',
         'browser/custom_home_pages_table_model.cc',
diff --git a/chrome/chrome_tests.gypi b/chrome/chrome_tests.gypi
index b1b18a1..226c2c1 100644
--- a/chrome/chrome_tests.gypi
+++ b/chrome/chrome_tests.gypi
@@ -755,6 +755,7 @@
         'browser/cocoa/view_resizer_pong.mm',
         'browser/cocoa/web_drop_target_unittest.mm',
         'browser/cocoa/window_size_autosaver_unittest.mm',
+        'browser/config_dir_policy_provider_unittest.cc',
         'browser/command_updater_unittest.cc',
         'browser/configuration_policy_pref_store_unittest.cc',
         'browser/cookies_tree_model_unittest.cc',
@@ -846,6 +847,7 @@
         'browser/metrics/metrics_log_unittest.cc',
         'browser/metrics/metrics_response_unittest.cc',
         'browser/metrics/metrics_service_unittest.cc',
+        'browser/mock_configuration_policy_store.h',
         'browser/net/chrome_url_request_context_unittest.cc',
         'browser/net/connection_tester_unittest.cc',
         'browser/net/dns_host_info_unittest.cc',