AutoFill suggestions for billing and credit card fields should show derived labels.

Changes suggestions for forms with billing information to show derived labels based on contents of suggestions, not the labels stored in the matched profiles.  Also, the suggestion logic has been updated to properly take account of the billing "relation" in the credit card info.  Credit card only suggestions now use derived labels as well.

BUG=50261
TEST=AutoFillManagerTest.GetBillingSuggestionsAddress1, AutoFillManagerTest.GetCreditCardSuggestions*

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

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

chrome/browser/autofill/autofill_manager.cc

@@ -39,10 +39,21 @@ const int kAutoFillPhoneNumberPrefixCount = 3;
 const int kAutoFillPhoneNumberSuffixOffset = 3;
 const int kAutoFillPhoneNumberSuffixCount = 4;
 
-
-const string16::value_type kCreditCardLabelPrefix[] = {'*', 0};
+const string16::value_type kCreditCardPrefix[] = {'*', 0};
 const string16::value_type kLabelSeparator[] = {';',' ', '*', 0};
 
+// Combines the |label| string with the last four digits of the credit card
+// |cc|.  If one, the other, or both are empty strings we omit the separator.
+string16 CombineLabelAndCreditCard(const string16& label,
+                                   const CreditCard* cc) {
+  if (label.empty())
+    return kCreditCardPrefix + cc->LastFourDigits();
+  else if (cc->LastFourDigits().empty())
+    return label;
+  else
+    return label + kLabelSeparator + cc->LastFourDigits();
+}
+
 // The name of the generic credit card icon, which maps to the image resource ID
 // in webkit/glue:WebKitClientImpl.
 // TODO(jhawkins): Move the images to chrome/common and implement the resource
@@ -515,13 +526,15 @@ void AutoFillManager::GetProfileSuggestions(FormStructure* form,
          personal_data_->credit_cards().begin();
          cc != personal_data_->credit_cards().end(); ++cc) {
       expanded_values.push_back((*values)[i]);
-      string16 label = (*labels)[i] + kLabelSeparator + (*cc)->LastFourDigits();
+      string16 label = CombineLabelAndCreditCard((*labels)[i], *cc);
       expanded_labels.push_back(label);
       expanded_ids.push_back(PackIDs((*cc)->unique_id(), profile->unique_id()));
     }
   }
   expanded_labels.swap(*labels);
   expanded_values.swap(*values);
+  // No CC, so no icons.
+  icons->resize(values->size());
   expanded_ids.swap(*unique_ids);
 }
 
