Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(343)

Unified Diff: chrome/browser/sync/profile_sync_service_bookmark_unittest.cc

Issue 906403003: Revert of Use external ID to match native model nodes during bookmark association. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Created 5 years, 10 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « chrome/browser/sync/glue/bookmark_model_associator.cc ('k') | sync/internal_api/delete_journal.cc » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
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 b678b6e43bfc3f6204eb901063d83483bb2cd832..3f8f10350df1878975ca62c642975acbc2a23098 100644
--- a/chrome/browser/sync/profile_sync_service_bookmark_unittest.cc
+++ b/chrome/browser/sync/profile_sync_service_bookmark_unittest.cc
@@ -43,7 +43,7 @@
#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"
@@ -374,17 +374,16 @@
// 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);
@@ -789,12 +788,12 @@
{
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));
}
}
}
@@ -805,189 +804,6 @@
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);
« no previous file with comments | « chrome/browser/sync/glue/bookmark_model_associator.cc ('k') | sync/internal_api/delete_journal.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698