Propagate correct data to the Toolbar servers

chrome/browser/autofill/autofill_manager.h

 // Copyright (c) 2010 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_AUTOFILL_MANAGER_H_
 #define CHROME_BROWSER_AUTOFILL_AUTOFILL_MANAGER_H_
 #pragma once
 
 #include <list>
 #include <string>
@@ skipping 49 matching lines @@
   virtual void OnLoadedAutoFillHeuristics(const std::string& heuristic_xml);
   virtual void OnUploadedAutoFillHeuristics(const std::string& form_signature);
   virtual void OnHeuristicsRequestError(
       const std::string& form_signature,
       AutoFillDownloadManager::AutoFillRequestType request_type,
       int http_error);
 
   // Returns the value of the AutoFillEnabled pref.
   virtual bool IsAutoFillEnabled() const;
 
-  // Handles the form data submitted by the user.
-  void HandleSubmit();
+  // Handles the form data submitted by the user. |data_present| indicates
+  // bitfield of the fields that we autofilled.
+  void HandleSubmit(std::string const& data_present);

[dhollowa, 2011/01/14 18:40:58]

I'd like to see a bit of refactoring here.  This is a snake-pit.  Here goes:

  1. Rename |HandleSubmit| to be |SaveFormData|
  2. Remove the calls to |UploadFormData| from methods |HandleSubmit| and
|OnInfoBarClosed|
  3. Add calls to |UploadFormData| and |SaveFormData| to |OnFormSubmitted|
  4. Remove the |data_autofilled_| member as we no longer need it.

Currently, the AutoFillManager *pushes* data into the download manager.  I'd
like to see more of the computation done in the download manager and a move to
more of a *pull* model.  I know this is somewhat vague... so I'm working on
concrete expressions in code.

But the spirit of it is this...  AutoFillManager should "notify" the
AutoFillDownloadManager that it is time to upload.  The AutoFillDownloadManager
should, in turn, query any necessary information from the AFM and the PDM and
process it into the XML it needs.

The AFM, PDM, and FormStructure classes should not have to know about
"datapresent" concepts, or how to encode upload/download requests.  This should
all happen in the AutofillDownloadManager based on information provided by the
AFM, PDM, and FormStructure classes.

[GeorgeY, 2011/01/18 21:04:56]

Done.

[dhollowa, 2011/01/18 21:48:09]

Almost. #4 didn't get done.

Here's a patch of what I had in mind.  http://codereview.chromium.org/6286003

On 2011/01/18 21:04:56, GeorgeY wrote:

 
   // Uploads the form data to the AutoFill server.
-  void UploadFormData();
+  void UploadFormData(std::string const& data_present);
 
   // Reset cache.
   void Reset();
 
+  // Set |field_type| presence bit in the |bitfield|.
+  void SetPresenceBit(AutoFillFieldType field_type,
+                      std::vector<uint8>* bitfield);
+  // Converts |bitfield| to string for uploading.
+  std::string ConvertPresenceBitsToString(std::vector<uint8> const& bitfield);
+
  protected:
   // For tests.
   AutoFillManager();
   AutoFillManager(TabContents* tab_contents,
                   PersonalDataManager* personal_data);
 
   void set_personal_data_manager(PersonalDataManager* personal_data) {
     personal_data_ = personal_data;
   }
 
@@ skipping 77 matching lines @@
 
   // Set |field| argument's value for phone number based on contents of the
   // |profile|.
   void FillPhoneNumberField(const AutoFillProfile* profile,
                             webkit_glue::FormField* field);
 
   // Parses the forms using heuristic matching and querying the AutoFill server.
   void ParseForms(const std::vector<webkit_glue::FormData>& forms);
 
   // Uses existing personal data to determine possible field types for the
-  // |upload_form_structure_|.
-  void DeterminePossibleFieldTypesForUpload(
+  // |upload_form_structure_|. Returns string representation of the bitfield of
+  // all of the fields types that we will send information for the form.
+  std::string DeterminePossibleFieldTypesForUpload(
       const FormStructure* cached_upload_form_structure);
 
   // The TabContents hosting this AutoFillManager.
   // Weak reference.
   // May not be NULL.
   TabContents* tab_contents_;
 
   // The personal data manager, used to save and load personal data to/from the
   // web database.  This is overridden by the AutoFillManagerTest.
   // Weak reference.
@@ skipping 16 matching lines @@
   // Our copy of the form data.
   ScopedVector<FormStructure> form_structures_;
 
   // The form data the user has submitted.
   scoped_ptr<FormStructure> upload_form_structure_;
 
   // The InfoBar that asks for permission to store credit card information.
   // Deletes itself when closed.
   AutoFillCCInfoBarDelegate* cc_infobar_;
 
+  // Bitfield that indicates which fields were autofilled for last Credit Card.
+  std::string data_autofilled_;
+
   // GUID to ID mapping.  We keep two maps to convert back and forth.
   std::map<std::string, int> guid_id_map_;
   std::map<int, std::string> id_guid_map_;
 
   friend class AutoFillManagerTest;
   friend class FormStructureBrowserTest;
   FRIEND_TEST_ALL_PREFIXES(AutoFillManagerTest, FillCreditCardForm);
   FRIEND_TEST_ALL_PREFIXES(AutoFillManagerTest, FillAddressForm);
   FRIEND_TEST_ALL_PREFIXES(AutoFillManagerTest, FillAddressAndCreditCardForm);
   FRIEND_TEST_ALL_PREFIXES(AutoFillManagerTest, FillAutoFilledForm);
   FRIEND_TEST_ALL_PREFIXES(AutoFillManagerTest, FillPhoneNumber);
   FRIEND_TEST_ALL_PREFIXES(AutoFillManagerTest, FormChangesRemoveField);
   FRIEND_TEST_ALL_PREFIXES(AutoFillManagerTest, FormChangesAddField);
   FRIEND_TEST_ALL_PREFIXES(AutoFillMetricsTest, QualityMetrics);
   FRIEND_TEST_ALL_PREFIXES(AutoFillMetricsTest,
                            NoQualityMetricsForNonAutoFillableForms);
   FRIEND_TEST_ALL_PREFIXES(AutoFillMetricsTest, QualityMetricsForFailure);
 
   DISALLOW_COPY_AND_ASSIGN(AutoFillManager);
 };
 
 #endif  // CHROME_BROWSER_AUTOFILL_AUTOFILL_MANAGER_H_