sync: Test and improve bookmark performance

chrome/browser/sync/profile_sync_service_bookmark_unittest.cc

Index: chrome/browser/sync/profile_sync_service_bookmark_unittest.cc
diff --git a/chrome/browser/sync/profile_sync_service_bookmark_unittest.cc b/chrome/browser/sync/profile_sync_service_bookmark_unittest.cc
index 6711fe6cfd2805920d1d500084a17866b0da7bcb..7677fb375551db048ef07490c5c3f023e9d369a8 100644
--- a/chrome/browser/sync/profile_sync_service_bookmark_unittest.cc
+++ b/chrome/browser/sync/profile_sync_service_bookmark_unittest.cc
@@ -19,6 +19,7 @@
 #include "base/message_loop.h"
 #include "base/string16.h"
 #include "base/string_util.h"
+#include "base/stringprintf.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/time.h"
 #include "base/utf_string_conversions.h"
@@ -287,6 +288,46 @@ class ProfileSyncServiceBookmarkTest : public testing::Test {
     test_user_share_.TearDown();
   }
 
+  // Inserts a folder directly to the share.
+  // Do not use this after model association is complete.
+  int64 AddFolderToShare(syncer::WriteTransaction* trans, std::string title) {

[tim (not reviewing), 2013/05/20 18:26:20]

It seems this is actually hard-wired to add a folder directly to the
bookmark_bar.

Can't this be made to be a special case of FakeServerChange?  Why do we need
multiple ways of adding bookmarks in these tests?  

[rlarocque, 2013/05/20 20:09:54]

It is hard-coded to add folders to the bookmark bar.  I don't see that as a
problem.  

It can't use the FakeServerChange infrastructure because FakeServerChanges are
interpreted by the ChangeProcessor.  The intent of the test I've added is to see
what happens when a pre-populated sync share is associated with an empty native
model.  This requires that the directory be populated before the change
processor exists.

+    EXPECT_FALSE(model_associator_);
+
+    // Be sure to call CreatePermanentBookmarkNodes(), otherwise this will fail.
+    syncer::ReadNode bookmark_bar(trans);
+    EXPECT_EQ(BaseNode::INIT_OK, bookmark_bar.InitByTagLookup("bookmark_bar"));
+
+    syncer::WriteNode node(trans);
+    EXPECT_TRUE(node.InitBookmarkByCreation(bookmark_bar, NULL));
+    node.SetIsFolder(true);
+    node.SetTitle(ASCIIToWide(title));
+
+    return node.GetId();
+  }
+
+  // Inserts a bookmark directly to the share.
+  // Do not use this after model association is complete.
+  int64 AddBookmarkToShare(syncer::WriteTransaction *trans,
+                           int64 parent_id,
+                           std::string title) {
+    EXPECT_FALSE(model_associator_);
+
+    syncer::ReadNode parent(trans);
+    EXPECT_EQ(BaseNode::INIT_OK, parent.InitByIdLookup(parent_id));
+
+    sync_pb::BookmarkSpecifics specifics;
+    specifics.set_url("http://www.google.com/search?q=" + title);
+    specifics.set_title(title);
+
+    syncer::WriteNode node(trans);
+    EXPECT_TRUE(node.InitBookmarkByCreation(parent, NULL));
+    node.SetIsFolder(false);
+    node.SetTitle(ASCIIToWide(title));
+    node.SetBookmarkSpecifics(specifics);
+
+    return node.GetId();
+  }
+
   // Load (or re-load) the bookmark model.  |load| controls use of the
   // bookmarks file on disk.  |save| controls whether the newly loaded
   // bookmark model will write out a bookmark file as it goes.
@@ -625,6 +666,35 @@ TEST_F(ProfileSyncServiceBookmarkTest, InitialState) {
   ExpectModelMatch();
 }
 
+// Populate the sync database then start model association.  Sync's bookmarks
+// should end up being copied into the native model, resulting in a successful
+// "ExpectModelMatch()".
+TEST_F(ProfileSyncServiceBookmarkTest, InitialModelAssociate) {
+  const int kNumBookmarksPerFolder = 10;
+  const int kNumFolders = 10;
+
+  CreatePermanentBookmarkNodes();
+
+  {
+    syncer::WriteTransaction trans(FROM_HERE, test_user_share_.user_share());
+    for (int i = 0; i < kNumFolders; ++i) {
+      int64 folder_id = AddFolderToShare(&trans,
+                                         base::StringPrintf("folder%05d", i));
+      for (int j = 0; j < kNumBookmarksPerFolder; ++j) {
+        AddBookmarkToShare(&trans,
+                           folder_id,
+                           base::StringPrintf("bookmark%05d", j));
+      }
+    }
+  }
+
+  LoadBookmarkModel(DELETE_EXISTING_STORAGE, DONT_SAVE_TO_STORAGE);
+  StartSync();
+
+  ExpectModelMatch();
+}
+
+
 TEST_F(ProfileSyncServiceBookmarkTest, BookmarkModelOperations) {
   LoadBookmarkModel(DELETE_EXISTING_STORAGE, DONT_SAVE_TO_STORAGE);
   StartSync();