Revert of Use external ID to match native model nodes during bookmark association.

chrome/browser/sync/profile_sync_service_bookmark_unittest.cc

 // Copyright (c) 2012 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.
 
 // TODO(akalin): This file is basically just a unit test for
 // BookmarkChangeProcessor.  Write unit tests for
 // BookmarkModelAssociator separately.
 
 #include <map>
 #include <queue>
@@ skipping 25 matching lines @@
 #include "components/sync_driver/data_type_error_handler_mock.h"
 #include "content/public/test/test_browser_thread_bundle.h"
 #include "sync/api/sync_error.h"
 #include "sync/internal_api/public/change_record.h"
 #include "sync/internal_api/public/read_node.h"
 #include "sync/internal_api/public/read_transaction.h"
 #include "sync/internal_api/public/test/test_user_share.h"
 #include "sync/internal_api/public/write_node.h"
 #include "sync/internal_api/public/write_transaction.h"
 #include "sync/internal_api/syncapi_internal.h"
-#include "sync/syncable/mutable_entry.h"
+#include "sync/syncable/mutable_entry.h"  // TODO(tim): Remove. Bug 131130.
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace browser_sync {
 
 using bookmarks::BookmarkModel;
 using bookmarks::BookmarkNode;
 using syncer::BaseNode;
 using testing::_;
 using testing::InvokeWithoutArgs;
@@ skipping 310 matching lines @@
   }
 
   // Inserts a bookmark directly to the share.
   // Do not use this after model association is complete.
   //
   // This function differs from the AddURL() function declared elsewhere in this
   // file in that it only affects the sync model.  It would be invalid to change
   // the sync model directly after ModelAssociation.  This function can be
   // invoked prior to model association to set up first-time sync model
   // association scenarios.
-  int64 AddBookmarkToShare(syncer::WriteTransaction* trans,
+  int64 AddBookmarkToShare(syncer::WriteTransaction *trans,
                            int64 parent_id,
-                           const std::string& title,
-                           const std::string& url) {
+                           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(url);
+    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(title);
     node.SetBookmarkSpecifics(specifics);
 
     return node.GetId();
   }
@@ skipping 384 matching lines @@
 // association code.
 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));
+      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),
-            base::StringPrintf("http://www.google.com/search?q=%05d", j));
+        AddBookmarkToShare(&trans,
+                           folder_id,
+                           base::StringPrintf("bookmark%05d", j));
       }
     }
   }
 
   LoadBookmarkModel(DELETE_EXISTING_STORAGE, DONT_SAVE_TO_STORAGE);
   StartSync();
 
   ExpectModelMatch();
 }
 
