Create the managed_node at the ChromeBookmarkClient.

This change wires the ManagedBookmarksTracker to the ChromeBookmarkClient.
The tracker keeps the new managed_node in sync with the ManagedBookmarks
policy, and the ChromeBookmarkClient publishes that node as an extra node
for the BookmarkModel.

Also cleaned up some duplicated constants used by the managed bookmarks code.

BUG=49598

Review URL: https://codereview.chromium.org/319543003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@275382 0039d316-1c4b-4281-b951-d872f2087c98

15 files changed, 500 insertions, 111 deletions

diff --git a/chrome/browser/android/bookmarks/managed_bookmarks_shim.cc b/chrome/browser/android/bookmarks/managed_bookmarks_shim.cc
index 644fd2d..c2b8105 100644
--- a/chrome/browser/android/bookmarks/managed_bookmarks_shim.cc
+++ b/chrome/browser/android/bookmarks/managed_bookmarks_shim.cc
@@ -8,15 +8,13 @@
 #include "base/bind_helpers.h"
 #include "base/prefs/pref_service.h"
 #include "base/strings/utf_string_conversions.h"
-#include "chrome/browser/policy/managed_bookmarks_policy_handler.h"
 #include "chrome/common/pref_names.h"
 #include "components/bookmarks/browser/bookmark_model.h"
+#include "components/policy/core/browser/managed_bookmarks_tracker.h"
 #include "google_apis/gaia/gaia_auth_util.h"
 #include "grit/components_strings.h"
 #include "ui/base/l10n/l10n_util.h"
 
