Reorganized phone field parsing, fixing parsing for fax numbers in the process - bug "Fax number is populated wrong"

BUG=63736
TEST=Form in the bug + unit-tested.
Review URL: http://codereview.chromium.org/6286127

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

12 files changed, 295 insertions, 211 deletions

diff --git a/chrome/browser/autofill/autofill_manager.cc b/chrome/browser/autofill/autofill_manager.cc
index 748b09a..bc2ebf8 100644
--- a/chrome/browser/autofill/autofill_manager.cc
+++ b/chrome/browser/autofill/autofill_manager.cc
@@ -577,6 +577,7 @@ void AutoFillManager::DeterminePossibleFieldTypesForUpload(
     const AutoFillField* field = submitted_form->field(i);
     FieldTypeSet field_types;
     personal_data_->GetPossibleFieldTypes(field->value(), &field_types);
+
     DCHECK(!field_types.empty());
     submitted_form->set_possible_types(i, field_types);
   }
@@ -909,7 +910,7 @@ void AutoFillManager::FillFormField(const AutoFillProfile* profile,
   DCHECK(field);
 
   if (type.subgroup() == AutoFillType::PHONE_NUMBER) {
-    FillPhoneNumberField(profile, field);
+    FillPhoneNumberField(profile, type, field);
   } else {
     if (field->form_control_type() == ASCIIToUTF16("select-one"))
       autofill::FillSelectControl(profile, type, field);
@@ -919,10 +920,11 @@ void AutoFillManager::FillFormField(const AutoFillProfile* profile,
 }
 
 void AutoFillManager::FillPhoneNumberField(const AutoFillProfile* profile,
+                                           AutoFillType type,
                                            webkit_glue::FormField* field) {
   // If we are filling a phone number, check to see if the size field
   // matches the "prefix" or "suffix" sizes and fill accordingly.
-  string16 number = profile->GetFieldText(AutoFillType(PHONE_HOME_NUMBER));
+  string16 number = profile->GetFieldText(AutoFillType(type));
   bool has_valid_suffix_and_prefix = (number.length() ==
       static_cast<size_t>(PhoneNumber::kPrefixLength +
                           PhoneNumber::kSuffixLength));
diff --git a/chrome/browser/autofill/autofill_manager.h b/chrome/browser/autofill/autofill_manager.h
index 9d44a92..aff0abc 100644
--- a/chrome/browser/autofill/autofill_manager.h
+++ b/chrome/browser/autofill/autofill_manager.h
@@ -171,9 +171,10 @@ class AutoFillManager : public TabContentsObserver,
                      AutoFillType type,
                      webkit_glue::FormField* field);
 
-  // Set |field| argument's value for phone number based on contents of the
-  // |profile|.
+  // Set |field| argument's value for phone/fax number based on contents of the
+  // |profile|. |type| is the type of the phone.
   void FillPhoneNumberField(const AutoFillProfile* profile,
+                            AutoFillType type,
                             webkit_glue::FormField* field);
 
   // Parses the forms using heuristic matching and querying the AutoFill server.
diff --git a/chrome/browser/autofill/fax_field.cc b/chrome/browser/autofill/fax_field.cc
deleted file mode 100644
index 7c948d1..0000000
--- a/chrome/browser/autofill/fax_field.cc
+++ /dev/null
@@ -1,32 +0,0 @@
-// Copyright (c) 2010 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 "chrome/browser/autofill/fax_field.h"
-
-#include "base/logging.h"
-#include "base/scoped_ptr.h"
-#include "base/utf_string_conversions.h"
-#include "chrome/browser/autofill/autofill_field.h"
-#include "grit/autofill_resources.h"
-#include "ui/base/l10n/l10n_util.h"
-
-// static
-FaxField* FaxField::Parse(std::vector<AutoFillField*>::const_iterator* iter) {
-  DCHECK(iter);
-
-  scoped_ptr<FaxField> fax_field(new FaxField);
-  if (ParseText(iter,
-                l10n_util::GetStringUTF16(IDS_AUTOFILL_FAX_RE),
-                &fax_field->number_)) {
-    return fax_field.release();
-  }
-
-  return NULL;
-}
-
-bool FaxField::GetFieldInfo(FieldTypeMap* field_type_map) const {
-  return Add(field_type_map, number_, AutoFillType(PHONE_FAX_WHOLE_NUMBER));
-}
-
-FaxField::FaxField() : number_(NULL) {}
diff --git a/chrome/browser/autofill/fax_field.h b/chrome/browser/autofill/fax_field.h
deleted file mode 100644
index e954ee1..0000000
--- a/chrome/browser/autofill/fax_field.h
+++ /dev/null
@@ -1,32 +0,0 @@
-// Copyright (c) 2010 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.
-
-#ifndef CHROME_BROWSER_AUTOFILL_FAX_FIELD_H_
-#define CHROME_BROWSER_AUTOFILL_FAX_FIELD_H_
-#pragma once
-
-#include <vector>
-
-#include "chrome/browser/autofill/autofill_type.h"
-#include "chrome/browser/autofill/form_field.h"
-
-class AutoFillField;
-
-// A class that identifies itself as a fax field using heuristics.
-class FaxField : public FormField {
- public:
-  static FaxField* Parse(std::vector<AutoFillField*>::const_iterator* iter);
-
-  virtual bool GetFieldInfo(FieldTypeMap* field_type_map) const;
-
- private:
-  FaxField();
-
-  // The fax number field.
-  AutoFillField* number_;
-
-  DISALLOW_COPY_AND_ASSIGN(FaxField);
-};
-
-#endif  // CHROME_BROWSER_AUTOFILL_FAX_FIELD_H_
diff --git a/chrome/browser/autofill/fax_field_unittest.cc b/chrome/browser/autofill/fax_field_unittest.cc
deleted file mode 100644
index 724456a..0000000
--- a/chrome/browser/autofill/fax_field_unittest.cc
+++ /dev/null
@@ -1,62 +0,0 @@
-// Copyright (c) 2010 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 "base/scoped_ptr.h"
-#include "base/scoped_vector.h"
-#include "base/utf_string_conversions.h"
-#include "chrome/browser/autofill/fax_field.h"
-#include "testing/gtest/include/gtest/gtest.h"
-#include "webkit/glue/form_field.h"
-
-namespace {
-
-class FaxFieldTest : public testing::Test {
- public:
-  FaxFieldTest() {}
-
- protected:
-  ScopedVector<AutoFillField> list_;
-  scoped_ptr<FaxField> field_;
-  FieldTypeMap field_type_map_;
-  std::vector<AutoFillField*>::const_iterator iter_;
-
- private:
-  DISALLOW_COPY_AND_ASSIGN(FaxFieldTest);
-};
-
-TEST_F(FaxFieldTest, Empty) {
-  list_.push_back(NULL);
-  iter_ = list_.begin();
-  field_.reset(FaxField::Parse(&iter_));
-  ASSERT_EQ(static_cast<FaxField*>(NULL), field_.get());
-}
-
-TEST_F(FaxFieldTest, NonParse) {
-  list_.push_back(new AutoFillField);
-  list_.push_back(NULL);
-  iter_ = list_.begin();
-  field_.reset(FaxField::Parse(&iter_));
-  ASSERT_EQ(static_cast<FaxField*>(NULL), field_.get());
-}
-
-TEST_F(FaxFieldTest, ParseOneLineFax) {
-  list_.push_back(
-      new AutoFillField(webkit_glue::FormField(ASCIIToUTF16("Fax"),
-                                               ASCIIToUTF16("faxnumber"),
-                                               string16(),
-                                               ASCIIToUTF16("text"),
-                                               0,
-                                               false),
-                        ASCIIToUTF16("fax1")));
-  list_.push_back(NULL);
-  iter_ = list_.begin();
-  field_.reset(FaxField::Parse(&iter_));
-  ASSERT_NE(static_cast<FaxField*>(NULL), field_.get());
-  ASSERT_TRUE(field_->GetFieldInfo(&field_type_map_));
-  ASSERT_TRUE(
-      field_type_map_.find(ASCIIToUTF16("fax1")) != field_type_map_.end());
-  EXPECT_EQ(PHONE_FAX_WHOLE_NUMBER, field_type_map_[ASCIIToUTF16("fax1")]);
-}
-
-}  // namespace
diff --git a/chrome/browser/autofill/form_field.cc b/chrome/browser/autofill/form_field.cc
index f364af0..e2d24dd 100644
--- a/chrome/browser/autofill/form_field.cc
+++ b/chrome/browser/autofill/form_field.cc
@@ -9,7 +9,6 @@
 #include "chrome/browser/autofill/address_field.h"
 #include "chrome/browser/autofill/autofill_field.h"
 #include "chrome/browser/autofill/credit_card_field.h"
-#include "chrome/browser/autofill/fax_field.h"
 #include "chrome/browser/autofill/name_field.h"
 #include "chrome/browser/autofill/phone_field.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebRegularExpression.h"
@@ -148,12 +147,10 @@ FormField* FormField::ParseFormField(
   field = EmailField::Parse(iter, is_ecml);
   if (field != NULL)
     return field;
+  // Parses both phone and fax.
   field = PhoneField::Parse(iter, is_ecml);
   if (field != NULL)
     return field;
-  field = FaxField::Parse(iter);
-  if (field != NULL)
-    return field;
   field = AddressField::Parse(iter, is_ecml);
   if (field != NULL)
     return field;
diff --git a/chrome/browser/autofill/phone_field.cc b/chrome/browser/autofill/phone_field.cc
index 7ce6c7c..9928b2a 100644
--- a/chrome/browser/autofill/phone_field.cc
+++ b/chrome/browser/autofill/phone_field.cc
@@ -10,6 +10,8 @@
 #include "base/string_util.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/browser/autofill/autofill_field.h"
+#include "chrome/browser/autofill/fax_number.h"
+#include "chrome/browser/autofill/home_phone_number.h"
 #include "grit/autofill_resources.h"
 #include "ui/base/l10n/l10n_util.h"
 
@@ -23,70 +25,18 @@ PhoneField* PhoneField::Parse(std::vector<AutoFillField*>::const_iterator* iter,
   if (is_ecml)
     return ParseECML(iter);
 
-  std::vector<AutoFillField*>::const_iterator q = *iter;
-  AutoFillField* phone = NULL;
-  AutoFillField* phone2 = NULL;
-  AutoFillField* phone3 = NULL;
-  bool area_code;  // true if we've parsed an area code field.
-
-  // Some pages, such as BloomingdalesShipping.html, have a field labeled
-  // "Area Code and Phone"; we want to parse this as a phone number field so
-  // we look for "phone" before we look for "area code".
-  if (ParseText(&q, l10n_util::GetStringUTF16(IDS_AUTOFILL_PHONE_RE), &phone)) {
-    area_code = false;
-  } else {
-    if (!ParseText(&q,
-                   l10n_util::GetStringUTF16(IDS_AUTOFILL_AREA_CODE_RE),
-                   &phone))
-      return NULL;
-    area_code = true;
-    ParseText(&q, l10n_util::GetStringUTF16(IDS_AUTOFILL_PHONE_RE), &phone2);
-  }
-
-  // Sometimes phone number fields are separated by "-" (e.g. test page
-  // Crate and Barrel Check Out.html).  Also, area codes are sometimes
-  // surrounded by parentheses, so a ")" may appear after the area code field.
-  //
-  // We used to match "tel" here, which we've seen in field names (e.g. on
-  // Newegg2.html), but that's too general: some pages (e.g.
-  // uk/Furniture123-1.html) have several phone numbers in succession and we
-  // don't want those to be parsed as components of a single phone number.
-  if (phone2 == NULL)
-    ParseText(&q,
-              l10n_util::GetStringUTF16(IDS_AUTOFILL_PHONE_PREFIX_RE),
-              &phone2);
-
-  // Look for a third text box.
-  if (phone2)
-    ParseText(&q,
-              l10n_util::GetStringUTF16(IDS_AUTOFILL_PHONE_SUFFIX_RE),
-              &phone3);
-
-  // Now we have one, two, or three phone number text fields.  Package them
-  // up into a PhoneField object.
-
   scoped_ptr<PhoneField> phone_field(new PhoneField);
-  if (phone2 == NULL) {  // only one field
-    if (area_code)  // it's an area code
-      return NULL;  // doesn't make sense
-    phone_field->phone_ = phone;
-  } else {
-    phone_field->area_code_ = phone;
-    if (phone3 == NULL) {  // two fields
-      phone_field->phone_ = phone2;
-    } else {  // three boxes: area code, prefix and suffix
-      phone_field->prefix_ = phone2;
-      phone_field->phone_ = phone3;
-    }
-  }
 
-  // Now look for an extension.
-  ParseText(&q,
-            l10n_util::GetStringUTF16(IDS_AUTOFILL_PHONE_EXTENSION_RE),
-            &phone_field->extension_);
+  // Go through the phones in order HOME, FAX, attempting to match. HOME should
+  // be the last as it is a catch all case ("fax" and "faxarea" parsed as FAX,
+  // but "area" and "someotherarea" parsed as HOME, for example).
+  for (int i = PHONE_TYPE_MAX - 1; i >= PHONE_TYPE_FIRST; --i) {
+    phone_field->SetPhoneType(static_cast<PhoneField::PHONE_TYPE>(i));
+    if (ParseInternal(phone_field.get(), iter, i == HOME_PHONE))
+      return phone_field.release();
+  }
 
-  *iter = q;
-  return phone_field.release();
+  return NULL;
 }
 
 // static
@@ -108,7 +58,8 @@ bool PhoneField::GetFieldInfo(FieldTypeMap* field_type_map) const {
   bool ok;
 
   if (area_code_ != NULL) {
-    ok = Add(field_type_map, area_code_, AutoFillType(PHONE_HOME_CITY_CODE));
+    ok = Add(field_type_map, area_code_,
+             AutoFillType(number_->GetCityCodeType()));
     DCHECK(ok);
 
     if (prefix_ != NULL) {
@@ -116,20 +67,24 @@ bool PhoneField::GetFieldInfo(FieldTypeMap* field_type_map) const {
       // we fill only the prefix depending on the size of the input field.
       ok = ok && Add(field_type_map,
                      prefix_,
-                     AutoFillType(PHONE_HOME_NUMBER));
+                     AutoFillType(number_->GetNumberType()));
       DCHECK(ok);
       // We tag the suffix as PHONE_HOME_NUMBER, then when filling the form
       // we fill only the suffix depending on the size of the input field.
       ok = ok && Add(field_type_map,
                      phone_,
-                     AutoFillType(PHONE_HOME_NUMBER));
+                     AutoFillType(number_->GetNumberType()));
       DCHECK(ok);
     } else {
-      ok = ok && Add(field_type_map, phone_, AutoFillType(PHONE_HOME_NUMBER));
+      ok = ok && Add(field_type_map,
+                     phone_,
+                     AutoFillType(number_->GetNumberType()));
       DCHECK(ok);
     }
   } else {
-    ok = Add(field_type_map, phone_, AutoFillType(PHONE_HOME_WHOLE_NUMBER));
+    ok = Add(field_type_map,
+             phone_,
+             AutoFillType(number_->GetWholeNumberType()));
     DCHECK(ok);
   }
 
@@ -141,4 +96,136 @@ PhoneField::PhoneField()
       area_code_(NULL),
       prefix_(NULL),
       extension_(NULL) {
+  SetPhoneType(HOME_PHONE);
+}
+
+string16 PhoneField::GetAreaRegex() const {
+  // This one is the same for Home and Fax numbers.
+  return l10n_util::GetStringUTF16(IDS_AUTOFILL_AREA_CODE_RE);
+}
+
+string16 PhoneField::GetPhoneRegex() const {
+  if (phone_type_ == HOME_PHONE)
+    return l10n_util::GetStringUTF16(IDS_AUTOFILL_PHONE_RE);
+  else if (phone_type_ == FAX_PHONE)
+    return l10n_util::GetStringUTF16(IDS_AUTOFILL_FAX_RE);
+  else
+    NOTREACHED();
+  return string16();
+}
+
+string16 PhoneField::GetPrefixRegex() const {
+  // This one is the same for Home and Fax numbers.
+  return l10n_util::GetStringUTF16(IDS_AUTOFILL_PHONE_PREFIX_RE);
+}
+
+string16 PhoneField::GetSuffixRegex() const {
+  // This one is the same for Home and Fax numbers.
+  return l10n_util::GetStringUTF16(IDS_AUTOFILL_PHONE_SUFFIX_RE);
+}
+
+string16 PhoneField::GetExtensionRegex() const {
+  // This one is the same for Home and Fax numbers.
+  return l10n_util::GetStringUTF16(IDS_AUTOFILL_PHONE_EXTENSION_RE);
 }
+
+// static
+bool PhoneField::ParseInternal(
+    PhoneField *phone_field,
+    std::vector<AutoFillField*>::const_iterator* iter,
+    bool regular_phone) {
+  DCHECK(iter);
+
+  DCHECK(phone_field);
+  if (!phone_field)
+    return false;
+
+  std::vector<AutoFillField*>::const_iterator q = *iter;
+  // The form owns the following variables, so they should not be deleted.
+  AutoFillField* phone = NULL;
+  AutoFillField* phone2 = NULL;
+  AutoFillField* phone3 = NULL;
+  bool area_code = false;  // true if we've parsed an area code field.
+
+  // Some pages, such as BloomingdalesShipping.html, have a field labeled
+  // "Area Code and Phone"; we want to parse this as a phone number field so
+  // we look for "phone" before we look for "area code".
+  if (ParseText(&q, phone_field->GetPhoneRegex(), &phone)) {
+    area_code = false;
+    // Check the case when the match is for non-home phone and area code, e.g.
+    // first field is a "Fax area code" and the subsequent is "Fax phone".
+    if (!regular_phone) {
+      // Attempt parsing of the same field as an area code and then phone:
+      std::vector<AutoFillField*>::const_iterator temp_it = *iter;
+      AutoFillField* tmp_phone1 = NULL;
+      AutoFillField* tmp_phone2 = NULL;
+      if (ParseText(&temp_it, phone_field->GetAreaRegex(), &tmp_phone1) &&
+          ParseText(&temp_it, phone_field->GetPhoneRegex(), &tmp_phone2)) {
+        phone = tmp_phone1;
+        phone2 = tmp_phone2;
+        q = temp_it;
+        area_code = true;
+      }
+    }
+  } else {
+    if (!ParseText(&q, phone_field->GetAreaRegex(), &phone))
+      return false;
+    area_code = true;
+    // If this is not a home phone and there was no specification before
+    // the phone number actually starts (e.g. field 1 "Area code:", field 2
+    // "Fax:"), we skip searching for preffix and suffix and bail out.
+    if (!ParseText(&q, phone_field->GetPhoneRegex(), &phone2) && !regular_phone)
+      return false;
+  }
+
+  // Sometimes phone number fields are separated by "-" (e.g. test page
+  // Crate and Barrel Check Out.html).  Also, area codes are sometimes
+  // surrounded by parentheses, so a ")" may appear after the area code field.
+  //
+  // We used to match "tel" here, which we've seen in field names (e.g. on
+  // Newegg2.html), but that's too general: some pages (e.g.
+  // uk/Furniture123-1.html) have several phone numbers in succession and we
+  // don't want those to be parsed as components of a single phone number.
+  if (phone2 == NULL)
+    ParseText(&q, phone_field->GetPrefixRegex(), &phone2);
+
+  // Look for a third text box.
+  if (phone2)
+    ParseText(&q, phone_field->GetSuffixRegex(), &phone3);
+
+  // Now we have one, two, or three phone number text fields.  Package them
+  // up into a PhoneField object.
+
+  if (phone2 == NULL) {  // only one field
+    if (area_code) {
+      // It's an area code - it doesn't make sense.
+      return false;
+    }
+    phone_field->phone_ = phone;
+  } else {
+    phone_field->area_code_ = phone;
+    if (phone3 == NULL) {  // two fields
+      phone_field->phone_ = phone2;
+    } else {  // three boxes: area code, prefix and suffix
+      phone_field->prefix_ = phone2;
+      phone_field->phone_ = phone3;
+    }
+  }
+
+  // Now look for an extension.
+  ParseText(&q, phone_field->GetExtensionRegex(), &phone_field->extension_);
+
+  *iter = q;
+  return true;
+}
+
+void PhoneField::SetPhoneType(PHONE_TYPE phone_type) {
+  // Field types are different as well, so we create a temporary phone number,
+  // to get relevant field types.
+  if (phone_type == HOME_PHONE)
+    number_.reset(new HomePhoneNumber);
+  else
+    number_.reset(new FaxNumber);
+  phone_type_ = phone_type;
+}
+
diff --git a/chrome/browser/autofill/phone_field.h b/chrome/browser/autofill/phone_field.h
index 2642a5c..53b0f02 100644
--- a/chrome/browser/autofill/phone_field.h
+++ b/chrome/browser/autofill/phone_field.h
@@ -8,8 +8,10 @@
 
 #include <vector>
 
+#include "base/scoped_ptr.h"
 #include "chrome/browser/autofill/autofill_type.h"
 #include "chrome/browser/autofill/form_field.h"
+#include "chrome/browser/autofill/phone_number.h"
 
 class AutoFillField;
 
@@ -29,6 +31,39 @@ class PhoneField : public FormField {
  private:
   PhoneField();
 
+  enum PHONE_TYPE {
+    PHONE_TYPE_FIRST = 0,
+    HOME_PHONE = PHONE_TYPE_FIRST,
+    FAX_PHONE,
+
+    // Must be last.
+    PHONE_TYPE_MAX,
+  };
+
+  // Some field names are different for phone and fax.
+  string16 GetAreaRegex() const;
+  string16 GetPhoneRegex() const;
+  string16 GetPrefixRegex() const;
+  string16 GetSuffixRegex() const;
+  string16 GetExtensionRegex() const;
+
+  // |field| - field to fill up on successful parsing.
+  // |iter| - in/out. Form field iterator, points to the first field that is
+  //   attempted to be parsed. If parsing successful, points to the first field
+  //   after parsed fields.
+  // |regular_phone| - true if the parsed phone is a HOME phone, false
+  //   otherwise.
+  static bool ParseInternal(PhoneField* field,
+                            std::vector<AutoFillField*>::const_iterator* iter,
+                            bool regular_phone);
+
+  void SetPhoneType(PHONE_TYPE phone_type);
+
+  // Field types are different as well, so we create a temporary phone number,
+  // to get relevant field types.
+  scoped_ptr<PhoneNumber> number_;
+  PHONE_TYPE phone_type_;
+
   // Always present; holds suffix if prefix is present.
   AutoFillField* phone_;
 
diff --git a/chrome/browser/autofill/phone_field_unittest.cc b/chrome/browser/autofill/phone_field_unittest.cc
index e79d64a..96b02f3 100644
--- a/chrome/browser/autofill/phone_field_unittest.cc
+++ b/chrome/browser/autofill/phone_field_unittest.cc
@@ -263,4 +263,95 @@ TEST_F(PhoneFieldTest, ThreePartPhoneNumberPrefixSuffix) {
   EXPECT_EQ(PHONE_HOME_NUMBER, field_type_map_[ASCIIToUTF16("suffix1")]);
 }
 
+TEST_F(PhoneFieldTest, ParseOneLineFax) {
+  list_.push_back(
+      new AutoFillField(webkit_glue::FormField(ASCIIToUTF16("Fax"),
+                                               ASCIIToUTF16("fax"),
+                                               string16(),
+                                               ASCIIToUTF16("text"),
+                                               0,
+                                               false),
+                        ASCIIToUTF16("fax1")));
+  list_.push_back(NULL);
+  iter_ = list_.begin();
+  field_.reset(PhoneField::Parse(&iter_, false));
+  ASSERT_NE(static_cast<PhoneField*>(NULL), field_.get());
+  ASSERT_TRUE(field_->GetFieldInfo(&field_type_map_));
+  ASSERT_TRUE(
+      field_type_map_.find(ASCIIToUTF16("fax1")) != field_type_map_.end());
+  EXPECT_EQ(PHONE_FAX_WHOLE_NUMBER, field_type_map_[ASCIIToUTF16("fax1")]);
+}
+
+TEST_F(PhoneFieldTest, ParseTwoLineFax) {
+  list_.push_back(
+      new AutoFillField(webkit_glue::FormField(ASCIIToUTF16("Area Code"),
+                                               ASCIIToUTF16("area code"),
+                                               string16(),
+                                               ASCIIToUTF16("text"),
+                                               0,
+                                               false),
+                        ASCIIToUTF16("areacode1")));
+  list_.push_back(
+      new AutoFillField(webkit_glue::FormField(ASCIIToUTF16("Fax"),
+                                               ASCIIToUTF16("fax"),
+                                               string16(),
+                                               ASCIIToUTF16("text"),
+                                               0,
+                                               false),
+                        ASCIIToUTF16("fax1")));
+  list_.push_back(NULL);
+  iter_ = list_.begin();
+  field_.reset(PhoneField::Parse(&iter_, false));
+  ASSERT_NE(static_cast<PhoneField*>(NULL), field_.get());
+  ASSERT_TRUE(field_->GetFieldInfo(&field_type_map_));
+  ASSERT_TRUE(
+      field_type_map_.find(ASCIIToUTF16("areacode1")) != field_type_map_.end());
+  // It should be FAX, based on the other phone in the group.
+  EXPECT_EQ(PHONE_FAX_CITY_CODE, field_type_map_[ASCIIToUTF16("areacode1")]);
+  ASSERT_TRUE(
+      field_type_map_.find(ASCIIToUTF16("fax1")) != field_type_map_.end());
+  EXPECT_EQ(PHONE_FAX_NUMBER, field_type_map_[ASCIIToUTF16("fax1")]);
+}
+
+TEST_F(PhoneFieldTest, ThreePartFaxNumberPrefixSuffix) {
+  list_.push_back(
+      new AutoFillField(webkit_glue::FormField(ASCIIToUTF16("Fax:"),
+                                               ASCIIToUTF16("area"),
+                                               string16(),
+                                               ASCIIToUTF16("text"),
+                                               0,
+                                               false),
+                        ASCIIToUTF16("areacode1")));
+  list_.push_back(
+      new AutoFillField(webkit_glue::FormField(string16(),
+                                               ASCIIToUTF16("prefix"),
+                                               string16(),
+                                               ASCIIToUTF16("text"),
+                                               0,
+                                               false),
+                        ASCIIToUTF16("prefix1")));
+  list_.push_back(
+      new AutoFillField(webkit_glue::FormField(string16(),
+                                               ASCIIToUTF16("suffix"),
+                                               string16(),
+                                               ASCIIToUTF16("text"),
+                                               0,
+                                               false),
+                        ASCIIToUTF16("suffix1")));
+  list_.push_back(NULL);
+  iter_ = list_.begin();
+  field_.reset(PhoneField::Parse(&iter_, false));
+  ASSERT_NE(static_cast<PhoneField*>(NULL), field_.get());
+  ASSERT_TRUE(field_->GetFieldInfo(&field_type_map_));
+  ASSERT_TRUE(
+      field_type_map_.find(ASCIIToUTF16("areacode1")) != field_type_map_.end());
+  EXPECT_EQ(PHONE_FAX_CITY_CODE, field_type_map_[ASCIIToUTF16("areacode1")]);
+  ASSERT_TRUE(
+      field_type_map_.find(ASCIIToUTF16("prefix1")) != field_type_map_.end());
+  EXPECT_EQ(PHONE_FAX_NUMBER, field_type_map_[ASCIIToUTF16("prefix1")]);
+  ASSERT_TRUE(
+      field_type_map_.find(ASCIIToUTF16("suffix1")) != field_type_map_.end());
+  EXPECT_EQ(PHONE_FAX_NUMBER, field_type_map_[ASCIIToUTF16("suffix1")]);
+}
+
 }  // namespace
diff --git a/chrome/browser/autofill/phone_number.h b/chrome/browser/autofill/phone_number.h
index 25648bc..3bb94ca 100644
--- a/chrome/browser/autofill/phone_number.h
+++ b/chrome/browser/autofill/phone_number.h
@@ -45,6 +45,14 @@ class PhoneNumber : public FormGroup {
   static const int kSuffixOffset = 3;
   static const int kSuffixLength = 4;
 
+  // The following functions should return the field type for each part of the
+  // phone number.  Currently, these are either fax or home phone number types.
+  virtual AutoFillFieldType GetNumberType() const = 0;
+  virtual AutoFillFieldType GetCityCodeType() const = 0;
+  virtual AutoFillFieldType GetCountryCodeType() const = 0;
+  virtual AutoFillFieldType GetCityAndNumberType() const = 0;
+  virtual AutoFillFieldType GetWholeNumberType() const = 0;
+
  protected:
   explicit PhoneNumber(const PhoneNumber& phone_number);
 
@@ -84,14 +92,6 @@ class PhoneNumber : public FormGroup {
   bool IsCityAndNumber(const string16& text) const;
   bool IsWholeNumber(const string16& text) const;
 
-  // The following functions should return the field type for each part of the
-  // phone number.  Currently, these are either fax or home phone number types.
-  virtual AutoFillFieldType GetNumberType() const = 0;
-  virtual AutoFillFieldType GetCityCodeType() const = 0;
-  virtual AutoFillFieldType GetCountryCodeType() const = 0;
-  virtual AutoFillFieldType GetCityAndNumberType() const = 0;
-  virtual AutoFillFieldType GetWholeNumberType() const = 0;
-
   // Verifies that |number| is a valid phone number.
   bool Validate(const string16& number) const;
 
diff --git a/chrome/chrome_browser.gypi b/chrome/chrome_browser.gypi
index ea4e102..350cba1 100644
--- a/chrome/chrome_browser.gypi
+++ b/chrome/chrome_browser.gypi
@@ -194,8 +194,6 @@
         'browser/autofill/credit_card.h',
         'browser/autofill/credit_card_field.cc',
         'browser/autofill/credit_card_field.h',
-        'browser/autofill/fax_field.cc',
-        'browser/autofill/fax_field.h',
         'browser/autofill/fax_number.cc',
         'browser/autofill/fax_number.h',
         'browser/autofill/field_types.h',
diff --git a/chrome/chrome_tests.gypi b/chrome/chrome_tests.gypi
index 2b56187..a561870 100644
--- a/chrome/chrome_tests.gypi
+++ b/chrome/chrome_tests.gypi
@@ -1096,7 +1096,6 @@
         'browser/autofill/contact_info_unittest.cc',
         'browser/autofill/credit_card_field_unittest.cc',
         'browser/autofill/credit_card_unittest.cc',
-        'browser/autofill/fax_field_unittest.cc',
         'browser/autofill/form_field_unittest.cc',
         'browser/autofill/form_structure_unittest.cc',
         'browser/autofill/name_field_unittest.cc',