AutoFill: Parse the ignored fields at the end of ParseAddress, so we don't

accidentally match an ignored field instead of matching a field we care about.

BUG=43260
TEST=FormStructureTest.HeuristicsStateWithProvince

Review URL: http://codereview.chromium.org/2006005

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@46701 0039d316-1c4b-4281-b951-d872f2087c98

2 files changed, 42 insertions, 9 deletions

diff --git a/chrome/browser/autofill/address_field.cc b/chrome/browser/autofill/address_field.cc
index 81d4aa1..c0adb4c 100644
--- a/chrome/browser/autofill/address_field.cc
+++ b/chrome/browser/autofill/address_field.cc
@@ -81,15 +81,7 @@ AddressField* AddressField::Parse(
 
   // Allow address fields to appear in any order.
   while (true) {
-    // We ignore the following:
-    // * Company/Business.
-    // * Attention.
-    // * Province/Region/Other.
-    if (ParseText(&q, ASCIIToUTF16("company|business name")) ||
-        ParseText(&q, ASCIIToUTF16("attention|attn.")) ||
-        ParseText(&q, ASCIIToUTF16("province|region|other"))) {
-      continue;
-    } else if (ParseAddressLines(&q, is_ecml, &address_field) ||
+    if (ParseAddressLines(&q, is_ecml, &address_field) ||
                ParseCity(&q, is_ecml, &address_field) ||
                ParseZipCode(&q, is_ecml, &address_field) ||
                ParseCountry(&q, is_ecml, &address_field)) {
@@ -97,6 +89,14 @@ AddressField* AddressField::Parse(
     } else if ((!address_field.state_ || address_field.state_->IsEmpty()) &&
                address_field.ParseState(&q, is_ecml, &address_field)) {
       continue;
+    } else if (ParseText(&q, ASCIIToUTF16("company|business name")) ||
+        ParseText(&q, ASCIIToUTF16("attention|attn.")) ||
+        ParseText(&q, ASCIIToUTF16("province|region|other"))) {
+      // We ignore the following:
+      // * Company/Business.
+      // * Attention.
+      // * Province/Region/Other.
+      continue;
     } else if (*q != **iter && ParseEmpty(&q)) {
       // Ignore non-labeled fields within an address; the page
       // MapQuest Driving Directions North America.html contains such a field.
diff --git a/chrome/browser/autofill/form_structure_unittest.cc b/chrome/browser/autofill/form_structure_unittest.cc
index 4835509..1286f9a 100644
--- a/chrome/browser/autofill/form_structure_unittest.cc
+++ b/chrome/browser/autofill/form_structure_unittest.cc
@@ -664,4 +664,37 @@ TEST(FormStructureTest, ThreeAddressLines) {
   EXPECT_EQ(ADDRESS_HOME_CITY, form_structure->field(3)->heuristic_type());
 }
 
+TEST(FormStructureTest, HeuristicsStateWithProvince) {
+  scoped_ptr<FormStructure> form_structure;
+  FormData form;
+
+  form.method = ASCIIToUTF16("post");
+  form.fields.push_back(
+      webkit_glue::FormField(ASCIIToUTF16("Address Line1"),
+                             ASCIIToUTF16("Address"),
+                             string16(),
+                             ASCIIToUTF16("text")));
+  form.fields.push_back(
+      webkit_glue::FormField(ASCIIToUTF16("Address Line2"),
+                             ASCIIToUTF16("Address"),
+                             string16(),
+                             ASCIIToUTF16("text")));
+  form.fields.push_back(
+      webkit_glue::FormField(ASCIIToUTF16("State/Province/Region"),
+                             ASCIIToUTF16("State"),
+                             string16(),
+                             ASCIIToUTF16("text")));
+  form_structure.reset(new FormStructure(form));
+  EXPECT_TRUE(form_structure->IsAutoFillable());
+  ASSERT_EQ(3U, form_structure->field_count());
+  ASSERT_EQ(3U, form_structure->autofill_count());
+
+  // Address Line 1.
+  EXPECT_EQ(ADDRESS_HOME_LINE1, form_structure->field(0)->heuristic_type());
+  // Address Line 2.
+  EXPECT_EQ(ADDRESS_HOME_LINE2, form_structure->field(1)->heuristic_type());
+  // State.
+  EXPECT_EQ(ADDRESS_HOME_STATE, form_structure->field(2)->heuristic_type());
+}
+
 }  // namespace