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

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;
       }

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",
   ],
 }

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();
 }

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);
 };
 

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;

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();
 

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,9 +2032,12 @@ 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));
     }
+  }
 }
 
 void ProfileSyncServiceBookmarkTestWithData::ExpectTransactionVersionMatch(

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));

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;
 }

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',

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',
       ],

components/bookmarks/test/mock_bookmark_model_observer.cc

+// 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() {}

components/bookmarks/test/mock_bookmark_model_observer.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.
+
+#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>&));
+};

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_);
   }