Implement the ConfigDirPolicyProvider based on the AsyncPolicyLoader

chrome/browser/policy/config_dir_policy_provider.cc

Index: chrome/browser/policy/config_dir_policy_provider.cc
diff --git a/chrome/browser/policy/config_dir_policy_provider.cc b/chrome/browser/policy/config_dir_policy_provider.cc
index c86fa5093645c1bf06a131b1ad9fe1b2319b0de2..debc126cf28e088f689224dbe2b8363d854f2625 100644
--- a/chrome/browser/policy/config_dir_policy_provider.cc
+++ b/chrome/browser/policy/config_dir_policy_provider.cc
@@ -18,18 +18,64 @@
 
 namespace policy {
 
+namespace {
+
+// Subdirectories of the ConfigDirPolicyProvider config dir that contain
+// managed and recommended policies.
+const char kManagedPath[] = "managed";
+const char kRecommendedPath[] = "recommended";
+
+}  // namespace
+
 ConfigDirPolicyProviderDelegate::ConfigDirPolicyProviderDelegate(
     const FilePath& config_dir,
-    PolicyLevel level,
     PolicyScope scope)
     : FileBasedPolicyProvider::ProviderDelegate(config_dir),
-      level_(level),
       scope_(scope) {}
 
 scoped_ptr<PolicyBundle> ConfigDirPolicyProviderDelegate::Load() {
+  scoped_ptr<PolicyBundle> bundle(new PolicyBundle());
+  LoadFromPath(config_file_path().Append(FILE_PATH_LITERAL(kManagedPath)),
+               POLICY_LEVEL_MANDATORY,
+               bundle.get());
+  LoadFromPath(config_file_path().Append(FILE_PATH_LITERAL(kRecommendedPath)),
+               POLICY_LEVEL_RECOMMENDED,
+               bundle.get());
+  return bundle.Pass();
+}
+
+base::Time ConfigDirPolicyProviderDelegate::GetLastModification() {
+  base::Time last_modification = base::Time();
+  base::PlatformFileInfo file_info;
+
+  // If the path does not exist or points to a directory, it's safe to load.

[Mattias Nissler (ping if slow), 2012/05/31 13:58:09]

should be "doesn't point to a directory", no?

[Joao da Silva, 2012/06/03 15:02:01]

I didn't even see this; this diff is smaller, but the change was to split Load()
into LoadFromPath(), which was moved to after this function to keep the
declaration order.

I've fixed this comment anyway :-)

+  if (!file_util::GetFileInfo(config_file_path(), &file_info) ||

[Mattias Nissler (ping if slow), 2012/05/31 13:58:09]

You need to do that on both of the directories. Have you tested this?
Continuously writing the policy file should stop Chrome from reloading. Until
you don't touch the file for 5 seconds.

[Joao da Silva, 2012/06/03 15:02:01]

Watching both paths after rebasing on AsyncPolicyLoader.

+      !file_info.is_directory) {
+    return last_modification;
+  }
+
+  // Enumerate the files and find the most recent modification timestamp.
+  file_util::FileEnumerator file_enumerator(config_file_path(),
+                                            false,
+                                            file_util::FileEnumerator::FILES);
+  for (FilePath config_file = file_enumerator.Next();
+       !config_file.empty();
+       config_file = file_enumerator.Next()) {
+    if (file_util::GetFileInfo(config_file, &file_info) &&
+        !file_info.is_directory) {
+      last_modification = std::max(last_modification, file_info.last_modified);
+    }
+  }
+
+  return last_modification;
+}
+
+void ConfigDirPolicyProviderDelegate::LoadFromPath(const FilePath& path,
+                                                   PolicyLevel level,
+                                                   PolicyBundle* bundle) {
   // Enumerate the files and sort them lexicographically.
   std::set<FilePath> files;
-  file_util::FileEnumerator file_enumerator(config_file_path(), false,
+  file_util::FileEnumerator file_enumerator(path, false,
                                             file_util::FileEnumerator::FILES);
   for (FilePath config_file_path = file_enumerator.Next();
        !config_file_path.empty(); config_file_path = file_enumerator.Next())
@@ -39,7 +85,6 @@ scoped_ptr<PolicyBundle> ConfigDirPolicyProviderDelegate::Load() {
   // The files are processed in reverse order because |MergeFrom| gives priority
   // to existing keys, but the ConfigDirPolicyProvider gives priority to the
   // last file in lexicographic order.
-  scoped_ptr<PolicyBundle> bundle(new PolicyBundle());
   for (std::set<FilePath>::reverse_iterator config_file_iter = files.rbegin();
        config_file_iter != files.rend(); ++config_file_iter) {
     JSONFileValueSerializer deserializer(*config_file_iter);
@@ -63,48 +108,21 @@ scoped_ptr<PolicyBundle> ConfigDirPolicyProviderDelegate::Load() {
     // Detach the "3rdparty" node.
     base::Value* third_party = NULL;
     if (dictionary_value->Remove("3rdparty", &third_party)) {
-      Merge3rdPartyPolicy(bundle.get(), third_party);
+      Merge3rdPartyPolicy(third_party, level, bundle);
       delete third_party;
     }
 
     // Add chrome policy.
     PolicyMap policy_map;
-    policy_map.LoadFrom(dictionary_value, level_, scope_);
+    policy_map.LoadFrom(dictionary_value, level, scope_);
     bundle->Get(POLICY_DOMAIN_CHROME, "").MergeFrom(policy_map);
   }
-
-  return bundle.Pass();
-}
-
-base::Time ConfigDirPolicyProviderDelegate::GetLastModification() {
-  base::Time last_modification = base::Time();
-  base::PlatformFileInfo file_info;
-
-  // If the path does not exist or points to a directory, it's safe to load.
-  if (!file_util::GetFileInfo(config_file_path(), &file_info) ||
-      !file_info.is_directory) {
-    return last_modification;
-  }
-
-  // Enumerate the files and find the most recent modification timestamp.
-  file_util::FileEnumerator file_enumerator(config_file_path(),
-                                            false,
-                                            file_util::FileEnumerator::FILES);
-  for (FilePath config_file = file_enumerator.Next();
-       !config_file.empty();
-       config_file = file_enumerator.Next()) {
-    if (file_util::GetFileInfo(config_file, &file_info) &&
-        !file_info.is_directory) {
-      last_modification = std::max(last_modification, file_info.last_modified);
-    }
-  }
-
-  return last_modification;
 }
 
 void ConfigDirPolicyProviderDelegate::Merge3rdPartyPolicy(
-    PolicyBundle* bundle,
-    const base::Value* value) {
+    const base::Value* value,
+    PolicyLevel level,
+    PolicyBundle* bundle) {
   // The first-level entries in |value| are PolicyDomains. The second-level
   // entries are component IDs, and the third-level entries are the policies
   // for that domain/component namespace.
@@ -145,7 +163,7 @@ void ConfigDirPolicyProviderDelegate::Merge3rdPartyPolicy(
       }
 
       PolicyMap policy;
-      policy.LoadFrom(policy_dictionary, level_, scope_);
+      policy.LoadFrom(policy_dictionary, level, scope_);
       bundle->Get(domain, components_it.key()).MergeFrom(policy);
     }
   }
@@ -153,11 +171,10 @@ void ConfigDirPolicyProviderDelegate::Merge3rdPartyPolicy(
 
 ConfigDirPolicyProvider::ConfigDirPolicyProvider(
     const PolicyDefinitionList* policy_list,
-    PolicyLevel level,
     PolicyScope scope,
     const FilePath& config_dir)
     : FileBasedPolicyProvider(
         policy_list,
-        new ConfigDirPolicyProviderDelegate(config_dir, level, scope)) {}
+        new ConfigDirPolicyProviderDelegate(config_dir, scope)) {}
 
 }  // namespace policy