Http cache: Modify the Vary header behavior.

Prioritize vary headers over LOAD_PREFERRING_CACHE. This means that back
navigations would fail after reaching a vary header, and a new network request
will be issued.

BUG=94369
TEST=net_unittests
Review URL: https://codereview.chromium.org/11358260

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

3 files changed, 57 insertions, 8 deletions

diff --git a/net/http/http_cache_transaction.cc b/net/http/http_cache_transaction.cc
index abc055d..ae98cc6 100644
--- a/net/http/http_cache_transaction.cc
+++ b/net/http/http_cache_transaction.cc
@@ -1698,8 +1698,7 @@ int HttpCache::Transaction::BeginCacheRead() {
 int HttpCache::Transaction::BeginCacheValidation() {
   DCHECK(mode_ == READ_WRITE);
 
-  bool skip_validation = effective_load_flags_ & LOAD_PREFERRING_CACHE ||
-                         !RequiresValidation();
+  bool skip_validation = !RequiresValidation();
 
   if (truncated_) {
     // Truncated entries can cause partial gets, so we shouldn't record this
@@ -1869,10 +1868,19 @@ bool HttpCache::Transaction::RequiresValidation() {
   // TODO(darin): need to do more work here:
   //  - make sure we have a matching request method
   //  - watch out for cached responses that depend on authentication
+
   // In playback mode, nothing requires validation.
   if (cache_->mode() == net::HttpCache::PLAYBACK)
     return false;
 
+  if (response_.vary_data.is_valid() &&
+      !response_.vary_data.MatchesRequest(*request_, *response_.headers)) {
+    return true;
+  }
+
+  if (effective_load_flags_ & LOAD_PREFERRING_CACHE)
+    return false;
+
   if (effective_load_flags_ & LOAD_VALIDATE_CACHE)
     return true;
 
@@ -1884,12 +1892,6 @@ bool HttpCache::Transaction::RequiresValidation() {
     return true;
   }
 
-  // Since Vary header computation is fairly expensive, we save it for last.
-  if (response_.vary_data.is_valid() &&
-      !response_.vary_data.MatchesRequest(*request_, *response_.headers)) {
-    return true;
-  }
-
   return false;
 }
 
diff --git a/net/http/http_cache_unittest.cc b/net/http/http_cache_unittest.cc
index 05ef842..73348255 100644
--- a/net/http/http_cache_unittest.cc
+++ b/net/http/http_cache_unittest.cc
@@ -772,6 +772,52 @@ TEST(HttpCache, SimpleGET_LoadPreferringCache_Miss) {
   EXPECT_EQ(1, cache.disk_cache()->create_count());
 }
 
+// Tests LOAD_PREFERRING_CACHE in the presence of vary headers.
+TEST(HttpCache, SimpleGET_LoadPreferringCache_VaryMatch) {
+  MockHttpCache cache;
+
+  // Write to the cache.
+  MockTransaction transaction(kSimpleGET_Transaction);
+  transaction.request_headers = "Foo: bar\n";
+  transaction.response_headers = "Cache-Control: max-age=10000\n"
+                                 "Vary: Foo\n";
+  AddMockTransaction(&transaction);
+  RunTransactionTest(cache.http_cache(), transaction);
+
+  // Read from the cache.
+  transaction.load_flags |= net::LOAD_PREFERRING_CACHE;
+  RunTransactionTest(cache.http_cache(), transaction);
+
+  EXPECT_EQ(1, cache.network_layer()->transaction_count());
+  EXPECT_EQ(1, cache.disk_cache()->open_count());
+  EXPECT_EQ(1, cache.disk_cache()->create_count());
+  RemoveMockTransaction(&transaction);
+}
+
+// Tests LOAD_PREFERRING_CACHE in the presence of vary headers.
+TEST(HttpCache, SimpleGET_LoadPreferringCache_VaryMismatch) {
+  MockHttpCache cache;
+
+  // Write to the cache.
+  MockTransaction transaction(kSimpleGET_Transaction);
+  transaction.request_headers = "Foo: bar\n";
+  transaction.response_headers = "Cache-Control: max-age=10000\n"
+                                 "Vary: Foo\n";
+  AddMockTransaction(&transaction);
+  RunTransactionTest(cache.http_cache(), transaction);
+
+  // Attempt to read from the cache... this is a vary mismatch that must reach
+  // the network again.
+  transaction.load_flags |= net::LOAD_PREFERRING_CACHE;
+  transaction.request_headers = "Foo: none\n";
+  RunTransactionTest(cache.http_cache(), transaction);
+
+  EXPECT_EQ(2, cache.network_layer()->transaction_count());
+  EXPECT_EQ(1, cache.disk_cache()->open_count());
+  EXPECT_EQ(1, cache.disk_cache()->create_count());
+  RemoveMockTransaction(&transaction);
+}
+
 TEST(HttpCache, SimpleGET_LoadBypassCache) {
   MockHttpCache cache;
 
diff --git a/net/http/http_transaction_unittest.cc b/net/http/http_transaction_unittest.cc
index 19f0189..b956245 100644
--- a/net/http/http_transaction_unittest.cc
+++ b/net/http/http_transaction_unittest.cc
@@ -251,6 +251,7 @@ int MockNetworkTransaction::Start(const net::HttpRequestInfo* request,
     response_.response_time = t->response_time;
 
   response_.headers = new net::HttpResponseHeaders(header_data);
+  response_.vary_data.Init(*request, *response_.headers);
   response_.ssl_info.cert_status = t->cert_status;
   data_ = resp_data;
   test_mode_ = t->test_mode;