Fix for Autofill popup labels should reflect the contents of the HTML form

BUG=58887
TEST=unit-test and in the bug
Review URL: http://codereview.chromium.org/4448004

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

4 files changed, 172 insertions, 29 deletions

diff --git a/chrome/browser/autofill/autofill_manager.cc b/chrome/browser/autofill/autofill_manager.cc
index 25b3192..e5056e2 100644
--- a/chrome/browser/autofill/autofill_manager.cc
+++ b/chrome/browser/autofill/autofill_manager.cc
@@ -500,8 +500,19 @@ void AutoFillManager::GetProfileSuggestions(FormStructure* form,
     }
   }
 
+  std::vector<AutoFillFieldType> form_fields;
+  form_fields.reserve(form->field_count());
+  for (std::vector<AutoFillField*>::const_iterator iter = form->begin();
+       iter != form->end(); ++iter) {
+    // The field list is terminated with a NULL AutoFillField, so don't try to
+    // dereference it.
+    if (!*iter)
+      break;
+    form_fields.push_back((*iter)->type());
+  }
+
   AutoFillProfile::CreateInferredLabels(&matched_profiles, labels, 0,
-                                        type.field_type());
+                                        type.field_type(), &form_fields);
 
   // No icons for profile suggestions.
   icons->resize(values->size());
