Changed parsing code for the phonenumbers fields to incorporate different com...

chrome/browser/autofill/phone_field.h

Index: chrome/browser/autofill/phone_field.h
===================================================================
--- chrome/browser/autofill/phone_field.h	(revision 74701)
+++ chrome/browser/autofill/phone_field.h	(working copy)
@@ -41,12 +41,39 @@
   };
 
   // Some field names are different for phone and fax.
+  string16 GetCountryRegex() const;
+  // This string includes all area code separators, including NoText.
   string16 GetAreaRegex() const;
+  // Separator of the area code in the case fields are formatted without
+  // any text indicating what fields are (e.g. field1 "(" field2 ")" field3 "-"
+  // field4 means Country Code, Area Code, Preffix, Suffix)

[Ilya Sherman, 2011/02/16 09:33:22]

nit: "Prefix" (one 'f')

[GeorgeY, 2011/02/16 20:53:34]

Done.

+  string16 GetAreaNoTextRegex() const;
   string16 GetPhoneRegex() const;
+  string16 GetPrefixSeparatorRegex() const;
   string16 GetPrefixRegex() const;
+  string16 GetSuffixSeparatorRegex() const;
   string16 GetSuffixRegex() const;
   string16 GetExtensionRegex() const;
 
+  // This is for easy description of the possible parsing paths of the phone
+  // fields.
+  enum REGEX_MATCH {

[dhollowa, 2011/02/16 00:25:37]

Types should be mixed case, not all-caps.  So: s/REGEX_MATCH/RegexMatch/ or
maybe "RegexType"?

[GeorgeY, 2011/02/16 20:53:34]

Done. (RegexType)

+    REGEX_COUNTRY,
+    REGEX_AREA,
+    REGEX_AREA_NOTEXT,
+    REGEX_PHONE,
+    REGEX_PREFIX_SEPARATOR,
+    REGEX_PREFIX,
+    REGEX_SUFFIX_SEPARATOR,
+    REGEX_SUFFIX,
+    REGEX_EXTENSION,
+
+    // Separates regexps in grammar.
+    REGEX_SEPARATOR,
+  };
+
+  string16 GetRegExp(REGEX_MATCH regex_id) 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
@@ -64,13 +91,28 @@
   scoped_ptr<PhoneNumber> number_;
   PHONE_TYPE phone_type_;
 
-  // Always present; holds suffix if prefix is present.
-  AutoFillField* phone_;
 
-  AutoFillField* area_code_;  // optional
-  AutoFillField* prefix_;     // optional
-  AutoFillField* extension_;  // optional
+  // Parsed fields.
+  enum {

[dhollowa, 2011/02/16 00:25:37]

Let's add a type to this enum, "PhonePart" say (?), and then use that this type
definition on line 112 to type the |field_id|.  And maybe change that from
|field_id| to |phone_part|.

[GeorgeY, 2011/02/16 20:53:34]

Done.

+    FIELD_COUNTRY_CODE,

[dhollowa, 2011/02/16 00:25:37]

I notice the |phone_field_grammars_| member is initialized with 0 for the
PhoneField::REGEX_SEPARATOR.  It would be preferable to have an explicit
FIELD_NULL = -1, at the start to avoid value overloading of 0 ==
FIELD_COUNTRY_CODE.

[GeorgeY, 2011/02/16 20:53:34]

Done.

+    FIELD_AREA_CODE,
+    FIELD_PHONE,
+    FIELD_SUFFIX,
+    FIELD_EXTENSION,
 
+    FIELD_MAX,
+  };
+
+  // FIELD_PHONE is always present; holds suffix if prefix is present.
+  // The rest could be NULL.
+  AutoFillField* parsed_phone_fields_[FIELD_MAX];
+
+  static struct Parser {
+    REGEX_MATCH regex;  // Field matching reg-ex.
+    size_t field_id;    // Index of the field.

[dhollowa, 2011/02/16 00:25:37]

As mentioned above, size_t -> PhonePart, and field_id -> phone_part.

[GeorgeY, 2011/02/16 20:53:34]

Done.

+    int max_size;       // Max size of the field to match. 0 means any.
+  } phone_field_grammars_[];
+
   DISALLOW_COPY_AND_ASSIGN(PhoneField);
 };