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

components/autofill/core/browser/autofill_field_unittest.cc

Index: components/autofill/core/browser/autofill_field_unittest.cc
diff --git a/components/autofill/core/browser/autofill_field_unittest.cc b/components/autofill/core/browser/autofill_field_unittest.cc
index 9d0f3ac3ae77f8a6f714fa431a99ced4add3481b..e7c5ff46cc38ba39ecacf5c3d089046017d484ab 100644
--- a/components/autofill/core/browser/autofill_field_unittest.cc
+++ b/components/autofill/core/browser/autofill_field_unittest.cc
@@ -34,6 +34,20 @@ FormFieldData GenerateSelectFieldWithOptions(const char* const* options,
   return form_field;
 }
 
+struct TestCase {
+  std::string card_number_;
+  int total_spilts_;

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

nit: Prefer size_t over int for all of the ints in this struct.

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

Done.

+  std::vector<int> splits_;
+  std::vector<std::string> expected_results_;
+
+  size_t GetStartIndex(int index) {
+    size_t ret = 0;
+    for (int i = 0; i < index; i++)

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

nit: Prefer "++i" to "i++".

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

Done.

+      ret += splits_[i];
+    return ret;
+  }
+};

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

It looks like this is only used in a single test, so please define this locally
to that test.

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

Done.

+
 TEST(AutofillFieldTest, Type) {
   AutofillField field;
   ASSERT_EQ(NO_SERVER_DATA, field.server_type());
@@ -503,5 +517,82 @@ TEST(AutofillFieldTest, FillStreetAddressTextField) {
   EXPECT_EQ(UTF8ToUTF16("桜丘町26-1セルリアンタワー6階"), field.value);
 }
 
+TEST(AutofillFieldTest, FillCreditCardNumber) {
+  // Populate test cases.
+  std::vector<TestCase> test_cases;
+
+  // Case 1: card number without any spilt.
+  TestCase test1;
+  test1.card_number_ = "4111111111111111";
+  test1.total_spilts_ = 0;
+  test_cases.push_back(test1);
+
+  // Case 2: card number broken up into four equal groups, of length 4.
+  test1.card_number_ = "5187654321098765";
+  test1.total_spilts_ = 4;
+  int splits1[] = {4, 4, 4, 4};
+  test1.splits_ =
+      std::vector<int>(splits1, splits1 + sizeof(splits1) / sizeof(int));

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

nit: Please use the arraysize macro rather than implementing it yourself.

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

Done.

+  std::string results1[] = {"5187654321098765", "654321098765", "21098765",
+                            "8765"};

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

Why aren't the expected results four digits each?

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

As we are not copying exact size substring, instead just moving the start index
and leaving the max_length for form field to do rest.

+  test1.expected_results_ = std::vector<std::string>(
+      results1, results1 + sizeof(results1) / sizeof(std::string));
+  test_cases.push_back(test1);
+
+  // Case 3: card with 15 digits number, broken up into three unequal groups, of
+  // lengths 4, 6, and 5.

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

Rather than writing a single TEST to cover three different cases, I think it
would be clearer to simply write three separate TESTs.  That way, the logic
within each test would be simpler and easier to follow.  In general, "dumb" but
easy-to-follow test code is better than clever but harder-to-follow test code,
since (1) tests are a form of documentation, and so readability matters quite a
bit; and (2) simpler tests are less likely to have their own bugs.

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

Done.

+  test1.card_number_ = "423456789012345";
+  test1.total_spilts_ = 3;
+  int splits4[] = {4, 6, 5};
+  test1.splits_ =
+      std::vector<int>(splits4, splits4 + sizeof(splits4) / sizeof(int));
+  std::string results4[] = {"423456789012345", "56789012345", "12345"};
+  test1.expected_results_ = std::vector<std::string>(
+      results4, results4 + sizeof(results4) / sizeof(std::string));
+  test_cases.push_back(test1);
+
+  // Start executing test cases to verify parts and full credit card number.
+  for (std::vector<TestCase>::iterator it = test_cases.begin();
+       it != test_cases.end();
+       ++it) {
+    TestCase current_case = *it;
+
+    for (int i = 0; i < current_case.total_spilts_; i++) {

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

nit: Prefer "++i" to "i++".

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

nit: int -> size_t

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

Done.

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

Done.

+      AutofillField cc_number_part;
+      cc_number_part.SetHtmlType(HTML_TYPE_UNKNOWN, HtmlFieldMode());

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

Does the test fail if you omit this line?  If not, please omit it.

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

Done.

+      cc_number_part.set_heuristic_type(CREDIT_CARD_NUMBER);
+      cc_number_part.label = ASCIIToUTF16("Card Number");
+      std::ostringstream str;
+      str << "card_number_part" << i;

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

If you really need this, please use base::StringPrintf

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

Done.

+      cc_number_part.name = ASCIIToUTF16(str.str());

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

Do you really need to set the name or label?

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

Done.

+      cc_number_part.max_length = current_case.splits_[i];
+      cc_number_part.set_credit_card_number_start_index(
+          current_case.GetStartIndex(i));
+
+      // Fill with a card-number; should fill just the card_number_part.
+      AutofillField::FillFormField(cc_number_part,
+                                   ASCIIToUTF16(current_case.card_number_),
+                                   "en-US",
+                                   "en-US",
+                                   &cc_number_part);
+      EXPECT_EQ(ASCIIToUTF16(current_case.expected_results_[i]),
+                cc_number_part.value);
+    }
+
+    // Verify that full card-number shall get fill properly as well.
+    AutofillField cc_number_full;
+    cc_number_full.SetHtmlType(HTML_TYPE_UNKNOWN, HtmlFieldMode());
+    cc_number_full.set_heuristic_type(CREDIT_CARD_NUMBER);
+    cc_number_full.label = ASCIIToUTF16("Card Number");
+    cc_number_full.name = ASCIIToUTF16("card_number");
+    AutofillField::FillFormField(cc_number_full,
+                                 ASCIIToUTF16(current_case.card_number_),
+                                 "en-US",
+                                 "en-US",
+                                 &cc_number_full);
+    EXPECT_EQ(ASCIIToUTF16(current_case.card_number_), cc_number_full.value);
+  }
+}
+
 }  // namespace
 }  // namespace autofill