diff --git a/chrome/browser/autofill/autofill_profile.cc b/chrome/browser/autofill/autofill_profile.cc
index 35be858..7bc473d 100644
--- a/chrome/browser/autofill/autofill_profile.cc
+++ b/chrome/browser/autofill/autofill_profile.cc
@@ -171,7 +171,7 @@ bool AutoFillProfile::AdjustInferredLabels(
   std::vector<string16> created_labels;
   const size_t kMinimalFieldsShown = 2;
   CreateInferredLabels(profiles, &created_labels, kMinimalFieldsShown,
-                       UNKNOWN_TYPE);
+                       UNKNOWN_TYPE, NULL);
   DCHECK(profiles->size() == created_labels.size());
   bool updated_labels = false;
   for (size_t i = 0; i < profiles->size(); ++i) {
@@ -188,10 +188,12 @@ void AutoFillProfile::CreateInferredLabels(
     const std::vector<AutoFillProfile*>* profiles,
     std::vector<string16>* created_labels,
     size_t minimal_fields_shown,
-    AutoFillFieldType exclude_field) {
-  // These fields are use to distinguish between two profiles in the order of
-  // importance, e. g. if both EMAIL_ADDRESS and COMPANY_NAME are different,
-  // EMAIL_ADDRESS will be used to distinguish them.
+    AutoFillFieldType exclude_field,
+    const std::vector<AutoFillFieldType>* suggested_fields) {
+  // |suggested_fields| may be NULL.
+  // The following fields are use to distinguish between two profiles in the
+  // order of importance, e. g. if both EMAIL_ADDRESS and COMPANY_NAME are
+  // different, EMAIL_ADDRESS will be used to distinguish them.
   const AutoFillFieldType distinguishing_fields[] = {
     // First non empty data are always present in the label, even if it matches.
     NAME_FULL,
@@ -205,15 +207,47 @@ void AutoFillProfile::CreateInferredLabels(
     PHONE_FAX_WHOLE_NUMBER,
     COMPANY_NAME,
   };
-  if (exclude_field == NAME_FIRST || exclude_field == NAME_LAST)
-    exclude_field = NAME_FULL;
+
   DCHECK(profiles);
   DCHECK(created_labels);
+
+  std::vector<AutoFillFieldType> fields_to_use;
+  if (suggested_fields) {
+    // Limiting the number of possible fields simplifies the following code,
+    // and 10 fields more than enough to create clear label.
+    fields_to_use.reserve(std::min(suggested_fields->size(),
+                                   arraysize(distinguishing_fields)));
+    bool name_selected = false;
+    for (size_t i = 0; i < suggested_fields->size() &&
+         fields_to_use.size() < arraysize(distinguishing_fields); ++i) {
+      if (suggested_fields->at(i) == NAME_FIRST ||
+          suggested_fields->at(i) == NAME_LAST ||
+          suggested_fields->at(i) == NAME_MIDDLE ||
+          suggested_fields->at(i) == NAME_FULL) {
+        if (name_selected)
+          continue;
+        name_selected = true;
+        fields_to_use.push_back(NAME_FULL);
+      } else {
+        fields_to_use.push_back(suggested_fields->at(i));
+      }
+    }
+  } else {
+    fields_to_use.resize(arraysize(distinguishing_fields));
+    for (size_t i = 0; i < arraysize(distinguishing_fields); ++i) {
+      fields_to_use[i] = distinguishing_fields[i];
+    }
+  }
+
+  if (exclude_field == NAME_FIRST || exclude_field == NAME_LAST ||
+      exclude_field == NAME_MIDDLE)
+    exclude_field = NAME_FULL;
   created_labels->resize(profiles->size());
   std::map<string16, std::list<size_t> > labels;
   for (size_t it = 0; it < profiles->size(); ++it) {
-    labels[
-        profiles->at(it)->GetFieldText(AutoFillType(NAME_FULL))].push_back(it);
+    labels[profiles->at(it)->GetFieldText(
+        AutoFillType(fields_to_use.size() ? fields_to_use[0] :
+                                            NAME_FULL))].push_back(it);
   }
   std::map<string16, std::list<size_t> >::iterator label_iterator;
   for (label_iterator = labels.begin(); label_iterator != labels.end();
@@ -222,13 +256,14 @@ void AutoFillProfile::CreateInferredLabels(
       // We have more than one item with the same preview, add differentiating
       // data.
       std::list<size_t>::iterator similar_profiles;
+      DCHECK(fields_to_use.size() <= arraysize(distinguishing_fields));
       std::map<string16, int> tested_fields[arraysize(distinguishing_fields)];
       for (similar_profiles = label_iterator->second.begin();
            similar_profiles != label_iterator->second.end();
            ++similar_profiles) {
-        for (size_t i = 0; i < arraysize(distinguishing_fields); ++i) {
+        for (size_t i = 0; i < fields_to_use.size(); ++i) {
           string16 key = profiles->at(*similar_profiles)->GetFieldText(
-              AutoFillType(distinguishing_fields[i]));
+              AutoFillType(fields_to_use[i]));
           std::map<string16, int>::iterator tested_field =
               tested_fields[i].find(key);
           if (tested_field == tested_fields[i].end())
@@ -242,7 +277,7 @@ void AutoFillProfile::CreateInferredLabels(
       size_t added_fields = 0;
       bool matched_necessary = false;
       // Leave it as a candidate if it is not the same for everybody.
-      for (size_t i = 0; i < arraysize(distinguishing_fields); ++i) {
+      for (size_t i = 0; i < fields_to_use.size(); ++i) {
         if (tested_fields[i].size() == label_iterator->second.size()) {
           // This field is different for everybody.
           if (!matched_necessary) {
@@ -259,26 +294,26 @@ void AutoFillProfile::CreateInferredLabels(
           } else {
             ++added_fields;
           }
-          fields.push_back(distinguishing_fields[i]);
+          fields.push_back(fields_to_use[i]);
           if (added_fields >= minimal_fields_shown)
             break;
         } else if (tested_fields[i].size() != 1) {
           // this field is different for some.
           if (added_fields < minimal_fields_shown) {
-            first_non_empty_fields.push_back(distinguishing_fields[i]);
+            first_non_empty_fields.push_back(fields_to_use[i]);
             ++added_fields;
             if (added_fields == minimal_fields_shown && matched_necessary)
               break;
           }
-          fields.push_back(distinguishing_fields[i]);
+          fields.push_back(fields_to_use[i]);
         } else if (added_fields < minimal_fields_shown &&
-                   exclude_field != distinguishing_fields[i] &&
+                   exclude_field != fields_to_use[i] &&
                    !label_iterator->first.empty()) {
-          fields.push_back(distinguishing_fields[i]);
-          first_non_empty_fields.push_back(distinguishing_fields[i]);
+          fields.push_back(fields_to_use[i]);
+          first_non_empty_fields.push_back(fields_to_use[i]);
           ++added_fields;
           if (added_fields == minimal_fields_shown && matched_necessary)
-           break;
+            break;
         }
       }
       // Update labels if needed.
@@ -287,12 +322,12 @@ void AutoFillProfile::CreateInferredLabels(
            ++similar_profiles) {
         size_t field_it = 0;
         for (size_t field_id = 0;
-             field_id < arraysize(distinguishing_fields) &&
+             field_id < fields_to_use.size() &&
              field_it < fields.size(); ++field_id) {
-          if (fields[field_it] == distinguishing_fields[field_id]) {
+          if (fields[field_it] == fields_to_use[field_id]) {
             if ((tested_fields[field_id])[
                 profiles->at(*similar_profiles)->GetFieldText(
-                AutoFillType(distinguishing_fields[field_id]))] == 1) {
+                AutoFillType(fields_to_use[field_id]))] == 1) {
               // this field is unique among the subset.
               break;
             }
@@ -319,12 +354,12 @@ void AutoFillProfile::CreateInferredLabels(
       std::vector<AutoFillFieldType> non_empty_fields;
       size_t include_fields = minimal_fields_shown ? minimal_fields_shown : 1;
       non_empty_fields.reserve(include_fields);
-      for (size_t i = 0; i < arraysize(distinguishing_fields); ++i) {
-        if (exclude_field == distinguishing_fields[i])
+      for (size_t i = 0; i < fields_to_use.size(); ++i) {
+        if (exclude_field == fields_to_use[i])
           continue;
         if (!profiles->at(label_iterator->second.front())->GetFieldText(
-            AutoFillType(distinguishing_fields[i])).empty()) {
-          non_empty_fields.push_back(distinguishing_fields[i]);
+            AutoFillType(fields_to_use[i])).empty()) {
+          non_empty_fields.push_back(fields_to_use[i]);
           if (non_empty_fields.size() >= include_fields)
             break;
         }
diff --git a/chrome/browser/autofill/autofill_profile.h b/chrome/browser/autofill/autofill_profile.h
index ea83730..7244641 100644
--- a/chrome/browser/autofill/autofill_profile.h
+++ b/chrome/browser/autofill/autofill_profile.h
@@ -70,12 +70,14 @@ class AutoFillProfile : public FormGroup {
   // Created inferred labels for |profiles|, according to the rules above and
   // stores them in |created_labels|. |minimal_fields_shown| minimal number of
   // fields that need to be shown for the label. |exclude_field| is excluded
-  // from the label.
+  // from the label. If |suggested_fields| is not NULL it is used to generate
+  // labels appropriate to the actual fields in a given form.
   static void CreateInferredLabels(
       const std::vector<AutoFillProfile*>* profiles,
       std::vector<string16>* created_labels,
       size_t minimal_fields_shown,
-      AutoFillFieldType exclude_field);
+      AutoFillFieldType exclude_field,
+      const std::vector<AutoFillFieldType>* suggested_fields);
 
   // Returns true if there are no values (field types) set.
   bool IsEmpty() const;
diff --git a/chrome/browser/autofill/autofill_profile_unittest.cc b/chrome/browser/autofill/autofill_profile_unittest.cc
index a290242..3c500a3 100644
--- a/chrome/browser/autofill/autofill_profile_unittest.cc
+++ b/chrome/browser/autofill/autofill_profile_unittest.cc
@@ -305,6 +305,101 @@ TEST(AutoFillProfileTest, AdjustInferredLabels) {
   STLDeleteContainerPointers(profiles.begin(), profiles.end());
 }
 
+TEST(AutoFillProfileTest, CreateInferredLabels) {
+  std::vector<AutoFillProfile*> profiles;
+  profiles.push_back(new AutoFillProfile);
+  autofill_test::SetProfileInfo(profiles[0],
+                                "",
+                                "John",
+                                "",
+                                "Doe",
+                                "johndoe@hades.com",
+                                "Underworld",
+                                "666 Erebus St.",
+                                "",
+                                "Elysium", "CA",
+                                "91111",
+                                "US",
+                                "11111111111",
+                                "22222222222");
+  profiles.push_back(new AutoFillProfile);
+  autofill_test::SetProfileInfo(profiles[1],
+                                "",
+                                "Jane",
+                                "",
+                                "Doe",
+                                "janedoe@tertium.com",
+                                "Pluto Inc.",
+                                "123 Letha Shore.",
+                                "",
+                                "Dis", "CA",
+                                "91222",
+                                "US",
+                                "12345678910",
+                                "01987654321");
+  std::vector<string16> labels;
+  // Two fields at least - no filter.
+  AutoFillProfile::CreateInferredLabels(&profiles, &labels, 2, UNKNOWN_TYPE,
+                                        NULL);
+  EXPECT_EQ(string16(ASCIIToUTF16("John Doe, 666 Erebus St.")), labels[0]);
+  EXPECT_EQ(string16(ASCIIToUTF16("Jane Doe, 123 Letha Shore.")), labels[1]);
+
+  // Three fields at least - no filter.
+  AutoFillProfile::CreateInferredLabels(&profiles, &labels, 3, UNKNOWN_TYPE,
+                                        NULL);
+  EXPECT_EQ(string16(ASCIIToUTF16("John Doe, 666 Erebus St., Elysium")),
+            labels[0]);
+  EXPECT_EQ(string16(ASCIIToUTF16("Jane Doe, 123 Letha Shore., Dis")),
+            labels[1]);
+
+  // Two fields at least - filter out the name.
+  AutoFillProfile::CreateInferredLabels(&profiles, &labels, 2, NAME_FULL, NULL);
+  EXPECT_EQ(string16(ASCIIToUTF16("666 Erebus St., Elysium")), labels[0]);
+  EXPECT_EQ(string16(ASCIIToUTF16("123 Letha Shore., Dis")), labels[1]);
+
+  std::vector<AutoFillFieldType> suggested_fields;
+  suggested_fields.push_back(ADDRESS_HOME_CITY);
+  suggested_fields.push_back(ADDRESS_HOME_STATE);
+  suggested_fields.push_back(ADDRESS_HOME_ZIP);
+
+  // Two fields at least, from suggested fields - no filter.
+  AutoFillProfile::CreateInferredLabels(&profiles, &labels, 2, UNKNOWN_TYPE,
+                                        &suggested_fields);
+  EXPECT_EQ(string16(ASCIIToUTF16("Elysium, CA")), labels[0]);
+  EXPECT_EQ(string16(ASCIIToUTF16("Dis, CA")), labels[1]);
+
+  // Three fields at least, from suggested fields - no filter.
+  AutoFillProfile::CreateInferredLabels(&profiles, &labels, 3, UNKNOWN_TYPE,
+                                        &suggested_fields);
+  EXPECT_EQ(string16(ASCIIToUTF16("Elysium, CA, 91111")), labels[0]);
+  EXPECT_EQ(string16(ASCIIToUTF16("Dis, CA, 91222")), labels[1]);
+
+  // Three fields at least, from suggested fields - but filter reduces available
+  // fields to two.
+  AutoFillProfile::CreateInferredLabels(&profiles, &labels, 3,
+                                        ADDRESS_HOME_STATE, &suggested_fields);
+  EXPECT_EQ(string16(ASCIIToUTF16("Elysium, 91111")), labels[0]);
+  EXPECT_EQ(string16(ASCIIToUTF16("Dis, 91222")), labels[1]);
+
+  suggested_fields.clear();
+  // In our implementation we always display NAME_FULL for all NAME* fields...
+  suggested_fields.push_back(NAME_MIDDLE);
+  // One field at least, from suggested fields - no filter.
+  AutoFillProfile::CreateInferredLabels(&profiles, &labels, 1, UNKNOWN_TYPE,
+                                        &suggested_fields);
+  EXPECT_EQ(string16(ASCIIToUTF16("John Doe")), labels[0]);
+  EXPECT_EQ(string16(ASCIIToUTF16("Jane Doe")), labels[1]);
+
+  // One field at least, from suggested fields - filter the same as suggested
+  // field.
+  AutoFillProfile::CreateInferredLabels(&profiles, &labels, 1, NAME_MIDDLE,
+                                        &suggested_fields);
+  EXPECT_EQ(string16(ASCIIToUTF16("")), labels[0]);
+  EXPECT_EQ(string16(ASCIIToUTF16("")), labels[1]);
+  // Clean up.
+  STLDeleteContainerPointers(profiles.begin(), profiles.end());
+}
+
 TEST(AutoFillProfileTest, IsSubsetOf) {
   scoped_ptr<AutoFillProfile> a, b;