Enable the ManagedBookmarks policy on all platforms.

chrome/browser/policy/managed_bookmarks_policy_handler.cc

Index: chrome/browser/policy/managed_bookmarks_policy_handler.cc
diff --git a/chrome/browser/policy/managed_bookmarks_policy_handler.cc b/chrome/browser/policy/managed_bookmarks_policy_handler.cc
index bf493ce83aeb6bc13ae63802d6973537e9728c29..dc5b72ab255bede589136ffd07e050a9e6b9174d 100644
--- a/chrome/browser/policy/managed_bookmarks_policy_handler.cc
+++ b/chrome/browser/policy/managed_bookmarks_policy_handler.cc
@@ -7,7 +7,7 @@
 #include "base/prefs/pref_value_map.h"
 #include "base/values.h"
 #include "chrome/common/net/url_fixer_upper.h"
-#include "chrome/common/pref_names.h"
+#include "components/bookmarks/common/bookmark_pref_names.h"
 #include "components/policy/core/browser/policy_error_map.h"
 #include "components/policy/core/common/policy_map.h"
 #include "grit/components_strings.h"
@@ -16,88 +16,68 @@
 
 namespace policy {
 
-namespace {
-
-bool GetBookmark(const base::Value& value,
-                 std::string* name,
-                 std::string* url) {
-  const base::DictionaryValue* dict = NULL;
-  if (!value.GetAsDictionary(&dict))
-    return false;
-  std::string url_string;
-  if (!dict->GetStringWithoutPathExpansion(ManagedBookmarksPolicyHandler::kName,
-                                           name) ||
-      !dict->GetStringWithoutPathExpansion(ManagedBookmarksPolicyHandler::kUrl,
-                                           &url_string)) {
-    return false;
-  }
-  GURL gurl = URLFixerUpper::FixupURL(url_string, "");
-  if (!gurl.is_valid())
-    return false;
-  *url = gurl.spec();
-  return true;
-}
-
-}  // namespace
-
 const char ManagedBookmarksPolicyHandler::kName[] = "name";
 const char ManagedBookmarksPolicyHandler::kUrl[] = "url";
+const char ManagedBookmarksPolicyHandler::kChildren[] = "children";
 
-ManagedBookmarksPolicyHandler::ManagedBookmarksPolicyHandler()
-    : TypeCheckingPolicyHandler(key::kManagedBookmarks,
-                                base::Value::TYPE_LIST) {}
+ManagedBookmarksPolicyHandler::ManagedBookmarksPolicyHandler(
+    Schema chrome_schema)
+    : SchemaValidatingPolicyHandler(
+          key::kManagedBookmarks,
+          chrome_schema.GetKnownProperty(key::kManagedBookmarks),
+          SCHEMA_ALLOW_INVALID) {}
 
 ManagedBookmarksPolicyHandler::~ManagedBookmarksPolicyHandler() {}
 
-bool ManagedBookmarksPolicyHandler::CheckPolicySettings(
-    const PolicyMap& policies,
-    PolicyErrorMap* errors) {
-  const base::Value* value = NULL;
-  if (!CheckAndGetValue(policies, errors, &value))
-    return false;
-
-  if (!value)
-    return true;
-
-  const base::ListValue* list = NULL;
-  value->GetAsList(&list);
-  DCHECK(list);
-
-  for (base::ListValue::const_iterator it = list->begin();
-       it != list->end(); ++it) {
-    std::string name;
-    std::string url;
-    if (!*it || !GetBookmark(**it, &name, &url)) {
-      size_t index = it - list->begin();
-      errors->AddError(policy_name(), index, IDS_POLICY_INVALID_BOOKMARK);
-    }
-  }
-
-  return true;
-}
-
 void ManagedBookmarksPolicyHandler::ApplyPolicySettings(
     const PolicyMap& policies,
     PrefValueMap* prefs) {
-  const base::Value* value = policies.GetValue(policy_name());
-  const base::ListValue* list = NULL;
+  scoped_ptr<base::Value> value;
+  if (!CheckAndGetValue(policies, NULL, &value))
+    return;
+
+  base::ListValue* list = NULL;
   if (!value || !value->GetAsList(&list))
     return;
 
-  base::ListValue* bookmarks = new base::ListValue();
-  for (base::ListValue::const_iterator it = list->begin();
-       it != list->end(); ++it) {
+  FilterBookmarks(list);
+  prefs->SetValue(prefs::kManagedBookmarks, value.release());
+}
+
+void ManagedBookmarksPolicyHandler::FilterBookmarks(base::ListValue* list) {
+  base::ListValue::iterator it = list->begin();
+  while (it != list->end()) {
+    base::DictionaryValue* dict = NULL;

[pastarmovj, 2014/05/30 14:07:13]

I think having a short one liners about the logic these checks do to delete
entries in the list and dictionary will help maintaining this code. The checks
are rather trivial and yet they are plenty.

[Joao da Silva, 2014/05/30 20:54:35]

Done.

+    if (!*it || !(*it)->GetAsDictionary(&dict)) {
+      it = list->Erase(it, NULL);
+      continue;
+    }
+
     std::string name;
     std::string url;
-    if (*it && GetBookmark(**it, &name, &url)) {
-      base::DictionaryValue* dict = new base::DictionaryValue();
-      dict->SetString(kName, name);
-      dict->SetString(kUrl, url);
-      bookmarks->Append(dict);
+    base::ListValue* children = NULL;
+    if (!dict->GetString(kName, &name) ||
+        (!dict->GetList(kChildren, &children) &&
+         !dict->GetString(kUrl, &url))) {
+      it = list->Erase(it, NULL);
+      continue;
     }
-  }
 
-  prefs->SetValue(prefs::kManagedBookmarks, bookmarks);
+    if (children) {
+      dict->Remove(kUrl, NULL);
+      FilterBookmarks(children);
+    } else {
+      dict->Remove(kChildren, NULL);
+      GURL gurl = URLFixerUpper::FixupURL(url, "");
+      if (!gurl.is_valid()) {
+        it = list->Erase(it, NULL);
+        continue;
+      }
+      dict->SetString(kUrl, gurl.spec());
+    }
+
+    ++it;
+  }
 }
 
 }  // namespace policy