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

components/autofill/core/browser/autofill_field.cc

Index: components/autofill/core/browser/autofill_field.cc
diff --git a/components/autofill/core/browser/autofill_field.cc b/components/autofill/core/browser/autofill_field.cc
index c2a744203af2321856cbc617e84fece0c7a1c114..96da4d5f9aec8b8fc77c2195eae8b14439b1b62b 100644
--- a/components/autofill/core/browser/autofill_field.cc
+++ b/components/autofill/core/browser/autofill_field.cc
@@ -389,6 +389,26 @@ std::string Hash32Bit(const std::string& str) {
   return base::UintToString(hash32);
 }
 
+void FillCreditCardNumberField(const AutofillField& field,
+                               const base::string16& cc_number,
+                               FormFieldData* field_data) {

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

nit: Please add documentation for this function.

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

Done.

+  base::string16 value = cc_number;
+  size_t length = value.length();
+  AutofillField::CreditCardNumberInfo* number_info =
+      const_cast<AutofillField::CreditCardNumberInfo*>(
+          field.credit_card_number_info());
+  if (field_data->max_length && field_data->max_length < length &&

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

max_length is always set, isn't it?

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

Done.

For HTML forms i didn't notice this being coming as zero. However, there are
many test-cases where max_length is not specified, being FormFieldData structure
member it's initialize to "0". 
[https://code.google.com/p/chromium/codesearch#chromium/src/components/autofill/core/common/form_field_data.cc&l=62-70]

+      number_info) {

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

Is it possible to have the number_info set on a field that doesn't have a
max_length?  It seems like we should already know how many characters we expect
to fill into this field; otherwise, what we fill into subsequent fields probably
won't make sense.

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

Done.

+    base::RemoveChars(cc_number, base::ASCIIToUTF16("- "), &value);

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

Why is this needed?  I would have expected that the cc_number is already a digit
string at this point.

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

Done.

+    length = value.length();

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

Why do you set this again?

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

Done.

+    length =
+        std::min(field_data->max_length, length - number_info->start_index_);
+    value = value.substr(number_info->start_index_, length);

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

I don't think you need to set the end index manually -- the field will enforce
max_length on its own.

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

Done.

+  }
+
+  field_data->value = value;
+}

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

Please move this to be adjacent to the FillPhoneNumberField method.

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

Done.

+
 }  // namespace
 
 AutofillField::AutofillField()
@@ -396,7 +416,8 @@ AutofillField::AutofillField()
       heuristic_type_(UNKNOWN_TYPE),
       html_type_(HTML_TYPE_UNKNOWN),
       html_mode_(HTML_MODE_NONE),
-      phone_part_(IGNORED) {
+      phone_part_(IGNORED),
+      credit_card_number_info_(NULL) {
 }
 
 AutofillField::AutofillField(const FormFieldData& field,
@@ -407,10 +428,16 @@ AutofillField::AutofillField(const FormFieldData& field,
       heuristic_type_(UNKNOWN_TYPE),
       html_type_(HTML_TYPE_UNKNOWN),
       html_mode_(HTML_MODE_NONE),
-      phone_part_(IGNORED) {
+      phone_part_(IGNORED),
+      credit_card_number_info_(NULL) {
 }
 
-AutofillField::~AutofillField() {}
+AutofillField::~AutofillField() {
+  if (credit_card_number_info_) {
+    delete credit_card_number_info_;

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

You should pretty much always prefer a scoped_ptr or other smart pointer to
manual resource management.

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

Done.

+    credit_card_number_info_ = NULL;
+  }
+}
 
 void AutofillField::set_heuristic_type(ServerFieldType type) {
   if (type >= 0 && type < MAX_VALID_FIELD_TYPE &&
@@ -486,6 +513,9 @@ bool AutofillField::FillFormField(const AutofillField& field,
   } else if (type.GetStorableType() == ADDRESS_HOME_STREET_ADDRESS) {
     FillStreetAddress(value, address_language_code, field_data);
     return true;
+  } else if (type.GetStorableType() == CREDIT_CARD_NUMBER) {
+    FillCreditCardNumberField(field, value, field_data);
+    return true;
   }
 
   field_data->value = value;