Net module changes to support caching responses to a POST request.

The solution is to add a user-defined identifier to UploadData.  If that identifier is set, and if the request method is POST, then HttpCache will enable caching for the response.  (The cache key will be a composition of the identifier and the URL.)  A subsequent POST request to the same URL with the same identifier will "hit" the previously generated cache entry.  Reuse from the cache is subject to all of the standard rules.

BUG=2636
R=wtc

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

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

5 files changed, 120 insertions, 48 deletions

diff --git a/net/base/upload_data.h b/net/base/upload_data.h
index 3174b77..231b40b 100644
--- a/net/base/upload_data.h
+++ b/net/base/upload_data.h
@@ -14,7 +14,7 @@ namespace net {
 
 class UploadData : public base::RefCounted<UploadData> {
  public:
-  UploadData() {}
+  UploadData() : identifier_(0) {}
 
   enum Type {
     TYPE_BYTES,
@@ -91,8 +91,19 @@ class UploadData : public base::RefCounted<UploadData> {
     elements_ = elements;
   }
 
+  void swap_elements(std::vector<Element>* elements) {
+    elements_.swap(*elements);
+  }
+
+  // Identifies a particular upload instance, which is used by the cache to
+  // formulate a cache key.  This value should be unique across browser
+  // sessions.  A value of 0 is used to indicate an unspecified identifier.
+  void set_identifier(int64 id) { identifier_ = id; }
+  int64 identifier() const { return identifier_; }
+
  private:
-   std::vector<Element> elements_;
+  std::vector<Element> elements_;
+  int64 identifier_;
 };
 
 }  // namespace net
diff --git a/net/http/http_cache.cc b/net/http/http_cache.cc
index 8637f2d..be1347c 100644
--- a/net/http/http_cache.cc
+++ b/net/http/http_cache.cc
@@ -118,38 +118,6 @@ static bool HeaderMatches(const HttpUtil::HeadersIterator& h,
 
 //-----------------------------------------------------------------------------
 
-std::string HttpCache::GenerateCacheKey(const HttpRequestInfo* request) {
-  std::string url = request->url.spec();
-  if (request->url.has_ref())
-    url.erase(url.find_last_of('#'));
-
-  if (mode_ == NORMAL) {
-    return url;
-  }
-
-  // In playback and record mode, we cache everything.
-
-  // Lazily initialize.
-  if (playback_cache_map_ == NULL)
-    playback_cache_map_.reset(new PlaybackCacheMap());
-
-  // Each time we request an item from the cache, we tag it with a
-  // generation number.  During playback, multiple fetches for the same
-  // item will use the same generation number and pull the proper
-  // instance of an URL from the cache.
-  int generation = 0;
-  DCHECK(playback_cache_map_ != NULL);
-  if (playback_cache_map_->find(url) != playback_cache_map_->end())
-    generation = (*playback_cache_map_)[url];
-  (*playback_cache_map_)[url] = generation + 1;
-
-  // The key into the cache is GENERATION # + METHOD + URL.
-  std::string result = IntToString(generation);
-  result.append(request->method);
-  result.append(url);
-  return result;
-}
-
 //-----------------------------------------------------------------------------
 
 HttpCache::ActiveEntry::ActiveEntry(disk_cache::Entry* e)
@@ -667,11 +635,15 @@ bool HttpCache::Transaction::ShouldPassThrough() {
   if (effective_load_flags_ & LOAD_DISABLE_CACHE)
     return true;
 
-  // TODO(darin): add support for caching HEAD and POST responses
-  if (request_->method != "GET")
-    return true;
+  if (request_->method == "GET")
+    return false;
 
-  return false;
+  if (request_->method == "POST" &&
+      request_->upload_data && request_->upload_data->identifier())
+    return false;
+
+  // TODO(darin): add support for caching HEAD responses
+  return true;
 }
 
 int HttpCache::Transaction::BeginCacheRead() {
@@ -1207,6 +1179,43 @@ bool HttpCache::WriteResponseInfo(disk_cache::Entry* disk_entry,
                                true) == len;
 }
 
+// Generate a key that can be used inside the cache.
+std::string HttpCache::GenerateCacheKey(const HttpRequestInfo* request) {
+  std::string url = request->url.spec();
+  if (request->url.has_ref())
+    url.erase(url.find_last_of('#'));
+
+  if (mode_ == NORMAL) {
+    // No valid URL can begin with numerals, so we should not have to worry
+    // about collisions with normal URLs.
+    if (request->upload_data && request->upload_data->identifier())
+      url.insert(0, StringPrintf("%lld/", request->upload_data->identifier()));
+    return url;
+  }
+
+  // In playback and record mode, we cache everything.
+
+  // Lazily initialize.
+  if (playback_cache_map_ == NULL)
+    playback_cache_map_.reset(new PlaybackCacheMap());
+
+  // Each time we request an item from the cache, we tag it with a
+  // generation number.  During playback, multiple fetches for the same
+  // item will use the same generation number and pull the proper
+  // instance of an URL from the cache.
+  int generation = 0;
+  DCHECK(playback_cache_map_ != NULL);
+  if (playback_cache_map_->find(url) != playback_cache_map_->end())
+    generation = (*playback_cache_map_)[url];
+  (*playback_cache_map_)[url] = generation + 1;
+
+  // The key into the cache is GENERATION # + METHOD + URL.
+  std::string result = IntToString(generation);
+  result.append(request->method);
+  result.append(url);
+  return result;
+}
+
 void HttpCache::DoomEntry(const std::string& key) {
   // Need to abandon the ActiveEntry, but any transaction attached to the entry
   // should not be impacted.  Dooming an entry only means that it will no
diff --git a/net/http/http_cache.h b/net/http/http_cache.h
index 297c8d5..eb47b44 100644
--- a/net/http/http_cache.h
+++ b/net/http/http_cache.h
@@ -106,9 +106,6 @@ class HttpCache : public HttpTransactionFactory {
                                 const HttpResponseInfo* response_info,
                                 bool skip_transient_headers);
 
-  // Generate a key that can be used inside the cache.
-  std::string GenerateCacheKey(const HttpRequestInfo* request);
-
   // Get/Set the cache's mode.
   void set_mode(Mode value) { mode_ = value; }
   Mode mode() { return mode_; }
@@ -143,6 +140,7 @@ class HttpCache : public HttpTransactionFactory {
 
   // Methods ------------------------------------------------------------------
 
+  std::string GenerateCacheKey(const HttpRequestInfo*);
   void DoomEntry(const std::string& key);
   void FinalizeDoomedEntry(ActiveEntry* entry);
   ActiveEntry* FindActiveEntry(const std::string& key);
diff --git a/net/http/http_cache_unittest.cc b/net/http/http_cache_unittest.cc
index 61c2908..2d343f8 100644
--- a/net/http/http_cache_unittest.cc
+++ b/net/http/http_cache_unittest.cc
@@ -33,7 +33,23 @@ class MockDiskEntry : public disk_cache::Entry,
 
   MockDiskEntry(const std::string& key)
       : key_(key), doomed_(false), platform_file_(global_platform_file_) {
-    const MockTransaction* t = FindMockTransaction(GURL(key));
+    //
+    // 'key' is prefixed with an identifier if it corresponds to a cached POST.
+    // Skip past that to locate the actual URL.
+    //
+    // TODO(darin): It breaks the abstraction a bit that we assume 'key' is an
+    // URL corresponding to a registered MockTransaction.  It would be good to
+    // have another way to access the test_mode.
+    //
+    GURL url;
+    if (isdigit(key[0])) {
+      size_t slash = key.find('/');
+      DCHECK(slash != std::string::npos);
+      url = GURL(key.substr(slash + 1));
+    } else {
+      url = GURL(key);
+    }
+    const MockTransaction* t = FindMockTransaction(url);
     DCHECK(t);
     test_mode_ = t->test_mode;
   }
@@ -274,9 +290,9 @@ void ReadAndVerifyTransaction(net::HttpTransaction* trans,
   EXPECT_EQ(0, memcmp(trans_info.data, content.data(), content.size()));
 }
 
-void RunTransactionTest(net::HttpCache* cache,
-                        const MockTransaction& trans_info) {
-  MockHttpRequest request(trans_info);
+void RunTransactionTestWithRequest(net::HttpCache* cache,
+                                   const MockTransaction& trans_info,
+                                   const MockHttpRequest& request) {
   TestCompletionCallback callback;
 
   // write to the cache
@@ -295,6 +311,12 @@ void RunTransactionTest(net::HttpCache* cache,
   ReadAndVerifyTransaction(trans.get(), trans_info);
 }
 
+void RunTransactionTest(net::HttpCache* cache,
+                        const MockTransaction& trans_info) {
+  return RunTransactionTestWithRequest(
+      cache, trans_info, MockHttpRequest(trans_info));
+}
+
 // This class provides a handler for kFastNoStoreGET_Transaction so that the
 // no-store header can be included on demand.
 class FastTransactionServer {
@@ -897,9 +919,8 @@ TEST(HttpCache, ETagGET_ConditionalRequest_304_NoStore) {
 TEST(HttpCache, SimplePOST_SkipsCache) {
   MockHttpCache cache;
 
-  // Test that we skip the cache for POST requests.  Eventually, we will want
-  // to cache these, but we'll still have cases where skipping the cache makes
-  // sense, so we want to make sure that it works properly.
+  // Test that we skip the cache for POST requests that do not have an upload
+  // identifier.
 
   RunTransactionTest(cache.http_cache(), kSimplePOST_Transaction);
 
@@ -937,6 +958,36 @@ TEST(HttpCache, SimplePOST_LoadOnlyFromCache_Miss) {
   EXPECT_EQ(0, cache.disk_cache()->create_count());
 }
 
+TEST(HttpCache, SimplePOST_LoadOnlyFromCache_Hit) {
+  MockHttpCache cache;
+
+  // Test that we hit the cache for POST requests.
+
+  MockTransaction transaction(kSimplePOST_Transaction);
+
+  const int64 kUploadId = 1;  // Just a dummy value.
+
+  MockHttpRequest request(transaction);
+  request.upload_data = new net::UploadData();
+  request.upload_data->set_identifier(kUploadId);
+  request.upload_data->AppendBytes("hello", 5);
+
+  // Populate the cache.
+  RunTransactionTestWithRequest(cache.http_cache(), transaction, request);
+
+  EXPECT_EQ(1, cache.network_layer()->transaction_count());
+  EXPECT_EQ(0, cache.disk_cache()->open_count());
+  EXPECT_EQ(1, cache.disk_cache()->create_count());
+
+  // Load from cache.
+  request.load_flags |= net::LOAD_ONLY_FROM_CACHE;
+  RunTransactionTestWithRequest(cache.http_cache(), transaction, request);
+
+  EXPECT_EQ(1, cache.network_layer()->transaction_count());
+  EXPECT_EQ(1, cache.disk_cache()->open_count());
+  EXPECT_EQ(1, cache.disk_cache()->create_count());
+}
+
 TEST(HttpCache, RangeGET_SkipsCache) {
   MockHttpCache cache;
 
diff --git a/net/http/http_request_info.h b/net/http/http_request_info.h
index 8a680b6..de817fa 100644
--- a/net/http/http_request_info.h
+++ b/net/http/http_request_info.h
@@ -13,6 +13,9 @@ namespace net {
 
 class HttpRequestInfo {
  public:
+  HttpRequestInfo() : load_flags(0) {
+  }
+
   // The requested URL.
   GURL url;