Managed bookmarks for supervised users

components/policy/core/browser/managed_bookmarks_tracker.cc

Index: components/policy/core/browser/managed_bookmarks_tracker.cc
diff --git a/components/policy/core/browser/managed_bookmarks_tracker.cc b/components/policy/core/browser/managed_bookmarks_tracker.cc
index 42ebc76d7e21c604b9fdcd8a0187c9f9db1215de..b41ed787dd1d05f21a227296c4bb33cc65d4038f 100644
--- a/components/policy/core/browser/managed_bookmarks_tracker.cc
+++ b/components/policy/core/browser/managed_bookmarks_tracker.cc
@@ -28,8 +28,10 @@ const char ManagedBookmarksTracker::kChildren[] = "children";
 ManagedBookmarksTracker::ManagedBookmarksTracker(
     BookmarkModel* model,
     PrefService* prefs,
+    bool is_supervised,
     const GetManagementDomainCallback& callback)
     : model_(model),
+      is_supervised_(is_supervised),
       managed_node_(NULL),
       prefs_(prefs),
       get_management_domain_callback_(callback) {
@@ -39,8 +41,7 @@ ManagedBookmarksTracker::~ManagedBookmarksTracker() {}
 
 scoped_ptr<base::ListValue>
 ManagedBookmarksTracker::GetInitialManagedBookmarks() {
-  const base::ListValue* list =
-      prefs_->GetList(bookmarks::prefs::kManagedBookmarks);
+  const base::ListValue* list = prefs_->GetList(GetPrefName());
   return make_scoped_ptr(list->DeepCopy());
 }
 
@@ -75,30 +76,46 @@ int64 ManagedBookmarksTracker::LoadInitial(BookmarkNode* folder,
 void ManagedBookmarksTracker::Init(BookmarkPermanentNode* managed_node) {
   managed_node_ = managed_node;
   registrar_.Init(prefs_);
-  registrar_.Add(bookmarks::prefs::kManagedBookmarks,
+  registrar_.Add(GetPrefName(),
                  base::Bind(&ManagedBookmarksTracker::ReloadManagedBookmarks,
                             base::Unretained(this)));
   // Reload now just in case something changed since the initial load started.
   ReloadManagedBookmarks();
 }
 
-void ManagedBookmarksTracker::ReloadManagedBookmarks() {
-  // Update the managed bookmarks folder title, in case the user just signed
-  // into or out of a managed account.
-  base::string16 title;
-  std::string domain = get_management_domain_callback_.Run();
-  if (domain.empty()) {
-    title =
-        l10n_util::GetStringUTF16(IDS_BOOKMARK_BAR_MANAGED_FOLDER_DEFAULT_NAME);
+// static
+const char* ManagedBookmarksTracker::GetPrefName(bool is_supervised) {
+  return is_supervised ? bookmarks::prefs::kSupervisedBookmarks
+                       : bookmarks::prefs::kManagedBookmarks;
+}
+
+const char* ManagedBookmarksTracker::GetPrefName() const {
+  return GetPrefName(is_supervised_);
+}
+
+base::string16 ManagedBookmarksTracker::GetBookmarksFolderTitle() const {
+  if (is_supervised_) {
+    return l10n_util::GetStringUTF16(
+        IDS_BOOKMARK_BAR_SUPERVISED_FOLDER_DEFAULT_NAME);
   } else {
-    title = l10n_util::GetStringFUTF16(
-        IDS_BOOKMARK_BAR_MANAGED_FOLDER_DOMAIN_NAME, base::UTF8ToUTF16(domain));
+    std::string domain = get_management_domain_callback_.Run();
+    if (domain.empty()) {
+      return l10n_util::GetStringUTF16(
+          IDS_BOOKMARK_BAR_MANAGED_FOLDER_DEFAULT_NAME);
+    } else {
+      return l10n_util::GetStringFUTF16(
+          IDS_BOOKMARK_BAR_MANAGED_FOLDER_DOMAIN_NAME,
+          base::UTF8ToUTF16(domain));
+    }
   }
-  model_->SetTitle(managed_node_, title);
+}
+
+void ManagedBookmarksTracker::ReloadManagedBookmarks() {
+  // In case the user just signed into or out of a managed account.
+  model_->SetTitle(managed_node_, GetBookmarksFolderTitle());
 
   // Recursively update all the managed bookmarks and folders.
-  const base::ListValue* list =
-      prefs_->GetList(bookmarks::prefs::kManagedBookmarks);
+  const base::ListValue* list = prefs_->GetList(GetPrefName());
   UpdateBookmarks(managed_node_, list);
 
   // The managed bookmarks folder isn't visible when that policy isn't present.
@@ -175,8 +192,11 @@ bool ManagedBookmarksTracker::LoadBookmark(const base::ListValue* list,
     NOTREACHED();
     return false;
   }
-  if (!*children)
+  if (!*children) {
     *url = GURL(spec);
+    if (!url->is_valid())
+      return false;

[Pam (message me for reviews), 2015/01/14 14:03:37]

Adding validation to an existing set of users' data may have unintended side
effects. Will this trip on JS bookmarklets, for example? Or is anyone using the
bookmark system to push random other snippets of text to their users?

[Marc Treib, 2015/01/14 16:40:50]

The policy checks for valid URLs elsewhere, so this shouldn't ever get
triggered. Our code didn't (it does now), and putting an invalid URL here
resulted in a crash somewhere in the bookmarks code, which is why I added this
check.
I've replaced it with a DCHECK.

+  }
   return true;
 }