Managed bookmarks for supervised users

chrome/browser/bookmarks/chrome_bookmark_client.h

Index: chrome/browser/bookmarks/chrome_bookmark_client.h
diff --git a/chrome/browser/bookmarks/chrome_bookmark_client.h b/chrome/browser/bookmarks/chrome_bookmark_client.h
index 363a58ebfe5371128fefe1bde9dafd374badb92b..cb55b2d8a6d0d52ab79cd17b4d6e3ee3d9b79c07 100644
--- a/chrome/browser/bookmarks/chrome_bookmark_client.h
+++ b/chrome/browser/bookmarks/chrome_bookmark_client.h
@@ -35,16 +35,32 @@ class ChromeBookmarkClient : public bookmarks::BookmarkClient,
   // KeyedService:
   void Shutdown() override;
 
-  // Returns the managed_node.
+  // The top-level managed bookmarks folder, defined by an enterprise policy.
   const BookmarkNode* managed_node() { return managed_node_; }
+  // The top-level supervised bookmarks folder, defined by the custodian of a
+  // supervised user.
+  const BookmarkNode* supervised_node() { return supervised_node_; }
 
   // Returns true if the given node belongs to the managed bookmarks tree.
   bool IsDescendantOfManagedNode(const BookmarkNode* node);
-
   // Returns true if there is at least one managed node in the |list|.
   bool HasDescendantsOfManagedNode(
       const std::vector<const BookmarkNode*>& list);
 
+  // Returns true if the given node belongs to the supervised bookmarks tree.
+  bool IsDescendantOfSupervisedNode(const BookmarkNode* node);

[sky, 2015/02/04 18:36:58]

Can these new functions (and the two above it) be const? Also, I tend to think
it best to keep this class solely about exposing/maintaining the nodes. Any
functions that need to query these should be moved into their own file.

[Marc Treib, 2015/02/05 12:57:06]

They can indeed be const, and now are. Thanks!

What exactly are you suggesting when you say their callers should move to their
own file? CanSetPermanentNodeTitle, CanSyncNode, and CanBeEditedByUser query
these. They're overridden from BookmarkClient, so they need to stay in this
class.

[sky, 2015/02/05 17:57:11]

What I meant is that these functions should move to their own file, take the
supervised/managed node and the other node as arguments. Why? The functions
don't need any state from this class (other than the nodes you get from it).
Basically, prefer non-member non-friend functions to member functions (see
effective C++ for specifics).

How about moving IsDescendantOf to bookmark_utils and changing call sites to:

bookmark_utils::IsDescendantOf(supervised_node(), node) ?

[Marc Treib, 2015/02/06 13:40:37]

Ah, I see now, thanks! Done.

+  // Returns true if there is at least one supervised node in the |list|.
+  bool HasDescendantsOfSupervisedNode(
+      const std::vector<const BookmarkNode*>& list);
+
+  // Returns true if the given node belongs to either the managed or the
+  // supervised bookmarks tree.
+  bool IsDescendantOfManagedOrSupervisedNode(const BookmarkNode* node);
+  // Returns true if there is at least one managed or supervised node in the
+  // |list|.
+  bool HasDescendantsOfManagedOrSupervisedNode(
+      const std::vector<const BookmarkNode*>& list);
+
   // bookmarks::BookmarkClient:
   bool PreferTouchIcon() override;
   base::CancelableTaskTracker::TaskId GetFaviconImageForPageURL(
@@ -83,6 +99,8 @@ class ChromeBookmarkClient : public bookmarks::BookmarkClient,
   static bookmarks::BookmarkPermanentNodeList LoadExtraNodes(
       scoped_ptr<BookmarkPermanentNode> managed_node,
       scoped_ptr<base::ListValue> initial_managed_bookmarks,
+      scoped_ptr<BookmarkPermanentNode> supervised_node,
+      scoped_ptr<base::ListValue> initial_supervised_bookmarks,
       int64* next_node_id);
 
   // Returns the management domain that configured the managed bookmarks,
@@ -103,9 +121,16 @@ class ChromeBookmarkClient : public bookmarks::BookmarkClient,
   // the call to Shutdown. Must be valid for the whole interval.
   bookmarks::BookmarkModel* model_;
 
+  // Managed bookmarks are defined by an enterprise policy.
   scoped_ptr<policy::ManagedBookmarksTracker> managed_bookmarks_tracker_;
+  // The top-level managed bookmarks folder.
   BookmarkPermanentNode* managed_node_;
 
+  // Supervised bookmarks are defined by the custodian of a supervised user.
+  scoped_ptr<policy::ManagedBookmarksTracker> supervised_bookmarks_tracker_;
+  // The top-level supervised bookmarks folder.
+  BookmarkPermanentNode* supervised_node_;
+
   DISALLOW_COPY_AND_ASSIGN(ChromeBookmarkClient);
 };