Make AutofillField::FindValueInSelectControl() handle non-ASCII values.

BUG=470726

Review URL: https://codereview.chromium.org/1059173002

Cr-Commit-Position: refs/heads/master@{#323844}

2 files changed, 100 insertions, 27 deletions

diff --git a/components/autofill/core/browser/autofill_field.cc b/components/autofill/core/browser/autofill_field.cc
index 8e8c047..1aa3bd3 100644
--- a/components/autofill/core/browser/autofill_field.cc
+++ b/components/autofill/core/browser/autofill_field.cc
@@ -5,6 +5,7 @@
 #include "components/autofill/core/browser/autofill_field.h"
 
 #include "base/command_line.h"
+#include "base/i18n/string_compare.h"
 #include "base/logging.h"
 #include "base/metrics/field_trial.h"
 #include "base/sha1.h"
@@ -379,6 +380,27 @@ std::string Hash32Bit(const std::string& str) {
   return base::UintToString(hash32);
 }
 
+scoped_ptr<icu::Collator> CreateCaseInsensitiveCollator() {
+  UErrorCode error = U_ZERO_ERROR;
+  scoped_ptr<icu::Collator> collator(icu::Collator::createInstance(error));
+  DCHECK(U_SUCCESS(error));
+  collator->setStrength(icu::Collator::PRIMARY);
+  return collator;
+}
+
+base::string16 RemoveWhitespace(const base::string16& value) {
+  base::string16 stripped_value;
+  base::RemoveChars(value, base::kWhitespaceUTF16, &stripped_value);
+  return stripped_value;
+}
+
+bool StringsEqualWithCollator(const base::string16& lhs,
+                              const base::string16& rhs,
+                              icu::Collator* collator) {
+  return base::i18n::CompareString16WithCollator(collator, lhs, rhs) ==
+      UCOL_EQUAL;
+}
+
 }  // namespace
 
 AutofillField::AutofillField()
