Support autocompletion for HTMl5 tags:"email", "month" and "tel".

chrome/browser/autofill/autofill_manager_unittest.cc

Index: chrome/browser/autofill/autofill_manager_unittest.cc
diff --git a/chrome/browser/autofill/autofill_manager_unittest.cc b/chrome/browser/autofill/autofill_manager_unittest.cc
index b4541432843268906f37f62fff19f16fe656a6c2..2332e93ff2267beaf9002a48552a0aaee89000fe 100644
--- a/chrome/browser/autofill/autofill_manager_unittest.cc
+++ b/chrome/browser/autofill/autofill_manager_unittest.cc
@@ -48,9 +48,12 @@ typedef Tuple5<int,
 
 class TestPersonalDataManager : public PersonalDataManager {
  public:
-  TestPersonalDataManager() {
+  explicit TestPersonalDataManager(bool four_credit_cards_with_year_month) {

[Ilya Sherman, 2011/01/07 07:28:28]

Rather than adding a boolean here, please add an |AddCreditCard()| method and
use that + the ClearCreditCards() method.

     CreateTestAutoFillProfiles(&web_profiles_);
-    CreateTestCreditCards(&credit_cards_);
+    if (four_credit_cards_with_year_month)
+      CreateTestCreditCardsYearMonth(&credit_cards_);
+    else
+      CreateTestCreditCards(&credit_cards_);
   }
 
   virtual void InitializeIfNeeded() {}
@@ -131,6 +134,27 @@ class TestPersonalDataManager : public PersonalDataManager {
     credit_cards->push_back(credit_card);
   }
 
+  void CreateTestCreditCardsYearMonth(ScopedVector<CreditCard>* credit_cards) {
+    // Create four credit cards with year month combination as following,
+    // 1. year empty, month empty
+    // 2. year empty, month non-empty
+    // 3. year non-empty, month empty
+    // 4. both non-empty
+    for (int i = 0; i < 4; i++) {

[dhollowa, 2011/01/07 03:47:15]

I'm a lazy reader.  It'd be easier on me if we unroll this loop and set the
values explicitly.  I like to save the thinking for "real" code.  ;-)

+      const char* year  = i%2 ? "" : "2012";
+      const char* month = i/2 ? "" : "04";
+      std::string name = base::StringPrintf("Miku%d",i);
+      CreditCard* credit_card = new CreditCard;
+      autofill_test::SetCreditCardInfo(credit_card, name.c_str(),
+          "Miku Hatsune",
+          "4234567890654321", // Visa
+          month, year);
+      credit_card->set_guid("00000000-0000-0000-0000-" +
+                            base::StringPrintf("%012d", 7 + i));
+      credit_cards->push_back(credit_card);
+    }
+  }

[honten.org, 2011/01/06 07:04:54]

Make four credit cards for year month combination checking.

+
   DISALLOW_COPY_AND_ASSIGN(TestPersonalDataManager);
 };
 
@@ -181,12 +205,20 @@ void CreateTestAddressFormData(FormData* form) {
   autofill_test::CreateTestFormField(
       "Email", "email", "", "text", &field);
   form->fields.push_back(field);
-}
+  autofill_test::CreateTestFormField(
+      "Email", "email2", "", "email", &field);
+  form->fields.push_back(field);
+  autofill_test::CreateTestFormField(
+      "Phone Number", "phonenumber2", "", "tel", &field);
+  form->fields.push_back(field);
+ }
 
 // Populates |form| with data corresponding to a simple credit card form.
 // Note that this actually appends fields to the form data, which can be useful
 // for building up more complex test forms.
