Download autocheckout whitelist and enable autocheckout for whitelisted sites only.

chrome/browser/autofill/form_structure.h

Index: chrome/browser/autofill/form_structure.h
diff --git a/chrome/browser/autofill/form_structure.h b/chrome/browser/autofill/form_structure.h
index 72485af67cde2e0f9eeafa9d54975952f945c312..b4f3194d20189e3522d28ab1c1ebadb209ce6b7a 100644
--- a/chrome/browser/autofill/form_structure.h
+++ b/chrome/browser/autofill/form_structure.h
@@ -45,7 +45,7 @@ class XmlElement;
 // in the fields along with additional information needed by Autofill.
 class FormStructure {
  public:
-  explicit FormStructure(const FormData& form);
+  FormStructure(const FormData& form, bool autocheckout_enabled);

[Ilya Sherman, 2013/01/28 23:21:39]

nit: Rather than passing in a boolean, which is hard to understand semantically
at the call-site, please define an enumerated type with two constants,
AUTOCHECKOUT_ENABLED and AUTOCHECKOUT_DISABLED.  My apologies for not mentioning
that earlier, prior to you adding the false param to all of the FormData
constructor call sites.

[benquan, 2013/01/29 03:28:30]

It will not be consistent with other methods in this file which take bool as
parameters.

[Ilya Sherman, 2013/01/29 05:56:35]

That's ok.  It's worthwhile to move code in that direction, and definitely to
implement new code this way, for the readability benefits,

[benquan, 2013/01/30 00:48:42]

Is it a new coding style guide requires to replace bool with specific enums or
something? 

Personally I don't think this won't gain much on readability. There is also a
class memeber named autocheckout_enabled_, how could we name it properly? Also,
we can not check the field like this:

if (autocheckout_enabled_) {
  // do something
}

instead we have to use this form:
if (autocheckout_enabled_ == AUTOCHECKOUT_ENABLED) {
  // do someting
}

Overall I think a simple bool fit this usecase naturally and much easier.

[Ilya Sherman, 2013/01/30 00:52:48]

Specifically, booleans as parameters to functions, where the meaning of the
boolean is not clear at the callsite.
You can keep the boolean member variable, and initialize it as
"""autocheckout_enabled_(autocheckout_enabled == AUTOCHECKOUT_ENABLED)""" in the
initializer list.

[benquan, 2013/01/30 18:39:00]

Bascially we have the same issue with all constants not just for booleans. For
example, foo(2), the meaning of number 1 is not clear at the caller site either.
I guess you have read item 40 in http://go/effectivejava, "Prefer two-element
enum types to boolean parameters". It makes sense for a lot of things when you
may need a third value. The example the author gave was: use TemperatureScale
enum:
public enum TemperatureScale { FAHRENHEIT, CELSIUS }
rather than :
bool is_fahrenheit;
The reason is that you may want to include KELVIN as the third scale.

But if you use it for a bi-state flag, you breaks the attitude of a boolean that
is nothing more than representing a bi-state of true and false and converse with
each other. What does !AUTOCHECKOUT_ENABLED mean? It should be equivalent to 
AUTOCHECKOUT_DISABLED, but it is illegal in syntax.

I think it is not ideal to convert autocheckout_enabled_ to an enum, but if you
insist that this is the direction we want to take, I can make the code change.

[Ilya Sherman, 2013/01/30 21:05:31]

I do insist that if you keep this as a parameter to the constructor, the
parameter should be an enumerated constant rather than a boolean.

[benquan, 2013/01/31 03:21:23]

Replaced autochout_enabled with autocheckout_url_prefix, which is the matched
url prefix in the whitelist. The hashcode of the prefix wil be passed down to
autofillserver in the other CL: https://codereview.chromium.org/11953100/

   virtual ~FormStructure();
 
   // Runs several heuristics against the form fields to determine their possible
@@ -172,6 +172,7 @@ class FormStructure {
  private:
   friend class FormStructureTest;
   FRIEND_TEST_ALL_PREFIXES(AutofillDownloadTest, QueryAndUploadTest);
+
   // 64-bit hash of the string - used in FormSignature and unit-tests.
   static std::string Hash64Bit(const std::string& str);
 
@@ -194,6 +195,9 @@ class FormStructure {
   // distinguishing credit card sections from non-credit card ones -- is made.
   void IdentifySections(bool has_author_specified_sections);
 
+  // Returns the minimal number of fillable fields required to start autofill.
+  size_t RequiredFillableFields() const;
+
   // The name of the form.
   string16 form_name_;
 
@@ -243,8 +247,9 @@ class FormStructure {
   // author, via the |autocompletetype| attribute.
   bool has_author_specified_types_;
 
-  // State of the kEnableExperimentalFormFilling flag.
-  bool experimental_form_filling_enabled_;
+  // Whether the autocheckout feature is enabled for the site which contains
+  // this form.
+  bool autocheckout_enabled_;
 
   DISALLOW_COPY_AND_ASSIGN(FormStructure);
 };