Autofill Multi-Value: Additional profiles with same name and address not added to DOM UI

Separates out the merging from |PersonalDataManager::AddProfile| so that merging only happens on form submission, and not when the user edits profiles manually via preferences.

BUG=80341
TEST=PersonalDataManagerTest.AddProfile

Review URL: http://codereview.chromium.org/6903091

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@83270 0039d316-1c4b-4281-b951-d872f2087c98

5 files changed, 103 insertions, 17 deletions

diff --git a/chrome/browser/autofill/autofill-inl.h b/chrome/browser/autofill/autofill-inl.h
index 66e0df2..4d90324 100644
--- a/chrome/browser/autofill/autofill-inl.h
+++ b/chrome/browser/autofill/autofill-inl.h
@@ -14,11 +14,11 @@ class FormGroupMatchesByCompareFunctor {
   }
 
   bool operator()(const T* form_group) {
-    return form_group->Compare(form_group_) == 0;
+    return form_group->CompareMulti(form_group_) == 0;
   }
 
   bool operator()(const T& form_group) {
-    return form_group.Compare(form_group_) == 0;
+    return form_group.CompareMulti(form_group_) == 0;
   }
 
  private:
diff --git a/chrome/browser/autofill/credit_card.cc b/chrome/browser/autofill/credit_card.cc
index afb20e0..4018fe4 100644
--- a/chrome/browser/autofill/credit_card.cc
+++ b/chrome/browser/autofill/credit_card.cc
@@ -372,6 +372,10 @@ int CreditCard::Compare(const CreditCard& credit_card) const {
   return 0;
 }
 
