[Autofill] Autofill fails to fill credit card number when split across fields.

components/autofill/core/browser/credit_card_field.cc

Index: components/autofill/core/browser/credit_card_field.cc
diff --git a/components/autofill/core/browser/credit_card_field.cc b/components/autofill/core/browser/credit_card_field.cc
index 09dab1e48d796800fda5065ee6cb54075fd5697a..17cf8897c5d3cf4b678d0c338a6302bc21fb782a 100644
--- a/components/autofill/core/browser/credit_card_field.cc
+++ b/components/autofill/core/browser/credit_card_field.cc
@@ -89,8 +89,27 @@ FormField* CreditCardField::Parse(AutofillScanner* scanner) {
     }
 
     pattern = base::UTF8ToUTF16(autofill::kCardNumberRe);
-    if (!credit_card_field->number_ &&
-        ParseField(scanner, pattern, &credit_card_field->number_)) {
+    const AutofillField* cc_number_field = NULL;
+    if (ParseField(scanner, pattern, &cc_number_field)) {
+      int start_index = 0;
+      if (!credit_card_field->numbers_.empty()) {
+        AutofillField* last_number_field =
+            const_cast<AutofillField*>(credit_card_field->numbers_.back());
+        AutofillField::CreditCardNumberInfo* last_number_info =
+            const_cast<AutofillField::CreditCardNumberInfo*>(

[Ilya Sherman, 2014/08/07 20:57:25]

Note: If you find yourself using const_cast, that's generally a sign that your
code is not const-correct.

[Pritam Nikam, 2014/08/08 14:14:34]

ParseField() accepts const Autofill**, and hence we *cannot* get rid of
const_cast<>.
Moreover, other const_cast<> I've removed.
[Ref:
https://code.google.com/p/chromium/codesearch#chromium/src/components/autofil...

[Ilya Sherman, 2014/08/12 04:23:41]

The correct change is to modify the signature of ParseField, if need be.  Using
const_cast subverts the type system, and hence should not be used except in some
very rare circumstances.

+                last_number_field->credit_card_number_info());
+        start_index =
+            last_number_info->start_index_ + last_number_field->max_length;

[Ilya Sherman, 2014/08/07 20:57:25]

It's possible for this code to assign very large start_index_ values.  We should
probably have some sort of error correction and bail for such cases.

[Pritam Nikam, 2014/08/08 14:14:34]

Done.

+      }
+
+      AutofillField* current_number_field =
+          const_cast<AutofillField*>(cc_number_field);
+      AutofillField::CreditCardNumberInfo* number_info =
+          new AutofillField::CreditCardNumberInfo;

[Ilya Sherman, 2014/08/07 20:57:25]

If this is always non-null, why use a pointer, rather than just passing and
setting by value?

[Pritam Nikam, 2014/08/08 14:14:34]

Done.
std::min(std::numeric_limits<size_t>::max(),
last_number_field->max_length+last_number_field->start_index)

And added a check inside FillCreditCardNumberField().

+      number_info->part_ = 1 + credit_card_field->numbers_.size();
+      number_info->start_index_ = start_index;
+      current_number_field->set_credit_card_number_info(number_info);
+      credit_card_field->numbers_.push_back(current_number_field);
       continue;
     }
 
@@ -167,7 +186,8 @@ FormField* CreditCardField::Parse(AutofillScanner* scanner) {
   // a strong enough signal that this is a credit card.  It is possible that
   // the number and name were parsed in a separate part of the form.  So if
   // the cvc and date were found independently they are returned.
-  if ((credit_card_field->number_ || credit_card_field->verification_) &&
+  if ((!credit_card_field->numbers_.empty() ||
+       credit_card_field->verification_) &&
       (credit_card_field->expiration_date_ ||
        (credit_card_field->expiration_month_ &&
         credit_card_field->expiration_year_))) {
@@ -182,7 +202,6 @@ CreditCardField::CreditCardField()
     : cardholder_(NULL),
       cardholder_last_(NULL),
       type_(NULL),
-      number_(NULL),
       verification_(NULL),
       expiration_month_(NULL),
       expiration_year_(NULL),
@@ -190,8 +209,17 @@ CreditCardField::CreditCardField()
       is_two_digit_year_(false) {
 }
 
+CreditCardField::~CreditCardField() {
+  numbers_.clear();

[Ilya Sherman, 2014/08/07 20:57:25]

This is called automatically.  Why did you add it explicitly?

[Pritam Nikam, 2014/08/08 14:14:34]

Done.

+}
+
 bool CreditCardField::ClassifyField(ServerFieldTypeMap* map) const {
-  bool ok = AddClassification(number_, CREDIT_CARD_NUMBER, map);
+  bool ok = true;
+  for (std::vector<const AutofillField*>::const_iterator it = numbers_.begin();
+       it != numbers_.end();
+       ++it)
+    ok = ok && AddClassification(*it, CREDIT_CARD_NUMBER, map);
+
   ok = ok && AddClassification(type_, CREDIT_CARD_TYPE, map);
   ok = ok && AddClassification(verification_, CREDIT_CARD_VERIFICATION_CODE,
                                map);