Autofill refactor form_field.h/cc.

chrome/browser/autofill/form_field.h

Index: chrome/browser/autofill/form_field.h
diff --git a/chrome/browser/autofill/form_field.h b/chrome/browser/autofill/form_field.h
index 3bb5c0eca47528862efea72e7ff0e44fb250544f..f6db247a4a56b2c893f1ffb293171f5afb7f35f3 100644
--- a/chrome/browser/autofill/form_field.h
+++ b/chrome/browser/autofill/form_field.h
@@ -9,54 +9,13 @@
 #include <vector>
 
 #include "base/basictypes.h"
+#include "base/gtest_prod_util.h"
 #include "base/string16.h"
 #include "chrome/browser/autofill/autofill_type.h"
 
 class AutofillField;
 class AutofillScanner;
 
-extern const char kEcmlShipToTitle[];
-extern const char kEcmlShipToFirstName[];
-extern const char kEcmlShipToMiddleName[];
-extern const char kEcmlShipToLastName[];
-extern const char kEcmlShipToNameSuffix[];
-extern const char kEcmlShipToCompanyName[];
-extern const char kEcmlShipToAddress1[];
-extern const char kEcmlShipToAddress2[];
-extern const char kEcmlShipToAddress3[];
-extern const char kEcmlShipToCity[];
-extern const char kEcmlShipToStateProv[];
-extern const char kEcmlShipToPostalCode[];
-extern const char kEcmlShipToCountry[];
-extern const char kEcmlShipToPhone[];
-extern const char kEcmlShipToEmail[];
-
-// billing name/address fields
-extern const char kEcmlBillToTitle[];
-extern const char kEcmlBillToFirstName[];
-extern const char kEcmlBillToMiddleName[];
-extern const char kEcmlBillToLastName[];
-extern const char kEcmlBillToNameSuffix[];
-extern const char kEcmlBillToCompanyName[];
-extern const char kEcmlBillToAddress1[];
-extern const char kEcmlBillToAddress2[];
-extern const char kEcmlBillToAddress3[];
-extern const char kEcmlBillToCity[];
-extern const char kEcmlBillToStateProv[];
-extern const char kEcmlBillToPostalCode[];
-extern const char kEcmlBillToCountry[];
-extern const char kEcmlBillToPhone[];
-extern const char kEcmlBillToEmail[];
-
-// credit card fields
-extern const char kEcmlCardHolder[];
-extern const char kEcmlCardType[];
-extern const char kEcmlCardNumber[];
-extern const char kEcmlCardVerification[];
-extern const char kEcmlCardExpireDay[];
-extern const char kEcmlCardExpireMonth[];
-extern const char kEcmlCardExpireYear[];
-
 // Represents a logical form field in a web form.  Classes that implement this
 // interface can identify themselves as a particular type of form field, e.g.
 // name, phone number, or address field.
@@ -64,82 +23,60 @@ class FormField {
  public:
   virtual ~FormField() {}
 
-  // Associates each field in |fields| with its heuristically detected type.
-  // The association is stored into |field_type_map|.
+  // Classifies each field in |fields| with its heuristically detected type.
+  // The association is stored into |map|.  Each field has a |unique_name| that

[Ilya Sherman, 2011/05/19 05:44:16]

nit: I don't see "unique_name" used as a variable below, so I would omit the
pipes.

[dhollowa, 2011/05/19 17:53:08]

Done.

+  // is used as the key into the |map|.
   static void ParseFormFields(const std::vector<AutofillField*>& fields,
-                              FieldTypeMap* field_type_map);
-
-  // Associates the available AutofillTypes of a FormField into
-  // |field_type_map|.
-  virtual bool GetFieldInfo(FieldTypeMap* field_type_map) const = 0;
-
-  // Returns true if |field| contains the regexp |pattern| in the name or label.
-  // If |match_label_only| is true, then only the field's label is considered.
-  static bool Match(const AutofillField* field,
-                    const string16& pattern,
-                    bool match_label_only);
-
-  // Parses a field using the different field views we know about.  |is_ecml|
-  // should be true when the field conforms to the ECML specification.
-  static FormField* ParseFormField(AutofillScanner* scanner, bool is_ecml);
-
-  // Attempts to parse a text field with the given pattern; returns true on
-  // success, but doesn't return the actual text field itself.
-  static bool ParseText(AutofillScanner* scanner, const string16& pattern);
-
-  // Attempts to parse a text field with the given pattern.  Returns true on
-  // success and fills |dest| with a pointer to the field.
-  static bool ParseText(AutofillScanner* scanner,
-                        const string16& pattern,
-                        const AutofillField** dest);
-
-  // Attempts to parse a text field with an empty name or label.  Returns true
-  // on success and fills |dest| with a pointer to the field.
-  static bool ParseEmptyText(AutofillScanner* scanner,
-                             const AutofillField** dest);
-
-  // Attempts to parse a text field label with the given pattern.  Returns true
-  // on success and fills |dest| with a pointer to the field.
-  static bool ParseLabelText(AutofillScanner* scanner,
-                             const string16& pattern,
-                             const AutofillField** dest);
-
-  // Attempts to parse a control with an empty label.
-  static bool ParseEmpty(AutofillScanner* scanner);
-
-  // Adds an association between a field and a type to |field_type_map|.
-  static bool Add(FieldTypeMap* field_type_map,
-                  const AutofillField* field,
-                  AutofillFieldType type);
+                              FieldTypeMap* map);
 
  protected:
+  // A bit-field used for matching specific parts of a field in question.
+  enum MatchType {
+    MATCH_LABEL  = 1 << 0,
+    MATCH_NAME   = 1 << 1,
+    MATCH_ALL    = MATCH_LABEL | MATCH_NAME
+  };
+
   // Only derived classes may instantiate.
   FormField() {}
 
-  // Note: ECML compliance checking has been modified to accommodate Google
-  // Checkout field name limitation. All ECML compliant web forms will be
-  // recognized correctly as such however the restrictions on having exactly
-  // ECML compliant names have been loosened to only require that field names
-  // be prefixed with an ECML compliant name in order to accommodate checkout.
-  // Additionally we allow the use of '.' as a word delimiter in addition to the
-  // ECML standard '_' (see FormField::FormField for details).
-  static string16 GetEcmlPattern(const char* ecml_name);
-  static string16 GetEcmlPattern(const char* ecml_name1,
-                                 const char* ecml_name2,
-                                 char pattern_operator);
-
+  // Attempts to parse a form field with the given pattern.  Returns true on
+  // success and fills |match| with a pointer to the field.
+  static bool ParseField(AutofillScanner* scanner,
+                         const string16& pattern,
+                         const AutofillField** match);
+
+  // Parses the stream of fields in |scanner| with regular expression |pattern|
+  // as specified in the |match_type| bit field (see |MatchType|).  If |match|
+  // is non-NULL and the pattern matches, the matched field is returned.

[Ilya Sherman, 2011/05/19 05:44:16]

nit: When does this return true/false?  We should probably mention that in the
comment.

[dhollowa, 2011/05/19 17:53:08]

Done.

+  static bool ParseFieldSpecifics(AutofillScanner* scanner,

[Ilya Sherman, 2011/05/19 05:44:16]

I don't really get much out of the term "Specifics" -- other than that this is
mostly like |ParseField|, but with something to be wary of.  I don't have any
better suggestions right now, but perhaps you had some alternate names in mind?

[dhollowa, 2011/05/19 17:53:08]

I had a few: ParseFieldVariant, ParseFieldWithMatch... But ParseFieldSpecifics
seems most accurate and descriptive to me.

On 2011/05/19 05:44:16, Ilya Sherman wrote:

[Ilya Sherman, 2011/05/19 20:24:09]

Ok.  "Specifics" still doesn't evoke "label and/or name" for me; but as I don't
have any better suggestions, let's keep this for now.

On 2011/05/19 17:53:08, dhollowa wrote:

+                                  const string16& pattern,
+                                  int match_type,
+                                  const AutofillField** match);
+
+  // Attempts to parse a field with an empty label.  Returns true
+  // on success and fills |match| with a pointer to the field.
+  static bool ParseEmptyLabel(AutofillScanner* scanner,
+                              const AutofillField** match);
+
+  // Adds an association between a field and a type to |map|.
+  static bool AddClassification(const AutofillField* field,
+                                AutofillFieldType type,
+                                FieldTypeMap* map);
  private:
-  static bool ParseText(AutofillScanner* scanner,
-                        const string16& pattern,
-                        const AutofillField** dest,
-                        bool match_label_only);
-
-  // For empty strings we need to test that both label and name are empty.
-  static bool ParseLabelAndName(AutofillScanner* scanner,
-                                const string16& pattern,
-                                const AutofillField** dest);
-  static bool MatchName(const AutofillField* field, const string16& pattern);
-  static bool MatchLabel(const AutofillField* field, const string16& pattern);
+  FRIEND_TEST_ALL_PREFIXES(FormFieldTest, Match);
+
+  // Derived classes must implement this interface to supply field type
+  // information.  |ParseFormFields| coordinates the parsing and extraction
+  // of types from an input vector of |AutofillField| objects and delegates
+  // the type extraction via this method.
+  virtual bool ClassifyField(FieldTypeMap* map) const = 0;

[Ilya Sherman, 2011/05/19 05:44:16]

nit: It seems odd to me to give a virtual method |private| visibility, even if
it's pure-virtual.  If subclasses are expected to implement this method, I'd
kinda expect to see it in the |protected| section of the class.

[dhollowa, 2011/05/19 17:53:08]

I was following a principle of least visibility, where the visibility is
determined by the needs of the caller, not the subclass.  And since the only
caller is |FormField| then "private" is most correct.

However, I see your point.  It does seem a little strange from the derivation
point of view.  I've pulled up to "protected".

+
+  // Matches the regular expression |pattern| against the components of |field|
+  // as specified in the |match_type| bit field (see |MatchType|).
+  static bool Match(const AutofillField* field,
+                    const string16& pattern,
+                    int match_type);
 
   DISALLOW_COPY_AND_ASSIGN(FormField);
 };