AutoFill: Handle non-unique profile labels by appending an increasing number to

the label in the PersonalDatabaseManager.

BUG=41002
TEST=PersonalDatabaseManager.SetUnique*

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

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

3 files changed, 144 insertions, 11 deletions

diff --git a/chrome/browser/autofill/personal_data_manager.cc b/chrome/browser/autofill/personal_data_manager.cc
index 8a711cd..664bbed 100644
--- a/chrome/browser/autofill/personal_data_manager.cc
+++ b/chrome/browser/autofill/personal_data_manager.cc
@@ -238,6 +238,8 @@ void PersonalDataManager::SetProfiles(std::vector<AutoFillProfile>* profiles) {
   if (profile_->IsOffTheRecord())
     return;
 
+  SetUniqueProfileLabels(profiles);
+
   WebDataService* wds = profile_->GetWebDataService(Profile::EXPLICIT_ACCESS);
   if (!wds)
     return;
@@ -300,6 +302,8 @@ void PersonalDataManager::SetCreditCards(
   if (profile_->IsOffTheRecord())
     return;
 
+  SetUniqueCreditCardLabels(credit_cards);
+
   WebDataService* wds = profile_->GetWebDataService(Profile::EXPLICIT_ACCESS);
   if (!wds)
     return;
@@ -470,6 +474,21 @@ int PersonalDataManager::DefaultCreditCard() const {
     return 0;
 }
 
+AutoFillProfile* PersonalDataManager::CreateNewEmptyAutoFillProfileForDBThread(
+    const string16& label) {
+  // See comment in header for thread details.
+  DCHECK(ChromeThread::CurrentlyOn(ChromeThread::DB));
+  AutoLock lock(unique_ids_lock_);
+  AutoFillProfile* p = new AutoFillProfile(label,
+      CreateNextUniqueID(&unique_profile_ids_));
+  return p;
+}
+
+void PersonalDataManager::Refresh() {
+  LoadProfiles();
+  LoadCreditCards();
+}
+
 PersonalDataManager::PersonalDataManager()
     : profile_(NULL),
       is_initialized_(false),
@@ -589,17 +608,46 @@ void PersonalDataManager::CancelPendingQuery(WebDataService::Handle* handle) {
   *handle = 0;
 }
 
-AutoFillProfile* PersonalDataManager::CreateNewEmptyAutoFillProfileForDBThread(
-    const string16& label) {
-  // See comment in header for thread details.
-  DCHECK(ChromeThread::CurrentlyOn(ChromeThread::DB));
-  AutoLock lock(unique_ids_lock_);
-  AutoFillProfile* p = new AutoFillProfile(label,
-      CreateNextUniqueID(&unique_profile_ids_));
-  return p;
+void PersonalDataManager::SetUniqueProfileLabels(
+    std::vector<AutoFillProfile>* profiles) {
+  std::map<string16, std::vector<AutoFillProfile*> > label_map;
+  for (std::vector<AutoFillProfile>::iterator iter = profiles->begin();
+       iter != profiles->end(); ++iter) {
+    label_map[iter->Label()].push_back(&(*iter));
+  }
+
+  for (std::map<string16, std::vector<AutoFillProfile*> >::iterator iter =
+           label_map.begin();
+       iter != label_map.end(); ++iter) {
+    // Start at the second element because the first label should not be
+    // renamed.  The appended label number starts at 2, because the first label
+    // has an implicit index of 1.
+    for (size_t i = 1; i < iter->second.size(); ++i) {
+      string16 newlabel = iter->second[i]->Label() +
+          UintToString16(static_cast<unsigned int>(i + 1));
+      iter->second[i]->set_label(newlabel);
+    }
+  }
 }
 
-void PersonalDataManager::Refresh() {
-  LoadProfiles();
-  LoadCreditCards();
+void PersonalDataManager::SetUniqueCreditCardLabels(
+    std::vector<CreditCard>* credit_cards) {
+  std::map<string16, std::vector<CreditCard*> > label_map;
+  for (std::vector<CreditCard>::iterator iter = credit_cards->begin();
+       iter != credit_cards->end(); ++iter) {
+    label_map[iter->Label()].push_back(&(*iter));
+  }
+
+  for (std::map<string16, std::vector<CreditCard*> >::iterator iter =
+           label_map.begin();
+       iter != label_map.end(); ++iter) {
+    // Start at the second element because the first label should not be
+    // renamed.  The appended label number starts at 2, because the first label
+    // has an implicit index of 1.
+    for (size_t i = 1; i < iter->second.size(); ++i) {
+      string16 newlabel = iter->second[i]->Label() +
+          UintToString16(static_cast<unsigned int>(i + 1));
+      iter->second[i]->set_label(newlabel);
+    }
+  }
 }
diff --git a/chrome/browser/autofill/personal_data_manager.h b/chrome/browser/autofill/personal_data_manager.h
index fa0eb20..1c161d9 100644
--- a/chrome/browser/autofill/personal_data_manager.h
+++ b/chrome/browser/autofill/personal_data_manager.h
@@ -184,6 +184,13 @@ class PersonalDataManager : public WebDataServiceConsumer,
   // query handle.
   void CancelPendingQuery(WebDataService::Handle* handle);
 
+  // Ensures that all profile labels are unique by appending an increasing digit
+  // to the end of non-unique labels.
+  // TODO(jhawkins): Create a new interface for labeled entities and turn these
+  // two methods into one.
+  void SetUniqueProfileLabels(std::vector<AutoFillProfile>* profiles);
+  void SetUniqueCreditCardLabels(std::vector<CreditCard>* credit_cards);
+
   // The profile hosting this PersonalDataManager.
   Profile* profile_;
 
diff --git a/chrome/browser/autofill/personal_data_manager_unittest.cc b/chrome/browser/autofill/personal_data_manager_unittest.cc
index 10d43a3..8656e25 100644
--- a/chrome/browser/autofill/personal_data_manager_unittest.cc
+++ b/chrome/browser/autofill/personal_data_manager_unittest.cc
@@ -559,3 +559,81 @@ TEST_F(PersonalDataManagerTest, DefaultCreditCards) {
   // setting to label of creditcard1.
   EXPECT_EQ(1, personal_data_->DefaultCreditCard());
 }
+
+TEST_F(PersonalDataManagerTest, SetUniqueProfileLabels) {
+  AutoFillProfile profile0(ASCIIToUTF16("Home"), 0);
+  AutoFillProfile profile1(ASCIIToUTF16("Home"), 0);
+  AutoFillProfile profile2(ASCIIToUTF16("Home"), 0);
+  AutoFillProfile profile3(ASCIIToUTF16("NotHome"), 0);
+  AutoFillProfile profile4(ASCIIToUTF16("Work"), 0);
+  AutoFillProfile profile5(ASCIIToUTF16("Work"), 0);
+
+  // 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();
+
+  // Add the test profiles to the database.
+  std::vector<AutoFillProfile> update;
+  update.push_back(profile0);
+  update.push_back(profile1);
+  update.push_back(profile2);
+  update.push_back(profile3);
+  update.push_back(profile4);
+  update.push_back(profile5);
+  personal_data_->SetProfiles(&update);
+
+  // And wait for the refresh.
+  EXPECT_CALL(personal_data_observer_,
+      OnPersonalDataLoaded()).WillOnce(QuitUIMessageLoop());
+
+  MessageLoop::current()->Run();
+
+  const std::vector<AutoFillProfile*>& results = personal_data_->profiles();
+  ASSERT_EQ(6U, results.size());
+  EXPECT_EQ(ASCIIToUTF16("Home"), results[0]->Label());
+  EXPECT_EQ(ASCIIToUTF16("Home2"), results[1]->Label());
+  EXPECT_EQ(ASCIIToUTF16("Home3"), results[2]->Label());
+  EXPECT_EQ(ASCIIToUTF16("NotHome"), results[3]->Label());
+  EXPECT_EQ(ASCIIToUTF16("Work"), results[4]->Label());
+  EXPECT_EQ(ASCIIToUTF16("Work2"), results[5]->Label());
+}
+
+TEST_F(PersonalDataManagerTest, SetUniqueCreditCardLabels) {
+  CreditCard profile0(ASCIIToUTF16("Home"), 0);
+  CreditCard profile1(ASCIIToUTF16("Home"), 0);
+  CreditCard profile2(ASCIIToUTF16("Home"), 0);
+  CreditCard profile3(ASCIIToUTF16("NotHome"), 0);
+  CreditCard profile4(ASCIIToUTF16("Work"), 0);
+  CreditCard profile5(ASCIIToUTF16("Work"), 0);
+
+  // 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();
+
+  // Add the test profiles to the database.
+  std::vector<CreditCard> update;
+  update.push_back(profile0);
+  update.push_back(profile1);
+  update.push_back(profile2);
+  update.push_back(profile3);
+  update.push_back(profile4);
+  update.push_back(profile5);
+  personal_data_->SetCreditCards(&update);
+
+  const std::vector<CreditCard*>& results = personal_data_->credit_cards();
+  ASSERT_EQ(6U, results.size());
+  EXPECT_EQ(ASCIIToUTF16("Home"), results[0]->Label());
+  EXPECT_EQ(ASCIIToUTF16("Home2"), results[1]->Label());
+  EXPECT_EQ(ASCIIToUTF16("Home3"), results[2]->Label());
+  EXPECT_EQ(ASCIIToUTF16("NotHome"), results[3]->Label());
+  EXPECT_EQ(ASCIIToUTF16("Work"), results[4]->Label());
+  EXPECT_EQ(ASCIIToUTF16("Work2"), results[5]->Label());
+}