diff options
-rw-r--r-- | net/base/upload_data.h | 15 | ||||
-rw-r--r-- | net/http/http_cache.cc | 81 | ||||
-rw-r--r-- | net/http/http_cache.h | 4 | ||||
-rw-r--r-- | net/http/http_cache_unittest.cc | 65 | ||||
-rw-r--r-- | net/http/http_request_info.h | 3 |
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; |