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

components/autofill/core/browser/autofill_field.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "components/autofill/core/browser/autofill_field.h"
 
 #include "base/logging.h"
 #include "base/sha1.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_split.h"
@@ skipping 371 matching lines @@
   DCHECK_EQ(20U, hash_bin.length());
 
   uint32 hash32 = ((hash_bin[0] & 0xFF) << 24) |
                   ((hash_bin[1] & 0xFF) << 16) |
                   ((hash_bin[2] & 0xFF) << 8) |
                    (hash_bin[3] & 0xFF);
 
   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()
     : server_type_(NO_SERVER_DATA),
       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,
                              const base::string16& unique_name)
     : FormFieldData(field),
       unique_name_(unique_name),
       server_type_(NO_SERVER_DATA),
       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 &&
       type != FIELD_WITH_DEFAULT_VALUE) {
     heuristic_type_ = type;
   } else {
     NOTREACHED();
     // This case should not be reachable; but since this has potential
     // implications on data uploaded to the server, better safe than sorry.
     heuristic_type_ = UNKNOWN_TYPE;
@@ skipping 55 matching lines @@
   if (type.GetStorableType() == PHONE_HOME_NUMBER) {
     FillPhoneNumberField(field, value, field_data);
     return true;
   } else if (field_data->form_control_type == "select-one") {
     return FillSelectControl(type, value, app_locale, field_data);
   } else if (field_data->form_control_type == "month") {
     return FillMonthControl(value, field_data);
   } 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;
   return true;
 }
 
 }  // namespace autofill