BookmarkModel cleanup. synced_node is now mobile_node and I'm nuking

IsVisible as it's no longer needed. This cl resulted in a ton of
changes, the majority are renames though.

BUG=102714
TEST=covered by tests

Review URL: http://codereview.chromium.org/8759017

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

3 files changed, 53 insertions, 70 deletions

diff --git a/chrome/browser/sync/engine/download_updates_command.cc b/chrome/browser/sync/engine/download_updates_command.cc
index f97ae98..f0ceb30 100644
--- a/chrome/browser/sync/engine/download_updates_command.cc
+++ b/chrome/browser/sync/engine/download_updates_command.cc
@@ -6,13 +6,11 @@
 
 #include <string>
 
-#include "base/command_line.h"
 #include "chrome/browser/sync/engine/syncer.h"
 #include "chrome/browser/sync/engine/syncer_proto_util.h"
 #include "chrome/browser/sync/engine/syncproto.h"
 #include "chrome/browser/sync/syncable/directory_manager.h"
 #include "chrome/browser/sync/syncable/model_type_payload_map.h"
-#include "chrome/common/chrome_switches.h"
 
 using syncable::ScopedDirLookup;
 
@@ -37,10 +35,8 @@ void DownloadUpdatesCommand::ExecuteImpl(SyncSession* session) {
       ClientToServerMessage::GET_UPDATES);
   GetUpdatesMessage* get_updates =
       client_to_server_message.mutable_get_updates();
-  if (CommandLine::ForCurrentProcess()->HasSwitch(
-      switches::kEnableSyncedBookmarksFolder)) {
-    get_updates->set_include_syncable_bookmarks(true);
-  }
+  // TODO: make this default to true.
+  get_updates->set_include_syncable_bookmarks(true);
 
   ScopedDirLookup dir(session->context()->directory_manager(),
                       session->context()->account_name());
