Limit access to ChromeBookmarkClient to bookmarks code

components/bookmarks/browser/bookmark_model.h

Index: components/bookmarks/browser/bookmark_model.h
diff --git a/components/bookmarks/browser/bookmark_model.h b/components/bookmarks/browser/bookmark_model.h
index 50760c804766b570f7dd781a3955b118f4056f0e..3268b459a3da687ea2d1dad180209e48d8733dbf 100644
--- a/components/bookmarks/browser/bookmark_model.h
+++ b/components/bookmarks/browser/bookmark_model.h
@@ -90,15 +90,23 @@ class BookmarkModel : public KeyedService {
   // children of the root node.
   const BookmarkNode* root_node() const { return &root_; }
 
-  // Returns the 'bookmark bar' node. This is NULL until loaded.
+  // Returns the 'bookmark bar' node. This is null until loaded.
   const BookmarkNode* bookmark_bar_node() const { return bookmark_bar_node_; }
 
-  // Returns the 'other' node. This is NULL until loaded.
+  // Returns the 'other' node. This is null until loaded.
   const BookmarkNode* other_node() const { return other_node_; }
 
-  // Returns the 'mobile' node. This is NULL until loaded.
+  // Returns the 'mobile' node. This is null until loaded.
   const BookmarkNode* mobile_node() const { return mobile_node_; }
 
+  // The top-level managed bookmarks foled defined by an enterprise policy. This

[sky, 2015/06/23 22:03:53]

I don't think these should be here. These are loaded externally and not part of
the core bookmarkmodel.

[sdefresne, 2015/06/23 22:13:05]

Were would you recommend putting them? Should I create a new
ManagedBookmarkModel with just those two and put it into
//components/bookmarks/manager/? I cannot leave them in ChromeBookmarkClient
(without causing code duplication) since the client will not be shared between
//ios/chrome/browser and //chrome/browser.

With my patches they are still created and managed by the embedder through the
client (this is the purpose of the |callback| parameter to
HistoryClient::GetLoadExtraNodesCallback => allow the client to set
|managed_node_| and |supervised_node_|), but accessor are provided through the
BookmarkModel for convenience.

+  // is null until loaded.
+  const BookmarkNode* managed_node() const { return managed_node_; }
+
+  // The top-level supervised bookmarks folder, defined by the custodian of a
+  // supervised user. This is null until loaded.
+  const BookmarkNode* supervised_node() const { return supervised_node_; }
+
   bool is_root_node(const BookmarkNode* node) const { return node == &root_; }
 
   // Returns whether the given |node| is one of the permanent nodes - root node,
@@ -300,8 +308,21 @@ class BookmarkModel : public KeyedService {
   // be refetched. This notification is sent by BookmarkClient.
   void OnFaviconChanged(const std::set<GURL>& urls);
 
-  // Returns the client used by this BookmarkModel.
-  BookmarkClient* client() const { return client_; }
+  // Returns true if |node| should sync.

[sky, 2015/06/23 22:03:53]

Same comment for all of these. They are policy decisions and not part of the
core model.

+  bool CanSyncNode(const BookmarkNode* node);
+
+  // Returns true if this node can be edited by the user.
+  // TODO(joaodasilva): the model should check this more aggressively, and
+  // should give the client a means to temporarily disable those checks.
+  // http://crbug.com/49598
+  bool CanBeEditedByUser(const BookmarkNode* node);
+
+  // Returns true if the permanent node |node| should always be visible.
+  bool IsPermanentNodeVisible(const BookmarkPermanentNode* node);
+
+  // Forward the |action| to |client_| to record it. See
+  // BookmarkClient::RecordAction for more information.
+  void RecordAction(const base::UserMetricsAction& action);
 
  private:
   friend class BookmarkCodecTest;
@@ -399,6 +420,19 @@ class BookmarkModel : public KeyedService {
   scoped_ptr<BookmarkLoadDetails> CreateLoadDetails(
       const std::string& accept_languages);
 
+  // Called upon completion of extra nodes initialization.
+  void OnExtraNodeLoaded(BookmarkPermanentNode* managed_node,
+                         BookmarkPermanentNode* supervised_node);
+
+  // Returns whether the permanent node is a top-level permanent node.
+  bool IsTopLevelPermanentNode(const BookmarkPermanentNode* node);
+
+  // Returns true if the permanent node can have its title updated.
+  bool CanSetPermanentNodeTitle(const BookmarkNode* node);
+
+  // Returns true if touch icons are preferred over favicons.
+  bool PreferTouchIcon();
+
   BookmarkClient* const client_;
 
   // Whether the initial set of data has been loaded.
@@ -411,6 +445,8 @@ class BookmarkModel : public KeyedService {
   BookmarkPermanentNode* bookmark_bar_node_;
   BookmarkPermanentNode* other_node_;
   BookmarkPermanentNode* mobile_node_;
+  BookmarkPermanentNode* managed_node_;
+  BookmarkPermanentNode* supervised_node_;
 
   // The maximum ID assigned to the bookmark nodes in the model.
   int64 next_node_id_;