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

Side by Side Diff: chrome/browser/sync/profile_sync_service_bookmark_unittest.cc

Issue 912693002: 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 unified diff | Download patch
OLDNEW
1 // Copyright (c) 2012 The Chromium Authors. All rights reserved. 1 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
2 // Use of this source code is governed by a BSD-style license that can be 2 // Use of this source code is governed by a BSD-style license that can be
3 // found in the LICENSE file. 3 // found in the LICENSE file.
4 4
5 // TODO(akalin): This file is basically just a unit test for 5 // TODO(akalin): This file is basically just a unit test for
6 // BookmarkChangeProcessor. Write unit tests for 6 // BookmarkChangeProcessor. Write unit tests for
7 // BookmarkModelAssociator separately. 7 // BookmarkModelAssociator separately.
8 8
9 #include <map> 9 #include <map>
10 #include <queue> 10 #include <queue>
(...skipping 25 matching lines...) Expand all
36 #include "components/sync_driver/data_type_error_handler_mock.h" 36 #include "components/sync_driver/data_type_error_handler_mock.h"
37 #include "content/public/test/test_browser_thread_bundle.h" 37 #include "content/public/test/test_browser_thread_bundle.h"
38 #include "sync/api/sync_error.h" 38 #include "sync/api/sync_error.h"
39 #include "sync/internal_api/public/change_record.h" 39 #include "sync/internal_api/public/change_record.h"
40 #include "sync/internal_api/public/read_node.h" 40 #include "sync/internal_api/public/read_node.h"
41 #include "sync/internal_api/public/read_transaction.h" 41 #include "sync/internal_api/public/read_transaction.h"
42 #include "sync/internal_api/public/test/test_user_share.h" 42 #include "sync/internal_api/public/test/test_user_share.h"
43 #include "sync/internal_api/public/write_node.h" 43 #include "sync/internal_api/public/write_node.h"
44 #include "sync/internal_api/public/write_transaction.h" 44 #include "sync/internal_api/public/write_transaction.h"
45 #include "sync/internal_api/syncapi_internal.h" 45 #include "sync/internal_api/syncapi_internal.h"
46 #include "sync/syncable/mutable_entry.h" // TODO(tim): Remove. Bug 131130. 46 #include "sync/syncable/mutable_entry.h"
47 #include "testing/gmock/include/gmock/gmock.h" 47 #include "testing/gmock/include/gmock/gmock.h"
48 #include "testing/gtest/include/gtest/gtest.h" 48 #include "testing/gtest/include/gtest/gtest.h"
49 49
50 namespace browser_sync { 50 namespace browser_sync {
51 51
52 using bookmarks::BookmarkModel; 52 using bookmarks::BookmarkModel;
53 using bookmarks::BookmarkNode; 53 using bookmarks::BookmarkNode;
54 using syncer::BaseNode; 54 using syncer::BaseNode;
55 using testing::_; 55 using testing::_;
56 using testing::InvokeWithoutArgs; 56 using testing::InvokeWithoutArgs;
(...skipping 317 matching lines...) Expand 10 before | Expand all | Expand 10 after
374 } 374 }
375 375
376 // Inserts a bookmark directly to the share. 376 // Inserts a bookmark directly to the share.
377 // Do not use this after model association is complete. 377 // Do not use this after model association is complete.
378 // 378 //
379 // This function differs from the AddURL() function declared elsewhere in this 379 // This function differs from the AddURL() function declared elsewhere in this
380 // file in that it only affects the sync model. It would be invalid to change 380 // file in that it only affects the sync model. It would be invalid to change
381 // the sync model directly after ModelAssociation. This function can be 381 // the sync model directly after ModelAssociation. This function can be
382 // invoked prior to model association to set up first-time sync model 382 // invoked prior to model association to set up first-time sync model
383 // association scenarios. 383 // association scenarios.
384 int64 AddBookmarkToShare(syncer::WriteTransaction *trans, 384 int64 AddBookmarkToShare(syncer::WriteTransaction* trans,
385 int64 parent_id, 385 int64 parent_id,
386 std::string title) { 386 const std::string& title,
387 const std::string& url) {
387 EXPECT_FALSE(model_associator_); 388 EXPECT_FALSE(model_associator_);
388 389
389 syncer::ReadNode parent(trans); 390 syncer::ReadNode parent(trans);
390 EXPECT_EQ(BaseNode::INIT_OK, parent.InitByIdLookup(parent_id)); 391 EXPECT_EQ(BaseNode::INIT_OK, parent.InitByIdLookup(parent_id));
391 392
392 sync_pb::BookmarkSpecifics specifics; 393 sync_pb::BookmarkSpecifics specifics;
393 specifics.set_url("http://www.google.com/search?q=" + title); 394 specifics.set_url(url);
394 specifics.set_title(title); 395 specifics.set_title(title);
395 396
396 syncer::WriteNode node(trans); 397 syncer::WriteNode node(trans);
397 EXPECT_TRUE(node.InitBookmarkByCreation(parent, NULL)); 398 EXPECT_TRUE(node.InitBookmarkByCreation(parent, NULL));
398 node.SetIsFolder(false); 399 node.SetIsFolder(false);
399 node.SetTitle(title); 400 node.SetTitle(title);
400 node.SetBookmarkSpecifics(specifics); 401 node.SetBookmarkSpecifics(specifics);
401 402
402 return node.GetId(); 403 return node.GetId();
403 } 404 }
(...skipping 384 matching lines...) Expand 10 before | Expand all | Expand 10 after
788 // association code. 789 // association code.
789 TEST_F(ProfileSyncServiceBookmarkTest, InitialModelAssociate) { 790 TEST_F(ProfileSyncServiceBookmarkTest, InitialModelAssociate) {
790 const int kNumBookmarksPerFolder = 10; 791 const int kNumBookmarksPerFolder = 10;
791 const int kNumFolders = 10; 792 const int kNumFolders = 10;
792 793
793 CreatePermanentBookmarkNodes(); 794 CreatePermanentBookmarkNodes();
794 795
795 { 796 {
796 syncer::WriteTransaction trans(FROM_HERE, test_user_share_.user_share()); 797 syncer::WriteTransaction trans(FROM_HERE, test_user_share_.user_share());
797 for (int i = 0; i < kNumFolders; ++i) { 798 for (int i = 0; i < kNumFolders; ++i) {
798 int64 folder_id = AddFolderToShare(&trans, 799 int64 folder_id =
799 base::StringPrintf("folder%05d", i)); 800 AddFolderToShare(&trans, base::StringPrintf("folder%05d", i));
800 for (int j = 0; j < kNumBookmarksPerFolder; ++j) { 801 for (int j = 0; j < kNumBookmarksPerFolder; ++j) {
801 AddBookmarkToShare(&trans, 802 AddBookmarkToShare(
802 folder_id, 803 &trans, folder_id, base::StringPrintf("bookmark%05d", j),
803 base::StringPrintf("bookmark%05d", j)); 804 base::StringPrintf("http://www.google.com/search?q=%05d", j));
804 } 805 }
805 } 806 }
806 } 807 }
807 808
808 LoadBookmarkModel(DELETE_EXISTING_STORAGE, DONT_SAVE_TO_STORAGE); 809 LoadBookmarkModel(DELETE_EXISTING_STORAGE, DONT_SAVE_TO_STORAGE);
809 StartSync(); 810 StartSync();
810 811
811 ExpectModelMatch(); 812 ExpectModelMatch();
812 } 813 }
813 814
815 // Tests bookmark association when nodes exists in the native model only.
816 // These entries should be copied to Sync directory during association process.
817 TEST_F(ProfileSyncServiceBookmarkTest,
818 InitialModelAssociateWithBookmarkModelNodes) {
819 LoadBookmarkModel(DELETE_EXISTING_STORAGE, DONT_SAVE_TO_STORAGE);
820 const BookmarkNode* folder =
821 model_->AddFolder(model_->other_node(), 0, base::ASCIIToUTF16("foobar"));
822 model_->AddFolder(folder, 0, base::ASCIIToUTF16("nested"));
823 model_->AddURL(folder, 0, base::ASCIIToUTF16("Internets #1 Pies Site"),
824 GURL("http://www.easypie.com/"));
825 model_->AddURL(folder, 1, base::ASCIIToUTF16("Airplanes"),
826 GURL("http://www.easyjet.com/"));
827
828 StartSync();
829 ExpectModelMatch();
830 }
831
832 // Tests bookmark association case when there is an entry in the delete journal
833 // that matches one of native bookmarks.
834 TEST_F(ProfileSyncServiceBookmarkTest, InitialModelAssociateWithDeleteJournal) {
835 LoadBookmarkModel(DELETE_EXISTING_STORAGE, DONT_SAVE_TO_STORAGE);
836 const BookmarkNode* folder = model_->AddFolder(model_->bookmark_bar_node(), 0,
837 base::ASCIIToUTF16("foobar"));
838 const BookmarkNode* bookmark =
839 model_->AddURL(folder, 0, base::ASCIIToUTF16("Airplanes"),
840 GURL("http://www.easyjet.com/"));
841
842 CreatePermanentBookmarkNodes();
843
844 // Create entries matching the folder and the bookmark above.
845 int64 folder_id, bookmark_id;
846 {
847 syncer::WriteTransaction trans(FROM_HERE, test_user_share_.user_share());
848 folder_id = AddFolderToShare(&trans, "foobar");
849 bookmark_id = AddBookmarkToShare(&trans, folder_id, "Airplanes",
850 "http://www.easyjet.com/");
851 }
852
853 // Associate the bookmark sync node with the native model one and make
854 // it deleted.
855 {
856 syncer::WriteTransaction trans(FROM_HERE, test_user_share_.user_share());
857 syncer::WriteNode node(&trans);
858 EXPECT_EQ(BaseNode::INIT_OK, node.InitByIdLookup(bookmark_id));
859
860 node.GetMutableEntryForTest()->PutLocalExternalId(bookmark->id());
861 node.GetMutableEntryForTest()->PutServerIsDel(true);
862 node.GetMutableEntryForTest()->PutIsDel(true);
863 }
864
865 ASSERT_TRUE(AssociateModels());
866 ExpectModelMatch();
867
868 // The bookmark node should be deleted.
869 EXPECT_EQ(0, folder->child_count());
870 }
871
872 // Tests that the external ID is used to match the right folder amoung
873 // multiple folders with the same name during the native model traversal.
874 // Also tests that the external ID is used to match the right bookmark
875 // among multiple identical bookmarks when dealing with the delete journal.
876 TEST_F(ProfileSyncServiceBookmarkTest,
877 InitialModelAssociateVerifyExternalIdMatch) {
878 LoadBookmarkModel(DELETE_EXISTING_STORAGE, DONT_SAVE_TO_STORAGE);
879 const int kNumFolders = 10;
880 const int kNumBookmarks = 10;
881 const int kFolderToIncludeBookmarks = 7;
882 const int kBookmarkToDelete = 4;
883
884 int64 folder_ids[kNumFolders];
885 int64 bookmark_ids[kNumBookmarks];
886
887 // Create native folders and bookmarks with identical names. Only
888 // one of the folders contains bookmarks and others are empty. Here is the
889 // expected tree shape:
890 // Bookmarks bar
891 // +- folder (#0)
892 // +- folder (#1)
893 // ...
894 // +- folder (#7) <-- Only this folder contains bookmarks.
895 // +- bookmark (#0)
896 // +- bookmark (#1)
897 // ...
898 // +- bookmark (#4) <-- Only this one bookmark should be removed later.
899 // ...
900 // +- bookmark (#9)
901 // ...
902 // +- folder (#9)
903
904 const BookmarkNode* parent_folder = nullptr;
905 for (int i = 0; i < kNumFolders; i++) {
906 const BookmarkNode* folder = model_->AddFolder(
907 model_->bookmark_bar_node(), i, base::ASCIIToUTF16("folder"));
908 folder_ids[i] = folder->id();
909 if (i == kFolderToIncludeBookmarks) {
910 parent_folder = folder;
911 }
912 }
913
914 for (int i = 0; i < kNumBookmarks; i++) {
915 const BookmarkNode* bookmark =
916 model_->AddURL(parent_folder, i, base::ASCIIToUTF16("bookmark"),
917 GURL("http://www.google.com/"));
918 bookmark_ids[i] = bookmark->id();
919 }
920
921 // Number of nodes in bookmark bar before association.
922 int total_node_count = model_->bookmark_bar_node()->GetTotalNodeCount();
923
924 CreatePermanentBookmarkNodes();
925
926 int64 sync_bookmark_id_to_delete = 0;
927 {
928 // Create sync folders matching native folders above.
929 int64 parent_id = 0;
930 syncer::WriteTransaction trans(FROM_HERE, test_user_share_.user_share());
931 // Create in reverse order because AddFolderToShare passes NULL for
932 // |predecessor| argument.
933 for (int i = kNumFolders - 1; i >= 0; i--) {
934 int64 id = AddFolderToShare(&trans, "folder");
935
936 // Pre-map sync folders to native folders by setting
937 // external ID. This will verify that the association algorithm picks
938 // the right ones despite all of them having identical names.
939 // More specifically this will help to avoid cloning bookmarks from
940 // a wrong folder.
941 syncer::WriteNode node(&trans);
942 EXPECT_EQ(BaseNode::INIT_OK, node.InitByIdLookup(id));
943 node.GetMutableEntryForTest()->PutLocalExternalId(folder_ids[i]);
944
945 if (i == kFolderToIncludeBookmarks) {
946 parent_id = id;
947 }
948 }
949
950 // Create sync bookmark matching native bookmarks above in reverse order
951 // because AddBookmarkToShare passes NULL for |predecessor| argument.
952 for (int i = kNumBookmarks - 1; i >= 0; i--) {
953 int id = AddBookmarkToShare(&trans, parent_id, "bookmark",
954 "http://www.google.com/");
955
956 // Pre-map sync bookmarks to native bookmarks by setting
957 // external ID. This will verify that the association algorithm picks
958 // the right ones despite all of them having identical names and URLs.
959 syncer::WriteNode node(&trans);
960 EXPECT_EQ(BaseNode::INIT_OK, node.InitByIdLookup(id));
961 node.GetMutableEntryForTest()->PutLocalExternalId(bookmark_ids[i]);
962
963 if (i == kBookmarkToDelete) {
964 sync_bookmark_id_to_delete = id;
965 }
966 }
967 }
968
969 // Make one bookmark deleted.
970 {
971 syncer::WriteTransaction trans(FROM_HERE, test_user_share_.user_share());
972 syncer::WriteNode node(&trans);
973 EXPECT_EQ(BaseNode::INIT_OK,
974 node.InitByIdLookup(sync_bookmark_id_to_delete));
975
976 node.GetMutableEntryForTest()->PutServerIsDel(true);
977 node.GetMutableEntryForTest()->PutIsDel(true);
978 }
979
980 // Perform association.
981 ASSERT_TRUE(AssociateModels());
982 ExpectModelMatch();
983
984 // Only one native node should have been deleted and no nodes cloned due to
985 // matching folder names.
986 EXPECT_EQ(kNumFolders, model_->bookmark_bar_node()->child_count());
987 EXPECT_EQ(kNumBookmarks - 1, parent_folder->child_count());
988 EXPECT_EQ(total_node_count - 1,
989 model_->bookmark_bar_node()->GetTotalNodeCount());
990
991 // Verify that the right bookmark got deleted and no bookmarks reordered.
992 for (int i = 0; i < parent_folder->child_count(); i++) {
993 int index_in_bookmark_ids = (i < kBookmarkToDelete) ? i : i + 1;
994 EXPECT_EQ(bookmark_ids[index_in_bookmark_ids],
995 parent_folder->GetChild(i)->id());
996 }
997 }
814 998
815 TEST_F(ProfileSyncServiceBookmarkTest, BookmarkModelOperations) { 999 TEST_F(ProfileSyncServiceBookmarkTest, BookmarkModelOperations) {
816 LoadBookmarkModel(DELETE_EXISTING_STORAGE, DONT_SAVE_TO_STORAGE); 1000 LoadBookmarkModel(DELETE_EXISTING_STORAGE, DONT_SAVE_TO_STORAGE);
817 StartSync(); 1001 StartSync();
818 1002
819 // Test addition. 1003 // Test addition.
820 const BookmarkNode* folder = 1004 const BookmarkNode* folder =
821 model_->AddFolder(model_->other_node(), 0, base::ASCIIToUTF16("foobar")); 1005 model_->AddFolder(model_->other_node(), 0, base::ASCIIToUTF16("foobar"));
822 ExpectSyncerNodeMatching(folder); 1006 ExpectSyncerNodeMatching(folder);
823 ExpectModelMatch(); 1007 ExpectModelMatch();
(...skipping 1422 matching lines...) Expand 10 before | Expand all | Expand 10 after
2246 ExpectModelMatch(); 2430 ExpectModelMatch();
2247 2431
2248 // Then simulate the add call arriving late. 2432 // Then simulate the add call arriving late.
2249 change_processor_->BookmarkNodeAdded(model_, model_->bookmark_bar_node(), 0); 2433 change_processor_->BookmarkNodeAdded(model_, model_->bookmark_bar_node(), 0);
2250 ExpectModelMatch(); 2434 ExpectModelMatch();
2251 } 2435 }
2252 2436
2253 } // namespace 2437 } // namespace
2254 2438
2255 } // namespace browser_sync 2439 } // namespace browser_sync
OLDNEW
« 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