Managed bookmarks for supervised users

chrome/browser/bookmarks/chrome_bookmark_client.h

 // Copyright 2014 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_BOOKMARKS_CHROME_BOOKMARK_CLIENT_H_
 #define CHROME_BROWSER_BOOKMARKS_CHROME_BOOKMARK_CLIENT_H_
 
 #include <set>
 #include <vector>
 
@@ skipping 17 matching lines @@
                              public bookmarks::BaseBookmarkModelObserver {
  public:
   explicit ChromeBookmarkClient(Profile* profile);
   ~ChromeBookmarkClient() override;
 
   void Init(bookmarks::BookmarkModel* model);
 
   // 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(
       const GURL& page_url,
       favicon_base::IconType type,
       const favicon_base::FaviconImageCallback& callback,
       base::CancelableTaskTracker* tracker) override;
   bool SupportsTypedCountForNodes() override;
   void GetTypedCountForNodes(
       const NodeSet& nodes,
@@ skipping 18 matching lines @@
                            const std::set<GURL>& removed_urls) override;
   void BookmarkAllUserNodesRemoved(bookmarks::BookmarkModel* model,
                                    const std::set<GURL>& removed_urls) override;
   void BookmarkModelLoaded(bookmarks::BookmarkModel* model,
                            bool ids_reassigned) override;
 
   // Helper for GetLoadExtraNodesCallback().
   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,
   // or an empty string.
   std::string GetManagedBookmarksDomain();
 
   Profile* profile_;
 
   // HistoryService associated to the Profile. Due to circular dependency, this
   // cannot be passed to the constructor, nor lazily fetched. Instead the value
   // is initialized from HistoryServiceFactory.
   HistoryService* history_service_;
 
   scoped_ptr<base::CallbackList<void(const std::set<GURL>&)>::Subscription>
       favicon_changed_subscription_;
 
   // Pointer to the BookmarkModel. Will be non-NULL from the call to Init to
   // 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);
 };
 
 #endif  // CHROME_BROWSER_BOOKMARKS_CHROME_BOOKMARK_CLIENT_H_