AutoFill: Remove duplicate suggestions when providing suggestions for a field

in a form that has already been auto-filled.

BUG=47436
TEST=AutoFillManagerTest.GetFieldSuggestionsWithDuplicateValues

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

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

2 files changed, 92 insertions, 18 deletions

diff --git a/chrome/browser/autofill/autofill_manager.cc b/chrome/browser/autofill/autofill_manager.cc
index 5343db2..dbe01d0 100644
--- a/chrome/browser/autofill/autofill_manager.cc
+++ b/chrome/browser/autofill/autofill_manager.cc
@@ -37,6 +37,27 @@ const int kAutoFillPhoneNumberSuffixCount = 4;
 
 const string16::value_type kLabelSeparator[] = {';',' ',0};
 
+// Removes duplicate elements whilst preserving original order of |elements|.
+void RemoveDuplicateElements(std::vector<string16>* elements) {
+  std::vector<string16> copy;
+  for (std::vector<string16>::iterator iter = elements->begin();
+       iter != elements->end(); ++iter) {
+    bool unique = true;
+    for (std::vector<string16>::const_iterator copy_iter = copy.begin();
+         copy_iter != copy.end(); ++copy_iter) {
+      if (*iter == *copy_iter) {
+        unique = false;
+        break;
+      }
+    }
+
+    if (unique)
+      copy.push_back(*iter);
+  }
+
+  elements->assign(copy.begin(), copy.end());
+}
+
 }  // namespace
 
 // TODO(jhawkins): Maybe this should be in a grd file?
