[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..304ba43610c3cdfc37e164b7ab903a4c2d67791f 100644
--- a/components/autofill/core/browser/credit_card_field.cc
+++ b/components/autofill/core/browser/credit_card_field.cc
@@ -5,6 +5,7 @@
 #include "components/autofill/core/browser/credit_card_field.h"
 
 #include <stddef.h>

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

nit: Please include a blank line after this one, to separate the C header
includes from the C++ ones.

[Pritam Nikam, 2014/08/12 14:00:37]

Done.

+#include <algorithm>
 
 #include "base/logging.h"
 #include "base/memory/scoped_ptr.h"
@@ -89,8 +90,21 @@ 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;
+    if (ParseField(scanner, pattern, &cc_number_field)) {
+      size_t start_index = 0;
+      AutofillField* current_number_field =
+          const_cast<AutofillField*>(cc_number_field);

[Pritam Nikam, 2014/08/12 14:00:38]

I didn't get any alternative approach to fix const_cast<> used here.

Changing ParseField() signature will have lots of signature changes as its just
a wrapper API. And may be doing that will have ripple effect across [form-field,
scanner and cursor modules].

I'm kind of blocked here, please guide me how can I solve this problem.

+      // Avoid having very large start_index.
+      if (!credit_card_field->numbers_.empty())
+        start_index =
+            std::min(std::numeric_limits<size_t>::max(),

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

This std::min call does not do anything, since the max size_t value is by
definitely the largest possible value, so std::min will always pick the other
option (or possibly both, if the two inputs are equal).

[Pritam Nikam, 2014/08/12 14:00:37]

Done.

+                     credit_card_field->numbers_.back()
+                             ->credit_card_number_start_index() +
+                         credit_card_field->numbers_.back()->max_length);

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

You should probably check whether the resulting start_index + max_length for the
last field is a small value (say, less than or equal to 19, which according to [
https://code.google.com/p/chromium/codesearch#chromium/src/components/autofil...
] is the maximum length of a credit card number).  At the least, you should
check whether the max_length is the default (very large) value, or something
smaller.  If we see four fields in a row that don't have a max_length set, we
should probably bail on parsing the form.

[Pritam Nikam, 2014/08/12 14:00:37]

Done.

+
+      current_number_field->set_credit_card_number_start_index(start_index);
+      credit_card_field->numbers_.push_back(current_number_field);
       continue;
     }
 
@@ -167,7 +181,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 +197,6 @@ CreditCardField::CreditCardField()
     : cardholder_(NULL),
       cardholder_last_(NULL),
       type_(NULL),
-      number_(NULL),
       verification_(NULL),
       expiration_month_(NULL),
       expiration_year_(NULL),
@@ -190,8 +204,16 @@ CreditCardField::CreditCardField()
       is_two_digit_year_(false) {
 }
 
+CreditCardField::~CreditCardField() {
+}
+
 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);

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

nit: Please add curly braces.

[Pritam Nikam, 2014/08/12 14:00:37]

Done.

+
   ok = ok && AddClassification(type_, CREDIT_CARD_TYPE, map);
   ok = ok && AddClassification(verification_, CREDIT_CARD_VERIFICATION_CODE,
                                map);