AutoFill: Clean up the server response handling code.

* Move the server response handler to FormStructure.
* Update the |autofill_count_| when we receive server data.

TBR=georgey (vacation)
BUG=none
TEST=none

Review URL: http://codereview.chromium.org/1965002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@46530 0039d316-1c4b-4281-b951-d872f2087c98

7 files changed, 88 insertions, 68 deletions

diff --git a/chrome/browser/autofill/autofill_download.cc b/chrome/browser/autofill/autofill_download.cc
index 8316597..eacf477 100644
--- a/chrome/browser/autofill/autofill_download.cc
+++ b/chrome/browser/autofill/autofill_download.cc
@@ -245,7 +245,7 @@ void AutoFillDownloadManager::OnURLFetchComplete(const URLFetcher* source,
         " request has succeeded";
     if (it->second.request_type == AutoFillDownloadManager::REQUEST_QUERY) {
       if (observer_)
-        observer_->OnLoadedAutoFillHeuristics(it->second.form_signatures, data);
+        observer_->OnLoadedAutoFillHeuristics(data);
     } else {
       double new_positive_upload_rate = 0;
       double new_negative_upload_rate = 0;
diff --git a/chrome/browser/autofill/autofill_download.h b/chrome/browser/autofill/autofill_download.h
index a3511dd..397aae7 100644
--- a/chrome/browser/autofill/autofill_download.h
+++ b/chrome/browser/autofill/autofill_download.h
@@ -31,11 +31,9 @@ class AutoFillDownloadManager : public URLFetcher::Delegate {
   // Notifications are *not* guaranteed to be called.
   class Observer {
    public:
-    // Called when heuristic successfully received from server.
-    // |form_signatures| - the signatures of the requesting forms.
+    // Called when field types are successfully received from the server.
     // |heuristic_xml| - server response.
     virtual void OnLoadedAutoFillHeuristics(
-        const std::vector<std::string>& form_signatures,
         const std::string& heuristic_xml) = 0;
     // Called when heuristic either successfully considered for upload and
     // not send or uploaded.
@@ -45,7 +43,7 @@ class AutoFillDownloadManager : public URLFetcher::Delegate {
     // Called when there was an error during the request.
     // |form_signature| - the signature of the requesting form.
     // |request_type| - type of request that failed.
-    // |http_error| - http error code.
+    // |http_error| - HTTP error code.
     virtual void OnHeuristicsRequestError(const std::string& form_signature,
                                           AutoFillRequestType request_type,
                                           int http_error) = 0;
diff --git a/chrome/browser/autofill/autofill_download_unittest.cc b/chrome/browser/autofill/autofill_download_unittest.cc
index 3738213..549e48b 100644
--- a/chrome/browser/autofill/autofill_download_unittest.cc
+++ b/chrome/browser/autofill/autofill_download_unittest.cc
@@ -17,7 +17,7 @@ using webkit_glue::FormData;
 using WebKit::WebInputElement;
 
 // This tests AutoFillDownloadManager. AutoFillDownloadTestHelper implements
-// AutoFillDownloadManager::Observer and creates instance of
+// AutoFillDownloadManager::Observer and creates an instance of
 // AutoFillDownloadManager. Then it records responses to different initiated
 // requests, which are verified later. To mock network requests
 // TestURLFetcherFactory is used, which creates URLFetchers that do not
@@ -38,14 +38,8 @@ class AutoFillDownloadTestHelper : public AutoFillDownloadManager::Observer {
 
   // AutoFillDownloadManager::Observer overridables:
   virtual void OnLoadedAutoFillHeuristics(
-      const std::vector<std::string>& form_signatures,
       const std::string& heuristic_xml) {
     ResponseData response;
-    for (size_t i = 0; i < form_signatures.size(); ++i) {
-      if (i)
-        response.signature += ",";
-      response.signature += form_signatures[i];
-    }
     response.response = heuristic_xml;
     response.type_of_response = QUERY_SUCCESSFULL;
     responses_.push_back(response);
@@ -53,7 +47,6 @@ class AutoFillDownloadTestHelper : public AutoFillDownloadManager::Observer {
 
   virtual void OnUploadedAutoFillHeuristics(const std::string& form_signature) {
     ResponseData response;
-    response.signature = form_signature;
     response.type_of_response = UPLOAD_SUCCESSFULL;
     responses_.push_back(response);
   }
@@ -208,10 +201,9 @@ TEST(AutoFillDownloadTest, QueryAndUploadTest) {
   EXPECT_EQ(AutoFillDownloadTestHelper::UPLOAD_SUCCESSFULL,
             helper.responses_.front().type_of_response);
   EXPECT_EQ(0, helper.responses_.front().error);
-  EXPECT_EQ(form_structures[0]->FormSignature(),
-            helper.responses_.front().signature);
+  EXPECT_EQ(std::string(), helper.responses_.front().signature);
   // Expected response on non-query request is an empty string.
-  EXPECT_EQ(std::string(""), helper.responses_.front().response);
+  EXPECT_EQ(std::string(), helper.responses_.front().response);
   helper.responses_.pop_front();
 
   EXPECT_EQ(AutoFillDownloadTestHelper::REQUEST_UPLOAD_FAILED,
@@ -220,16 +212,13 @@ TEST(AutoFillDownloadTest, QueryAndUploadTest) {
   EXPECT_EQ(form_structures[1]->FormSignature(),
             helper.responses_.front().signature);
   // Expected response on non-query request is an empty string.
-  EXPECT_EQ(std::string(""), helper.responses_.front().response);
+  EXPECT_EQ(std::string(), helper.responses_.front().response);
   helper.responses_.pop_front();
 
   EXPECT_EQ(helper.responses_.front().type_of_response,
             AutoFillDownloadTestHelper::QUERY_SUCCESSFULL);
   EXPECT_EQ(0, helper.responses_.front().error);
-  std::string signature(form_structures[0]->FormSignature());
-  signature.append(",");
-  signature.append(form_structures[1]->FormSignature());
-  EXPECT_EQ(signature, helper.responses_.front().signature);
+  EXPECT_EQ(std::string(), helper.responses_.front().signature);
   EXPECT_EQ(responses[0], helper.responses_.front().response);
   helper.responses_.pop_front();
 
@@ -260,7 +249,7 @@ TEST(AutoFillDownloadTest, QueryAndUploadTest) {
             helper.responses_.front().type_of_response);
   EXPECT_EQ(500, helper.responses_.front().error);
   // Expected response on non-query request is an empty string.
-  EXPECT_EQ(std::string(""), helper.responses_.front().response);
+  EXPECT_EQ(std::string(), helper.responses_.front().response);
   helper.responses_.pop_front();
 
   // Query requests should be ignored for the next 10 seconds.
diff --git a/chrome/browser/autofill/autofill_manager.cc b/chrome/browser/autofill/autofill_manager.cc
index 4f654b7..f3a2d4f 100644
--- a/chrome/browser/autofill/autofill_manager.cc
+++ b/chrome/browser/autofill/autofill_manager.cc
@@ -9,7 +9,6 @@
 #include "base/command_line.h"
 #include "chrome/browser/autofill/autofill_dialog.h"
 #include "chrome/browser/autofill/autofill_infobar_delegate.h"
-#include "chrome/browser/autofill/autofill_xml_parser.h"
 #include "chrome/browser/autofill/form_structure.h"
 #include "chrome/browser/pref_service.h"
 #include "chrome/browser/profile.h"
@@ -299,49 +298,12 @@ void AutoFillManager::Reset() {
 }
 
 void AutoFillManager::OnLoadedAutoFillHeuristics(
-    const std::vector<std::string>& form_signatures,
     const std::string& heuristic_xml) {
-  // Create a vector of AutoFillFieldTypes,
-  // to assign the parsed field types to.
-  std::vector<AutoFillFieldType> field_types;
-  UploadRequired upload_required = USE_UPLOAD_RATES;
-
-  // Create a parser.
-  AutoFillQueryXmlParser parse_handler(&field_types, &upload_required);
-  buzz::XmlParser parser(&parse_handler);
-  parser.Parse(heuristic_xml.c_str(), heuristic_xml.length(), true);
-  if (!parse_handler.succeeded()) {
-    return;
-  }
-
-  // For multiple forms requested, returned field types are in one array.
-  // |field_shift| indicates start of the fields for current form.
-  size_t field_shift = 0;
-  // form_signatures should mirror form_structures_ unless new request is
-  // initiated. So if there is a discrepancy we just ignore data and return.
-  ScopedVector<FormStructure>::iterator it_forms;
-  std::vector<std::string>::const_iterator it_signatures;
-  for (it_forms = form_structures_.begin(),
-       it_signatures = form_signatures.begin();
-       it_forms != form_structures_.end() &&
-       it_signatures != form_signatures.end() &&
-       (*it_forms)->FormSignature() == *it_signatures;
-       ++it_forms, ++it_signatures) {
-    // In some cases *successful* response does not return all the fields.
-    // Quit the update of the types then.
-    if (field_types.size() - field_shift < (*it_forms)->field_count()) {
-      break;
-    }
-    for (size_t i = 0; i < (*it_forms)->field_count(); ++i) {
-      if (field_types[i + field_shift] != NO_SERVER_DATA &&
-          field_types[i + field_shift] != UNKNOWN_TYPE) {
-        FieldTypeSet types = (*it_forms)->field(i)->possible_types();
-        types.insert(field_types[i + field_shift]);
-        (*it_forms)->set_possible_types(i, types);
-      }
-    }
-    field_shift += (*it_forms)->field_count();
-  }
+  // TODO(jhawkins): Store |upload_required| in the AutoFillManager.
+  UploadRequired upload_required;
+  FormStructure::ParseQueryResponse(heuristic_xml,
+                                    form_structures_.get(),
+                                    &upload_required);
 }
 
 void AutoFillManager::OnUploadedAutoFillHeuristics(
diff --git a/chrome/browser/autofill/autofill_manager.h b/chrome/browser/autofill/autofill_manager.h
index 4eac435..0a7a523 100644
--- a/chrome/browser/autofill/autofill_manager.h
+++ b/chrome/browser/autofill/autofill_manager.h
@@ -68,9 +68,7 @@ class AutoFillManager : public RenderViewHostDelegate::AutoFill,
   virtual void Reset();
 
   // AutoFillDownloadManager::Observer implementation:
-  virtual void OnLoadedAutoFillHeuristics(
-      const std::vector<std::string>& form_signatures,
-      const std::string& heuristic_xml);
+  virtual void OnLoadedAutoFillHeuristics(const std::string& heuristic_xml);
   virtual void OnUploadedAutoFillHeuristics(const std::string& form_signature);
   virtual void OnHeuristicsRequestError(
       const std::string& form_signature,
diff --git a/chrome/browser/autofill/form_structure.cc b/chrome/browser/autofill/form_structure.cc
index c67b95f..35d2820 100644
--- a/chrome/browser/autofill/form_structure.cc
+++ b/chrome/browser/autofill/form_structure.cc
@@ -9,6 +9,7 @@
 #include "base/sha1.h"
 #include "base/string_util.h"
 #include "base/utf_string_conversions.h"
+#include "chrome/browser/autofill/autofill_xml_parser.h"
 #include "chrome/browser/autofill/field_types.h"
 #include "chrome/browser/autofill/form_field.h"
 #include "third_party/libjingle/files/talk/xmllite/xmlelement.h"
@@ -131,6 +132,7 @@ bool FormStructure::EncodeUploadRequest(bool auto_fill_used,
   return true;
 }
 
+// static
 bool FormStructure::EncodeQueryRequest(const ScopedVector<FormStructure>& forms,
                                        std::string* encoded_xml) {
   buzz::XmlElement autofil_request_xml(buzz::QName("autofillquery"));
@@ -160,6 +162,52 @@ bool FormStructure::EncodeQueryRequest(const ScopedVector<FormStructure>& forms,
   return true;
 }
 
+// static
+void FormStructure::ParseQueryResponse(const std::string& response_xml,
+                                       const std::vector<FormStructure*>& forms,
+                                       UploadRequired* upload_required) {
+  // Parse the field types from the server response to the query.
+  std::vector<AutoFillFieldType> field_types;
+  AutoFillQueryXmlParser parse_handler(&field_types, upload_required);
+  buzz::XmlParser parser(&parse_handler);
+  parser.Parse(response_xml.c_str(), response_xml.length(), true);
+  if (!parse_handler.succeeded())
+    return;
+
+  // Copy the field types into the actual form.
+  std::vector<AutoFillFieldType>::iterator current_type = field_types.begin();
+  for (std::vector<FormStructure*>::const_iterator iter = forms.begin();
+       iter != forms.end(); ++iter) {
+    FormStructure* form = *iter;
+    form->has_credit_card_field_ = false;
+    form->has_autofillable_field_ = false;
+
+    for (std::vector<AutoFillField*>::iterator field = form->fields_.begin();
+         field != form->fields_.end(); ++field) {
+      // The field list is terminated by a NULL AutoFillField.
+      if (!*field)
+        break;
+
+      // In some cases *successful* response does not return all the fields.
+      // Quit the update of the types then.
+      if (current_type == field_types.end())
+        break;
+
+      (*field)->set_server_type(*current_type);
+      AutoFillType autofill_type((*field)->type());
+      if (autofill_type.group() == AutoFillType::CREDIT_CARD)
+        form->has_credit_card_field_ = true;
+      if (autofill_type.field_type() != UNKNOWN_TYPE)
+        form->has_autofillable_field_ = true;
+      ++current_type;
+    }
+
+    form->UpdateAutoFillCount();
+  }
+
+  return;
+}
+
 std::string FormStructure::FormSignature() const {
   std::string form_string = target_url_.scheme() +
                             "://" +
@@ -188,6 +236,9 @@ bool FormStructure::IsAutoFillable() const {
 }
 
 bool FormStructure::HasAutoFillableValues() const {
+  if (autofill_count_ == 0)
+    return false;
+
   for (std::vector<AutoFillField*>::const_iterator iter = begin();
        iter != end(); ++iter) {
     AutoFillField* field = *iter;
@@ -197,6 +248,16 @@ bool FormStructure::HasAutoFillableValues() const {
   return false;
 }
 
+void FormStructure::UpdateAutoFillCount() {
+  autofill_count_ = 0;
+  for (std::vector<AutoFillField*>::const_iterator iter = begin();
+       iter != end(); ++iter) {
+    AutoFillField* field = *iter;
+    if (field && field->IsFieldFillable())
+      ++autofill_count_;
+  }
+}
+
 void FormStructure::set_possible_types(int index, const FieldTypeSet& types) {
   int num_fields = static_cast<int>(field_count());
   DCHECK(index >= 0 && index < num_fields);
diff --git a/chrome/browser/autofill/form_structure.h b/chrome/browser/autofill/form_structure.h
index c9a4856..a0740a0 100644
--- a/chrome/browser/autofill/form_structure.h
+++ b/chrome/browser/autofill/form_structure.h
@@ -48,6 +48,12 @@ class FormStructure {
   static bool EncodeQueryRequest(const ScopedVector<FormStructure>& forms,
                                  std::string* encoded_xml);
 
+  // Parses the field types from the server query response. |forms| must be the
+  // same as the one passed to EncodeQueryRequest when constructing the query.
+  static void ParseQueryResponse(const std::string& response_xml,
+                                 const std::vector<FormStructure*>& forms,
+                                 UploadRequired* upload_required);
+
   // The unique signature for this form, composed of the target url domain,
   // the form name, and the form field names in a 64-bit hash.
   std::string FormSignature() const;
@@ -60,6 +66,12 @@ class FormStructure {
   // is not empty.
   bool HasAutoFillableValues() const;
 
+  // Resets |autofill_count_| and counts the number of auto-fillable fields.
+  // This is used when we receive server data for form fields.  At that time,
+  // we may have more known fields than just the number of fields we matched
+  // heuristically.
+  void UpdateAutoFillCount();
+
   // Sets the possible types for the field at |index|.
   void set_possible_types(int index, const FieldTypeSet& types);