Refactor webkit_glue::FormField to remove hacky methods

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 2a2c329c95b5c2400c7aec04bc5fa86a7d32ab0c..f5d3ddaf569b8d46cc9b05dabfd89fe8f154a27f 100644
--- a/chrome/browser/autofill/autofill_manager_unittest.cc
+++ b/chrome/browser/autofill/autofill_manager_unittest.cc
@@ -257,6 +257,15 @@ void ExpectSuggestions(int page_id,
   }
 }
 
+void ExpectFieldEquals(const webkit_glue::FormField& expected,
+                       const webkit_glue::FormField& actual) {
+  EXPECT_EQ(expected.label, actual.label);
+  EXPECT_EQ(expected.name, actual.name);
+  EXPECT_EQ(expected.value, actual.value);
+  EXPECT_EQ(expected.form_control_type, actual.form_control_type);
+  EXPECT_EQ(expected.max_length, actual.max_length);

[dhollowa, 2011/08/08 21:40:39]

This test is repeated in autofill_browsertest.cc and
form_manager_browsertest.cc.  It would be better to have a single definition (in
form_field.cc).  Also this version is missing a comparison of |is_autofilled|.

[Ilya Sherman, 2011/08/09 00:13:43]

This version is intentionally missing the |is_autofilled| check.  I've rewritten
this function to be more explicit in its expectations -- this also makes the
below code cleaner.

+}
+
 // 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
@@ -310,50 +319,92 @@ void ExpectFilledForm(int page_id,
   if (has_address_fields) {
     autofill_test::CreateTestFormField(
         "First Name", "firstname", first, "text", &field);
-    EXPECT_TRUE(field.StrictlyEqualsHack(filled_form.fields[0]));
+    {
+      SCOPED_TRACE("First Name");
+      ExpectFieldEquals(field, filled_form.fields[0]);
+    }
     autofill_test::CreateTestFormField(
         "Middle Name", "middlename", middle, "text", &field);
-    EXPECT_TRUE(field.StrictlyEqualsHack(filled_form.fields[1]));
+    {
+      SCOPED_TRACE("Middle Name");
+      ExpectFieldEquals(field, filled_form.fields[1]);
+    }
     autofill_test::CreateTestFormField(
         "Last Name", "lastname", last, "text", &field);
-    EXPECT_TRUE(field.StrictlyEqualsHack(filled_form.fields[2]));
+    {
+      SCOPED_TRACE("Last Name");
+      ExpectFieldEquals(field, filled_form.fields[2]);
+    }
     autofill_test::CreateTestFormField(
         "Address Line 1", "addr1", address1, "text", &field);
-    EXPECT_TRUE(field.StrictlyEqualsHack(filled_form.fields[3]));
+    {
+      SCOPED_TRACE("Address Line 1");
+      ExpectFieldEquals(field, filled_form.fields[3]);
+    }
     autofill_test::CreateTestFormField(
         "Address Line 2", "addr2", address2, "text", &field);
-    EXPECT_TRUE(field.StrictlyEqualsHack(filled_form.fields[4]));
+    {
+      SCOPED_TRACE("Address Line 2");
+      ExpectFieldEquals(field, filled_form.fields[4]);
+    }
     autofill_test::CreateTestFormField(
         "City", "city", city, "text", &field);
-    EXPECT_TRUE(field.StrictlyEqualsHack(filled_form.fields[5]));
+    {
+      SCOPED_TRACE("City");
+      ExpectFieldEquals(field, filled_form.fields[5]);
+    }
     autofill_test::CreateTestFormField(
         "State", "state", state, "text", &field);
-    EXPECT_TRUE(field.StrictlyEqualsHack(filled_form.fields[6]));
+    {
+      SCOPED_TRACE("State");
+      ExpectFieldEquals(field, filled_form.fields[6]);
+    }
     autofill_test::CreateTestFormField(
         "Postal Code", "zipcode", postal_code, "text", &field);
-    EXPECT_TRUE(field.StrictlyEqualsHack(filled_form.fields[7]));
+    {
+      SCOPED_TRACE("Postal Code");
+      ExpectFieldEquals(field, filled_form.fields[7]);
+    }
     autofill_test::CreateTestFormField(
         "Country", "country", country, "text", &field);
-    EXPECT_TRUE(field.StrictlyEqualsHack(filled_form.fields[8]));
+    {
+      SCOPED_TRACE("Country");
+      ExpectFieldEquals(field, filled_form.fields[8]);
+    }
     autofill_test::CreateTestFormField(
         "Phone Number", "phonenumber", phone, "tel", &field);
-    EXPECT_TRUE(field.StrictlyEqualsHack(filled_form.fields[9]));
+    {
+      SCOPED_TRACE("Phone Number");
+      ExpectFieldEquals(field, filled_form.fields[9]);
+    }
     autofill_test::CreateTestFormField(
         "Fax", "fax", fax, "text", &field);
-    EXPECT_TRUE(field.StrictlyEqualsHack(filled_form.fields[10]));
+    {
+      SCOPED_TRACE("Fax");
+      ExpectFieldEquals(field, filled_form.fields[10]);
+    }
     autofill_test::CreateTestFormField(
         "Email", "email", email, "email", &field);
-    EXPECT_TRUE(field.StrictlyEqualsHack(filled_form.fields[11]));
+    {
+      SCOPED_TRACE("Email");
+      ExpectFieldEquals(field, filled_form.fields[11]);
+    }
   }
 
   if (has_credit_card_fields) {
     size_t offset = has_address_fields? kAddressFormSize : 0;
     autofill_test::CreateTestFormField(
         "Name on Card", "nameoncard", name_on_card, "text", &field);
-    EXPECT_TRUE(field.StrictlyEqualsHack(filled_form.fields[offset + 0]));
+    {
+      SCOPED_TRACE("Name on Card");
+      ExpectFieldEquals(field, filled_form.fields[offset + 0]);
+    }
     autofill_test::CreateTestFormField(
         "Card Number", "cardnumber", card_number, "text", &field);
-    EXPECT_TRUE(field.StrictlyEqualsHack(filled_form.fields[offset + 1]));
+    {
+      SCOPED_TRACE("Card Number");
+      ExpectFieldEquals(field, filled_form.fields[offset + 1]);
+    }
     if (use_month_type) {
       std::string exp_year = expiration_year;
       std::string exp_month = expiration_month;
@@ -362,14 +413,23 @@ void ExpectFilledForm(int page_id,
         date = exp_year + "-" + exp_month;
       autofill_test::CreateTestFormField(
           "Expiration Date", "ccmonth", date.c_str(), "month", &field);
-      EXPECT_TRUE(field.StrictlyEqualsHack(filled_form.fields[offset + 2]));
+      {
+        SCOPED_TRACE("Expiration Date");
+        ExpectFieldEquals(field, 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]));
+      {
+        SCOPED_TRACE("Expiration Date");
+        ExpectFieldEquals(field, filled_form.fields[offset + 2]);
+      }
       autofill_test::CreateTestFormField(
           "", "ccyear", expiration_year, "text", &field);
-      EXPECT_TRUE(field.StrictlyEqualsHack(filled_form.fields[offset + 3]));
+      {
+        SCOPED_TRACE("Expiration Year");
+        ExpectFieldEquals(field, filled_form.fields[offset + 3]);
+      }
     }
   }
 }