-using policy::ManagedBookmarksPolicyHandler;
-
 ManagedBookmarksShim::ManagedBookmarksShim(PrefService* prefs)
     : prefs_(prefs) {
   registrar_.Init(prefs_);
@@ -94,8 +92,8 @@ void ManagedBookmarksShim::Reload() {
 
       base::string16 name;
       std::string url;
-      if (!dict->GetString(ManagedBookmarksPolicyHandler::kName, &name) ||
-          !dict->GetString(ManagedBookmarksPolicyHandler::kUrl, &url)) {
+      if (!dict->GetString(policy::ManagedBookmarksTracker::kName, &name) ||
+          !dict->GetString(policy::ManagedBookmarksTracker::kUrl, &url)) {
         NOTREACHED();
         continue;
       }
diff --git a/chrome/browser/bookmarks/DEPS b/chrome/browser/bookmarks/DEPS
index 2f599a7..4f9306e 100644
--- a/chrome/browser/bookmarks/DEPS
+++ b/chrome/browser/bookmarks/DEPS
@@ -7,6 +7,10 @@ include_rules = [
   "+chrome/browser/bookmarks",
   "+chrome/browser/favicon",
   "+chrome/browser/chrome_notification_types.h",
+  "+chrome/browser/policy/profile_policy_connector.h",
+  "+chrome/browser/policy/profile_policy_connector_factory.h",
+  "+chrome/browser/profiles/startup_task_runner_service.h",
+  "+chrome/browser/profiles/startup_task_runner_service_factory.h",
   "+chrome/browser/undo/bookmark_undo_service.h",
   "+chrome/browser/undo/bookmark_undo_service_factory.h",
 
@@ -19,8 +23,6 @@ include_rules = [
   "!chrome/browser/omnibox/omnibox_field_trial.h",
   "!chrome/browser/profiles/incognito_helpers.h",
   "!chrome/browser/profiles/profile.h",
-  "!chrome/browser/profiles/startup_task_runner_service.h",
-  "!chrome/browser/profiles/startup_task_runner_service_factory.h",
   # Do not add to the list of temporarily-allowed dependencies above,
   # and please do not introduce more #includes of these files.
 ]
@@ -28,7 +30,7 @@ include_rules = [
 specific_include_rules = {
   # For unit tests, it's fine to include utility process code.
   '.*test\.cc': [
-    "!chrome/test/base/testing_profile.h",
+    "+chrome/test/base/testing_profile.h",
     "+chrome/utility/importer/bookmark_html_reader.h",
   ],
 }
diff --git a/chrome/browser/bookmarks/chrome_bookmark_client.cc b/chrome/browser/bookmarks/chrome_bookmark_client.cc
index e601cd4..55a60bd 100644
--- a/chrome/browser/bookmarks/chrome_bookmark_client.cc
+++ b/chrome/browser/bookmarks/chrome_bookmark_client.cc
@@ -5,7 +5,9 @@
 #include "chrome/browser/bookmarks/chrome_bookmark_client.h"
 
 #include "base/bind.h"
+#include "base/bind_helpers.h"
 #include "base/logging.h"
+#include "base/values.h"
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/favicon/favicon_changed_details.h"
 #include "chrome/browser/favicon/favicon_service.h"
@@ -13,12 +15,20 @@
 #include "chrome/browser/history/history_service.h"
 #include "chrome/browser/history/history_service_factory.h"
 #include "chrome/browser/history/url_database.h"
+#include "chrome/browser/policy/profile_policy_connector.h"
+#include "chrome/browser/policy/profile_policy_connector_factory.h"
 #include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/profiles/startup_task_runner_service.h"
+#include "chrome/browser/profiles/startup_task_runner_service_factory.h"
 #include "components/bookmarks/browser/bookmark_model.h"
 #include "components/bookmarks/browser/bookmark_node.h"
+#include "content/public/browser/browser_thread.h"
 #include "content/public/browser/notification_details.h"
 #include "content/public/browser/notification_source.h"
 #include "content/public/browser/user_metrics.h"
+#include "grit/components_strings.h"
+#include "policy/policy_constants.h"
+#include "ui/base/l10n/l10n_util.h"
 
 namespace {
 
@@ -34,7 +44,13 @@ void NotifyHistoryOfRemovedURLs(Profile* profile,
 
 ChromeBookmarkClient::ChromeBookmarkClient(Profile* profile, bool index_urls)
     : profile_(profile),
-      model_(new BookmarkModel(this, index_urls)) {
+      model_(new BookmarkModel(this, index_urls)),
+      managed_bookmarks_tracker_(
+          model_.get(),
+          profile_->GetPrefs(),
+          base::Bind(&ChromeBookmarkClient::GetManagedBookmarksDomain,
+                     base::Unretained(this))),
+      managed_node_(NULL) {
   model_->AddObserver(this);
   // Listen for changes to favicons so that we can update the favicon of the
   // node appropriately.
@@ -49,6 +65,10 @@ ChromeBookmarkClient::~ChromeBookmarkClient() {
   registrar_.RemoveAll();
 }
 
+bool ChromeBookmarkClient::IsDescendantOfManagedNode(const BookmarkNode* node) {
+  return node && node->HasAncestor(managed_node_);
+}
+
 bool ChromeBookmarkClient::PreferTouchIcon() {
 #if !defined(OS_IOS)
   return false;
@@ -104,7 +124,10 @@ bool ChromeBookmarkClient::IsPermanentNodeVisible(
     const BookmarkPermanentNode* node) {
   DCHECK(node->type() == BookmarkNode::BOOKMARK_BAR ||
          node->type() == BookmarkNode::OTHER_NODE ||
-         node->type() == BookmarkNode::MOBILE);
+         node->type() == BookmarkNode::MOBILE ||
+         node == managed_node_);
+  if (node == managed_node_)
+    return false;
 #if !defined(OS_IOS)
   return node->type() != BookmarkNode::MOBILE;
 #else
@@ -117,25 +140,36 @@ void ChromeBookmarkClient::RecordAction(const base::UserMetricsAction& action) {
 }
 
 bookmarks::LoadExtraCallback ChromeBookmarkClient::GetLoadExtraNodesCallback() {
-  return base::Bind(&ChromeBookmarkClient::LoadExtraNodes);
+  // Create the managed_node now; it will be populated in the LoadExtraNodes
+  // callback.
+  managed_node_ = new BookmarkPermanentNode(0);
+  return base::Bind(
+      &ChromeBookmarkClient::LoadExtraNodes,
+      StartupTaskRunnerServiceFactory::GetForProfile(profile_)
+          ->GetBookmarkTaskRunner(),
+      managed_node_,
+      base::Passed(managed_bookmarks_tracker_.GetInitialManagedBookmarks()));
 }
 
 bool ChromeBookmarkClient::CanRemovePermanentNodeChildren(
     const BookmarkNode* node) {
-  return true;
+  return !IsDescendantOfManagedNode(node);
 }
 
 bool ChromeBookmarkClient::CanSetPermanentNodeTitle(
     const BookmarkNode* permanent_node) {
-  return false;
+  // The |managed_node_| can have its title updated if the user signs in or
+  // out.
+  return !IsDescendantOfManagedNode(permanent_node) ||
+         permanent_node == managed_node_;
 }
 
 bool ChromeBookmarkClient::CanSyncNode(const BookmarkNode* node) {
-  return true;
+  return !IsDescendantOfManagedNode(node);
 }
 
 bool ChromeBookmarkClient::CanReorderChildren(const BookmarkNode* parent) {
-  return true;
+  return !IsDescendantOfManagedNode(parent);
 }
 
 void ChromeBookmarkClient::Observe(
@@ -177,9 +211,40 @@ void ChromeBookmarkClient::BookmarkAllNodesRemoved(
   NotifyHistoryOfRemovedURLs(profile_, removed_urls);
 }
 
+void ChromeBookmarkClient::BookmarkModelLoaded(BookmarkModel* model,
+                                               bool ids_reassigned) {
+  // Start tracking the managed bookmarks. This will detect any changes that
+  // may have occurred while the initial managed bookmarks were being loaded
+  // on the background.
+  managed_bookmarks_tracker_.Init(managed_node_);
+}
+
 // static
 bookmarks::BookmarkPermanentNodeList ChromeBookmarkClient::LoadExtraNodes(
-    int64* next_id) {
-  // TODO(joaodasilva): load the managed node. http://crbug.com/49598
-  return bookmarks::BookmarkPermanentNodeList();
+    const scoped_refptr<base::DeferredSequencedTaskRunner>& profile_io_runner,
+    BookmarkPermanentNode* managed_node,
+    scoped_ptr<base::ListValue> initial_managed_bookmarks,
+    int64* next_node_id) {
+  DCHECK(profile_io_runner->RunsTasksOnCurrentThread());
+  // Load the initial contents of the |managed_node| now, and assign it an
+  // unused ID.
+  int64 managed_id = *next_node_id;
+  managed_node->set_id(managed_id);
+  *next_node_id = policy::ManagedBookmarksTracker::LoadInitial(
+      managed_node, initial_managed_bookmarks.get(), managed_id + 1);
+  managed_node->set_visible(!managed_node->empty());
+  managed_node->SetTitle(
+      l10n_util::GetStringUTF16(IDS_BOOKMARK_BAR_MANAGED_FOLDER_DEFAULT_NAME));
+
+  bookmarks::BookmarkPermanentNodeList extra_nodes;
+  extra_nodes.push_back(managed_node);
+  return extra_nodes.Pass();
+}
+
+std::string ChromeBookmarkClient::GetManagedBookmarksDomain() {
+  policy::ProfilePolicyConnector* connector =
+      policy::ProfilePolicyConnectorFactory::GetForProfile(profile_);
+  if (connector->IsPolicyFromCloudPolicy(policy::key::kManagedBookmarks))
+    return connector->GetManagementDomain();
+  return std::string();
 }
diff --git a/chrome/browser/bookmarks/chrome_bookmark_client.h b/chrome/browser/bookmarks/chrome_bookmark_client.h
index 512178b..de67a1b 100644
--- a/chrome/browser/bookmarks/chrome_bookmark_client.h
+++ b/chrome/browser/bookmarks/chrome_bookmark_client.h
@@ -6,9 +6,12 @@
 #define CHROME_BROWSER_BOOKMARKS_CHROME_BOOKMARK_CLIENT_H_
 
 #include "base/compiler_specific.h"
+#include "base/deferred_sequenced_task_runner.h"
+#include "base/memory/ref_counted.h"
 #include "components/bookmarks/browser/base_bookmark_model_observer.h"
 #include "components/bookmarks/browser/bookmark_client.h"
 #include "components/keyed_service/core/keyed_service.h"
+#include "components/policy/core/browser/managed_bookmarks_tracker.h"
 #include "content/public/browser/notification_observer.h"
 #include "content/public/browser/notification_registrar.h"
 
@@ -28,6 +31,12 @@ class ChromeBookmarkClient : public BookmarkClient,
   // Returns the BookmarkModel that corresponds to this ChromeBookmarkClient.
   BookmarkModel* model() { return model_.get(); }
 
+  // Returns the managed_node.
+  const BookmarkNode* managed_node() { return managed_node_; }
+
+  // Returns true if the given node belongs to the managed bookmarks tree.
+  bool IsDescendantOfManagedNode(const BookmarkNode* node);
+
   // BookmarkClient:
   virtual bool PreferTouchIcon() OVERRIDE;
   virtual base::CancelableTaskTracker::TaskId GetFaviconImageForURL(
@@ -70,9 +79,19 @@ class ChromeBookmarkClient : public BookmarkClient,
   virtual void BookmarkAllNodesRemoved(
       BookmarkModel* model,
       const std::set<GURL>& removed_urls) OVERRIDE;
+  virtual void BookmarkModelLoaded(BookmarkModel* model,
+                                   bool ids_reassigned) OVERRIDE;
 
   // Helper for GetLoadExtraNodesCallback().
-  static bookmarks::BookmarkPermanentNodeList LoadExtraNodes(int64* next_id);
+  static bookmarks::BookmarkPermanentNodeList LoadExtraNodes(
+      const scoped_refptr<base::DeferredSequencedTaskRunner>& profile_io_runner,
+      BookmarkPermanentNode* managed_node,
+      scoped_ptr<base::ListValue> initial_managed_bookmarks,
+      int64* next_node_id);
+
+  // Returns the management domain that configured the managed bookmarks,
+  // or an empty string.
+  std::string GetManagedBookmarksDomain();
 
   Profile* profile_;
 
@@ -80,6 +99,9 @@ class ChromeBookmarkClient : public BookmarkClient,
 
   scoped_ptr<BookmarkModel> model_;
 
+  policy::ManagedBookmarksTracker managed_bookmarks_tracker_;
+  BookmarkPermanentNode* managed_node_;
+
   DISALLOW_COPY_AND_ASSIGN(ChromeBookmarkClient);
 };
 
diff --git a/chrome/browser/bookmarks/chrome_bookmark_client_unittest.cc b/chrome/browser/bookmarks/chrome_bookmark_client_unittest.cc
new file mode 100644
index 0000000..2d95d4c
--- /dev/null
+++ b/chrome/browser/bookmarks/chrome_bookmark_client_unittest.cc
@@ -0,0 +1,268 @@
+// 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.
+
+#include "chrome/browser/bookmarks/chrome_bookmark_client.h"
+
+#include "base/macros.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/strings/utf_string_conversions.h"
+#include "base/values.h"
+#include "chrome/browser/bookmarks/bookmark_model_factory.h"
+#include "chrome/browser/bookmarks/chrome_bookmark_client.h"
+#include "chrome/test/base/testing_pref_service_syncable.h"
+#include "chrome/test/base/testing_profile.h"
+#include "components/bookmarks/browser/bookmark_model.h"
+#include "components/bookmarks/browser/bookmark_node.h"
+#include "components/bookmarks/common/bookmark_pref_names.h"
+#include "components/bookmarks/test/bookmark_test_helpers.h"
+#include "components/bookmarks/test/mock_bookmark_model_observer.h"
+#include "content/public/test/test_browser_thread_bundle.h"
+#include "grit/components_strings.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "ui/base/l10n/l10n_util.h"
+
+using testing::Mock;
+using testing::_;
+
+class ChromeBookmarkClientTest : public testing::Test {
+ public:
+  ChromeBookmarkClientTest() : client_(NULL), model_(NULL) {}
+  virtual ~ChromeBookmarkClientTest() {}
+
+  virtual void SetUp() OVERRIDE {
+    prefs_ = profile_.GetTestingPrefService();
+    ASSERT_FALSE(prefs_->HasPrefPath(prefs::kManagedBookmarks));
+
+    prefs_->SetManagedPref(prefs::kManagedBookmarks, CreateTestTree());
+    ResetModel();
+
+    // The managed node always exists.
+    ASSERT_TRUE(client_->managed_node());
+    ASSERT_TRUE(client_->managed_node()->parent() == model_->root_node());
+    EXPECT_NE(-1, model_->root_node()->GetIndexOf(client_->managed_node()));
+  }
+
+  virtual void TearDown() OVERRIDE {
+    model_->RemoveObserver(&observer_);
+  }
+
+  void ResetModel() {
+    profile_.CreateBookmarkModel(false);
+    client_ =
+        BookmarkModelFactory::GetChromeBookmarkClientForProfile(&profile_);
+    ASSERT_TRUE(client_);
+    model_ = client_->model();
+    test::WaitForBookmarkModelToLoad(model_);
+    model_->AddObserver(&observer_);
+  }
+
+  static base::DictionaryValue* CreateBookmark(const std::string& title,
+                                               const std::string& url) {
+    EXPECT_TRUE(GURL(url).is_valid());
+    base::DictionaryValue* dict = new base::DictionaryValue();
+    dict->SetString("name", title);
+    dict->SetString("url", GURL(url).spec());
+    return dict;
+  }
+
+  static base::DictionaryValue* CreateFolder(const std::string& title,
+                                             base::ListValue* children) {
+    base::DictionaryValue* dict = new base::DictionaryValue();
+    dict->SetString("name", title);
+    dict->Set("children", children);
+    return dict;
+  }
+
+  static base::ListValue* CreateTestTree() {
+    base::ListValue* folder = new base::ListValue();
+    base::ListValue* empty = new base::ListValue();
+    folder->Append(CreateFolder("Empty", empty));
+    folder->Append(CreateBookmark("Youtube", "http://youtube.com/"));
+
+    base::ListValue* list = new base::ListValue();
+    list->Append(CreateBookmark("Google", "http://google.com/"));
+    list->Append(CreateFolder("Folder", folder));
+
+    return list;
+  }
+
+  static base::DictionaryValue* CreateExpectedTree() {
+    return CreateFolder(GetManagedFolderTitle(), CreateTestTree());
+  }
+
+  static std::string GetManagedFolderTitle() {
+    return l10n_util::GetStringUTF8(
+        IDS_BOOKMARK_BAR_MANAGED_FOLDER_DEFAULT_NAME);
+  }
+
+  static bool NodeMatchesValue(const BookmarkNode* node,
+                               const base::DictionaryValue* dict) {
+    base::string16 title;
+    if (!dict->GetString("name", &title) || node->GetTitle() != title)
+      return false;
+
+    if (node->is_folder()) {
+      const base::ListValue* children = NULL;
+      if (!dict->GetList("children", &children) ||
+          node->child_count() != static_cast<int>(children->GetSize())) {
+        return false;
+      }
+      for (int i = 0; i < node->child_count(); ++i) {
+        const base::DictionaryValue* child = NULL;
+        if (!children->GetDictionary(i, &child) ||
+            !NodeMatchesValue(node->GetChild(i), child)) {
+          return false;
+        }
+      }
+    } else if (node->is_url()) {
+      std::string url;
+      if (!dict->GetString("url", &url) || node->url() != GURL(url))
+        return false;
+    } else {
+      return false;
+    }
+    return true;
+  }
+
+  content::TestBrowserThreadBundle thread_bundle_;
+  TestingProfile profile_;
+  TestingPrefServiceSyncable* prefs_;
+  MockBookmarkModelObserver observer_;
+  ChromeBookmarkClient* client_;
+  BookmarkModel* model_;
+
+  DISALLOW_COPY_AND_ASSIGN(ChromeBookmarkClientTest);
+};
+
+TEST_F(ChromeBookmarkClientTest, EmptyManagedNode) {
+  // Verifies that the managed node is empty and invisible when the policy is
+  // not set.
+  model_->RemoveObserver(&observer_);
+  prefs_->RemoveManagedPref(prefs::kManagedBookmarks);
+  ResetModel();
+
+  ASSERT_TRUE(client_->managed_node());
+  EXPECT_TRUE(client_->managed_node()->empty());
+  EXPECT_FALSE(client_->managed_node()->IsVisible());
+}
+
+TEST_F(ChromeBookmarkClientTest, LoadInitial) {
+  // Verifies that the initial load picks up the initial policy too.
+  EXPECT_TRUE(model_->bookmark_bar_node()->empty());
+  EXPECT_TRUE(model_->other_node()->empty());
+  EXPECT_FALSE(client_->managed_node()->empty());
+  EXPECT_TRUE(client_->managed_node()->IsVisible());
+
+  scoped_ptr<base::DictionaryValue> expected(CreateExpectedTree());
+  EXPECT_TRUE(NodeMatchesValue(client_->managed_node(), expected.get()));
+}
+
+TEST_F(ChromeBookmarkClientTest, SwapNodes) {
+  // Swap the Google bookmark with the Folder.
+  scoped_ptr<base::ListValue> updated(CreateTestTree());
+  scoped_ptr<base::Value> removed;
+  ASSERT_TRUE(updated->Remove(0, &removed));
+  updated->Append(removed.release());
+
+  // These two nodes should just be swapped.
+  const BookmarkNode* parent = client_->managed_node();
+  EXPECT_CALL(observer_, BookmarkNodeMoved(model_, parent, 1, parent, 0));
+  prefs_->SetManagedPref(prefs::kManagedBookmarks, updated->DeepCopy());
+  Mock::VerifyAndClearExpectations(&observer_);
+
+  // Verify the final tree.
+  scoped_ptr<base::DictionaryValue> expected(
+      CreateFolder(GetManagedFolderTitle(), updated.release()));
+  EXPECT_TRUE(NodeMatchesValue(client_->managed_node(), expected.get()));
+}
+
+TEST_F(ChromeBookmarkClientTest, RemoveNode) {
+  // Remove the Folder.
+  scoped_ptr<base::ListValue> updated(CreateTestTree());
+  ASSERT_TRUE(updated->Remove(1, NULL));
+
+  const BookmarkNode* parent = client_->managed_node();
+  EXPECT_CALL(observer_, BookmarkNodeRemoved(model_, parent, 1, _, _));
+  prefs_->SetManagedPref(prefs::kManagedBookmarks, updated->DeepCopy());
+  Mock::VerifyAndClearExpectations(&observer_);
+
+  // Verify the final tree.
+  scoped_ptr<base::DictionaryValue> expected(
+      CreateFolder(GetManagedFolderTitle(), updated.release()));
+  EXPECT_TRUE(NodeMatchesValue(client_->managed_node(), expected.get()));
+}
+
+TEST_F(ChromeBookmarkClientTest, CreateNewNodes) {
+  // Put all the nodes inside another folder.
+  scoped_ptr<base::ListValue> updated(new base::ListValue);
+  updated->Append(CreateFolder("Container", CreateTestTree()));
+
+  EXPECT_CALL(observer_, BookmarkNodeAdded(model_, _, _)).Times(5);
+  // The remaining nodes have been pushed to positions 1 and 2; they'll both be
+  // removed when at position 1.
+  const BookmarkNode* parent = client_->managed_node();
+  EXPECT_CALL(observer_, BookmarkNodeRemoved(model_, parent, 1, _, _))
+      .Times(2);
+  prefs_->SetManagedPref(prefs::kManagedBookmarks, updated->DeepCopy());
+  Mock::VerifyAndClearExpectations(&observer_);
+
+  // Verify the final tree.
+  scoped_ptr<base::DictionaryValue> expected(
+      CreateFolder(GetManagedFolderTitle(), updated.release()));
+  EXPECT_TRUE(NodeMatchesValue(client_->managed_node(), expected.get()));
+}
+
+TEST_F(ChromeBookmarkClientTest, RemoveAll) {
+  // Remove the policy.
+  const BookmarkNode* parent = client_->managed_node();
+  EXPECT_CALL(observer_, BookmarkNodeRemoved(model_, parent, 0, _, _))
+      .Times(2);
+  prefs_->RemoveManagedPref(prefs::kManagedBookmarks);
+  Mock::VerifyAndClearExpectations(&observer_);
+
+  EXPECT_TRUE(client_->managed_node()->empty());
+  EXPECT_FALSE(client_->managed_node()->IsVisible());
+}
+
+TEST_F(ChromeBookmarkClientTest, IsDescendantOfManagedNode) {
+  EXPECT_FALSE(client_->IsDescendantOfManagedNode(model_->root_node()));
+  EXPECT_FALSE(client_->IsDescendantOfManagedNode(model_->bookmark_bar_node()));
+  EXPECT_FALSE(client_->IsDescendantOfManagedNode(model_->other_node()));
+  EXPECT_FALSE(client_->IsDescendantOfManagedNode(model_->mobile_node()));
+  EXPECT_TRUE(client_->IsDescendantOfManagedNode(client_->managed_node()));
+
+  const BookmarkNode* parent = client_->managed_node();
+  ASSERT_EQ(2, parent->child_count());
+  EXPECT_TRUE(client_->IsDescendantOfManagedNode(parent->GetChild(0)));
+  EXPECT_TRUE(client_->IsDescendantOfManagedNode(parent->GetChild(1)));
+
+  parent = parent->GetChild(1);
+  ASSERT_EQ(2, parent->child_count());
+  EXPECT_TRUE(client_->IsDescendantOfManagedNode(parent->GetChild(0)));
+  EXPECT_TRUE(client_->IsDescendantOfManagedNode(parent->GetChild(1)));
+}
+
+TEST_F(ChromeBookmarkClientTest, RemoveAllDoesntRemoveManaged) {
+  EXPECT_EQ(2, client_->managed_node()->child_count());
+
+  EXPECT_CALL(observer_,
+              BookmarkNodeAdded(model_, model_->bookmark_bar_node(), 0));
+  EXPECT_CALL(observer_,
+              BookmarkNodeAdded(model_, model_->bookmark_bar_node(), 1));
+  model_->AddURL(model_->bookmark_bar_node(),
+                 0,
+                 base::ASCIIToUTF16("Test"),
+                 GURL("http://google.com/"));
+  model_->AddFolder(
+      model_->bookmark_bar_node(), 1, base::ASCIIToUTF16("Test Folder"));
+  EXPECT_EQ(2, model_->bookmark_bar_node()->child_count());
+  Mock::VerifyAndClearExpectations(&observer_);
+
+  EXPECT_CALL(observer_, BookmarkAllNodesRemoved(model_, _));
+  model_->RemoveAll();
+  EXPECT_EQ(2, client_->managed_node()->child_count());
+  EXPECT_EQ(0, model_->bookmark_bar_node()->child_count());
+  Mock::VerifyAndClearExpectations(&observer_);
+}
diff --git a/chrome/browser/policy/managed_bookmarks_policy_handler.cc b/chrome/browser/policy/managed_bookmarks_policy_handler.cc
index e037067..e85e08b 100644
--- a/chrome/browser/policy/managed_bookmarks_policy_handler.cc
+++ b/chrome/browser/policy/managed_bookmarks_policy_handler.cc
@@ -8,6 +8,7 @@
 #include "base/values.h"
 #include "chrome/common/net/url_fixer_upper.h"
 #include "components/bookmarks/common/bookmark_pref_names.h"
+#include "components/policy/core/browser/managed_bookmarks_tracker.h"
 #include "components/policy/core/browser/policy_error_map.h"
 #include "components/policy/core/common/policy_map.h"
 #include "grit/components_strings.h"
@@ -16,10 +17,6 @@
 
 namespace policy {
 
-const char ManagedBookmarksPolicyHandler::kName[] = "name";
-const char ManagedBookmarksPolicyHandler::kUrl[] = "url";
-const char ManagedBookmarksPolicyHandler::kChildren[] = "children";
-
 ManagedBookmarksPolicyHandler::ManagedBookmarksPolicyHandler(
     Schema chrome_schema)
     : SchemaValidatingPolicyHandler(
@@ -59,26 +56,26 @@ void ManagedBookmarksPolicyHandler::FilterBookmarks(base::ListValue* list) {
     base::ListValue* children = NULL;
     // Every bookmark must have a name, and then either a URL of a list of
     // child bookmarks.
-    if (!dict->GetString(kName, &name) ||
-        (!dict->GetList(kChildren, &children) &&
-         !dict->GetString(kUrl, &url))) {
+    if (!dict->GetString(ManagedBookmarksTracker::kName, &name) ||
+        (!dict->GetList(ManagedBookmarksTracker::kChildren, &children) &&
+         !dict->GetString(ManagedBookmarksTracker::kUrl, &url))) {
       it = list->Erase(it, NULL);
       continue;
     }
 
     if (children) {
       // Ignore the URL if this bookmark has child nodes.
-      dict->Remove(kUrl, NULL);
+      dict->Remove(ManagedBookmarksTracker::kUrl, NULL);
       FilterBookmarks(children);
     } else {
       // Make sure the URL is valid before passing a bookmark to the pref.
-      dict->Remove(kChildren, NULL);
+      dict->Remove(ManagedBookmarksTracker::kChildren, NULL);
       GURL gurl = URLFixerUpper::FixupURL(url, "");
       if (!gurl.is_valid()) {
         it = list->Erase(it, NULL);
         continue;
       }
-      dict->SetString(kUrl, gurl.spec());
+      dict->SetString(ManagedBookmarksTracker::kUrl, gurl.spec());
     }
 
     ++it;
diff --git a/chrome/browser/policy/managed_bookmarks_policy_handler.h b/chrome/browser/policy/managed_bookmarks_policy_handler.h
index 8fcb1c9..0a61cb4 100644
--- a/chrome/browser/policy/managed_bookmarks_policy_handler.h
+++ b/chrome/browser/policy/managed_bookmarks_policy_handler.h
@@ -16,10 +16,6 @@ namespace policy {
 // Handles the ManagedBookmarks policy.
 class ManagedBookmarksPolicyHandler : public SchemaValidatingPolicyHandler {
  public:
-  static const char kName[];
-  static const char kUrl[];
-  static const char kChildren[];
-
   explicit ManagedBookmarksPolicyHandler(Schema chrome_schema);
   virtual ~ManagedBookmarksPolicyHandler();
 
diff --git a/chrome/browser/sync/profile_sync_service_bookmark_unittest.cc b/chrome/browser/sync/profile_sync_service_bookmark_unittest.cc
index 04185677..b100114 100644
--- a/chrome/browser/sync/profile_sync_service_bookmark_unittest.cc
+++ b/chrome/browser/sync/profile_sync_service_bookmark_unittest.cc
@@ -23,6 +23,7 @@
 #include "base/strings/utf_string_conversions.h"
 #include "base/time/time.h"
 #include "chrome/browser/bookmarks/bookmark_model_factory.h"
+#include "chrome/browser/bookmarks/chrome_bookmark_client.h"
 #include "chrome/browser/sync/glue/bookmark_change_processor.h"
 #include "chrome/browser/sync/glue/bookmark_model_associator.h"
 #include "chrome/common/chrome_switches.h"
@@ -607,7 +608,9 @@ class ProfileSyncServiceBookmarkTest : public testing::Test {
       EXPECT_EQ(gnode.GetPredecessorId(), gprev.GetId());
       EXPECT_EQ(gnode.GetParentId(), gprev.GetParentId());
     }
-    if (browser_index == bnode->parent()->child_count() - 1) {
+    // Note: the managed node comes next to the mobile node but isn't synced.
+    if (browser_index == bnode->parent()->child_count() - 1 ||
+        bnode == model_->mobile_node()) {
       EXPECT_EQ(gnode.GetSuccessorId(), 0);
     } else {
       const BookmarkNode* bnext =
@@ -632,6 +635,12 @@ class ProfileSyncServiceBookmarkTest : public testing::Test {
     const BookmarkNode* bnode =
         model_associator_->GetChromeNodeFromSyncId(sync_id);
     ASSERT_TRUE(bnode);
+
+    ChromeBookmarkClient* client =
+        BookmarkModelFactory::GetChromeBookmarkClientForProfile(&profile_);
+    ASSERT_TRUE(client);
+    ASSERT_FALSE(client->IsDescendantOfManagedNode(bnode));
+
     int64 id = model_associator_->GetSyncIdFromChromeId(bnode->id());
     EXPECT_EQ(id, sync_id);
     ExpectSyncerNodeMatching(trans, bnode);
@@ -1312,6 +1321,11 @@ class ProfileSyncServiceBookmarkTestWithData
   void ExpectBookmarkModelMatchesTestData();
   void WriteTestDataToBookmarkModel();
 
+  // Output transaction versions of |node| and nodes under it to
+  // |node_versions|.
+  void GetTransactionVersions(const BookmarkNode* root,
+                              BookmarkNodeVersionMap* node_versions);
+
   // Verify transaction versions of bookmark nodes and sync nodes are equal
   // recursively. If node is in |version_expected|, versions should match
   // there, too.
@@ -2000,10 +2014,13 @@ TEST_F(ProfileSyncServiceBookmarkTestWithData, UpdateMetaInfoFromModel) {
   ExpectModelMatch();
 }
 
-// Output transaction versions of |node| and nodes under it to |node_versions|.
-void GetTransactionVersions(
+void ProfileSyncServiceBookmarkTestWithData::GetTransactionVersions(
     const BookmarkNode* root,
     BookmarkNodeVersionMap* node_versions) {
+  ChromeBookmarkClient* client =
+      BookmarkModelFactory::GetChromeBookmarkClientForProfile(&profile_);
+  ASSERT_TRUE(client);
+
   node_versions->clear();
   std::queue<const BookmarkNode*> nodes;
   nodes.push(root);
@@ -2015,8 +2032,11 @@ void GetTransactionVersions(
     EXPECT_NE(BookmarkNode::kInvalidSyncTransactionVersion, version);
 
     (*node_versions)[n->id()] = version;
-    for (int i = 0; i < n->child_count(); ++i)
+    for (int i = 0; i < n->child_count(); ++i) {
+      if (client->IsDescendantOfManagedNode(n->GetChild(i)))
+        continue;
       nodes.push(n->GetChild(i));
+    }
   }
 }
 
diff --git a/chrome/browser/ui/bookmarks/bookmark_context_menu_controller_unittest.cc b/chrome/browser/ui/bookmarks/bookmark_context_menu_controller_unittest.cc
index 3102158..9327fbb 100644
--- a/chrome/browser/ui/bookmarks/bookmark_context_menu_controller_unittest.cc
+++ b/chrome/browser/ui/bookmarks/bookmark_context_menu_controller_unittest.cc
@@ -52,19 +52,12 @@ class BookmarkContextMenuControllerTest : public testing::Test {
   }
 
   virtual void SetUp() OVERRIDE {
-    Reset(false);
-  }
-  void Reset(bool incognito) {
     TestingProfile::Builder builder;
-    if (incognito)
-      builder.SetIncognito();
     profile_ = builder.Build();
     profile_->CreateBookmarkModel(true);
-
     model_ = BookmarkModelFactory::GetForProfile(profile_.get());
     test::WaitForBookmarkModelToLoad(model_);
-
-    AddTestData();
+    AddTestData(model_);
   }
 
   virtual void TearDown() OVERRIDE {
@@ -74,15 +67,6 @@ class BookmarkContextMenuControllerTest : public testing::Test {
     message_loop_.RunUntilIdle();
   }
 
- protected:
-  base::MessageLoopForUI message_loop_;
-  content::TestBrowserThread ui_thread_;
-  content::TestBrowserThread file_thread_;
-  scoped_ptr<TestingProfile> profile_;
-  BookmarkModel* model_;
-  TestingPageNavigator navigator_;
-
- private:
   // Creates the following structure:
   // a
   // F1
@@ -93,19 +77,27 @@ class BookmarkContextMenuControllerTest : public testing::Test {
   // F3
   // F4
   //   f4a
-  void AddTestData() {
-    const BookmarkNode* bb_node = model_->bookmark_bar_node();
+  static void AddTestData(BookmarkModel* model) {
+    const BookmarkNode* bb_node = model->bookmark_bar_node();
     std::string test_base = "file:///c:/tmp/";
-    model_->AddURL(bb_node, 0, ASCIIToUTF16("a"), GURL(test_base + "a"));
-    const BookmarkNode* f1 = model_->AddFolder(bb_node, 1, ASCIIToUTF16("F1"));
-    model_->AddURL(f1, 0, ASCIIToUTF16("f1a"), GURL(test_base + "f1a"));
-    const BookmarkNode* f11 = model_->AddFolder(f1, 1, ASCIIToUTF16("F11"));
-    model_->AddURL(f11, 0, ASCIIToUTF16("f11a"), GURL(test_base + "f11a"));
-    model_->AddFolder(bb_node, 2, ASCIIToUTF16("F2"));
-    model_->AddFolder(bb_node, 3, ASCIIToUTF16("F3"));
-    const BookmarkNode* f4 = model_->AddFolder(bb_node, 4, ASCIIToUTF16("F4"));
-    model_->AddURL(f4, 0, ASCIIToUTF16("f4a"), GURL(test_base + "f4a"));
+    model->AddURL(bb_node, 0, ASCIIToUTF16("a"), GURL(test_base + "a"));
+    const BookmarkNode* f1 = model->AddFolder(bb_node, 1, ASCIIToUTF16("F1"));
+    model->AddURL(f1, 0, ASCIIToUTF16("f1a"), GURL(test_base + "f1a"));
+    const BookmarkNode* f11 = model->AddFolder(f1, 1, ASCIIToUTF16("F11"));
+    model->AddURL(f11, 0, ASCIIToUTF16("f11a"), GURL(test_base + "f11a"));
+    model->AddFolder(bb_node, 2, ASCIIToUTF16("F2"));
+    model->AddFolder(bb_node, 3, ASCIIToUTF16("F3"));
+    const BookmarkNode* f4 = model->AddFolder(bb_node, 4, ASCIIToUTF16("F4"));
+    model->AddURL(f4, 0, ASCIIToUTF16("f4a"), GURL(test_base + "f4a"));
   }
+
+ protected:
+  base::MessageLoopForUI message_loop_;
+  content::TestBrowserThread ui_thread_;
+  content::TestBrowserThread file_thread_;
+  scoped_ptr<TestingProfile> profile_;
+  BookmarkModel* model_;
+  TestingPageNavigator navigator_;
 };
 
 // Tests Deleting from the menu.
@@ -249,12 +241,24 @@ TEST_F(BookmarkContextMenuControllerTest, MultipleFoldersWithURLs) {
 
 // Tests the enabled state of open incognito.
 TEST_F(BookmarkContextMenuControllerTest, DisableIncognito) {
-  // Create a new incognito profile.
-  Reset(true);
+  // Create an incognito Profile. It must be associated with the original
+  // Profile, so that GetOriginalProfile() works as expected.
+  TestingProfile::Builder builder;
+  builder.SetIncognito();
+  scoped_ptr<TestingProfile> testing_incognito = builder.Build();
+  testing_incognito->SetOriginalProfile(profile_.get());
+  TestingProfile* incognito = testing_incognito.get();
+  profile_->SetOffTheRecordProfile(testing_incognito.PassAs<Profile>());
+
+  incognito->CreateBookmarkModel(true);
+  BookmarkModel* model = BookmarkModelFactory::GetForProfile(incognito);
+  test::WaitForBookmarkModelToLoad(model);
+  AddTestData(model);
+
   std::vector<const BookmarkNode*> nodes;
-  nodes.push_back(model_->bookmark_bar_node()->GetChild(0));
+  nodes.push_back(model->bookmark_bar_node()->GetChild(0));
   BookmarkContextMenuController controller(
-      NULL, NULL, NULL, profile_.get(), NULL, nodes[0]->parent(), nodes);
+      NULL, NULL, NULL, incognito, NULL, nodes[0]->parent(), nodes);
   EXPECT_FALSE(controller.IsCommandIdEnabled(IDC_BOOKMARK_BAR_OPEN_INCOGNITO));
   EXPECT_FALSE(
       controller.IsCommandIdEnabled(IDC_BOOKMARK_BAR_OPEN_ALL_INCOGNITO));
diff --git a/chrome/browser/ui/views/bookmarks/bookmark_editor_view.cc b/chrome/browser/ui/views/bookmarks/bookmark_editor_view.cc
index 4d4f363..d9be571 100644
--- a/chrome/browser/ui/views/bookmarks/bookmark_editor_view.cc
+++ b/chrome/browser/ui/views/bookmarks/bookmark_editor_view.cc
@@ -495,10 +495,10 @@ BookmarkEditorView::EditorNode* BookmarkEditorView::CreateRootNode() {
   EditorNode* root_node = new EditorNode(base::string16(), 0);
   const BookmarkNode* bb_root_node = bb_model_->root_node();
   CreateNodes(bb_root_node, root_node);
-  DCHECK(root_node->child_count() >= 2 && root_node->child_count() <= 3);
+  DCHECK(root_node->child_count() >= 2 && root_node->child_count() <= 4);
   DCHECK_EQ(BookmarkNode::BOOKMARK_BAR, bb_root_node->GetChild(0)->type());
   DCHECK_EQ(BookmarkNode::OTHER_NODE, bb_root_node->GetChild(1)->type());
-  if (root_node->child_count() == 3)
+  if (root_node->child_count() >= 3)
     DCHECK_EQ(BookmarkNode::MOBILE, bb_root_node->GetChild(2)->type());
   return root_node;
 }
diff --git a/chrome/chrome_tests_unit.gypi b/chrome/chrome_tests_unit.gypi
index f1a2cc3..c66d619 100644
--- a/chrome/chrome_tests_unit.gypi
+++ b/chrome/chrome_tests_unit.gypi
@@ -618,6 +618,7 @@
         'browser/background/background_contents_service_unittest.cc',
         'browser/background/background_mode_manager_unittest.cc',
         'browser/bookmarks/bookmark_html_writer_unittest.cc',
+        'browser/bookmarks/chrome_bookmark_client_unittest.cc',
         'browser/browser_about_handler_unittest.cc',
         'browser/browser_commands_unittest.cc',
         'browser/browsing_data/browsing_data_appcache_helper_unittest.cc',
diff --git a/components/bookmarks.gypi b/components/bookmarks.gypi
index 28e3038..5ebccb1 100644
--- a/components/bookmarks.gypi
+++ b/components/bookmarks.gypi
@@ -81,6 +81,7 @@
       ],
       'dependencies': [
         '../base/base.gyp:base',
+        '../testing/gmock.gyp:gmock',
         '../ui/events/platform/events_platform.gyp:events_platform',
         '../url/url.gyp:url_lib',
         'bookmarks_browser',
@@ -88,6 +89,8 @@
       'sources': [
         'bookmarks/test/bookmark_test_helpers.cc',
         'bookmarks/test/bookmark_test_helpers.h',
+        'bookmarks/test/mock_bookmark_model_observer.cc',
+        'bookmarks/test/mock_bookmark_model_observer.h',
         'bookmarks/test/test_bookmark_client.cc',
         'bookmarks/test/test_bookmark_client.h',
       ],
diff --git a/components/bookmarks/test/mock_bookmark_model_observer.cc b/components/bookmarks/test/mock_bookmark_model_observer.cc
new file mode 100644
index 0000000..bd967e9
--- /dev/null
+++ b/components/bookmarks/test/mock_bookmark_model_observer.cc
@@ -0,0 +1,9 @@
+// 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.
+
+#include "components/bookmarks/test/mock_bookmark_model_observer.h"
+
+MockBookmarkModelObserver::MockBookmarkModelObserver() {}
+
+MockBookmarkModelObserver::~MockBookmarkModelObserver() {}
diff --git a/components/bookmarks/test/mock_bookmark_model_observer.h b/components/bookmarks/test/mock_bookmark_model_observer.h
new file mode 100644
index 0000000..d2a8ccc
--- /dev/null
+++ b/components/bookmarks/test/mock_bookmark_model_observer.h
@@ -0,0 +1,42 @@
+// 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.
+
+#include "components/bookmarks/browser/bookmark_model_observer.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "url/gurl.h"
+
+class MockBookmarkModelObserver : public BookmarkModelObserver {
+ public:
+  MockBookmarkModelObserver();
+  virtual ~MockBookmarkModelObserver();
+
+  MOCK_METHOD2(BookmarkModelLoaded, void(BookmarkModel*, bool));
+
+  MOCK_METHOD5(BookmarkNodeMoved, void(BookmarkModel*,
+                                       const BookmarkNode*,
+                                       int,
+                                       const BookmarkNode*,
+                                       int));
+
+  MOCK_METHOD3(BookmarkNodeAdded, void(BookmarkModel*,
+                                       const BookmarkNode*,
+                                       int));
+
+  MOCK_METHOD5(BookmarkNodeRemoved, void(BookmarkModel*,
+                                         const BookmarkNode*,
+                                         int,
+                                         const BookmarkNode*,
+                                         const std::set<GURL>&));
+
+  MOCK_METHOD2(BookmarkNodeChanged, void(BookmarkModel*, const BookmarkNode*));
+
+  MOCK_METHOD2(BookmarkNodeFaviconChanged, void(BookmarkModel*,
+                                                const BookmarkNode*));
+
+  MOCK_METHOD2(BookmarkNodeChildrenReordered, void(BookmarkModel*,
+                                                   const BookmarkNode*));
+
+  MOCK_METHOD2(BookmarkAllNodesRemoved, void(BookmarkModel*,
+                                             const std::set<GURL>&));
+};
diff --git a/components/policy/core/browser/managed_bookmarks_tracker_unittest.cc b/components/policy/core/browser/managed_bookmarks_tracker_unittest.cc
index 7342581..7ac1003 100644
--- a/components/policy/core/browser/managed_bookmarks_tracker_unittest.cc
+++ b/components/policy/core/browser/managed_bookmarks_tracker_unittest.cc
@@ -16,6 +16,7 @@
 #include "components/bookmarks/browser/bookmark_node.h"
 #include "components/bookmarks/common/bookmark_pref_names.h"
 #include "components/bookmarks/test/bookmark_test_helpers.h"
+#include "components/bookmarks/test/mock_bookmark_model_observer.h"
 #include "components/bookmarks/test/test_bookmark_client.h"
 #include "grit/components_strings.h"
 #include "testing/gmock/include/gmock/gmock.h"
@@ -28,45 +29,6 @@ using testing::_;
 
 namespace policy {
 
-namespace {
-
-class MockBookmarkModelObserver : public BookmarkModelObserver {
- public:
-  MockBookmarkModelObserver() {}
-  virtual ~MockBookmarkModelObserver() {}
-
-  MOCK_METHOD2(BookmarkModelLoaded, void(BookmarkModel*, bool));
-
-  MOCK_METHOD5(BookmarkNodeMoved, void(BookmarkModel*,
-                                       const BookmarkNode*,
-                                       int,
-                                       const BookmarkNode*,
-                                       int));
-
-  MOCK_METHOD3(BookmarkNodeAdded, void(BookmarkModel*,
-                                       const BookmarkNode*,
-                                       int));
-
-  MOCK_METHOD5(BookmarkNodeRemoved, void(BookmarkModel*,
-                                         const BookmarkNode*,
-                                         int,
-                                         const BookmarkNode*,
-                                         const std::set<GURL>&));
-
-  MOCK_METHOD2(BookmarkNodeChanged, void(BookmarkModel*, const BookmarkNode*));
-
-  MOCK_METHOD2(BookmarkNodeFaviconChanged, void(BookmarkModel*,
-                                                const BookmarkNode*));
-
-  MOCK_METHOD2(BookmarkNodeChildrenReordered, void(BookmarkModel*,
-                                                   const BookmarkNode*));
-
-  MOCK_METHOD2(BookmarkAllNodesRemoved, void(BookmarkModel*,
-                                             const std::set<GURL>&));
-};
-
-}  // namespace
-
 class ManagedBookmarksTrackerTest : public testing::Test {
  public:
   ManagedBookmarksTrackerTest() : managed_node_(NULL) {}
@@ -77,7 +39,7 @@ class ManagedBookmarksTrackerTest : public testing::Test {
     prefs_.registry()->RegisterListPref(prefs::kBookmarkEditorExpandedNodes);
   }
 
-  virtual void TearDown() {
+  virtual void TearDown() OVERRIDE {
     if (model_)
       model_->RemoveObserver(&observer_);
   }