-void CreateTestCreditCardFormData(FormData* form, bool is_https) {
+void CreateTestCreditCardFormData(FormData* form,
+                                  bool is_https,
+                                  bool use_month_type) {
   form->name = ASCIIToUTF16("MyForm");
   form->method = ASCIIToUTF16("POST");
   if (is_https) {
@@ -205,12 +237,18 @@ void CreateTestCreditCardFormData(FormData* form, bool is_https) {
   autofill_test::CreateTestFormField(
       "Card Number", "cardnumber", "", "text", &field);
   form->fields.push_back(field);
-  autofill_test::CreateTestFormField(
-      "Expiration Date", "ccmonth", "", "text", &field);
-  form->fields.push_back(field);
-  autofill_test::CreateTestFormField(
-      "", "ccyear", "", "text", &field);
-  form->fields.push_back(field);
+  if (use_month_type) {
+    autofill_test::CreateTestFormField(
+        "Expiration Date", "ccmonth", "", "month", &field);
+    form->fields.push_back(field);
+  } else {
+    autofill_test::CreateTestFormField(
+        "Expiration Date", "ccmonth", "", "text", &field);
+    form->fields.push_back(field);
+    autofill_test::CreateTestFormField(
+        "", "ccyear", "", "text", &field);
+    form->fields.push_back(field);
+  }
 }
 
 void ExpectSuggestions(int page_id,
@@ -241,7 +279,7 @@ void ExpectSuggestions(int page_id,
 // Verifies that the |filled_form| has been filled with the given data.
 // Verifies address fields if |has_address_fields| is true, and verifies
 // credit card fields if |has_credit_card_fields| is true. Verifies both if both
-// are true.
+// are true. |use_month_type| is used for credit card input month type.
 void ExpectFilledForm(int page_id,
                       const FormData& filled_form,
                       int expected_page_id,
@@ -262,10 +300,11 @@ void ExpectFilledForm(int page_id,
                       const char* expiration_month,
                       const char* expiration_year,
                       bool has_address_fields,
-                      bool has_credit_card_fields) {
+                      bool has_credit_card_fields,
+                      int use_month_type) {

[Ilya Sherman, 2011/01/07 07:28:28]

nit: bool rather than int

   // The number of fields in the address and credit card forms created above.
-  const size_t kAddressFormSize = 12;
-  const size_t kCreditCardFormSize = 4;
+  const size_t kAddressFormSize = 14;
+  const size_t kCreditCardFormSize = use_month_type ? 3 : 4;
 
   EXPECT_EQ(expected_page_id, page_id);
   EXPECT_EQ(ASCIIToUTF16("MyForm"), filled_form.name);
@@ -324,7 +363,13 @@ void ExpectFilledForm(int page_id,
     autofill_test::CreateTestFormField(
          "Email", "email", email, "text", &field);
     EXPECT_TRUE(field.StrictlyEqualsHack(filled_form.fields[11]));
-  }
+    autofill_test::CreateTestFormField(
+         "Email", "email2", email, "email", &field);
+    EXPECT_TRUE(field.StrictlyEqualsHack(filled_form.fields[12]));
+    autofill_test::CreateTestFormField(
+        "Phone Number", "phonenumber2", phone, "tel", &field);
+    EXPECT_TRUE(field.StrictlyEqualsHack(filled_form.fields[13]));
+   }
 
   if (has_credit_card_fields) {
     size_t offset = has_address_fields? kAddressFormSize : 0;
@@ -334,12 +379,23 @@ void ExpectFilledForm(int page_id,
     autofill_test::CreateTestFormField(
         "Card Number", "cardnumber", card_number, "text", &field);
     EXPECT_TRUE(field.StrictlyEqualsHack(filled_form.fields[offset + 1]));
-    autofill_test::CreateTestFormField(
-        "Expiration Date", "ccmonth", expiration_month, "text", &field);
-    EXPECT_TRUE(field.StrictlyEqualsHack(filled_form.fields[offset + 2]));
-    autofill_test::CreateTestFormField(
-        "", "ccyear", expiration_year, "text", &field);
-    EXPECT_TRUE(field.StrictlyEqualsHack(filled_form.fields[offset + 3]));
+    if (use_month_type) {
+      std::string exp_year = expiration_year;
+      std::string exp_month = expiration_month;
+      std::string year_month;

[Ilya Sherman, 2011/01/07 07:28:28]

nit: "date" might be a better name

+      if (!exp_year.empty() && !exp_month.empty())
+        year_month = exp_year + "-" + exp_month;
+      autofill_test::CreateTestFormField("Expiration Date", "ccmonth",
+          year_month.c_str(), "month", &field);
+      EXPECT_TRUE(field.StrictlyEqualsHack(filled_form.fields[offset + 2]));
+    } else {
+      autofill_test::CreateTestFormField(
+          "Expiration Date", "ccmonth", expiration_month, "text", &field);
+      EXPECT_TRUE(field.StrictlyEqualsHack(filled_form.fields[offset + 2]));
+      autofill_test::CreateTestFormField(
+          "", "ccyear", expiration_year, "text", &field);
+      EXPECT_TRUE(field.StrictlyEqualsHack(filled_form.fields[offset + 3]));
+    }
   }
 }
 
@@ -351,7 +407,7 @@ void ExpectFilledAddressFormElvis(int page_id,
                    "Presley", "3734 Elvis Presley Blvd.", "Apt. 10", "Memphis",
                    "Tennessee", "38116", "USA", "12345678901", "",
                    "theking@gmail.com", "", "", "", "", true,
-                   has_credit_card_fields);
+                   has_credit_card_fields, false);
 }
 
 void ExpectFilledCreditCardFormElvis(int page_id,
@@ -361,7 +417,20 @@ void ExpectFilledCreditCardFormElvis(int page_id,
   ExpectFilledForm(page_id, filled_form, expected_page_id,
                    "", "", "", "", "", "", "", "", "", "", "", "",
                    "Elvis Presley", "4234567890123456", "04", "2012",
-                   has_address_fields, true);
+                   has_address_fields, true, false);
+}
+
+void ExpectFilledCreditCardYearMonthWithIndex(int page_id,
+                                              const FormData& filled_form,
+                                              int expected_page_id,
+                                              bool has_address_fields,
+                                              int i) {
+  const char* year  = i%2 ? "" : "2012";
+  const char* month = i/2 ? "" : "04";
+  ExpectFilledForm(page_id, filled_form, expected_page_id,
+                   "", "", "", "", "", "", "", "", "", "", "", "",
+                   "Miku Hatsune", "4234567890654321", month, year,
+                   has_address_fields, true, true);
 }
 
 class TestAutoFillManager : public AutoFillManager {
@@ -433,7 +502,7 @@ class AutoFillManagerTest : public RenderViewHostTestHarness {
 
   virtual void SetUp() {
     RenderViewHostTestHarness::SetUp();
-    test_personal_data_ = new TestPersonalDataManager();
+    test_personal_data_ = new TestPersonalDataManager(false);
     autofill_manager_.reset(new TestAutoFillManager(contents(),
                                                     test_personal_data_.get()));
   }
@@ -733,7 +802,7 @@ TEST_F(AutoFillManagerTest, GetProfileSuggestionsMethodGet) {
 TEST_F(AutoFillManagerTest, GetCreditCardSuggestionsEmptyValue) {
   // Set up our form data.
   FormData form;
-  CreateTestCreditCardFormData(&form, true);
+  CreateTestCreditCardFormData(&form, true, false);
   std::vector<FormData> forms(1, form);
   autofill_manager_->FormsSeen(forms);
 
@@ -777,7 +846,7 @@ TEST_F(AutoFillManagerTest, GetCreditCardSuggestionsEmptyValue) {
 TEST_F(AutoFillManagerTest, GetCreditCardSuggestionsMatchCharacter) {
   // Set up our form data.
   FormData form;
-  CreateTestCreditCardFormData(&form, true);
+  CreateTestCreditCardFormData(&form, true, false);
   std::vector<FormData> forms(1, form);
   autofill_manager_->FormsSeen(forms);
 
@@ -814,7 +883,7 @@ TEST_F(AutoFillManagerTest, GetCreditCardSuggestionsMatchCharacter) {
 TEST_F(AutoFillManagerTest, GetCreditCardSuggestionsNonCCNumber) {
   // Set up our form data.
   FormData form;
-  CreateTestCreditCardFormData(&form, true);
+  CreateTestCreditCardFormData(&form, true, false);
   std::vector<FormData> forms(1, form);
   autofill_manager_->FormsSeen(forms);
 
@@ -858,7 +927,7 @@ TEST_F(AutoFillManagerTest, GetCreditCardSuggestionsNonCCNumber) {
 TEST_F(AutoFillManagerTest, GetCreditCardSuggestionsNonHTTPS) {
   // Set up our form data.
   FormData form;
-  CreateTestCreditCardFormData(&form, false);
+  CreateTestCreditCardFormData(&form, false, false);
   std::vector<FormData> forms(1, form);
   autofill_manager_->FormsSeen(forms);
 
@@ -925,7 +994,7 @@ TEST_F(AutoFillManagerTest, GetAddressAndCreditCardSuggestions) {
   // Set up our form data.
   FormData form;
   CreateTestAddressFormData(&form);
-  CreateTestCreditCardFormData(&form, true);
+  CreateTestCreditCardFormData(&form, true, false);
   std::vector<FormData> forms(1, form);
   autofill_manager_->FormsSeen(forms);
 
@@ -1002,7 +1071,7 @@ TEST_F(AutoFillManagerTest, GetAddressAndCreditCardSuggestionsNonHttps) {
   // Set up our form data.
   FormData form;
   CreateTestAddressFormData(&form);
-  CreateTestCreditCardFormData(&form, false);
+  CreateTestCreditCardFormData(&form, false, false);
   std::vector<FormData> forms(1, form);
   autofill_manager_->FormsSeen(forms);
 
@@ -1268,7 +1337,7 @@ TEST_F(AutoFillManagerTest, FillAddressForm) {
 TEST_F(AutoFillManagerTest, FillCreditCardForm) {
   // Set up our form data.
   FormData form;
-  CreateTestCreditCardFormData(&form, true);
+  CreateTestCreditCardFormData(&form, true, false);
   std::vector<FormData> forms(1, form);
   autofill_manager_->FormsSeen(forms);
 
@@ -1283,12 +1352,42 @@ TEST_F(AutoFillManagerTest, FillCreditCardForm) {
   ExpectFilledCreditCardFormElvis(page_id, results, kDefaultPageID, false);
 }
 
+// Test that we correctly fill a credit card form with month input type.
+TEST_F(AutoFillManagerTest, FillCreditCardFormWithMonthInput) {
+  for (int i  = 0; i < 4; ++i) {

[dhollowa, 2011/01/07 03:47:15]

Same here.  I'd prefer to unroll the loop with these.

[Ilya Sherman, 2011/01/07 07:28:28]

I think it's reasonable to preserve the loop, but unroll the credit card
creation.  One way to do this is to create an array of the four credit cards and
the corresponding expectations, and loop over that.

[honten.org, 2011/01/07 08:06:17]

I see... I'll change it again.

On 2011/01/07 07:28:28, Ilya Sherman wrote:

[dhollowa, 2011/01/07 18:44:13]

No, don't!  No loops!

On 2011/01/07 08:06:17, honten wrote:

[honten.org, 2011/01/07 18:45:52]

So... which should I choose????

Could you discuss between you guys?

On 2011/01/07 18:44:13, dhollowa wrote:

[honten.org, 2011/01/07 18:48:31]

BTW, if the no-loop is fine, is my latest change Ok, dhollowa?

I separate this loop to four test functions. Is this fine? Please refer to the
latest patch.

Thanks,

On 2011/01/07 18:45:52, honten wrote:

[dhollowa, 2011/01/07 18:54:18]

Yes, I like the latest unrolled version.  Thanks.

On 2011/01/07 18:48:31, honten wrote:

[Ilya Sherman, 2011/01/10 20:08:48]

Unrolled is fine.  I prefer loops to extra boilerplate, but I don't feel
strongly about it. =)

+    // Same as the SetUp(), but generate 4 credit cards with year month
+    // combination.
+    RenderViewHostTestHarness::SetUp();
+    test_personal_data_ = new TestPersonalDataManager(true);
+    autofill_manager_.reset(new TestAutoFillManager(contents(),
+                            test_personal_data_.get()));
+
+    // Set up our form data.
+    FormData form;
+    CreateTestCreditCardFormData(&form, true, true);
+    std::vector<FormData> forms(1, form);
+    autofill_manager_->FormsSeen(forms);
+
+    std::string guid = autofill_manager_->GetLabeledCreditCard(
+        base::StringPrintf("Miku%d", i).c_str())->guid();
+    EXPECT_TRUE(autofill_manager_->FillAutoFillFormData(
+        kDefaultPageID, form, *form.fields.begin(),
+        autofill_manager_->PackGUIDs(guid, std::string())));
+
+    int page_id = 0;
+    FormData results;
+    EXPECT_TRUE(GetAutoFillFormDataFilledMessage(&page_id, &results));
+    ExpectFilledCreditCardYearMonthWithIndex(page_id, results, kDefaultPageID,
+                                             false, i);
+  }
+}
+
 // Test that we correctly fill a combined address and credit card form.
 TEST_F(AutoFillManagerTest, FillAddressAndCreditCardForm) {
   // Set up our form data.
   FormData form;
   CreateTestAddressFormData(&form);
-  CreateTestCreditCardFormData(&form, true);
+  CreateTestCreditCardFormData(&form, true, false);
   std::vector<FormData> forms(1, form);
   autofill_manager_->FormsSeen(forms);
 
@@ -1329,7 +1428,7 @@ TEST_F(AutoFillManagerTest, FillAutoFilledForm) {
   CreateTestAddressFormData(&form);
   // Mark one of the address fields as autofilled.
   form.fields[4].set_autofilled(true);
-  CreateTestCreditCardFormData(&form, true);
+  CreateTestCreditCardFormData(&form, true, false);
   std::vector<FormData> forms(1, form);
   autofill_manager_->FormsSeen(forms);
 
@@ -1346,7 +1445,7 @@ TEST_F(AutoFillManagerTest, FillAutoFilledForm) {
     SCOPED_TRACE("Address");
     ExpectFilledForm(page_id, results, kDefaultPageID,
                      "Elvis", "", "", "", "", "", "", "", "", "", "", "",
-                     "", "", "", "", true, true);
+                     "", "", "", "", true, true, false);
   }
 
   // Now fill the credit card data.
@@ -1384,7 +1483,7 @@ TEST_F(AutoFillManagerTest, FillAutoFilledForm) {
     SCOPED_TRACE("Credit card 2");
     ExpectFilledForm(page_id, results, kPageID3,
                    "", "", "", "", "", "", "", "", "", "", "", "",
-                   "", "", "", "2012", true, true);
+                   "", "", "", "2012", true, true, false);
   }
 }