@@ -529,22 +551,23 @@ base::string16 AutofillField::GetPhoneNumberValue(
 bool AutofillField::FindValueInSelectControl(const FormFieldData& field,
                                              const base::string16& value,
                                              size_t* index) {
-  // TODO(thestig): Improve this. See http://crbug.com/470726)
-  // Try stripping off spaces.
-  base::string16 value_stripped;
-  base::RemoveChars(base::StringToLowerASCII(value), base::kWhitespaceUTF16,
-                    &value_stripped);
+  scoped_ptr<icu::Collator> collator = CreateCaseInsensitiveCollator();
+
+  // Strip off spaces for all values in the comparisons.
+  const base::string16 value_stripped = RemoveWhitespace(value);
+
   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.
-    if (value_stripped == option_value_lowercase ||
-        value_stripped == option_contents_lowercase) {
+    base::string16 option_value = RemoveWhitespace(field.option_values[i]);
+    if (StringsEqualWithCollator(value_stripped, option_value,
+                                 collator.get())) {
+      if (index)
+        *index = i;
+      return true;
+    }
+
+    base::string16 option_contents = RemoveWhitespace(field.option_contents[i]);
+    if (StringsEqualWithCollator(value_stripped, option_contents,
+                                 collator.get())) {
       if (index)
         *index = i;
       return true;
diff --git a/components/autofill/core/browser/autofill_field_unittest.cc b/components/autofill/core/browser/autofill_field_unittest.cc
index cb61fcd..36096f2 100644
--- a/components/autofill/core/browser/autofill_field_unittest.cc
+++ b/components/autofill/core/browser/autofill_field_unittest.cc
@@ -4,9 +4,8 @@
 
 #include "components/autofill/core/browser/autofill_field.h"
 
-#include "base/format_macros.h"
+#include "base/strings/string_number_conversions.h"
 #include "base/strings/string_util.h"
-#include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
 #include "components/autofill/core/browser/autofill_type.h"
 #include "components/autofill/core/browser/field_types.h"
@@ -23,9 +22,8 @@ namespace {
 FormFieldData GenerateSelectFieldWithOptions(const char* const* options,
                                              size_t options_size) {
   std::vector<base::string16> options16(options_size);
-  for (size_t i = 0; i < options_size; ++i) {
-    options16[i] = ASCIIToUTF16(options[i]);
-  }
+  for (size_t i = 0; i < options_size; ++i)
+    options16[i] = UTF8ToUTF16(options[i]);
 
   FormFieldData form_field;
   form_field.form_control_type = "select-one";
@@ -171,9 +169,8 @@ TEST(AutofillFieldTest, FillSelectControlByValue) {
 
   // Set semantically empty contents for each option, so that only the values
   // can be used for matching.
-  for (size_t i = 0; i < field.option_contents.size(); ++i) {
-    field.option_contents[i] = ASCIIToUTF16(base::StringPrintf("%" PRIuS, i));
-  }
+  for (size_t i = 0; i < field.option_contents.size(); ++i)
+    field.option_contents[i] = base::SizeTToString16(i);
 
   AutofillField::FillFormField(
       field, ASCIIToUTF16("Meenie"), "en-US", "en-US", &field);
@@ -190,9 +187,8 @@ TEST(AutofillFieldTest, FillSelectControlByContents) {
 
   // Set semantically empty values for each option, so that only the contents
   // can be used for matching.
-  for (size_t i = 0; i < field.option_values.size(); ++i) {
-    field.option_values[i] = ASCIIToUTF16(base::StringPrintf("%" PRIuS, i));
-  }
+  for (size_t i = 0; i < field.option_values.size(); ++i)
+    field.option_values[i] = base::SizeTToString16(i);
 
   AutofillField::FillFormField(
       field, ASCIIToUTF16("Miney"), "en-US", "en-US", &field);
@@ -393,7 +389,7 @@ TEST(AutofillFieldTest, FillSelectControlWithAbbreviatedMonthName) {
 
 TEST(AutofillFieldTest, FillSelectControlWithFullMonthName) {
   const char* const kMonthsFull[] = {
-    "January","February", "March", "April", "May", "June",
+    "January", "February", "March", "April", "May", "June",
     "July", "August", "September", "October", "November", "December",
   };
   AutofillField field(
@@ -622,5 +618,59 @@ TEST(AutofillFieldTest, FillCreditCardNumberWithUnequalSizeSplits) {
   EXPECT_EQ(ASCIIToUTF16(test.card_number_), cc_number_full.value);
 }
 
+TEST(AutofillFieldTest, FindValueInSelectControl) {
+  const size_t kBadIndex = 1000;
+
+  {
+    const char* const kCountries[] = {
+      "Albania", "Canada"
+    };
+    FormFieldData field(
+        GenerateSelectFieldWithOptions(kCountries, arraysize(kCountries)));
+    size_t index = kBadIndex;
+    bool ret = AutofillField::FindValueInSelectControl(
+        field, ASCIIToUTF16("Canada"), &index);
+    EXPECT_TRUE(ret);
+    EXPECT_EQ(1U, index);
+
+    index = kBadIndex;
+    ret = AutofillField::FindValueInSelectControl(
+        field, ASCIIToUTF16("CANADA"), &index);
+    EXPECT_TRUE(ret);
+    EXPECT_EQ(1U, index);
+
+    index = kBadIndex;
+    ret = AutofillField::FindValueInSelectControl(
+        field, ASCIIToUTF16("Canadia"), &index);
+    EXPECT_FALSE(ret);
+    EXPECT_EQ(kBadIndex, index);
+  }
+
+  {
+    const char* const kProvinces[] = {
+      "ALBERTA", "QUÉBEC", "NOVA SCOTIA",
+    };
+    FormFieldData field(
+        GenerateSelectFieldWithOptions(kProvinces, arraysize(kProvinces)));
+    size_t index = kBadIndex;
+    bool ret = AutofillField::FindValueInSelectControl(
+        field, ASCIIToUTF16("alberta"), &index);
+    EXPECT_TRUE(ret);
+    EXPECT_EQ(0U, index);
+
+    index = kBadIndex;
+    ret = AutofillField::FindValueInSelectControl(
+        field, UTF8ToUTF16("québec"), &index);
+    EXPECT_TRUE(ret);
+    EXPECT_EQ(1U, index);
+
+    index = kBadIndex;
+    ret = AutofillField::FindValueInSelectControl(
+        field, UTF8ToUTF16("NoVaScOtIa"), &index);
+    EXPECT_TRUE(ret);
+    EXPECT_EQ(2U, index);
+  }
+}
+
 }  // namespace
 }  // namespace autofill