[Autofill] Dedupe similar profiles on insertion.

components/autofill/core/browser/personal_data_manager_unittest.cc

Index: components/autofill/core/browser/personal_data_manager_unittest.cc
diff --git a/components/autofill/core/browser/personal_data_manager_unittest.cc b/components/autofill/core/browser/personal_data_manager_unittest.cc
index dde71cf38938ff0d3449aa4c6f932903b6fdf77e..410d3b65df058e3c7d0c5ce87a0ffe0e412e3b5a 100644
--- a/components/autofill/core/browser/personal_data_manager_unittest.cc
+++ b/components/autofill/core/browser/personal_data_manager_unittest.cc
@@ -4366,4 +4366,236 @@ TEST_F(PersonalDataManagerTest, MergeProfile_UsageStats) {
             base::Time::Now() - profile.modification_date());
 }
 
+// Tests that using a profile results in a merge of with all similar profiles
+// and that all but the resulting profile gets deleted. Also tests that
+// non-similar profiles are not affected by the merge or the delete.
+TEST_F(PersonalDataManagerTest, DedupeOnInsert) {
+  // Created saved profiles.

[Mathieu, 2016/06/09 20:39:28]

*Create?

[sebsg, 2016/06/12 21:41:25]

Done.

+  // Create two very similar profiles that should be deduped.

[Mathieu, 2016/06/09 20:39:28]

nit: mention the difference for the reader 

"(one has a company name and the other does not)"

[sebsg, 2016/06/12 21:41:25]

Done.

+  AutofillProfile profile1(base::GenerateGUID(), "https://www.example.com");
+  test::SetProfileInfo(&profile1, "Homer", "Jay", "Simpson",
+                       "homer.simpson@abc.com", "", "742 Evergreen Terrace", "",
+                       "Springfield", "IL", "91601", "US", "12345678910");
+  profile1.set_use_count(2);
+  AutofillProfile profile2(base::GenerateGUID(), "https://www.example.com");
+  test::SetProfileInfo(&profile2, "Homer", "Jay", "Simpson",
+                       "homer.simpson@abc.com", "Fox", "742 Evergreen Terrace.",
+                       "", "Springfield", "IL", "91601", "US", "12345678910");
+  profile2.set_use_count(3);
+
+  // Create a different profile that should not be deduped (different address).
+  AutofillProfile profile3(base::GenerateGUID(), "https://www.example.com");
+  test::SetProfileInfo(&profile3, "Homer", "Jay", "Simpson",

[Mathieu, 2016/06/09 20:39:28]

Let's have one with no middle name.

[sebsg, 2016/06/12 21:41:25]

Done.

+                       "homer.simpson@abc.com", "Fox", "1234 Other Street", "",
+                       "Springfield", "IL", "91601", "US", "12345678910");
+  profile3.set_use_count(4);
+
+  // Create another different profile that should not be deduped (different
+  // name).
+  AutofillProfile profile4(base::GenerateGUID(), "https://www.example.com");
+  test::SetProfileInfo(&profile4, "Marjorie", "Jacqueline", "Simpson",
+                       "homer.simpson@abc.com", "Fox", "742 Evergreen Terrace",
+                       "", "Springfield", "IL", "91601", "US", "12345678910");
+  profile4.set_use_count(1);
+
+  personal_data_->AddProfile(profile1);
+  personal_data_->AddProfile(profile2);
+  personal_data_->AddProfile(profile3);
+  personal_data_->AddProfile(profile4);
+
+  EXPECT_CALL(personal_data_observer_, OnPersonalDataChanged())
+      .WillOnce(QuitMainMessageLoop());
+  base::MessageLoop::current()->Run();
+
+  EXPECT_EQ(4U, personal_data_->GetProfiles().size());
+
+  // Create a new imported profile with no company name. It is similar to
+  // profiles 1 and 2 and should be merged with them.
+  AutofillProfile imported_profile(base::GenerateGUID(),
+                                   "https://www.example.com");
+  test::SetProfileInfo(&imported_profile, "Homer", "Jay", "Simpson",
+                       "homer.simpson@abc.com", "", "742. Evergreen Terrace",
+                       "", "Springfield", "IL", "91601", "US", "12345678910");
+
+  // Save the imported profile (use it).
+  personal_data_->SaveImportedProfile(imported_profile);
+
+  EXPECT_CALL(personal_data_observer_, OnPersonalDataChanged())
+      .WillOnce(QuitMainMessageLoop());
+  base::MessageLoop::current()->Run();
+
+  std::vector<AutofillProfile*> profiles = personal_data_->GetProfiles();
+
+  // The imported profile and saved profiles 1 and 2 should be merged together.
+  // Therefore there should only be 3 saved profiles.
+  ASSERT_EQ(3U, profiles.size());
+
+  // Sort the profiles by frecency to have a deterministic order.
+  base::Time comparison_time = base::Time::Now();
+  std::sort(profiles.begin(), profiles.end(),
+            [comparison_time](const AutofillDataModel* a,
+                              const AutofillDataModel* b) {
+              return a->CompareFrecency(b, comparison_time);
+            });
+
+  // Since profiles with higher frecency scores are merged into profiles with
+  // lower frecency scores, the result of the merge should be contained in
+  // profile1 since it had a lower frecency score compared to profile2.
+  EXPECT_EQ(profile1.guid(), profiles[0]->guid());
+  // The address syntax that results from the merge should be the one from the
+  // imported profile (most recent).
+  EXPECT_EQ(UTF8ToUTF16("742. Evergreen Terrace"),
+            profiles[0]->GetRawInfo(ADDRESS_HOME_LINE1));
+  // Even though the imported profile had no company name, a merge should not
+  // result in a loss of information. Therefore the company name present in the
+  // saved profile should be kept.

[Mathieu, 2016/06/09 20:39:28]

nit: |profile2|

[sebsg, 2016/06/12 21:41:25]

Done.

+  EXPECT_EQ(UTF8ToUTF16("Fox"), profiles[0]->GetRawInfo(COMPANY_NAME));
+
+  // Make sure the two other remaining profiles are the expected ones.
+  EXPECT_EQ(profile3.guid(), profiles[1]->guid());
+  EXPECT_EQ(UTF8ToUTF16("1234 Other Street"),
+            profiles[1]->GetRawInfo(ADDRESS_HOME_LINE1));
+  EXPECT_EQ(profile4.guid(), profiles[2]->guid());
+  EXPECT_EQ(UTF8ToUTF16("Marjorie"), profiles[2]->GetRawInfo(NAME_FIRST));
+}
+
+// Tests that FindAndMergeDuplicateProfiles sets the correct profile guids to
+// delete after merging similar profiles.
+TEST_F(PersonalDataManagerTest,
+       FindAndMergeDuplicateProfiles_ProfilesToDelete) {
+  // Create the profile for which to find duplicates.
+  AutofillProfile profile1(base::GenerateGUID(), "https://www.example.com");
+  test::SetProfileInfo(&profile1, "Homer", "Jay", "Simpson",
+                       "homer.simpson@abc.com", "", "742. Evergreen Terrace",
+                       "", "Springfield", "IL", "91601", "US", "12345678910");
+
+  // Create a different profile that should not be deduped (different address).
+  AutofillProfile profile2(base::GenerateGUID(), "https://www.example.com");
+  test::SetProfileInfo(&profile2, "Homer", "Jay", "Simpson",
+                       "homer.simpson@abc.com", "Fox", "1234 Other Street", "",
+                       "Springfield", "IL", "91601", "US", "12345678910");
+
+  // Create a profile similar to profile1 which should be deduped.
+  AutofillProfile profile3(base::GenerateGUID(), "https://www.example.com");
+  test::SetProfileInfo(&profile3, "Homer", "Jay", "Simpson",
+                       "homer.simpson@abc.com", "", "742 Evergreen Terrace", "",
+                       "Springfield", "IL", "91601", "US", "12345678910");
+
+  // Create another different profile that should not be deduped (different
+  // name).
+  AutofillProfile profile4(base::GenerateGUID(), "https://www.example.com");
+  test::SetProfileInfo(&profile4, "Marjorie", "Jacqueline", "Simpson",
+                       "homer.simpson@abc.com", "Fox", "742 Evergreen Terrace",
+                       "", "Springfield", "IL", "91601", "US", "12345678910");
+
+  // Create another profile similar to profile1. Since that one is last, the
+  // result of the merge should be in this profile at the end of the test.
+  AutofillProfile profile5(base::GenerateGUID(), "https://www.example.com");
+  test::SetProfileInfo(&profile5, "Homer", "Jay", "Simpson",
+                       "homer.simpson@abc.com", "Fox", "742 Evergreen Terrace.",
+                       "", "Springfield", "IL", "91601", "US", "12345678910");
+
+  std::vector<AutofillProfile*> existing_profiles;
+  existing_profiles.push_back(&profile1);
+  existing_profiles.push_back(&profile2);
+  existing_profiles.push_back(&profile3);
+  existing_profiles.push_back(&profile4);
+  existing_profiles.push_back(&profile5);
+
+  std::vector<std::string> guids_to_delete;
+  personal_data_->FindAndMergeDuplicateProfiles(existing_profiles, &profile5,
+                                                &guids_to_delete, "US - EN");
+

[Mathieu, 2016/06/09 20:39:28]

Let's check that other guids are still present in |existing_profiles|

[sebsg, 2016/06/12 21:41:25]

Done.

+  // Profile1 should be deleted because it was sent as the profile to merge and
+  // thus was merged into profile3 and then into profile5.
+  EXPECT_TRUE(std::find(guids_to_delete.begin(), guids_to_delete.end(),
+                        profile1.guid()) != guids_to_delete.end());
+
+  // Profile3 should be deleted because profile1 was merged into it and the
+  // resulting profile was then merged into profile5.
+  EXPECT_TRUE(std::find(guids_to_delete.begin(), guids_to_delete.end(),
+                        profile3.guid()) != guids_to_delete.end());
+}
+
+// Tests that FindAndMergeDuplicateProfiles merges the profile values correctly,
+// ie: never lose information and keep the syntax of the profile with the higher
+// frecency score.
+TEST_F(PersonalDataManagerTest,
+       FindAndMergeDuplicateProfiles_MergedProfileValues) {
+  // Create a saved profile with a higher frecency score.
+  AutofillProfile profile1(base::GenerateGUID(), "https://www.example.com");
+  test::SetProfileInfo(&profile1, "Homer", "Jay", "Simpson",
+                       "homer.simpson@abc.com", "", "742 Evergreen Terrace", "",
+                       "Springfield", "IL", "91601", "", "12345678910");
+  profile1.set_use_count(5);
+  profile1.set_use_date(base::Time::Now() - base::TimeDelta::FromDays(3));
+
+  // Create a saved profile with a lower frecency score.
+  AutofillProfile profile2(base::GenerateGUID(), "https://www.example.com");
+  test::SetProfileInfo(&profile2, "Homer", "J", "Simpson",
+                       "homer.simpson@abc.com", "Fox", "742 Evergreen Terrace.",
+                       "", "Springfield", "IL", "91601", "", "");
+  profile2.set_use_count(3);
+  profile2.set_use_date(base::Time::Now() - base::TimeDelta::FromDays(5));
+
+  personal_data_->AddProfile(profile1);
+  personal_data_->AddProfile(profile2);
+
+  EXPECT_CALL(personal_data_observer_, OnPersonalDataChanged())
+      .WillOnce(QuitMainMessageLoop());
+  base::MessageLoop::current()->Run();
+
+  EXPECT_EQ(2U, personal_data_->GetProfiles().size());
+
+  // Create a new imported profile to be merged with the saved profiles.
+  AutofillProfile imported_profile(base::GenerateGUID(),
+                                   "https://www.example.com");
+  test::SetProfileInfo(&imported_profile, "Homer", "J", "Simpson",
+                       "homer.simpson@abc.com", "", "742. Evergreen Terrace",
+                       "", "Springfield", "IL", "91601", "US", "");
+
+  personal_data_->SaveImportedProfile(imported_profile);
+
+  EXPECT_CALL(personal_data_observer_, OnPersonalDataChanged())
+      .WillOnce(QuitMainMessageLoop());
+  base::MessageLoop::current()->Run();
+
+  std::vector<AutofillProfile*> profiles = personal_data_->GetProfiles();
+
+  // The imported profile and saved profiles 1 and 2 should be merged together.
+  // Therefore there should only be 1 saved profile.
+  ASSERT_EQ(1U, profiles.size());
+
+  // Since profiles with higher frecency scores are merged into profiles with
+  // lower frecency scores, the result of the merge should be contained in
+  // profile2 since it had a lower frecency score compared to profile1.
+  EXPECT_EQ(profile2.guid(), profiles[0]->guid());
+  // The address syntax that results from the merge should be the one from the
+  // imported profile (highest frecency).
+  EXPECT_EQ(UTF8ToUTF16("742. Evergreen Terrace"),
+            profiles[0]->GetRawInfo(ADDRESS_HOME_LINE1));
+  // The middle name should be full, even if the profile with the higher
+  // frecency only had an initial (no loss of information).
+  EXPECT_EQ(UTF8ToUTF16("Jay"), profiles[0]->GetRawInfo(NAME_MIDDLE));
+  // The specified phone number from profile1 should be kept (no loss of
+  // information).
+  EXPECT_EQ(UTF8ToUTF16("12345678910"),
+            profiles[0]->GetRawInfo(PHONE_HOME_WHOLE_NUMBER));
+  // The specified company name from profile2 should be kept (no loss of
+  // information).
+  EXPECT_EQ(UTF8ToUTF16("Fox"), profiles[0]->GetRawInfo(COMPANY_NAME));
+  // The specified country from the imported profile shoudl be kept (no loss of
+  // information).
+  EXPECT_EQ(UTF8ToUTF16("US"), profiles[0]->GetRawInfo(ADDRESS_HOME_COUNTRY));
+  // The use count that results from the merge should be the sum of the two
+  // saved profiles plus 1 (imported profile count).
+  EXPECT_EQ(profile1.use_count() + profile2.use_count() +
+                imported_profile.use_count(),
+            profiles[0]->use_count());
+  // The use date that results from the merge should be the one from the
+  // imported profile since it was used just now.
+  EXPECT_GT(base::Time::Now() - base::TimeDelta::FromSeconds(1),
+            profiles[0]->use_date());
+}
+
 }  // namespace autofill