Refactor webkit_glue::FormField to remove hacky methods

chrome/browser/autofill/autofill_manager_unittest.cc

 // Copyright (c) 2011 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 <vector>
 
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/scoped_vector.h"
 #include "base/string16.h"
@@ skipping 239 matching lines @@
   ASSERT_EQ(expected_num_suggestions, unique_ids.size());
   for (size_t i = 0; i < expected_num_suggestions; ++i) {
     SCOPED_TRACE(StringPrintf("i: %" PRIuS, i));
     EXPECT_EQ(expected_values[i], values[i]);
     EXPECT_EQ(expected_labels[i], labels[i]);
     EXPECT_EQ(expected_icons[i], icons[i]);
     EXPECT_EQ(expected_unique_ids[i], unique_ids[i]);
   }
 }
 
+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
 // 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,
                       const char* first,
                       const char* middle,
                       const char* last,
@@ skipping 33 matching lines @@
   if (has_address_fields)
     form_size += kAddressFormSize;
   if (has_credit_card_fields)
     form_size += kCreditCardFormSize;
   ASSERT_EQ(form_size, filled_form.fields.size());
 
   FormField field;
   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;
       std::string date;
       if (!exp_year.empty() && !exp_month.empty())
         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]);
+      }
     }
   }
 }
 
 void ExpectFilledAddressFormElvis(int page_id,
                                   const FormData& filled_form,
                                   int expected_page_id,
                                   bool has_credit_card_fields) {
   ExpectFilledForm(page_id, filled_form, expected_page_id, "Elvis", "Aaron",
                    "Presley", "3734 Elvis Presley Blvd.", "Apt. 10", "Memphis",
@@ skipping 2178 matching lines @@
   const FieldTypeSet& possible_types1 =
       form_structure.field(1)->possible_types();
   EXPECT_EQ(1U, possible_types1.size());
   EXPECT_TRUE(possible_types1.find(NAME_FIRST) != possible_types1.end());
   const FieldTypeSet& possible_types2 =
       form_structure.field(2)->possible_types();
   EXPECT_EQ(1U, possible_types2.size());
   EXPECT_TRUE(possible_types2.find(UNKNOWN_TYPE) !=
               possible_types2.end());
 }