Http cache: Don't attempt to revalidate entries after a vary header mismatch

if there is no etag.

BUG=79758
TEST=net_unittests
Review URL: https://codereview.chromium.org/11692013

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

3 files changed, 142 insertions, 7 deletions

diff --git a/net/http/http_cache_transaction.cc b/net/http/http_cache_transaction.cc
index 27792a1..624c08f 100644
--- a/net/http/http_cache_transaction.cc
+++ b/net/http/http_cache_transaction.cc
@@ -145,6 +145,7 @@ HttpCache::Transaction::Transaction(
       handling_206_(false),
       cache_pending_(false),
       done_reading_(false),
+      vary_mismatch_(false),
       io_buf_len_(0),
       read_offset_(0),
       effective_load_flags_(0),
@@ -732,7 +733,6 @@ int HttpCache::Transaction::DoGetBackendComplete(int result) {
     if (effective_load_flags_ & LOAD_ONLY_FROM_CACHE) {
       mode_ = READ;
     } else if (effective_load_flags_ & LOAD_BYPASS_CACHE) {
-      UpdateTransactionPattern(PATTERN_NOT_COVERED);
       mode_ = WRITE;
     } else {
       mode_ = READ_WRITE;
@@ -1917,6 +1917,7 @@ bool HttpCache::Transaction::RequiresValidation() {
 
   if (response_.vary_data.is_valid() &&
       !response_.vary_data.MatchesRequest(*request_, *response_.headers)) {
+    vary_mismatch_ = true;
     return true;
   }
 
@@ -1957,14 +1958,14 @@ bool HttpCache::Transaction::ConditionalizeRequest() {
   // TODO(darin): Or should we use the last?
 
   std::string etag_value;
-  response_.headers->EnumerateHeader(NULL, "etag", &etag_value);
+  if (response_.headers->GetHttpVersion() >= HttpVersion(1, 1))
+    response_.headers->EnumerateHeader(NULL, "etag", &etag_value);
 
   std::string last_modified_value;
-  response_.headers->EnumerateHeader(NULL, "last-modified",
-                                     &last_modified_value);
-
-  if (response_.headers->GetHttpVersion() < HttpVersion(1, 1))
-    etag_value.clear();
+  if (!vary_mismatch_) {
+    response_.headers->EnumerateHeader(NULL, "last-modified",
+                                       &last_modified_value);
+  }
 
   if (etag_value.empty() && last_modified_value.empty())
     return false;
diff --git a/net/http/http_cache_transaction.h b/net/http/http_cache_transaction.h
index 85d485e..86e42f6 100644
--- a/net/http/http_cache_transaction.h
+++ b/net/http/http_cache_transaction.h
@@ -405,6 +405,7 @@ class HttpCache::Transaction : public HttpTransaction {
   bool handling_206_;  // We must deal with this 206 response.
   bool cache_pending_;  // We are waiting for the HttpCache.
   bool done_reading_;
+  bool vary_mismatch_;  // The request doesn't match the stored vary data.
   scoped_refptr<IOBuffer> read_buf_;
   int io_buf_len_;
   int read_offset_;
diff --git a/net/http/http_cache_unittest.cc b/net/http/http_cache_unittest.cc
index d42ee56..51098b4 100644
--- a/net/http/http_cache_unittest.cc
+++ b/net/http/http_cache_unittest.cc
@@ -1712,6 +1712,139 @@ TEST(HttpCache, ETagGET_ConditionalRequest_304) {
   EXPECT_EQ(1, cache.disk_cache()->create_count());
 }
 
+class RevalidationServer {
+ public:
+  RevalidationServer() {
+    s_etag_used_ = false;
+    s_last_modified_used_ = false;
+  }
+
+  bool EtagUsed() { return s_etag_used_; }
+  bool LastModifiedUsed() { return s_last_modified_used_; }
+
+  static void Handler(const net::HttpRequestInfo* request,
+                      std::string* response_status,
+                      std::string* response_headers,
+                      std::string* response_data);
+
+ private:
+  static bool s_etag_used_;
+  static bool s_last_modified_used_;
+};
+bool RevalidationServer::s_etag_used_ = false;
+bool RevalidationServer::s_last_modified_used_ = false;
+
+void RevalidationServer::Handler(const net::HttpRequestInfo* request,
+                                 std::string* response_status,
+                                 std::string* response_headers,
+                                 std::string* response_data) {
+  if (request->extra_headers.HasHeader(net::HttpRequestHeaders::kIfNoneMatch))
+      s_etag_used_ = true;
+
+  if (request->extra_headers.HasHeader(
+          net::HttpRequestHeaders::kIfModifiedSince)) {
+      s_last_modified_used_ = true;
+  }
+
+  if (s_etag_used_ || s_last_modified_used_) {
+    response_status->assign("HTTP/1.1 304 Not Modified");
+    response_headers->assign(kTypicalGET_Transaction.response_headers);
+    response_data->clear();
+  } else {
+    response_status->assign(kTypicalGET_Transaction.status);
+    response_headers->assign(kTypicalGET_Transaction.response_headers);
+    response_data->assign(kTypicalGET_Transaction.data);
+  }
+}
+
+// Tests revalidation after a vary match.
+TEST(HttpCache, SimpleGET_LoadValidateCache_VaryMatch) {
+  MockHttpCache cache;
+
+  // Write to the cache.
+  MockTransaction transaction(kTypicalGET_Transaction);
+  transaction.request_headers = "Foo: bar\n";
+  transaction.response_headers =
+      "Date: Wed, 28 Nov 2007 09:40:09 GMT\n"
+      "Last-Modified: Wed, 28 Nov 2007 00:40:09 GMT\n"
+      "Etag: \"foopy\"\n"
+      "Cache-Control: max-age=0\n"
+      "Vary: Foo\n";
+  AddMockTransaction(&transaction);
+  RunTransactionTest(cache.http_cache(), transaction);
+
+  // Read from the cache.
+  RevalidationServer server;
+  transaction.handler = server.Handler;
+  RunTransactionTest(cache.http_cache(), transaction);
+
+  EXPECT_TRUE(server.EtagUsed());
+  EXPECT_TRUE(server.LastModifiedUsed());
+  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);
+}
+
+// Tests revalidation after a vary mismatch if etag is present.
+TEST(HttpCache, SimpleGET_LoadValidateCache_VaryMismatch) {
+  MockHttpCache cache;
+
+  // Write to the cache.
+  MockTransaction transaction(kTypicalGET_Transaction);
+  transaction.request_headers = "Foo: bar\n";
+  transaction.response_headers =
+      "Date: Wed, 28 Nov 2007 09:40:09 GMT\n"
+      "Last-Modified: Wed, 28 Nov 2007 00:40:09 GMT\n"
+      "Etag: \"foopy\"\n"
+      "Cache-Control: max-age=0\n"
+      "Vary: Foo\n";
+  AddMockTransaction(&transaction);
+  RunTransactionTest(cache.http_cache(), transaction);
+
+  // Read from the cache and revalidate the entry.
+  RevalidationServer server;
+  transaction.handler = server.Handler;
+  transaction.request_headers = "Foo: none\n";
+  RunTransactionTest(cache.http_cache(), transaction);
+
+  EXPECT_TRUE(server.EtagUsed());
+  EXPECT_FALSE(server.LastModifiedUsed());
+  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);
+}
+
+// Tests lack of revalidation after a vary mismatch and no etag.
+TEST(HttpCache, SimpleGET_LoadDontValidateCache_VaryMismatch) {
+  MockHttpCache cache;
+
+  // Write to the cache.
+  MockTransaction transaction(kTypicalGET_Transaction);
+  transaction.request_headers = "Foo: bar\n";
+  transaction.response_headers =
+      "Date: Wed, 28 Nov 2007 09:40:09 GMT\n"
+      "Last-Modified: Wed, 28 Nov 2007 00:40:09 GMT\n"
+      "Cache-Control: max-age=0\n"
+      "Vary: Foo\n";
+  AddMockTransaction(&transaction);
+  RunTransactionTest(cache.http_cache(), transaction);
+
+  // Read from the cache and don't revalidate the entry.
+  RevalidationServer server;
+  transaction.handler = server.Handler;
+  transaction.request_headers = "Foo: none\n";
+  RunTransactionTest(cache.http_cache(), transaction);
+
+  EXPECT_FALSE(server.EtagUsed());
+  EXPECT_FALSE(server.LastModifiedUsed());
+  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);
+}
+
 static void ETagGet_UnconditionalRequest_Handler(
     const net::HttpRequestInfo* request,
     std::string* response_status,