Download autocheckout whitelist and enable autocheckout for whitelisted sites only.

chrome/browser/autofill/form_structure.h

 // Copyright (c) 2011 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_FORM_STRUCTURE_H_
 #define CHROME_BROWSER_AUTOFILL_FORM_STRUCTURE_H_
 
 #include <string>
 #include <vector>
 
@@ skipping 27 matching lines @@
 }
 
 namespace buzz {
 class XmlElement;
 }
 
 // FormStructure stores a single HTML form together with the values entered
 // 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
   // types.
   void DetermineHeuristicTypes(const AutofillMetrics& metric_logger);
 
   // Encodes the XML upload request from this FormStructure.
   bool EncodeUploadRequest(const FieldTypeSet& available_field_types,
                            bool form_was_autofilled,
                            std::string* encoded_xml) const;
@@ skipping 106 matching lines @@
   // Returns a FormData containing the data this form structure knows about.
   // |user_submitted| is currently always false.
   FormData ToFormData() const;
 
   bool operator==(const FormData& form) const;
   bool operator!=(const FormData& form) const;
 
  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);
 
   enum EncodeRequestType {
     QUERY,
     UPLOAD,
   };
 
   // Adds form info to |encompassing_xml_element|. |request_type| indicates if
   // it is a query or upload.
   bool EncodeFormRequest(EncodeRequestType request_type,
                          buzz::XmlElement* encompassing_xml_element) const;
 
   // Classifies each field in |fields_| into a logical section.
   // Sections are identified by the heuristic that a logical section should not
   // include multiple fields of the same autofill type (with some exceptions, as
   // described in the implementation).  Sections are furthermore distinguished
   // as either credit card or non-credit card sections.
   // If |has_author_specified_sections| is true, only the second pass --
   // 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_;
 
   // The source URL.
   GURL source_url_;
 
   // The target URL.
   GURL target_url_;
 
   // The number of fields able to be auto-filled.
@@ skipping 29 matching lines @@
   // to any autofill flow, it is set to -1.
   int total_pages_;
 
   // Proceed element for multipage Autofill flow.
   scoped_ptr<autofill::WebElementDescriptor> proceed_element_descriptor_;
 
   // Whether the form includes any field types explicitly specified by the site
   // 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);
 };
 
 #endif  // CHROME_BROWSER_AUTOFILL_FORM_STRUCTURE_H_