Fix for: Autofill should not ping the server again for the same form

BUG=67039
TEST=unit-tested. Any network sniffer could be used to test that there no subsequent calls to the web for the same form.
Review URL: http://codereview.chromium.org/6366014

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

3 files changed, 286 insertions, 2 deletions

diff --git a/chrome/browser/autofill/autofill_download.cc b/chrome/browser/autofill/autofill_download.cc
index cc7e119..2c9cc4c 100644
--- a/chrome/browser/autofill/autofill_download.cc
+++ b/chrome/browser/autofill/autofill_download.cc
@@ -27,6 +27,10 @@
 #define AUTO_FILL_QUERY_SERVER_NAME_START_IN_HEADER "SOMESERVER/"
 #endif
 
+namespace {
+const size_t kMaxFormCacheSize = 16;
+};
+
 struct AutoFillDownloadManager::FormRequestData {
   std::vector<std::string> form_signatures;
   AutoFillRequestType request_type;
@@ -35,6 +39,7 @@ struct AutoFillDownloadManager::FormRequestData {
 AutoFillDownloadManager::AutoFillDownloadManager(Profile* profile)
     : profile_(profile),
       observer_(NULL),
+      max_form_cache_size_(kMaxFormCacheSize),
       next_query_request_(base::Time::Now()),
       next_upload_request_(base::Time::Now()),
       positive_upload_rate_(0),
@@ -76,12 +81,22 @@ bool AutoFillDownloadManager::StartQueryRequest(
   std::string form_xml;
   FormRequestData request_data;
   if (!FormStructure::EncodeQueryRequest(forms, &request_data.form_signatures,
-                                         &form_xml))
+                                         &form_xml)) {
     return false;
+  }
 
   request_data.request_type = AutoFillDownloadManager::REQUEST_QUERY;
   metric_logger.Log(AutoFillMetrics::QUERY_SENT);
 
+  std::string query_data;
+  if (CheckCacheForQueryRequest(request_data.form_signatures, &query_data)) {
+    VLOG(1) << "AutoFillDownloadManager: query request has been retrieved from"
+            << "the cache";
+    if (observer_)
+      observer_->OnLoadedAutoFillHeuristics(query_data);
+    return true;
+  }
+
   return StartRequest(form_xml, request_data);
 }
 
@@ -188,6 +203,60 @@ bool AutoFillDownloadManager::StartRequest(
   return true;
 }
 