+int CreditCard::CompareMulti(const CreditCard& credit_card) const {
+  return Compare(credit_card);
+}
+
 bool CreditCard::operator==(const CreditCard& credit_card) const {
   if (guid_ != credit_card.guid_)
     return false;
diff --git a/chrome/browser/autofill/credit_card.h b/chrome/browser/autofill/credit_card.h
index a96e0a0..1b8cc97 100644
--- a/chrome/browser/autofill/credit_card.h
+++ b/chrome/browser/autofill/credit_card.h
@@ -58,6 +58,10 @@ class CreditCard : public FormGroup {
   // credit cards themselves.
   int Compare(const CreditCard& credit_card) const;
 
+  // This is same as |Compare| for credit cards as they are single-valued.
+  // This is here to unify templated code that deals with |FormGroup|s.
+  int CompareMulti(const CreditCard& credit_card) const;
+
   // Used by tests.
   bool operator==(const CreditCard& credit_card) const;
   bool operator!=(const CreditCard& credit_card) const;
diff --git a/chrome/browser/autofill/personal_data_manager.cc b/chrome/browser/autofill/personal_data_manager.cc
index 6796a96..0684c72 100644
--- a/chrome/browser/autofill/personal_data_manager.cc
+++ b/chrome/browser/autofill/personal_data_manager.cc
@@ -450,19 +450,15 @@ void PersonalDataManager::SetCreditCards(
   FOR_EACH_OBSERVER(Observer, observers_, OnPersonalDataChanged());
 }
 
-// TODO(jhawkins): Refactor SetProfiles so this isn't so hacky.
+// TODO(dhollowa): Refactor to eliminate batch update of |SetProfiles|.
+// http://crbug.com/73068
 void PersonalDataManager::AddProfile(const AutofillProfile& profile) {
-  // Don't save a web profile if the data in the profile is a subset of an
-  // auxiliary profile.
-  for (std::vector<AutofillProfile*>::const_iterator iter =
-           auxiliary_profiles_.begin();
-       iter != auxiliary_profiles_.end(); ++iter) {
-    if (profile.IsSubsetOf(**iter))
-      return;
-  }
+  std::vector<AutofillProfile> profiles(web_profiles_.size());
+  std::transform(web_profiles_.begin(), web_profiles_.end(),
+                 profiles.begin(),
+                 DereferenceFunctor<AutofillProfile>());
 
-  std::vector<AutofillProfile> profiles;
-  MergeProfile(profile, web_profiles_.get(), &profiles);
+  profiles.push_back(profile);
   SetProfiles(&profiles);
 }
 
@@ -488,8 +484,9 @@ void PersonalDataManager::UpdateProfile(const AutofillProfile& profile) {
   FOR_EACH_OBSERVER(Observer, observers_, OnPersonalDataChanged());
 }
 
+// TODO(dhollowa): Refactor to eliminate batch update of |SetProfiles|.
+// http://crbug.com/73068
 void PersonalDataManager::RemoveProfile(const std::string& guid) {
-  // TODO(jhawkins): Refactor SetProfiles so this isn't so hacky.
   std::vector<AutofillProfile> profiles(web_profiles_.size());
   std::transform(web_profiles_.begin(), web_profiles_.end(),
                  profiles.begin(),
@@ -514,7 +511,8 @@ AutofillProfile* PersonalDataManager::GetProfileByGUID(
   return NULL;
 }
 
-// TODO(jhawkins): Refactor SetCreditCards so this isn't so hacky.
+// TODO(dhollowa): Refactor to eliminate batch update of |SetCreditCards|.
+// http://crbug.com/73068
 void PersonalDataManager::AddCreditCard(const CreditCard& credit_card) {
   std::vector<CreditCard> credit_cards(credit_cards_.size());
   std::transform(credit_cards_.begin(), credit_cards_.end(),
@@ -544,8 +542,9 @@ void PersonalDataManager::UpdateCreditCard(const CreditCard& credit_card) {
   FOR_EACH_OBSERVER(Observer, observers_, OnPersonalDataChanged());
 }
 
+// TODO(dhollowa): Refactor to eliminate batch update of |SetCreditCards|.
+// http://crbug.com/73068
 void PersonalDataManager::RemoveCreditCard(const std::string& guid) {
-  // TODO(jhawkins): Refactor SetCreditCards so this isn't so hacky.
   std::vector<CreditCard> credit_cards(credit_cards_.size());
   std::transform(credit_cards_.begin(), credit_cards_.end(),
                  credit_cards.begin(),
@@ -839,7 +838,18 @@ void PersonalDataManager::SaveImportedProfile(
   if (profile_->IsOffTheRecord())
     return;
 
-  AddProfile(imported_profile);
+  // Don't save a web profile if the data in the profile is a subset of an
+  // auxiliary profile.
+  for (std::vector<AutofillProfile*>::const_iterator iter =
+           auxiliary_profiles_.begin();
+       iter != auxiliary_profiles_.end(); ++iter) {
+    if (imported_profile.IsSubsetOf(**iter))
+      return;
+  }
+
+  std::vector<AutofillProfile> profiles;
+  MergeProfile(imported_profile, web_profiles_.get(), &profiles);
+  SetProfiles(&profiles);
 }
 
 
diff --git a/chrome/browser/autofill/personal_data_manager_unittest.cc b/chrome/browser/autofill/personal_data_manager_unittest.cc
index 4c031b6..458f0e5c 100644
--- a/chrome/browser/autofill/personal_data_manager_unittest.cc
+++ b/chrome/browser/autofill/personal_data_manager_unittest.cc
@@ -87,6 +87,74 @@ class PersonalDataManagerTest : public testing::Test {
   PersonalDataLoadedObserverMock personal_data_observer_;
 };
 
+TEST_F(PersonalDataManagerTest, AddProfile) {
+  // This will verify that the web database has been loaded and the notification
+  // sent out.
+  EXPECT_CALL(personal_data_observer_,
+              OnPersonalDataLoaded()).WillOnce(QuitUIMessageLoop());
+
+  // The message loop will exit when the mock observer is notified.
+  MessageLoop::current()->Run();
+
+  AutofillProfile profile0;
+  autofill_test::SetProfileInfo(&profile0,
+      "John", "Mitchell", "Smith",
+      "j@s.com", "Acme Inc.", "1 Main", "Apt A", "San Francisco", "CA",
+      "94102", "USA", "4158889999", "4152223333");
+
+  // Add profile0 to the database.
+  personal_data_->AddProfile(profile0);
+
+  // Reload the database.
+  ResetPersonalDataManager();
+  EXPECT_CALL(personal_data_observer_,
+              OnPersonalDataLoaded()).WillOnce(QuitUIMessageLoop());
+  MessageLoop::current()->Run();
+
+  // Verify the addition.
+  const std::vector<AutofillProfile*>& results1 = personal_data_->profiles();
+  ASSERT_EQ(1U, results1.size());
+  EXPECT_EQ(0, profile0.CompareMulti(*results1.at(0)));
+
+  // Add profile with identical values.  Duplicates should not get saved.
+  AutofillProfile profile0a = profile0;
+  profile0a.set_guid(guid::GenerateGUID());
+  personal_data_->AddProfile(profile0a);
+
+  // Reload the database.
+  ResetPersonalDataManager();
+  EXPECT_CALL(personal_data_observer_,
+              OnPersonalDataLoaded()).WillOnce(QuitUIMessageLoop());
+  MessageLoop::current()->Run();
+
+  // Verify the non-addition.
+  const std::vector<AutofillProfile*>& results2 = personal_data_->profiles();
+  ASSERT_EQ(1U, results2.size());
+  EXPECT_EQ(0, profile0.CompareMulti(*results2.at(0)));
+
+  // New profile with different email.
+  AutofillProfile profile1 = profile0;
+  profile1.set_guid(guid::GenerateGUID());
+  profile1.SetInfo(EMAIL_ADDRESS, ASCIIToUTF16("john@smith.com"));
+
+  // Add the different profile.  This should save as a separate profile.
+  // Note that if this same profile was "merged" it would collapse to one
+  // profile with a multi-valued entry for email.
+  personal_data_->AddProfile(profile1);
+
+  // Reload the database.
+  ResetPersonalDataManager();
+  EXPECT_CALL(personal_data_observer_,
+              OnPersonalDataLoaded()).WillOnce(QuitUIMessageLoop());
+  MessageLoop::current()->Run();
+
+  // Verify the addition.
+  const std::vector<AutofillProfile*>& results3 = personal_data_->profiles();
+  ASSERT_EQ(2U, results3.size());
+  EXPECT_EQ(0, profile0.CompareMulti(*results3.at(0)));
+  EXPECT_EQ(0, profile1.CompareMulti(*results3.at(1)));
+}
+
 // TODO(jhawkins): Test SetProfiles w/out a WebDataService in the profile.
 TEST_F(PersonalDataManagerTest, SetProfiles) {
   AutofillProfile profile0;