BookmarkClient can add extra nodes to BookmarkModel.

components/bookmarks/browser/bookmark_model.cc

Index: components/bookmarks/browser/bookmark_model.cc
diff --git a/components/bookmarks/browser/bookmark_model.cc b/components/bookmarks/browser/bookmark_model.cc
index cbf7a6a07fb24d2686cf6c471882a021a975d63d..7fd9ea507e032ff81a3cf3c463af96877cc2f6a7 100644
--- a/components/bookmarks/browser/bookmark_model.cc
+++ b/components/bookmarks/browser/bookmark_model.cc
@@ -13,6 +13,7 @@
 #include "base/logging.h"
 #include "base/macros.h"
 #include "base/strings/string_util.h"
+#include "base/values.h"
 #include "components/bookmarks/browser/bookmark_expanded_state_tracker.h"
 #include "components/bookmarks/browser/bookmark_index.h"
 #include "components/bookmarks/browser/bookmark_match.h"
@@ -20,6 +21,7 @@
 #include "components/bookmarks/browser/bookmark_node_data.h"
 #include "components/bookmarks/browser/bookmark_storage.h"
 #include "components/bookmarks/browser/bookmark_utils.h"
+#include "components/bookmarks/browser/managed_bookmarks_tracker.h"
 #include "components/favicon_base/favicon_types.h"
 #include "grit/components_strings.h"
 #include "ui/base/l10n/l10n_util.h"
@@ -98,6 +100,7 @@ BookmarkModel::BookmarkModel(BookmarkClient* client, bool index_urls)
       bookmark_bar_node_(NULL),
       other_node_(NULL),
       mobile_node_(NULL),
+      managed_node_(NULL),
       next_node_id_(1),
       observers_(ObserverList<BookmarkModelObserver>::NOTIFY_EXISTING_ONLY),
       index_urls_(index_urls),