@@ -170,9 +191,13 @@ bool AutoFillManager::GetAutoFillSuggestions(int query_id,
     return false;
 
   // If the form is auto-filled and the renderer is querying for suggestions,
-  // then the user is editing the value of a field.  In this case, we don't
-  // want to display the labels, as that information is redundant.
+  // then the user is editing the value of a field.  In this case, don't display
+  // labels, as that information is redundant. In addition, remove duplicate
+  // values.
   if (form_autofilled) {
+    RemoveDuplicateElements(&values);
+    labels.resize(values.size());
+
     for (size_t i = 0; i < labels.size(); ++i)
       labels[i] = string16();
   }
diff --git a/chrome/browser/autofill/autofill_manager_unittest.cc b/chrome/browser/autofill/autofill_manager_unittest.cc
index fbb5f3e..e989269 100644
--- a/chrome/browser/autofill/autofill_manager_unittest.cc
+++ b/chrome/browser/autofill/autofill_manager_unittest.cc
@@ -49,13 +49,7 @@ class TestPersonalDataManager : public PersonalDataManager {
     return NULL;
   }
 
-  void AddSemicolonAutoFillProfile() {
-    AutoFillProfile* profile = new AutoFillProfile;
-    autofill_unittest::SetProfileInfo(profile, "Home; 8765", "Joe", "", "Ely",
-                                      "flatlander@gmail.com", "MCA",
-                                      "916 16th St.", "Apt. 6", "Lubbock",
-                                      "Texas", "79401", "USA",
-                                      "12345678901", "");
+  void AddProfile(AutoFillProfile* profile) {
     web_profiles_->push_back(profile);
   }
 
@@ -115,8 +109,8 @@ class TestAutoFillManager : public AutoFillManager {
     return test_personal_data_->GetLabeledProfile(label);
   }
 
-  void AddSemicolonAutoFillProfile() {
-    test_personal_data_->AddSemicolonAutoFillProfile();
+  void AddProfile(AutoFillProfile* profile) {
+    test_personal_data_->AddProfile(profile);
   }
 
  private:
@@ -266,10 +260,6 @@ class AutoFillManagerTest : public RenderViewHostTestHarness {
     return true;
   }
 
-  void AddSemicolonAutoFillProfile() {
-    autofill_manager_->AddSemicolonAutoFillProfile();
-  }
-
  protected:
   scoped_ptr<TestAutoFillManager> autofill_manager_;
 
@@ -454,7 +444,14 @@ TEST_F(AutoFillManagerTest, GetCreditCardSuggestionsNonCCNumber) {
 }
 
 TEST_F(AutoFillManagerTest, GetCreditCardSuggestionsSemicolon) {
-  AddSemicolonAutoFillProfile();
+  // |profile| will be owned by the mock PersonalDataManager.
+  AutoFillProfile* profile = new AutoFillProfile;
+  autofill_unittest::SetProfileInfo(profile, "Home; 8765", "Joe", "", "Ely",
+                                    "flatlander@gmail.com", "MCA",
+                                    "916 16th St.", "Apt. 6", "Lubbock",
+                                    "Texas", "79401", "USA",
+                                    "12345678901", "");
+  autofill_manager_->AddProfile(profile);
 
   FormData form;
   CreateTestFormDataBilling(&form);
@@ -531,6 +528,44 @@ TEST_F(AutoFillManagerTest, GetFieldSuggestionsFormIsAutoFilled) {
   EXPECT_EQ(string16(), labels[1]);
 }
 
+TEST_F(AutoFillManagerTest, GetFieldSuggestionsWithDuplicateValues) {
+  FormData form;
+  CreateTestFormData(&form);
+
+  // Set up our FormStructures.
+  std::vector<FormData> forms;
+  forms.push_back(form);
+  autofill_manager_->FormsSeen(forms);
+
+  // |profile| will be owned by the mock PersonalDataManager.
+  AutoFillProfile* profile = new AutoFillProfile;
+  autofill_unittest::SetProfileInfo(profile, "Duplicate", "Elvis", "", "", "",
+                                    "", "", "", "", "", "", "", "", "");
+  autofill_manager_->AddProfile(profile);
+
+  // The page ID sent to the AutoFillManager from the RenderView, used to send
+  // an IPC message back to the renderer.
+  const int kPageID = 1;
+
+  webkit_glue::FormField field;
+  autofill_unittest::CreateTestFormField(
+      "First Name", "firstname", "", "text", &field);
+  EXPECT_TRUE(autofill_manager_->GetAutoFillSuggestions(kPageID, true, field));
+
+  // Test that we sent the right message to the renderer.
+  int page_id = 0;
+  std::vector<string16> values;
+  std::vector<string16> labels;
+  EXPECT_TRUE(GetAutoFillSuggestionsMessage(&page_id, &values, &labels));
+  EXPECT_EQ(kPageID, page_id);
+  ASSERT_EQ(2U, values.size());
+  EXPECT_EQ(ASCIIToUTF16("Elvis"), values[0]);
+  EXPECT_EQ(ASCIIToUTF16("Charles"), values[1]);
+  ASSERT_EQ(2U, labels.size());
+  EXPECT_EQ(string16(), labels[0]);
+  EXPECT_EQ(string16(), labels[1]);
+}
+
 TEST_F(AutoFillManagerTest, FillCreditCardForm) {
   FormData form;
   CreateTestFormDataBilling(&form);
@@ -608,7 +643,14 @@ TEST_F(AutoFillManagerTest, FillCreditCardForm) {
 }
 
 TEST_F(AutoFillManagerTest, FillNonBillingFormSemicolon) {
-  AddSemicolonAutoFillProfile();
+  // |profile| will be owned by the mock PersonalDataManager.
+  AutoFillProfile* profile = new AutoFillProfile;
+  autofill_unittest::SetProfileInfo(profile, "Home; 8765", "Joe", "", "Ely",
+                                    "flatlander@gmail.com", "MCA",
+                                    "916 16th St.", "Apt. 6", "Lubbock",
+                                    "Texas", "79401", "USA",
+                                    "12345678901", "");
+  autofill_manager_->AddProfile(profile);
 
   FormData form;
   CreateTestFormData(&form);
@@ -673,7 +715,14 @@ TEST_F(AutoFillManagerTest, FillNonBillingFormSemicolon) {
 }
 
 TEST_F(AutoFillManagerTest, FillBillFormSemicolon) {
-  AddSemicolonAutoFillProfile();
+  // |profile| will be owned by the mock PersonalDataManager.
+  AutoFillProfile* profile = new AutoFillProfile;
+  autofill_unittest::SetProfileInfo(profile, "Home; 8765", "Joe", "", "Ely",
+                                    "flatlander@gmail.com", "MCA",
+                                    "916 16th St.", "Apt. 6", "Lubbock",
+                                    "Texas", "79401", "USA",
+                                    "12345678901", "");
+  autofill_manager_->AddProfile(profile);
 
   FormData form;
   CreateTestFormDataBilling(&form);