summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--net/base/upload_data.h15
-rw-r--r--net/http/http_cache.cc81
-rw-r--r--net/http/http_cache.h4
-rw-r--r--net/http/http_cache_unittest.cc65
-rw-r--r--net/http/http_request_info.h3
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;