@@ -141,10 +144,22 @@ void BookmarkModel::Load(
 
   expanded_state_tracker_.reset(
       new BookmarkExpandedStateTracker(this, pref_service));
+  managed_bookmarks_tracker_.reset(
+      new ManagedBookmarksTracker(this, pref_service));
 
   // Load the bookmarks. BookmarkStorage notifies us when done.
   store_ = new BookmarkStorage(this, profile_path, io_task_runner.get());
-  store_->LoadBookmarks(CreateLoadDetails(accept_languages), ui_task_runner);
+  store_->LoadBookmarks(
+      CreateLoadDetails(accept_languages),
+      managed_bookmarks_tracker_->GetInitialManagedBookmarks(),
+      ui_task_runner);
+}
+
+bool BookmarkModel::IsManaged(const BookmarkNode* node) const {
+  DCHECK(loaded_);
+  while (node && node != managed_node())
+    node = node->parent();
+  return node == managed_node();
 }
 
 const BookmarkNode* BookmarkModel::GetParentForNewNodes() {
@@ -211,6 +226,11 @@ void BookmarkModel::RemoveAll() {
     base::AutoLock url_lock(url_lock_);
     for (int i = 0; i < root_.child_count(); ++i) {
       BookmarkNode* permanent_node = root_.GetChild(i);
+
+      // The managed bookmarks can't be removed.

[sky, 2014/05/30 22:31:37]

Not removing managed bookmarks makes this function name and the observer name
confusing. I think you should rename RemoveAll to RemoveAllUserEditableNodes and
update the observer functions similarly. That'll force you to make sure doing
this doesn't cause any problems. If you could do the rename in a separate patch,
that would be great.

[Joao da Silva, 2014/06/01 13:32:07]

Agreed, I'll rename in a subsequent CL.

+      if (permanent_node == managed_node())
+        continue;
+
       for (int j = permanent_node->child_count() - 1; j >= 0; --j) {
         BookmarkNode* child_node = permanent_node->GetChild(j);
         removed_nodes.push_back(child_node);
@@ -318,7 +338,8 @@ void BookmarkModel::SetTitle(const BookmarkNode* node,
   if (node->GetTitle() == title)
     return;
 
-  if (is_permanent_node(node)) {
+  // The managed node can change its title.

[sky, 2014/05/30 22:31:37]

Why do we want to allow the managed nodes title to change?

[Joao da Silva, 2014/06/01 13:32:07]

This was the UX decision when we first introduced managed bookmarks on the
mobile platforms: if the bookmarks come from cloud policy then the folder name
is "$DOMAIN bookmarks", otherwise it's "Managed bookmarks" (e.g. the policy came
via MDM).

The title can change at runtime if the user signs in or out of a managed
account.

+  if (is_permanent_node(node) && node != managed_node()) {
     NOTREACHED();
     return;
   }
@@ -430,6 +451,10 @@ void BookmarkModel::DeleteNodeMetaInfo(const BookmarkNode* node,
 void BookmarkModel::SetNodeSyncTransactionVersion(
     const BookmarkNode* node,
     int64 sync_transaction_version) {
+  if (IsManaged(node)) {
+    NOTREACHED();
+    return;
+  }
   if (sync_transaction_version == node->sync_transaction_version())
     return;
 
@@ -618,6 +643,11 @@ void BookmarkModel::SortChildren(const BookmarkNode* parent) {
     return;
   }
 
+  if (IsManaged(parent)) {

[sky, 2014/05/30 22:31:37]

Should this be a DCHECK? Same with 673 and 454.

[Joao da Silva, 2014/06/01 13:32:07]

Done.

+    NOTREACHED();
+    return;
+  }
+
   FOR_EACH_OBSERVER(BookmarkModelObserver, observers_,
                     OnWillReorderBookmarkNode(this, parent));
 
@@ -640,6 +670,11 @@ void BookmarkModel::SortChildren(const BookmarkNode* parent) {
 void BookmarkModel::ReorderChildren(
     const BookmarkNode* parent,
     const std::vector<const BookmarkNode*>& ordered_nodes) {
+  if (IsManaged(parent)) {
+    NOTREACHED();
+    return;
+  }
+
   // Ensure that all children in |parent| are in |ordered_nodes|.
   DCHECK_EQ(static_cast<size_t>(parent->child_count()), ordered_nodes.size());
   for (size_t i = 0; i < ordered_nodes.size(); ++i)
@@ -701,6 +736,8 @@ const BookmarkPermanentNode* BookmarkModel::PermanentNode(
       return other_node_;
     case BookmarkNode::MOBILE:
       return mobile_node_;
+    case BookmarkNode::MANAGED:
+      return managed_node_;
     default:
       NOTREACHED();
       return NULL;
@@ -755,13 +792,14 @@ void BookmarkModel::DoneLoading(scoped_ptr<BookmarkLoadDetails> details) {
   bookmark_bar_node_ = details->release_bb_node();
   other_node_ = details->release_other_folder_node();
   mobile_node_ = details->release_mobile_folder_node();
+  managed_node_ = details->release_managed_node();
   index_.reset(details->release_index());
 
   // WARNING: order is important here, various places assume the order is
   // constant (but can vary between embedders with the initial visibility
   // of permanent nodes).
   BookmarkPermanentNode* root_children[] = {
-      bookmark_bar_node_, other_node_, mobile_node_,
+      bookmark_bar_node_, other_node_, mobile_node_, managed_node_,
   };
   std::stable_sort(root_children,
                    root_children + arraysize(root_children),
@@ -786,6 +824,12 @@ void BookmarkModel::DoneLoading(scoped_ptr<BookmarkLoadDetails> details) {
   // Notify our direct observers.
   FOR_EACH_OBSERVER(BookmarkModelObserver, observers_,
                     BookmarkModelLoaded(this, details->ids_reassigned()));
+
+  // Start tracking the managed bookmarks. This will detect any changes that
+  // may have occurred while the |store_| loaded the initial managed bookmarks.
+  // May be NULL in unit tests.
+  if (managed_bookmarks_tracker_)
+    managed_bookmarks_tracker_->Init();
 }
 
 void BookmarkModel::RemoveAndDeleteNode(BookmarkNode* delete_me) {
@@ -879,7 +923,8 @@ BookmarkPermanentNode* BookmarkModel::CreatePermanentNode(
     BookmarkNode::Type type) {
   DCHECK(type == BookmarkNode::BOOKMARK_BAR ||
          type == BookmarkNode::OTHER_NODE ||
-         type == BookmarkNode::MOBILE);
+         type == BookmarkNode::MOBILE ||
+         type == BookmarkNode::MANAGED);
   BookmarkPermanentNode* node =
       new BookmarkPermanentNode(generate_next_node_id());
   node->set_visible(client_->IsPermanentNodeVisible(type));
@@ -895,6 +940,9 @@ BookmarkPermanentNode* BookmarkModel::CreatePermanentNode(
     case BookmarkNode::MOBILE:
       title_id = IDS_BOOKMARK_BAR_MOBILE_FOLDER_NAME;
       break;
+    case BookmarkNode::MANAGED:
+      title_id = IDS_BOOKMARK_BAR_MANAGED_FOLDER_DEFAULT_NAME;
+      break;
     default:
       NOTREACHED();
       title_id = IDS_BOOKMARK_BAR_FOLDER_NAME;
@@ -979,10 +1027,13 @@ scoped_ptr<BookmarkLoadDetails> BookmarkModel::CreateLoadDetails(
       CreatePermanentNode(BookmarkNode::OTHER_NODE);
   BookmarkPermanentNode* mobile_node =
       CreatePermanentNode(BookmarkNode::MOBILE);
+  BookmarkPermanentNode* managed_node =
+      CreatePermanentNode(BookmarkNode::MANAGED);
   return scoped_ptr<BookmarkLoadDetails>(new BookmarkLoadDetails(
       bb_node,
       other_node,
       mobile_node,
+      managed_node,
       new BookmarkIndex(client_, index_urls_, accept_languages),
       next_node_id_));
 }