+void AutoFillDownloadManager::CacheQueryRequest(
+    const std::vector<std::string>& forms_in_query,
+    const std::string& query_data) {
+  std::string signature = GetCombinedSignature(forms_in_query);
+  for (QueryRequestCache::iterator it = cached_forms_.begin();
+       it != cached_forms_.end(); ++it) {
+    if (it->first == signature) {
+      // We hit the cache, move to the first position and return.
+      std::pair<std::string, std::string> data = *it;
+      cached_forms_.erase(it);
+      cached_forms_.push_front(data);
+      return;
+    }
+  }
+  std::pair<std::string, std::string> data;
+  data.first = signature;
+  data.second = query_data;
+  cached_forms_.push_front(data);
+  while (cached_forms_.size() > max_form_cache_size_)
+    cached_forms_.pop_back();
+}
+
+bool AutoFillDownloadManager::CheckCacheForQueryRequest(
+    const std::vector<std::string>& forms_in_query,
+    std::string* query_data) const {
+  std::string signature = GetCombinedSignature(forms_in_query);
+  for (QueryRequestCache::const_iterator it = cached_forms_.begin();
+       it != cached_forms_.end(); ++it) {
+    if (it->first == signature) {
+      // We hit the cache, fill the data and return.
+      *query_data = it->second;
+      return true;
+    }
+  }
+  return false;
+}
+
+std::string AutoFillDownloadManager::GetCombinedSignature(
+    const std::vector<std::string>& forms_in_query) const {
+  size_t total_size = forms_in_query.size();
+  for (size_t i = 0; i < forms_in_query.size(); ++i)
+    total_size += forms_in_query[i].length();
+  std::string signature;
+
+  signature.reserve(total_size);
+
+  for (size_t i = 0; i < forms_in_query.size(); ++i) {
+    if (i)
+      signature.append(",");
+    signature.append(forms_in_query[i]);
+  }
+  return signature;
+}
+
 void AutoFillDownloadManager::OnURLFetchComplete(
     const URLFetcher* source,
     const GURL& url,
@@ -250,6 +319,7 @@ void AutoFillDownloadManager::OnURLFetchComplete(
     VLOG(1) << "AutoFillDownloadManager: " << type_of_request
             << " request has succeeded";
     if (it->second.request_type == AutoFillDownloadManager::REQUEST_QUERY) {
+      CacheQueryRequest(it->second.form_signatures, data);
       if (observer_)
         observer_->OnLoadedAutoFillHeuristics(data);
     } else {
diff --git a/chrome/browser/autofill/autofill_download.h b/chrome/browser/autofill/autofill_download.h
index 8937557..157062c 100644
--- a/chrome/browser/autofill/autofill_download.h
+++ b/chrome/browser/autofill/autofill_download.h
@@ -6,8 +6,10 @@
 #define CHROME_BROWSER_AUTOFILL_AUTOFILL_DOWNLOAD_H_
 #pragma once
 
+#include <list>
 #include <map>
 #include <string>
+#include <vector>
 
 #include "base/scoped_vector.h"
 #include "base/time.h"
@@ -96,6 +98,7 @@ class AutoFillDownloadManager : public URLFetcher::Delegate {
   friend class AutoFillDownloadTestHelper;  // unit-test.
 
   struct FormRequestData;
+  typedef std::list<std::pair<std::string, std::string> > QueryRequestCache;
 
   // Initiates request to AutoFill servers to download/upload heuristics.
   // |form_xml| - form structure XML to upload/download.
@@ -106,6 +109,25 @@ class AutoFillDownloadManager : public URLFetcher::Delegate {
   bool StartRequest(const std::string& form_xml,
                     const FormRequestData& request_data);
 
+  // Each request is page visited. We store last |max_form_cache_size|
+  // request, to avoid going over the wire. Set to 16 in constructor. Warning:
+  // the search is linear (newest first), so do not make the constant very big.
+  void set_max_form_cache_size(size_t max_form_cache_size) {
+    max_form_cache_size_ = max_form_cache_size;
+  }
+
+  // Caches query request. |forms_in_query| is a vector of form signatures in
+  // the query. |query_data| is the successful data returned over the wire.
+  void CacheQueryRequest(const std::vector<std::string>& forms_in_query,
+                         const std::string& query_data);
+  // Returns true if query is in the cache, while filling |query_data|, false
+  // otherwise. |forms_in_query| is a vector of form signatures in the query.
+  bool CheckCacheForQueryRequest(const std::vector<std::string>& forms_in_query,
+                                 std::string* query_data) const;
+  // Concatenates |forms_in_query| into one signature.
+  std::string GetCombinedSignature(
+      const std::vector<std::string>& forms_in_query) const;
+
   // URLFetcher::Delegate implementation:
   virtual void OnURLFetchComplete(const URLFetcher* source,
                                   const GURL& url,
@@ -123,6 +145,10 @@ class AutoFillDownloadManager : public URLFetcher::Delegate {
   std::map<URLFetcher*, FormRequestData> url_fetchers_;
   AutoFillDownloadManager::Observer *observer_;
 
+  // Cached QUERY requests.
+  QueryRequestCache cached_forms_;
+  size_t max_form_cache_size_;
+
   // Time when next query/upload requests are allowed. If 50x HTTP received,
   // exponential back off is initiated, so this times will be in the future
   // for awhile.
diff --git a/chrome/browser/autofill/autofill_download_unittest.cc b/chrome/browser/autofill/autofill_download_unittest.cc
index aaf2a5e..b9967d6 100644
--- a/chrome/browser/autofill/autofill_download_unittest.cc
+++ b/chrome/browser/autofill/autofill_download_unittest.cc
@@ -53,6 +53,10 @@ class AutoFillDownloadTestHelper : public AutoFillDownloadManager::Observer {
     download_manager.SetObserver(NULL);
   }
 
+  void LimitCache(size_t cache_size) {
+    download_manager.set_max_form_cache_size(cache_size);
+  }
+
   // AutoFillDownloadManager::Observer overridables:
   virtual void OnLoadedAutoFillHeuristics(
       const std::string& heuristic_xml) {
@@ -61,7 +65,6 @@ class AutoFillDownloadTestHelper : public AutoFillDownloadManager::Observer {
     response.type_of_response = QUERY_SUCCESSFULL;
     responses_.push_back(response);
   };
-
   virtual void OnUploadedAutoFillHeuristics(const std::string& form_signature) {
     ResponseData response;
     response.type_of_response = UPLOAD_SUCCESSFULL;
@@ -276,6 +279,16 @@ TEST(AutoFillDownloadTest, QueryAndUploadTest) {
   fetcher = factory.GetFetcherByID(3);
   EXPECT_EQ(NULL, fetcher);
 
+  // Modify form structures to miss the cache.
+  form.fields.push_back(webkit_glue::FormField(ASCIIToUTF16("Address line 2"),
+                                               ASCIIToUTF16("address2"),
+                                               string16(),
+                                               ASCIIToUTF16("text"),
+                                               0,
+                                               false));
+  form_structure = new FormStructure(form);
+  form_structures.push_back(form_structure);
+
   // Request with id 3.
   EXPECT_CALL(mock_metric_logger, Log(AutoFillMetrics::QUERY_SENT)).Times(1);
   EXPECT_TRUE(helper.download_manager.StartQueryRequest(form_structures,
@@ -329,3 +342,178 @@ TEST(AutoFillDownloadTest, QueryAndUploadTest) {
   // Make sure consumer of URLFetcher does the right thing.
   URLFetcher::set_factory(NULL);
 }
+
+TEST(AutoFillDownloadTest, CacheQueryTest) {
+  MessageLoopForUI message_loop;
+  AutoFillDownloadTestHelper helper;
+  // Create and register factory.
+  TestURLFetcherFactory factory;
+  URLFetcher::set_factory(&factory);
+
+  FormData form;
+  form.method = ASCIIToUTF16("post");
+  form.fields.push_back(webkit_glue::FormField(ASCIIToUTF16("username"),
+                                               ASCIIToUTF16("username"),
+                                               string16(),
+                                               ASCIIToUTF16("text"),
+                                               0,
+                                               false));
+  form.fields.push_back(webkit_glue::FormField(ASCIIToUTF16("First Name"),
+                                               ASCIIToUTF16("firstname"),
+                                               string16(),
+                                               ASCIIToUTF16("text"),
+                                               0,
+                                               false));
+  form.fields.push_back(webkit_glue::FormField(ASCIIToUTF16("Last Name"),
+                                               ASCIIToUTF16("lastname"),
+                                               string16(),
+                                               ASCIIToUTF16("text"),
+                                               0,
+                                               false));
+  FormStructure *form_structure = new FormStructure(form);
+  ScopedVector<FormStructure> form_structures0;
+  form_structures0.push_back(form_structure);
+
+  form.fields.push_back(webkit_glue::FormField(ASCIIToUTF16("email"),
+                                               ASCIIToUTF16("email"),
+                                               string16(),
+                                               ASCIIToUTF16("text"),
+                                               0,
+                                               false));
+  // Slightly different form - so different request.
+  form_structure = new FormStructure(form);
+  ScopedVector<FormStructure> form_structures1;
+  form_structures1.push_back(form_structure);
+
+  form.fields.push_back(webkit_glue::FormField(ASCIIToUTF16("email2"),
+                                               ASCIIToUTF16("email2"),
+                                               string16(),
+                                               ASCIIToUTF16("text"),
+                                               0,
+                                               false));
+  // Slightly different form - so different request.
+  form_structure = new FormStructure(form);
+  ScopedVector<FormStructure> form_structures2;
+  form_structures2.push_back(form_structure);
+
+  // Limit cache to two forms.
+  helper.LimitCache(2);
+
+  const char *responses[] = {
+    "<autofillqueryresponse>"
+      "<field autofilltype=\"0\" />"
+      "<field autofilltype=\"3\" />"
+      "<field autofilltype=\"5\" />"
+    "</autofillqueryresponse>",
+    "<autofillqueryresponse>"
+      "<field autofilltype=\"0\" />"
+      "<field autofilltype=\"3\" />"
+      "<field autofilltype=\"5\" />"
+      "<field autofilltype=\"9\" />"
+    "</autofillqueryresponse>",
+    "<autofillqueryresponse>"
+      "<field autofilltype=\"0\" />"
+      "<field autofilltype=\"3\" />"
+      "<field autofilltype=\"5\" />"
+      "<field autofilltype=\"9\" />"
+      "<field autofilltype=\"0\" />"
+    "</autofillqueryresponse>",
+  };
+
+  // Request with id 0.
+  MockAutoFillMetrics mock_metric_logger;
+  EXPECT_CALL(mock_metric_logger, Log(AutoFillMetrics::QUERY_SENT)).Times(1);
+  EXPECT_TRUE(helper.download_manager.StartQueryRequest(form_structures0,
+                                                        mock_metric_logger));
+  // No responses yet
+  EXPECT_EQ(static_cast<size_t>(0), helper.responses_.size());
+
+  TestURLFetcher* fetcher = factory.GetFetcherByID(0);
+  ASSERT_TRUE(fetcher);
+  fetcher->delegate()->OnURLFetchComplete(fetcher, GURL(),
+                                          net::URLRequestStatus(),
+                                          200, ResponseCookies(),
+                                          std::string(responses[0]));
+  ASSERT_EQ(static_cast<size_t>(1), helper.responses_.size());
+  EXPECT_EQ(responses[0], helper.responses_.front().response);
+
+  helper.responses_.clear();
+
+  // No actual request - should be a cache hit.
+  EXPECT_CALL(mock_metric_logger, Log(AutoFillMetrics::QUERY_SENT)).Times(1);
+  EXPECT_TRUE(helper.download_manager.StartQueryRequest(form_structures0,
+                                                        mock_metric_logger));
+  // Data is available immediately from cache - no over-the-wire trip.
+  ASSERT_EQ(static_cast<size_t>(1), helper.responses_.size());
+  EXPECT_EQ(responses[0], helper.responses_.front().response);
+  helper.responses_.clear();
+
+  // Request with id 1.
+  EXPECT_CALL(mock_metric_logger, Log(AutoFillMetrics::QUERY_SENT)).Times(1);
+  EXPECT_TRUE(helper.download_manager.StartQueryRequest(form_structures1,
+                                                        mock_metric_logger));
+  // No responses yet
+  EXPECT_EQ(static_cast<size_t>(0), helper.responses_.size());
+
+  fetcher = factory.GetFetcherByID(1);
+  ASSERT_TRUE(fetcher);
+  fetcher->delegate()->OnURLFetchComplete(fetcher, GURL(),
+                                          net::URLRequestStatus(),
+                                          200, ResponseCookies(),
+                                          std::string(responses[1]));
+  ASSERT_EQ(static_cast<size_t>(1), helper.responses_.size());
+  EXPECT_EQ(responses[1], helper.responses_.front().response);
+
+  helper.responses_.clear();
+
+  // Request with id 2.
+  EXPECT_CALL(mock_metric_logger, Log(AutoFillMetrics::QUERY_SENT)).Times(1);
+  EXPECT_TRUE(helper.download_manager.StartQueryRequest(form_structures2,
+                                                        mock_metric_logger));
+
+  fetcher = factory.GetFetcherByID(2);
+  ASSERT_TRUE(fetcher);
+  fetcher->delegate()->OnURLFetchComplete(fetcher, GURL(),
+                                          net::URLRequestStatus(),
+                                          200, ResponseCookies(),
+                                          std::string(responses[2]));
+  ASSERT_EQ(static_cast<size_t>(1), helper.responses_.size());
+  EXPECT_EQ(responses[2], helper.responses_.front().response);
+
+  helper.responses_.clear();
+
+  // No actual requests - should be a cache hit.
+  EXPECT_CALL(mock_metric_logger, Log(AutoFillMetrics::QUERY_SENT)).Times(1);
+  EXPECT_TRUE(helper.download_manager.StartQueryRequest(form_structures1,
+                                                        mock_metric_logger));
+
+  EXPECT_CALL(mock_metric_logger, Log(AutoFillMetrics::QUERY_SENT)).Times(1);
+  EXPECT_TRUE(helper.download_manager.StartQueryRequest(form_structures2,
+                                                        mock_metric_logger));
+
+  ASSERT_EQ(static_cast<size_t>(2), helper.responses_.size());
+  EXPECT_EQ(responses[1], helper.responses_.front().response);
+  EXPECT_EQ(responses[2], helper.responses_.back().response);
+  helper.responses_.clear();
+
+  // The first structure should've expired.
+  // Request with id 3.
+  EXPECT_CALL(mock_metric_logger, Log(AutoFillMetrics::QUERY_SENT)).Times(1);
+  EXPECT_TRUE(helper.download_manager.StartQueryRequest(form_structures0,
+                                                        mock_metric_logger));
+  // No responses yet
+  EXPECT_EQ(static_cast<size_t>(0), helper.responses_.size());
+
+  fetcher = factory.GetFetcherByID(3);
+  ASSERT_TRUE(fetcher);
+  fetcher->delegate()->OnURLFetchComplete(fetcher, GURL(),
+                                          net::URLRequestStatus(),
+                                          200, ResponseCookies(),
+                                          std::string(responses[0]));
+  ASSERT_EQ(static_cast<size_t>(1), helper.responses_.size());
+  EXPECT_EQ(responses[0], helper.responses_.front().response);
+
+  // Make sure consumer of URLFetcher does the right thing.
+  URLFetcher::set_factory(NULL);
+}
+