diff --git a/chrome/browser/sync/glue/bookmark_model_associator.cc b/chrome/browser/sync/glue/bookmark_model_associator.cc
index e50506c..a010fad 100644
--- a/chrome/browser/sync/glue/bookmark_model_associator.cc
+++ b/chrome/browser/sync/glue/bookmark_model_associator.cc
@@ -47,7 +47,7 @@ namespace browser_sync {
 // TODO(ncarter): Pull these tags from an external protocol specification
 // rather than hardcoding them here.
 static const char kBookmarkBarTag[] = "bookmark_bar";
-static const char kSyncedBookmarksTag[] = "synced_bookmarks";
+static const char kMobileBookmarksTag[] = "synced_bookmarks";
 static const char kOtherBookmarksTag[] = "other_bookmarks";
 static const char kServerError[] =
     "Server did not create top-level nodes.  Possibly we are running against "
@@ -243,7 +243,7 @@ void BookmarkModelAssociator::Disassociate(int64 sync_id) {
 bool BookmarkModelAssociator::SyncModelHasUserCreatedNodes(bool* has_nodes) {
   DCHECK(has_nodes);
   *has_nodes = false;
-  bool has_synced_folder = true;
+  bool has_mobile_folder = true;
   int64 bookmark_bar_sync_id;
   if (!GetSyncIdForTaggedNode(kBookmarkBarTag, &bookmark_bar_sync_id)) {
     return false;
@@ -252,9 +252,9 @@ bool BookmarkModelAssociator::SyncModelHasUserCreatedNodes(bool* has_nodes) {
   if (!GetSyncIdForTaggedNode(kOtherBookmarksTag, &other_bookmarks_sync_id)) {
     return false;
   }
-  int64 synced_bookmarks_sync_id;
-  if (!GetSyncIdForTaggedNode(kSyncedBookmarksTag, &synced_bookmarks_sync_id)) {
-    has_synced_folder = false;
+  int64 mobile_bookmarks_sync_id;
+  if (!GetSyncIdForTaggedNode(kMobileBookmarksTag, &mobile_bookmarks_sync_id)) {
+    has_mobile_folder = false;
   }
 
   sync_api::ReadTransaction trans(FROM_HERE, user_share_);
@@ -269,17 +269,17 @@ bool BookmarkModelAssociator::SyncModelHasUserCreatedNodes(bool* has_nodes) {
     return false;
   }
 
-  sync_api::ReadNode synced_bookmarks_node(&trans);
-  if (has_synced_folder &&
-      !synced_bookmarks_node.InitByIdLookup(synced_bookmarks_sync_id)) {
+  sync_api::ReadNode mobile_bookmarks_node(&trans);
+  if (has_mobile_folder &&
+      !mobile_bookmarks_node.InitByIdLookup(mobile_bookmarks_sync_id)) {
     return false;
   }
 
-  // Sync model has user created nodes if either one of the permanent nodes
-  // has children.
+  // Sync model has user created nodes if any of the permanent nodes has
+  // children.
   *has_nodes = bookmark_bar_node.HasChildren() ||
       other_bookmarks_node.HasChildren() ||
-      (has_synced_folder && synced_bookmarks_node.HasChildren());
+      (has_mobile_folder && mobile_bookmarks_node.HasChildren());
   return true;
 }
 
@@ -368,12 +368,8 @@ bool BookmarkModelAssociator::BuildAssociations(SyncError* error) {
     error->Reset(FROM_HERE, kServerError, model_type());
     return false;
   }
-  if (!AssociateTaggedPermanentNode(bookmark_model_->synced_node(),
-                                    kSyncedBookmarksTag) &&
-      // We only need to ensure that the "synced bookmarks" folder exists on the
-      // server if the command line flag is set.
-      CommandLine::ForCurrentProcess()->HasSwitch(
-      switches::kEnableSyncedBookmarksFolder)) {
+  if (!AssociateTaggedPermanentNode(bookmark_model_->mobile_node(),
+                                    kMobileBookmarksTag)) {
     error->Reset(FROM_HERE, kServerError, model_type());
     return false;
   }
@@ -383,16 +379,13 @@ bool BookmarkModelAssociator::BuildAssociations(SyncError* error) {
   int64 other_bookmarks_sync_id = GetSyncIdFromChromeId(
       bookmark_model_->other_node()->id());
   DCHECK_NE(other_bookmarks_sync_id, sync_api::kInvalidId);
-  int64 synced_bookmarks_sync_id = GetSyncIdFromChromeId(
-       bookmark_model_->synced_node()->id());
-  if (CommandLine::ForCurrentProcess()->HasSwitch(
-      switches::kEnableSyncedBookmarksFolder)) {
-    DCHECK_NE(synced_bookmarks_sync_id, sync_api::kInvalidId);
-  }
+  int64 mobile_bookmarks_sync_id = GetSyncIdFromChromeId(
+       bookmark_model_->mobile_node()->id());
+  DCHECK_NE(mobile_bookmarks_sync_id, sync_api::kInvalidId);
 
   std::stack<int64> dfs_stack;
-  if (synced_bookmarks_sync_id != sync_api::kInvalidId)
-    dfs_stack.push(synced_bookmarks_sync_id);
+  if (mobile_bookmarks_sync_id != sync_api::kInvalidId)
+    dfs_stack.push(mobile_bookmarks_sync_id);
   dfs_stack.push(other_bookmarks_sync_id);
   dfs_stack.push(bookmark_bar_sync_id);
 
@@ -531,11 +524,8 @@ bool BookmarkModelAssociator::LoadAssociations() {
     // We should always be able to find the permanent nodes.
     return false;
   }
-  int64 synced_bookmarks_id = -1;
-  if (!GetSyncIdForTaggedNode(kSyncedBookmarksTag, &synced_bookmarks_id) &&
-      CommandLine::ForCurrentProcess()->HasSwitch(
-      switches::kEnableSyncedBookmarksFolder)) {
-
+  int64 mobile_bookmarks_id = -1;
+  if (!GetSyncIdForTaggedNode(kMobileBookmarksTag, &mobile_bookmarks_id)) {
     // We should always be able to find the permanent nodes.
     return false;
   }
@@ -545,11 +535,10 @@ bool BookmarkModelAssociator::LoadAssociations() {
   BookmarkNodeIdIndex id_index;
   id_index.AddAll(bookmark_model_->bookmark_bar_node());
   id_index.AddAll(bookmark_model_->other_node());
-  id_index.AddAll(bookmark_model_->synced_node());
+  id_index.AddAll(bookmark_model_->mobile_node());
 
   std::stack<int64> dfs_stack;
-  if (synced_bookmarks_id != -1)
-    dfs_stack.push(synced_bookmarks_id);
+  dfs_stack.push(mobile_bookmarks_id);
   dfs_stack.push(other_bookmarks_id);
   dfs_stack.push(bookmark_bar_id);
 
@@ -578,7 +567,7 @@ bool BookmarkModelAssociator::LoadAssociations() {
     // Don't try to call NodesMatch on permanent nodes like bookmark bar and
     // other bookmarks. They are not expected to match.
     if (node != bookmark_model_->bookmark_bar_node() &&
-        node != bookmark_model_->synced_node() &&
+        node != bookmark_model_->mobile_node() &&
         node != bookmark_model_->other_node() &&
         !NodesMatch(node, &sync_parent))
       return false;
diff --git a/chrome/browser/sync/profile_sync_service_bookmark_unittest.cc b/chrome/browser/sync/profile_sync_service_bookmark_unittest.cc
index da29f98..57bdf2f 100644
--- a/chrome/browser/sync/profile_sync_service_bookmark_unittest.cc
+++ b/chrome/browser/sync/profile_sync_service_bookmark_unittest.cc
@@ -296,8 +296,6 @@ class ProfileSyncServiceBookmarkTest : public testing::Test {
 
   virtual void SetUp() {
     test_user_share_.SetUp();
-    CommandLine::ForCurrentProcess()->AppendSwitch(
-        switches::kEnableSyncedBookmarksFolder);
   }
 
   virtual void TearDown() {
@@ -465,7 +463,7 @@ class ProfileSyncServiceBookmarkTest : public testing::Test {
     const BookmarkNode* root = model_->root_node();
     EXPECT_EQ(root->GetIndexOf(model_->bookmark_bar_node()), 0);
     EXPECT_EQ(root->GetIndexOf(model_->other_node()), 1);
-    EXPECT_EQ(root->GetIndexOf(model_->synced_node()), 2);
+    EXPECT_EQ(root->GetIndexOf(model_->mobile_node()), 2);
 
     std::stack<int64> stack;
     stack.push(bookmark_bar_id());
@@ -488,9 +486,9 @@ class ProfileSyncServiceBookmarkTest : public testing::Test {
     ExpectModelMatch(&trans);
   }
 
-  int64 synced_bookmarks_id() {
+  int64 mobile_bookmarks_id() {
     return
-        model_associator_->GetSyncIdFromChromeId(model_->synced_node()->id());
+        model_associator_->GetSyncIdFromChromeId(model_->mobile_node()->id());
   }
 
   int64 other_bookmarks_id() {
@@ -526,7 +524,7 @@ TEST_F(ProfileSyncServiceBookmarkTest, InitialState) {
 
   EXPECT_TRUE(other_bookmarks_id());
   EXPECT_TRUE(bookmark_bar_id());
-  EXPECT_TRUE(synced_bookmarks_id());
+  EXPECT_TRUE(mobile_bookmarks_id());
 
   ExpectModelMatch();
 }
@@ -554,9 +552,9 @@ TEST_F(ProfileSyncServiceBookmarkTest, BookmarkModelOperations) {
   ExpectSyncerNodeMatching(url2);
   ExpectModelMatch();
   // Test addition.
-  const BookmarkNode* synced_folder =
-      model_->AddFolder(model_->synced_node(), 0, ASCIIToUTF16("pie"));
-  ExpectSyncerNodeMatching(synced_folder);
+  const BookmarkNode* mobile_folder =
+      model_->AddFolder(model_->mobile_node(), 0, ASCIIToUTF16("pie"));
+  ExpectSyncerNodeMatching(mobile_folder);
   ExpectModelMatch();
 
   // Test modification.
@@ -574,7 +572,7 @@ TEST_F(ProfileSyncServiceBookmarkTest, BookmarkModelOperations) {
   ExpectModelMatch();
   model_->Copy(url2, model_->bookmark_bar_node(), 0);
   ExpectModelMatch();
-  model_->SetTitle(synced_folder, ASCIIToUTF16("strawberry"));
+  model_->SetTitle(mobile_folder, ASCIIToUTF16("strawberry"));
   ExpectModelMatch();
 
   // Test deletion.
@@ -585,7 +583,7 @@ TEST_F(ProfileSyncServiceBookmarkTest, BookmarkModelOperations) {
   model_->Remove(folder2->parent(),
                  folder2->parent()->GetIndexOf(folder2));
   ExpectModelMatch();
-  model_->Remove(model_->synced_node(), 0);
+  model_->Remove(model_->mobile_node(), 0);
   ExpectModelMatch();
 }
 
@@ -614,7 +612,7 @@ TEST_F(ProfileSyncServiceBookmarkTest, ServerChangeProcessing) {
       "height=300,resizable');});");
   adds.AddURL(L"", javascript_url, other_bookmarks_id(), 0);
   int64 u6 = adds.AddURL(L"Sync1", "http://www.syncable.edu/",
-                         synced_bookmarks_id(), 0);
+                         mobile_bookmarks_id(), 0);
 
   sync_api::ChangeRecordList::const_iterator it;
   // The bookmark model shouldn't yet have seen any of the nodes of |adds|.
@@ -643,7 +641,7 @@ TEST_F(ProfileSyncServiceBookmarkTest, ServerChangeProcessing) {
   // Then add u3 after f1.
   int64 u3_old_parent = mods.ModifyPosition(u3, f2, f1);
 
-  std::wstring u6_old_title = mods.ModifyTitle(u6, L"Synced Folder A");
+  std::wstring u6_old_title = mods.ModifyTitle(u6, L"Mobile Folder A");
 
   // Test that the property changes have not yet taken effect.
   ExpectBrowserNodeTitle(u2, u2_old_title);
@@ -982,7 +980,7 @@ namespace {
 // |   +-- dup
 // |       +-- dupu2, http://www.dupu1.com/
 // |
-// +-- Synced bookmarks
+// +-- Mobile bookmarks
 //     |-- f5
 //     |   |-- f5u1, http://www.f5u1.com/
 //     |-- f6
@@ -1036,7 +1034,7 @@ static TestData kDup2Children[] = {
   { L"dupu2", "http://www.dupu2.com/" },
 };
 
-static TestData kSyncedBookmarkChildren[] = {
+static TestData kMobileBookmarkChildren[] = {
   { L"f5", NULL },
   { L"f6", NULL },
   { L"u5", "http://www.u5.com/" },
@@ -1117,15 +1115,15 @@ void ProfileSyncServiceBookmarkTestWithData::WriteTestDataToBookmarkModel() {
   dup_node = other_bookmarks_node->GetChild(5);
   PopulateFromTestData(dup_node, kDup2Children, arraysize(kDup2Children));
 
-  const BookmarkNode* synced_bookmarks_node = model_->synced_node();
-  PopulateFromTestData(synced_bookmarks_node,
-                       kSyncedBookmarkChildren,
-                       arraysize(kSyncedBookmarkChildren));
+  const BookmarkNode* mobile_bookmarks_node = model_->mobile_node();
+  PopulateFromTestData(mobile_bookmarks_node,
+                       kMobileBookmarkChildren,
+                       arraysize(kMobileBookmarkChildren));
 
-  ASSERT_GE(synced_bookmarks_node->child_count(), 3);
-  const BookmarkNode* f5_node = synced_bookmarks_node->GetChild(0);
+  ASSERT_GE(mobile_bookmarks_node->child_count(), 3);
+  const BookmarkNode* f5_node = mobile_bookmarks_node->GetChild(0);
   PopulateFromTestData(f5_node, kF5Children, arraysize(kF5Children));
-  const BookmarkNode* f6_node = synced_bookmarks_node->GetChild(1);
+  const BookmarkNode* f6_node = mobile_bookmarks_node->GetChild(1);
   PopulateFromTestData(f6_node, kF6Children, arraysize(kF6Children));
 
   ExpectBookmarkModelMatchesTestData();
@@ -1159,15 +1157,15 @@ void ProfileSyncServiceBookmarkTestWithData::
   dup_node = other_bookmarks_node->GetChild(5);
   CompareWithTestData(dup_node, kDup2Children, arraysize(kDup2Children));
 
-  const BookmarkNode* synced_bookmarks_node = model_->synced_node();
-  CompareWithTestData(synced_bookmarks_node,
-                      kSyncedBookmarkChildren,
-                      arraysize(kSyncedBookmarkChildren));
+  const BookmarkNode* mobile_bookmarks_node = model_->mobile_node();
+  CompareWithTestData(mobile_bookmarks_node,
+                      kMobileBookmarkChildren,
+                      arraysize(kMobileBookmarkChildren));
 
-  ASSERT_GE(synced_bookmarks_node->child_count(), 3);
-  const BookmarkNode* f5_node = synced_bookmarks_node->GetChild(0);
+  ASSERT_GE(mobile_bookmarks_node->child_count(), 3);
+  const BookmarkNode* f5_node = mobile_bookmarks_node->GetChild(0);
   CompareWithTestData(f5_node, kF5Children, arraysize(kF5Children));
-  const BookmarkNode* f6_node = synced_bookmarks_node->GetChild(1);
+  const BookmarkNode* f6_node = mobile_bookmarks_node->GetChild(1);
   CompareWithTestData(f6_node, kF6Children, arraysize(kF6Children));
 
 }
@@ -1236,7 +1234,7 @@ TEST_F(ProfileSyncServiceBookmarkTestWithData, MergeWithEmptyBookmarkModel) {
   LoadBookmarkModel(DELETE_EXISTING_STORAGE, DONT_SAVE_TO_STORAGE);
   EXPECT_EQ(model_->bookmark_bar_node()->child_count(), 0);
   EXPECT_EQ(model_->other_node()->child_count(), 0);
-  EXPECT_EQ(model_->synced_node()->child_count(), 0);
+  EXPECT_EQ(model_->mobile_node()->child_count(), 0);
 
   // Now restart the sync service.  Starting it should populate the bookmark
   // model -- test for consistency.