@@ -533,10 +546,6 @@ void AutoFillManager::GetBillingProfileSuggestions(
     std::vector<string16>* labels,
     std::vector<string16>* icons,
     std::vector<int>* unique_ids) {
-  std::vector<CreditCard*> matching_creditcards;
-  std::vector<AutoFillProfile*> matching_profiles;
-  std::vector<string16> cc_values;
-  std::vector<string16> cc_labels;
 
   // If the form is non-HTTPS, no CC suggestions are provided; however, give the
   // user the option of filling the billing address fields with regular address
@@ -547,6 +556,11 @@ void AutoFillManager::GetBillingProfileSuggestions(
     return;
   }
 
+  std::vector<CreditCard*> matching_creditcards;
+  std::vector<AutoFillProfile*> matching_profiles;
+
+  // Collect matching pairs of credit cards and related profiles, where profile
+  // field value matches form field value.
   for (std::vector<CreditCard*>::const_iterator cc =
            personal_data_->credit_cards().begin();
        cc != personal_data_->credit_cards().end(); ++cc) {
@@ -573,18 +587,27 @@ void AutoFillManager::GetBillingProfileSuggestions(
     if (!billing_profile)
       continue;
 
-    for (std::vector<AutoFillProfile*>::const_iterator iter =
-             personal_data_->profiles().begin();
-         iter != personal_data_->profiles().end(); ++iter) {
-      values->push_back(billing_profile->GetFieldText(type));
-
-      string16 label = (*iter)->Label() + kLabelSeparator +
-                       (*cc)->LastFourDigits();
-      labels->push_back(label);
-      icons->push_back(ASCIIToUTF16(kGenericCC));
-      unique_ids->push_back(
-          PackIDs((*cc)->unique_id(), (*iter)->unique_id()));
-    }
+    matching_creditcards.push_back(*cc);
+    matching_profiles.push_back(billing_profile);
+  }
+
+  std::vector<string16> inferred_labels;
+  AutoFillProfile::CreateInferredLabels(&matching_profiles, &inferred_labels, 0,
+                                        type.field_type());
+
+  DCHECK_EQ(matching_profiles.size(), matching_creditcards.size());
+  DCHECK_EQ(matching_profiles.size(), inferred_labels.size());
+
+  // Process the matching pairs into suggested |values|, |labels|, and
+  // |unique_ids|.
+  for (size_t i = 0; i < matching_profiles.size(); ++i) {
+    values->push_back(matching_profiles[i]->GetFieldText(type));
+    string16 label = CombineLabelAndCreditCard(inferred_labels[i],
+                                               matching_creditcards[i]);
+    labels->push_back(label);
+    icons->push_back(ASCIIToUTF16(kGenericCC));
+    unique_ids->push_back(PackIDs(matching_creditcards[i]->unique_id(),
+                                  matching_profiles[i]->unique_id()));
   }
 }
 
@@ -613,21 +636,28 @@ void AutoFillManager::GetCreditCardSuggestions(FormStructure* form,
 
       if (!form->HasNonBillingFields()) {
         values->push_back(creditcard_field_value);
-        labels->push_back(
-            kCreditCardLabelPrefix + credit_card->LastFourDigits());
+        labels->push_back(CombineLabelAndCreditCard(string16(), credit_card));
+        icons->push_back(ASCIIToUTF16(kGenericCC));
         unique_ids->push_back(PackIDs(credit_card->unique_id(), 0));
       } else {
-        for (std::vector<AutoFillProfile*>::const_iterator iter =
-                 personal_data_->profiles().begin();
-             iter != personal_data_->profiles().end(); ++iter) {
+        const std::vector<AutoFillProfile*>& profiles
+            = personal_data_->profiles();
+        std::vector<string16> inferred_labels;
+        AutoFillProfile::CreateInferredLabels(&profiles,
+                                              &inferred_labels,
+                                              0,
+                                              type.field_type());
+        DCHECK_EQ(profiles.size(), inferred_labels.size());
+
+        for (size_t i = 0; i < profiles.size(); ++i) {
           values->push_back(creditcard_field_value);
 
-          string16 label = (*iter)->Label() + kLabelSeparator +
-                           credit_card->LastFourDigits();
+          string16 label = CombineLabelAndCreditCard(inferred_labels[i],
+                                                     credit_card);
           labels->push_back(label);
           icons->push_back(ASCIIToUTF16(kGenericCC));
           unique_ids->push_back(
-            PackIDs(credit_card->unique_id(), (*iter)->unique_id()));
+              PackIDs(credit_card->unique_id(), profiles[i]->unique_id()));
         }
       }
     }

chrome/browser/autofill/autofill_manager_unittest.cc

@@ -92,7 +92,7 @@ class TestPersonalDataManager : public PersonalDataManager {
     credit_card = new CreditCard;
     autofill_unittest::SetCreditCardInfo(credit_card, "Second", "Buddy Holly",
                                          "Mastercard", "0987654321098765", "10",
-                                         "2014", "");
+                                         "2014", "Work");
     credit_card->set_unique_id(5);
     credit_cards->push_back(credit_card);
     credit_card = new CreditCard;
@@ -396,12 +396,12 @@ TEST_F(AutoFillManagerTest, SKIP_BRANDED(GetCreditCardSuggestionsEmptyValue)) {
   EXPECT_EQ(ASCIIToUTF16("************8765"), values[4]);
   EXPECT_EQ(ASCIIToUTF16("************8765"), values[5]);
   ASSERT_EQ(6U, labels.size());
-  EXPECT_EQ(ASCIIToUTF16("Home; *3456"), labels[0]);
-  EXPECT_EQ(ASCIIToUTF16("Work; *3456"), labels[1]);
-  EXPECT_EQ(ASCIIToUTF16("Empty; *3456"), labels[2]);
-  EXPECT_EQ(ASCIIToUTF16("Home; *8765"), labels[3]);
-  EXPECT_EQ(ASCIIToUTF16("Work; *8765"), labels[4]);
-  EXPECT_EQ(ASCIIToUTF16("Empty; *8765"), labels[5]);
+  EXPECT_EQ(ASCIIToUTF16("Elvis Aaron Presley; *3456"), labels[0]);
+  EXPECT_EQ(ASCIIToUTF16("Charles Hardin Holley; *3456"), labels[1]);
+  EXPECT_EQ(ASCIIToUTF16("*3456"), labels[2]);
+  EXPECT_EQ(ASCIIToUTF16("Elvis Aaron Presley; *8765"), labels[3]);
+  EXPECT_EQ(ASCIIToUTF16("Charles Hardin Holley; *8765"), labels[4]);
+  EXPECT_EQ(ASCIIToUTF16("*8765"), labels[5]);
 }
 
 TEST_F(AutoFillManagerTest,
@@ -438,9 +438,9 @@ TEST_F(AutoFillManagerTest,
   EXPECT_EQ(ASCIIToUTF16("************3456"), values[1]);
   EXPECT_EQ(ASCIIToUTF16("************3456"), values[2]);
   ASSERT_EQ(3U, labels.size());
-  EXPECT_EQ(ASCIIToUTF16("Home; *3456"), labels[0]);
-  EXPECT_EQ(ASCIIToUTF16("Work; *3456"), labels[1]);
-  EXPECT_EQ(ASCIIToUTF16("Empty; *3456"), labels[2]);
+  EXPECT_EQ(ASCIIToUTF16("Elvis Aaron Presley; *3456"), labels[0]);
+  EXPECT_EQ(ASCIIToUTF16("Charles Hardin Holley; *3456"), labels[1]);
+  EXPECT_EQ(ASCIIToUTF16("*3456"), labels[2]);
 }
 
 TEST_F(AutoFillManagerTest, SKIP_BRANDED(GetCreditCardSuggestionsNonCCNumber)) {
@@ -479,12 +479,12 @@ TEST_F(AutoFillManagerTest, SKIP_BRANDED(GetCreditCardSuggestionsNonCCNumber)) {
   EXPECT_EQ(ASCIIToUTF16("Buddy Holly"), values[4]);
   EXPECT_EQ(ASCIIToUTF16("Buddy Holly"), values[5]);
   ASSERT_EQ(6U, labels.size());
-  EXPECT_EQ(ASCIIToUTF16("Home; *3456"), labels[0]);
-  EXPECT_EQ(ASCIIToUTF16("Work; *3456"), labels[1]);
-  EXPECT_EQ(ASCIIToUTF16("Empty; *3456"), labels[2]);
-  EXPECT_EQ(ASCIIToUTF16("Home; *8765"), labels[3]);
-  EXPECT_EQ(ASCIIToUTF16("Work; *8765"), labels[4]);
-  EXPECT_EQ(ASCIIToUTF16("Empty; *8765"), labels[5]);
+  EXPECT_EQ(ASCIIToUTF16("Elvis Aaron Presley; *3456"), labels[0]);
+  EXPECT_EQ(ASCIIToUTF16("Charles Hardin Holley; *3456"), labels[1]);
+  EXPECT_EQ(ASCIIToUTF16("*3456"), labels[2]);
+  EXPECT_EQ(ASCIIToUTF16("Elvis Aaron Presley; *8765"), labels[3]);
+  EXPECT_EQ(ASCIIToUTF16("Charles Hardin Holley; *8765"), labels[4]);
+  EXPECT_EQ(ASCIIToUTF16("*8765"), labels[5]);
 }
 
 TEST_F(AutoFillManagerTest, SKIP_BRANDED(GetCreditCardSuggestionsSemicolon)) {
@@ -534,14 +534,14 @@ TEST_F(AutoFillManagerTest, SKIP_BRANDED(GetCreditCardSuggestionsSemicolon)) {
   EXPECT_EQ(ASCIIToUTF16("Buddy Holly"), values[6]);
   EXPECT_EQ(ASCIIToUTF16("Buddy Holly"), values[7]);
   ASSERT_EQ(8U, labels.size());
-  EXPECT_EQ(ASCIIToUTF16("Home; *3456"), labels[0]);
-  EXPECT_EQ(ASCIIToUTF16("Work; *3456"), labels[1]);
-  EXPECT_EQ(ASCIIToUTF16("Empty; *3456"), labels[2]);
-  EXPECT_EQ(ASCIIToUTF16("Home; 8765; *3456"), labels[3]);
-  EXPECT_EQ(ASCIIToUTF16("Home; *8765"), labels[4]);
-  EXPECT_EQ(ASCIIToUTF16("Work; *8765"), labels[5]);
-  EXPECT_EQ(ASCIIToUTF16("Empty; *8765"), labels[6]);
-  EXPECT_EQ(ASCIIToUTF16("Home; 8765; *8765"), labels[7]);
+  EXPECT_EQ(ASCIIToUTF16("Elvis Aaron Presley; *3456"), labels[0]);
+  EXPECT_EQ(ASCIIToUTF16("Charles Hardin Holley; *3456"), labels[1]);
+  EXPECT_EQ(ASCIIToUTF16("*3456"), labels[2]);
+  EXPECT_EQ(ASCIIToUTF16("Joe Ely; *3456"), labels[3]);
+  EXPECT_EQ(ASCIIToUTF16("Elvis Aaron Presley; *8765"), labels[4]);
+  EXPECT_EQ(ASCIIToUTF16("Charles Hardin Holley; *8765"), labels[5]);
+  EXPECT_EQ(ASCIIToUTF16("*8765"), labels[6]);
+  EXPECT_EQ(ASCIIToUTF16("Joe Ely; *8765"), labels[7]);
 }
 
 TEST_F(AutoFillManagerTest, SKIP_BRANDED(GetCreditCardSuggestionsNonHTTPS)) {
@@ -725,6 +725,42 @@ TEST_F(AutoFillManagerTest,
   EXPECT_EQ(string16(), labels[1]);
 }
 
+TEST_F(AutoFillManagerTest, GetBillingSuggestionsAddress1) {
+  FormData form;
+  CreateTestFormDataBilling(&form);
+
+  // Set up our FormStructures.
+  std::vector<FormData> forms;
+  forms.push_back(form);
+  autofill_manager_->FormsSeen(forms);
+
+  // 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(
+      "Address Line 1", "billingAddr1", "", "text", &field);
+  EXPECT_TRUE(autofill_manager_->GetAutoFillSuggestions(kPageID, false, field));
+
+  // No suggestions provided, so send an empty vector as the results.
+  // This triggers the combined message send.
+  rvh()->AutocompleteSuggestionsReturned(kPageID, std::vector<string16>());
+
+  // 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("3734 Elvis Presley Blvd."), values[0]);
+  EXPECT_EQ(ASCIIToUTF16("123 Apple St."), values[1]);
+  ASSERT_EQ(2U, labels.size());
+  EXPECT_EQ(ASCIIToUTF16("Elvis Aaron Presley; *3456"), labels[0]);
+  EXPECT_EQ(ASCIIToUTF16("Charles Hardin Holley; *8765"), labels[1]);
+}
+
 TEST_F(AutoFillManagerTest, SKIP_BRANDED(FillCreditCardForm)) {
   FormData form;
   CreateTestFormDataBilling(&form);
@@ -737,12 +773,9 @@ TEST_F(AutoFillManagerTest, SKIP_BRANDED(FillCreditCardForm)) {
   // The page ID sent to the AutoFillManager from the RenderView, used to send
   // an IPC message back to the renderer.
   const int kPageID = 1;
-  EXPECT_TRUE(
-      autofill_manager_->FillAutoFillFormData(kPageID,
-                                              form,
-                                              string16(),
-                                              ASCIIToUTF16("Home; *3456"),
-                                              AutoFillManager::PackIDs(4, 1)));
+  EXPECT_TRUE(autofill_manager_->FillAutoFillFormData(
+      kPageID, form, string16(), ASCIIToUTF16("Elvis Aaron Presley; *3456"),
+      AutoFillManager::PackIDs(4, 1)));
 
   int page_id = 0;
   FormData results;
@@ -824,12 +857,9 @@ TEST_F(AutoFillManagerTest, SKIP_BRANDED(FillNonBillingFormSemicolon)) {
   // The page ID sent to the AutoFillManager from the RenderView, used to send
   // an IPC message back to the renderer.
   const int kPageID = 1;
-  EXPECT_TRUE(
-      autofill_manager_->FillAutoFillFormData(kPageID,
-                                              form,
-                                              string16(),
-                                              ASCIIToUTF16("Home; 8765"),
-                                              AutoFillManager::PackIDs(4, 7)));
+  EXPECT_TRUE(autofill_manager_->FillAutoFillFormData(
+      kPageID, form, string16(), ASCIIToUTF16("Elvis Aaron Presley; 8765"),
+      AutoFillManager::PackIDs(4, 7)));
 
   int page_id = 0;
   FormData results;
@@ -1070,11 +1100,9 @@ TEST_F(AutoFillManagerTest, SKIP_BRANDED(FormChangesRemoveField)) {
   // The page ID sent to the AutoFillManager from the RenderView, used to send
   // an IPC message back to the renderer.
   const int kPageID = 1;
-  EXPECT_TRUE(autofill_manager_->FillAutoFillFormData(kPageID,
-                                                      form,
-                                                      ASCIIToUTF16("Elvis"),
-                                                      ASCIIToUTF16("Home"),
-                                                      1));
+  EXPECT_TRUE(autofill_manager_->FillAutoFillFormData(
+      kPageID, form, ASCIIToUTF16("Elvis"), ASCIIToUTF16("Elvis Aaron Presley"),
+      1));
 
   int page_id = 0;
   FormData results;
@@ -1137,11 +1165,9 @@ TEST_F(AutoFillManagerTest, SKIP_BRANDED(FormChangesAddField)) {
   // The page ID sent to the AutoFillManager from the RenderView, used to send
   // an IPC message back to the renderer.
   const int kPageID = 1;
-  EXPECT_TRUE(autofill_manager_->FillAutoFillFormData(kPageID,
-                                                      form,
-                                                      ASCIIToUTF16("Elvis"),
-                                                      ASCIIToUTF16("Home"),
-                                                      1));
+  EXPECT_TRUE(autofill_manager_->FillAutoFillFormData(
+      kPageID, form, ASCIIToUTF16("Elvis"), ASCIIToUTF16("Elvis Aaron Presley"),
+      1));
 
   int page_id = 0;
   FormData results;
@@ -1224,3 +1250,4 @@ TEST_F(AutoFillManagerTest, AuxiliaryProfilesReset) {
       prefs::kAutoFillAuxiliaryProfilesEnabled));
 #endif
 }
+