[Autofill] Autofill fails to show suggestions for credit card split across fields.

components/autofill/core/browser/autofill_manager.cc

Index: components/autofill/core/browser/autofill_manager.cc
diff --git a/components/autofill/core/browser/autofill_manager.cc b/components/autofill/core/browser/autofill_manager.cc
index 48cd5974c206b50c064630dace3cb85731b24497..7aceb1d0fe7f97c04a41f2c97c862f387e6ea458 100644
--- a/components/autofill/core/browser/autofill_manager.cc
+++ b/components/autofill/core/browser/autofill_manager.cc
@@ -156,6 +156,48 @@ void DeterminePossibleFieldTypesForUpload(
   }
 }
 
+// Helper function to get the suggetions for |user_input| matching to
+// |card_number_filed| split.
+void GetCredtCardSuggetionsForNumberSplit(
+    const base::string16& user_input,
+    const AutofillField& card_number_filed,
+    PersonalDataManager* personal_data,
+    std::vector<base::string16>* values,
+    std::vector<base::string16>* labels,
+    std::vector<base::string16>* icons,
+    std::vector<GUIDPair>* guid_pairs) {
+  const std::vector<CreditCard*>& credit_cards =
+      personal_data->GetCreditCards();
+  for (std::vector<CreditCard*>::const_iterator iter = credit_cards.begin();
+       iter != credit_cards.end();
+       ++iter) {
+    CreditCard* credit_card = *iter;
+    base::string16 stored_card_number =
+        CreditCard::StripSeparators(credit_card->number());
+
+    // Exclude first split, as it would have already included in the
+    // suggestions.
+    if (!stored_card_number.empty() &&
+        !StartsWith(stored_card_number, user_input, false)) {
+
+      // Make sure the suggetions get include for rest of the
+      // |card_number_filed|s.
+      stored_card_number = AutofillField::GetCreditCardNumberValue(
+          card_number_filed, stored_card_number);
+      if (StartsWith(stored_card_number, user_input, false)) {
+        personal_data->AppendCreditCardSuggetions(
+            credit_card,
+            AutofillType(CREDIT_CARD_NUMBER),
+            user_input,
+            values,
+            labels,
+            icons,
+            guid_pairs);
+      }
+    }
+  }
+}

[Ilya Sherman, 2014/10/07 21:49:10]

Can all of this logic be moved into the PersonalDataManager class, as it is for
phone numbers?

[Pritam Nikam, 2014/10/08 05:17:17]

In my opinion, we *cannot* move this logic to PersonalDataManager class straight
away.
Retrieving card number substring is poised on
AutofillField::credit_card_number_offset(). 

Moreover, changing API signature of
PersonalDataManager::GetCreditCardSuggestions() will apparently impact usages
from other code excerpts [1] where passing AutofillField reference as argument
won't be easy.

And as I understand, phone number prefix/suffix suggestions handling logic is
part of AutofillManager[2] as of now, and reason is the same mentioned [1].

[1].
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/ui/...

[2].
https://code.google.com/p/chromium/codesearch#chromium/src/components/autofil...

[Ilya Sherman, 2014/10/08 21:04:38]

Phone numbers are also handled within the PersonalDataManager code [1].  The
AutofillManager code that you linked to just adjusts what data will be filled;
it's much simpler and clearer than the credit card code that you've added in
this CL.  I'd prefer to align the two implementations.

[1]
https://code.google.com/p/chromium/codesearch#chromium/src/components/autofil...

[Pritam Nikam, 2014/10/09 06:11:11]

Acknowledged.

I'll align my changes along these logic i.e. sub-string match and upload patch
for review.

By I've query related to same, it's a sub-string matching instead of pre-fix
matching. But don't we need field match as well?

For instance, say we have 3 credit cards saved.
/*  Name On Card  |  Card Number          | mm  | yyyy  */
A. {"Miku Hatsune", "4111-1111-1111-1111", "07", "2015"}
B. {"Elvis Presley", "4231-2341-1111-1017", "04", "2012"}
C. {"Buddy Holly", "5632-1111-4111-2341", "10", "2014"}

If suppose, a HTML form having card number split across 4 input fields. And
autofilled with card [A] i.e. cc-number "4111-1111-1111-1111". In my opinion,
double click on 4'th cc-number input field may bring all 3 card suggestions, and
user may be expecting to see only A'th card instead, OR am I missing something
here?

[Ilya Sherman, 2014/10/09 21:35:12]

That's true.  It's such an edge case that I'm honestly not too worried about it
-- most users will have at most one credit card saved, and most websites only
have a single credit card field anyway.  If you really want to handle this case
correctly, I'd recommend (a) also improving the handling of phone numbers to
match, and (b) extending the PersonalDataManager::GetCreditCardSuggestions()
method with the necessary logic, possibly by passing an additional parameter for
the current field value.

[Pritam Nikam, 2014/10/13 04:33:52]

Currently, *current field value* (i.e. const base::string16& field_contents) is
getting pass to this function and that seems not enough to get the exact
suggestions for matching field input. I'll further investigate on these lines.

Moreover, I've added a patch addressing your earlier suggestions i.e. sub-string
instead of prefix match.

+
 }  // namespace
 
 AutofillManager::AutofillManager(
@@ -473,7 +515,7 @@ void AutofillManager::OnQueryFormFieldAutofill(int query_id,
     bool is_filling_credit_card = (type.group() == CREDIT_CARD);
     if (is_filling_credit_card) {
       GetCreditCardSuggestions(
-          field, type, &values, &labels, &icons, &unique_ids);
+          field, *autofill_field, &values, &labels, &icons, &unique_ids);
     } else {
       GetProfileSuggestions(*form_structure,
                             field,
@@ -1117,15 +1159,27 @@ void AutofillManager::GetProfileSuggestions(
 
 void AutofillManager::GetCreditCardSuggestions(
     const FormFieldData& field,
-    const AutofillType& type,
+    const AutofillField& autofill_field,
     std::vector<base::string16>* values,
     std::vector<base::string16>* labels,
     std::vector<base::string16>* icons,
     std::vector<int>* unique_ids) const {
   std::vector<GUIDPair> guid_pairs;
+  AutofillType type = autofill_field.Type();
   personal_data_->GetCreditCardSuggestions(
       type, field.value, values, labels, icons, &guid_pairs);
 
+  // Populate suggetions for number split case (http://crbug.com/420323).
+  if (!field.value.empty() &&
+      autofill_field.Type().GetStorableType() == CREDIT_CARD_NUMBER)
+    GetCredtCardSuggetionsForNumberSplit(field.value,
+                                         autofill_field,
+                                         personal_data_,
+                                         values,
+                                         labels,
+                                         icons,
+                                         &guid_pairs);

[Ilya Sherman, 2014/10/07 21:49:10]

nit: Please add curly braces, since the body spans multiple lines of text.

[Pritam Nikam, 2014/10/08 05:17:17]

Done.

+
   for (size_t i = 0; i < guid_pairs.size(); ++i) {
     unique_ids->push_back(PackGUIDs(guid_pairs[i], GUIDPair(std::string(), 0)));
   }