-// Tests bookmark association when nodes exists in the native model only.
-// These entries should be copied to Sync directory during association process.
-TEST_F(ProfileSyncServiceBookmarkTest,
-       InitialModelAssociateWithBookmarkModelNodes) {
-  LoadBookmarkModel(DELETE_EXISTING_STORAGE, DONT_SAVE_TO_STORAGE);
-  const BookmarkNode* folder =
-      model_->AddFolder(model_->other_node(), 0, base::ASCIIToUTF16("foobar"));
-  model_->AddFolder(folder, 0, base::ASCIIToUTF16("nested"));
-  model_->AddURL(folder, 0, base::ASCIIToUTF16("Internets #1 Pies Site"),
-                 GURL("http://www.easypie.com/"));
-  model_->AddURL(folder, 1, base::ASCIIToUTF16("Airplanes"),
-                 GURL("http://www.easyjet.com/"));
-
-  StartSync();
-  ExpectModelMatch();
-}
-
-// Tests bookmark association case when there is an entry in the delete journal
-// that matches one of native bookmarks.
-TEST_F(ProfileSyncServiceBookmarkTest, InitialModelAssociateWithDeleteJournal) {
-  LoadBookmarkModel(DELETE_EXISTING_STORAGE, DONT_SAVE_TO_STORAGE);
-  const BookmarkNode* folder = model_->AddFolder(model_->bookmark_bar_node(), 0,
-                                                 base::ASCIIToUTF16("foobar"));
-  const BookmarkNode* bookmark =
-      model_->AddURL(folder, 0, base::ASCIIToUTF16("Airplanes"),
-                     GURL("http://www.easyjet.com/"));
-
-  CreatePermanentBookmarkNodes();
-
-  // Create entries matching the folder and the bookmark above.
-  int64 folder_id, bookmark_id;
-  {
-    syncer::WriteTransaction trans(FROM_HERE, test_user_share_.user_share());
-    folder_id = AddFolderToShare(&trans, "foobar");
-    bookmark_id = AddBookmarkToShare(&trans, folder_id, "Airplanes",
-                                     "http://www.easyjet.com/");
-  }
-
-  // Associate the bookmark sync node with the native model one and make
-  // it deleted.
-  {
-    syncer::WriteTransaction trans(FROM_HERE, test_user_share_.user_share());
-    syncer::WriteNode node(&trans);
-    EXPECT_EQ(BaseNode::INIT_OK, node.InitByIdLookup(bookmark_id));
-
-    node.GetMutableEntryForTest()->PutLocalExternalId(bookmark->id());
-    node.GetMutableEntryForTest()->PutServerIsDel(true);
-    node.GetMutableEntryForTest()->PutIsDel(true);
-  }
-
-  ASSERT_TRUE(AssociateModels());
-  ExpectModelMatch();
-
-  // The bookmark node should be deleted.
-  EXPECT_EQ(0, folder->child_count());
-}
-
-// Tests that the external ID is used to match the right folder amoung
-// multiple folders with the same name during the native model traversal.
-// Also tests that the external ID is used to match the right bookmark
-// among multiple identical bookmarks when dealing with the delete journal.
-TEST_F(ProfileSyncServiceBookmarkTest,
-       InitialModelAssociateVerifyExternalIdMatch) {
-  LoadBookmarkModel(DELETE_EXISTING_STORAGE, DONT_SAVE_TO_STORAGE);
-  const int kNumFolders = 10;
-  const int kNumBookmarks = 10;
-  const int kFolderToIncludeBookmarks = 7;
-  const int kBookmarkToDelete = 4;
-
-  int64 folder_ids[kNumFolders];
-  int64 bookmark_ids[kNumBookmarks];
-
-  // Create native folders and bookmarks with identical names. Only
-  // one of the folders contains bookmarks and others are empty. Here is the
-  // expected tree shape:
-  // Bookmarks bar
-  // +- folder (#0)
-  // +- folder (#1)
-  // ...
-  // +- folder (#7) <-- Only this folder contains bookmarks.
-  //    +- bookmark (#0)
-  //    +- bookmark (#1)
-  //    ...
-  //    +- bookmark (#4) <-- Only this one bookmark should be removed later.
-  //    ...
-  //    +- bookmark (#9)
-  // ...
-  // +- folder (#9)
-
-  const BookmarkNode* parent_folder = nullptr;
-  for (int i = 0; i < kNumFolders; i++) {
-    const BookmarkNode* folder = model_->AddFolder(
-        model_->bookmark_bar_node(), i, base::ASCIIToUTF16("folder"));
-    folder_ids[i] = folder->id();
-    if (i == kFolderToIncludeBookmarks) {
-      parent_folder = folder;
-    }
-  }
-
-  for (int i = 0; i < kNumBookmarks; i++) {
-    const BookmarkNode* bookmark =
-        model_->AddURL(parent_folder, i, base::ASCIIToUTF16("bookmark"),
-                       GURL("http://www.google.com/"));
-    bookmark_ids[i] = bookmark->id();
-  }
-
-  // Number of nodes in bookmark bar before association.
-  int total_node_count = model_->bookmark_bar_node()->GetTotalNodeCount();
-
-  CreatePermanentBookmarkNodes();
-
-  int64 sync_bookmark_id_to_delete = 0;
-  {
-    // Create sync folders matching native folders above.
-    int64 parent_id = 0;
-    syncer::WriteTransaction trans(FROM_HERE, test_user_share_.user_share());
-    // Create in reverse order because AddFolderToShare passes NULL for
-    // |predecessor| argument.
-    for (int i = kNumFolders - 1; i >= 0; i--) {
-      int64 id = AddFolderToShare(&trans, "folder");
-
-      // Pre-map sync folders to native folders by setting
-      // external ID. This will verify that the association algorithm picks
-      // the right ones despite all of them having identical names.
-      // More specifically this will help to avoid cloning bookmarks from
-      // a wrong folder.
-      syncer::WriteNode node(&trans);
-      EXPECT_EQ(BaseNode::INIT_OK, node.InitByIdLookup(id));
-      node.GetMutableEntryForTest()->PutLocalExternalId(folder_ids[i]);
-
-      if (i == kFolderToIncludeBookmarks) {
-        parent_id = id;
-      }
-    }
-
-    // Create sync bookmark matching native bookmarks above in reverse order
-    // because AddBookmarkToShare passes NULL for |predecessor| argument.
-    for (int i = kNumBookmarks - 1; i >= 0; i--) {
-      int id = AddBookmarkToShare(&trans, parent_id, "bookmark",
-                                  "http://www.google.com/");
-
-      // Pre-map sync bookmarks to native bookmarks by setting
-      // external ID. This will verify that the association algorithm picks
-      // the right ones despite all of them having identical names and URLs.
-      syncer::WriteNode node(&trans);
-      EXPECT_EQ(BaseNode::INIT_OK, node.InitByIdLookup(id));
-      node.GetMutableEntryForTest()->PutLocalExternalId(bookmark_ids[i]);
-
-      if (i == kBookmarkToDelete) {
-        sync_bookmark_id_to_delete = id;
-      }
-    }
-  }
-
-  // Make one bookmark deleted.
-  {
-    syncer::WriteTransaction trans(FROM_HERE, test_user_share_.user_share());
-    syncer::WriteNode node(&trans);
-    EXPECT_EQ(BaseNode::INIT_OK,
-              node.InitByIdLookup(sync_bookmark_id_to_delete));
-
-    node.GetMutableEntryForTest()->PutServerIsDel(true);
-    node.GetMutableEntryForTest()->PutIsDel(true);
-  }
-
-  // Perform association.
-  ASSERT_TRUE(AssociateModels());
-  ExpectModelMatch();
-
-  // Only one native node should have been deleted and no nodes cloned due to
-  // matching folder names.
-  EXPECT_EQ(kNumFolders, model_->bookmark_bar_node()->child_count());
-  EXPECT_EQ(kNumBookmarks - 1, parent_folder->child_count());
-  EXPECT_EQ(total_node_count - 1,
-            model_->bookmark_bar_node()->GetTotalNodeCount());
-
-  // Verify that the right bookmark got deleted and no bookmarks reordered.
-  for (int i = 0; i < parent_folder->child_count(); i++) {
-    int index_in_bookmark_ids = (i < kBookmarkToDelete) ? i : i + 1;
-    EXPECT_EQ(bookmark_ids[index_in_bookmark_ids],
-              parent_folder->GetChild(i)->id());
-  }
-}
 
 TEST_F(ProfileSyncServiceBookmarkTest, BookmarkModelOperations) {
   LoadBookmarkModel(DELETE_EXISTING_STORAGE, DONT_SAVE_TO_STORAGE);
   StartSync();
 
   // Test addition.
   const BookmarkNode* folder =
       model_->AddFolder(model_->other_node(), 0, base::ASCIIToUTF16("foobar"));
   ExpectSyncerNodeMatching(folder);
   ExpectModelMatch();
@@ skipping 1370 matching lines @@
   ExpectModelMatch();
 
   // Then simulate the add call arriving late.
   change_processor_->BookmarkNodeAdded(model_, model_->bookmark_bar_node(), 0);
   ExpectModelMatch();
 }
 
 }  // namespace
 
 }  // namespace browser_sync