Autofill: Better recognize credit card type 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/command_line.h"
 #include "base/logging.h"
 #include "base/metrics/field_trial.h"
 #include "base/sha1.h"
@@ skipping 230 matching lines @@
     }
   }
 
   return false;
 }
 
 // Try to fill a credit card type |value| (Visa, MasterCard, etc.) into the
 // given |field|.
 bool FillCreditCardTypeSelectControl(const base::string16& value,
                                      FormFieldData* field) {
-  // Try stripping off spaces.
-  base::string16 value_stripped;
-  base::RemoveChars(base::StringToLowerASCII(value), base::kWhitespaceUTF16,
-                    &value_stripped);
-
-  for (size_t i = 0; i < field->option_values.size(); ++i) {
-    base::string16 option_value_lowercase;
-    base::RemoveChars(base::StringToLowerASCII(field->option_values[i]),
-                      base::kWhitespaceUTF16, &option_value_lowercase);
-    base::string16 option_contents_lowercase;
-    base::RemoveChars(base::StringToLowerASCII(field->option_contents[i]),
-                      base::kWhitespaceUTF16, &option_contents_lowercase);
-
-    // Perform a case-insensitive comparison; but fill the form with the
-    // original text, not the lowercased version.
-    if (value_stripped == option_value_lowercase ||
-        value_stripped == option_contents_lowercase) {
-      field->value = field->option_values[i];
-      return true;
-    }
+  size_t idx;
+  if (AutofillField::FindValueInCreditTypeSelectControl(*field, value, &idx)) {
+    field->value = field->option_values[idx];
+    return true;
   }
 
   // For American Express, also try filling as "AmEx".
   if (value == l10n_util::GetStringUTF16(IDS_AUTOFILL_CC_AMEX))
     return FillCreditCardTypeSelectControl(ASCIIToUTF16("AmEx"), field);
 
   return false;
 }
 
 // Set |field_data|'s value to |number|, or possibly an appropriate substring of
@@ skipping 253 matching lines @@
 
   if (field.phone_part() == AutofillField::PHONE_SUFFIX ||
       field_data.max_length == PhoneNumber::kSuffixLength) {
     return
         number.substr(PhoneNumber::kSuffixOffset, PhoneNumber::kSuffixLength);
   }
 
   return number;
 }
 
+// static
+bool AutofillField::FindValueInCreditTypeSelectControl(

[Evan Stade, 2015/03/25 21:25:51]

nothing about this looks credit card specific (?)

[Lei Zhang, 2015/03/25 22:18:48]

Done.

+    const FormFieldData& field,
+    const base::string16& value,
+    size_t* index) {
+  // Try stripping off spaces.
+  base::string16 value_stripped;
+  base::RemoveChars(base::StringToLowerASCII(value), base::kWhitespaceUTF16,
+                    &value_stripped);
+  for (size_t i = 0; i < field.option_values.size(); ++i) {
+    base::string16 option_value_lowercase;
+    base::RemoveChars(base::StringToLowerASCII(field.option_values[i]),
+                      base::kWhitespaceUTF16, &option_value_lowercase);
+    base::string16 option_contents_lowercase;
+    base::RemoveChars(base::StringToLowerASCII(field.option_contents[i]),

[Evan Stade, 2015/03/25 21:25:51]

I don't think assuming this is ASCII and failing on all non-ascii stuff is
correct; you can strip whitespace but after that compare should probably use
base::CaseInsensitiveCompare

[Lei Zhang, 2015/03/25 22:18:48]

base::StringToLowerASCII() doesn't fail if it encounters non-ASCII characters.
It only converts ASCII chars to lower case. Non-ASCII chars are unmodified. i.e.
ABCйцу becomes abcйцу.

[Evan Stade, 2015/03/25 22:23:08]

my point is that upper case accented character won't get converted to lower case
accented character.

[Lei Zhang, 2015/03/25 22:39:03]

Sure, that's a valid point, but I just want to move some code from
FillCreditCardTypeSelectControl() in this CL. Let's work on the non-ASCII lower
case issue separately.

[Evan Stade, 2015/03/25 22:39:53]

seems like it would only take about 5 minutes of effort. Is there something
tricky to it that I'm missing?

+                      base::kWhitespaceUTF16, &option_contents_lowercase);
+
+    // Perform a case-insensitive comparison.
+    if (value_stripped == option_value_lowercase ||
+        value_stripped == option_contents_lowercase) {
+      if (index)
+        *index = i;
+      return true;
+    }
+  }
+  return false;
+}
+
 }  // namespace autofill