[autofill] Fill in values on a successful run of interactive autocomplete.

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 127 matching lines @@
   std::vector<AutofillField*>::const_iterator end() const {
     return fields_.end();
   }
 
   const GURL& source_url() const { return source_url_; }
 
   UploadRequired upload_required() const { return upload_required_; }
 
   virtual std::string server_experiment_id() const;
 
+  // Puts the data this form structure knows about into |out_data|.
+  // |out_data->user_initiated| will always be false.

[Ilya Sherman, 2012/11/29 00:08:59]

nit: "user_initiated" -> "user_submitted"

+  void ToFormData(FormData* out_data) const;

[Evan Stade, 2012/11/28 22:45:51]

return FormData*

[Ilya Sherman, 2012/11/29 00:08:59]

Conceptually this function signature should be "FormData ToFormData() const". 
However, for efficiency's sake, Chrome code usually instead uses out-params,
i.e. "void ToFormData(FormData* result) const".

Returning an un-owned heap allocated pointer is definitely wrong, so the
signature "FormData* ToFormData() const" doesn't make sense.  The signature
"scoped_ptr<FormData> ToFormData()" makes sense, but requires an extra heap
allocation, which is otherwise unnecessary.  That's why I recommended using the
current method signature.

[Evan Stade, 2012/11/29 00:20:59]

yes, FormGroup is correct rather than FormGroup*. I don't think efficiency plays
a role here because the size of the structure is quite small. Do you have any
evidence that Chrome usually uses out params? I think out params are easier to
misuse and harder to read. They are acceptable under certain circumstances, for
example if failure is possible, if multiple values need to be output, etc. None
of those apply here though.

[Ilya Sherman, 2012/11/29 00:26:43]

Fair enough -- we're about to make another copy of this data when we send it
over IPC anyway, so a copy here isn't too bad.  I was under the impression that
Chrome code usually uses out-params based on this bit of the Chromium style
guide [1]: "After testing, it seems just as fast to return a std::[w]string by
value as by out param. So we encourage passing strings out as return values when
possible for the sake of readability."  To me that suggested that things bigger
than strings should normally be returned by out param.

[1] http://dev.chromium.org/developers/coding-style#Method_Signatures

+
   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 {
@@ skipping 47 matching lines @@
   RequestMethod method_;
 
   // Whether the form includes any field types explicitly specified by the site
   // author, via the |autocompletetype| attribute.
   bool has_author_specified_types_;
 
   DISALLOW_COPY_AND_ASSIGN(FormStructure);
 };
 
 #endif  // CHROME_BROWSER_AUTOFILL_FORM_STRUCTURE_H_