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

chrome/browser/autofill/autofill_download.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_AUTOFILL_DOWNLOAD_H_
 #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"
 #include "chrome/browser/autofill/autofill_profile.h"
 #include "chrome/browser/autofill/field_types.h"
 #include "chrome/browser/autofill/form_structure.h"
 #include "chrome/common/net/url_fetcher.h"
 
 class AutoFillMetrics;
 class Profile;
@@ skipping 78 matching lines @@
 
   // Initiates request to AutoFill servers to download/upload heuristics.
   // |form_xml| - form structure XML to upload/download.
   // |request_data| - form signature hash(es) and indicator if it was a query.
   // |request_data.query| - if true the data is queried and observer notified
   //   with new data, if available. If false heuristic data is uploaded to our
   //   servers.
   bool StartRequest(const std::string& form_xml,
                     const FormRequestData& request_data);
 
+  // Each request is page visited. We store last |number_of_forms_to_cache|
+  // 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_number_of_forms_to_cache(size_t number_of_forms_to_cache) {
+    number_of_forms_to_cache_ = number_of_forms_to_cache;
+  }
+
+  // Caches query request. |forms_in_query| is a vector of form signatures in
+  // the query. |query_data| is the successfull data returned over the wire.
+  void CacheQueryRequest(std::vector<std::string> const& forms_in_query,

[dhollowa, 2011/01/25 23:33:02]

|const| in front of argument type please.  And elsewhere.

[GeorgeY, 2011/01/25 23:51:30]

Done.

+                         std::string const& 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(std::vector<std::string> const& forms_in_query,
+                                 std::string* query_data) const;
+  // Concatenates |forms_in_query| into one signature.
+  std::string GetCombinedSignature(
+      std::vector<std::string> const& forms_in_query) const;
+
   // URLFetcher::Delegate implementation:
   virtual void OnURLFetchComplete(const URLFetcher* source,
                                   const GURL& url,
                                   const net::URLRequestStatus& status,
                                   int response_code,
                                   const ResponseCookies& cookies,
                                   const std::string& data);
 
   // Profile for preference storage.
   Profile* profile_;
 
   // For each requested form for both query and upload we create a separate
   // request and save its info. As url fetcher is identified by its address
   // we use a map between fetchers and info.
   std::map<URLFetcher*, FormRequestData> url_fetchers_;
   AutoFillDownloadManager::Observer *observer_;
 
+  // Cached QUERY requests.
+  std::list<std::pair<std::string, std::string> > cached_forms_;

[dhollowa, 2011/01/25 23:33:02]

Add a typedef for the cache type.  It'll make iterators more readable /
manageable.

[GeorgeY, 2011/01/25 23:51:30]

Sure, but as this used only in these two functions, explicit description
actually adds to readability.

+  size_t number_of_forms_to_cache_;

[dhollowa, 2011/01/25 23:33:02]

This seems a bit verbose.  How about |form_cache_size_|.  Similarly with
getter/setter.

[GeorgeY, 2011/01/25 23:51:30]

Done.

+
   // 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.
   base::Time next_query_request_;
   base::Time next_upload_request_;
 
   // |positive_upload_rate_| is for matched forms,
   // |negative_upload_rate_| for non matched.
   double positive_upload_rate_;
   double negative_upload_rate_;
 
   // Needed for unit-test.
   int fetcher_id_for_unittest_;
   bool is_testing_;
 };
 
 #endif  // CHROME_BROWSER_AUTOFILL_AUTOFILL_DOWNLOAD_H_