Http Cache: Update vary data after a 304.

net/http/http_cache_unittest.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "net/http/http_cache.h"
 
 #include <algorithm>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
@@ skipping 2090 matching lines @@
     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) {
+TEST(HttpCache, GET_ValidateCache_VaryMatch) {
   MockHttpCache cache;
 
   // Write to the cache.
   MockTransaction transaction(kTypicalGET_Transaction);
   transaction.request_headers = "Foo: bar\r\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"
@@ skipping 12 matching lines @@
   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());
   TestLoadTimingNetworkRequest(load_timing_info);
   RemoveMockTransaction(&transaction);
 }
 
 // Tests revalidation after a vary mismatch if etag is present.
-TEST(HttpCache, SimpleGET_LoadValidateCache_VaryMismatch) {
+TEST(HttpCache, GET_ValidateCache_VaryMismatch) {
   MockHttpCache cache;
 
   // Write to the cache.
   MockTransaction transaction(kTypicalGET_Transaction);
   transaction.request_headers = "Foo: bar\r\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"
@@ skipping 13 matching lines @@
   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());
   TestLoadTimingNetworkRequest(load_timing_info);
   RemoveMockTransaction(&transaction);
 }
 
 // Tests lack of revalidation after a vary mismatch and no etag.
-TEST(HttpCache, SimpleGET_LoadDontValidateCache_VaryMismatch) {
+TEST(HttpCache, GET_DontValidateCache_VaryMismatch) {
   MockHttpCache cache;
 
   // Write to the cache.
   MockTransaction transaction(kTypicalGET_Transaction);
   transaction.request_headers = "Foo: bar\r\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";
@@ skipping 11 matching lines @@
 
   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());
   TestLoadTimingNetworkRequest(load_timing_info);
   RemoveMockTransaction(&transaction);
 }
 
+// Tests that a new vary header provided when revalidating an entry is saved.
+TEST(HttpCache, GET_ValidateCache_VaryMatch_UpdateVary) {
+  MockHttpCache cache;
+
+  // Write to the cache.
+  ScopedMockTransaction transaction(kTypicalGET_Transaction);
+  transaction.request_headers = "Foo: bar\r\n Name: bar\r\n";

[Adam Rice, 2015/03/06 09:49:11]

What purpose does the space between \r\n and Name: serve?

[rvargas (doing something else), 2015/03/06 20:44:11]

Better readability. We skip white space when reading headers.

+  transaction.response_headers =
+      "Etag: \"foopy\"\n"
+      "Cache-Control: max-age=0\n"
+      "Vary: Foo\n";
+  RunTransactionTest(cache.http_cache(), transaction);
+
+  // Validate the entry and change the vary field in the response.
+  transaction.request_headers = "Foo: bar\r\n Name: none\r\n";
+  transaction.status = "HTTP/1.1 304 Not Modified";
+  transaction.response_headers =
+      "Etag: \"foopy\"\n"
+      "Cache-Control: max-age=3600\n"
+      "Vary: Name\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());
+
+  // Make sure that the ActiveEntry is gone.
+  base::MessageLoop::current()->RunUntilIdle();

[Adam Rice, 2015/03/06 09:49:11]

Optional: change these all to base::RunLoop().RunUntilIdle()

Even more optional: RunUntilActiveEntryGone() could be a method on MockHttpCache
?

[rvargas (doing something else), 2015/03/06 20:44:11]

I couldn't come up with a name that makes me happy (a method promising something
that doesn't do) so I went with using RunLoop

+
+  // Generate a vary mismatch.
+  transaction.request_headers = "Foo: bar\r\n Name: bar\r\n";
+  RunTransactionTest(cache.http_cache(), transaction);
+
+  EXPECT_EQ(3, cache.network_layer()->transaction_count());
+  EXPECT_EQ(2, cache.disk_cache()->open_count());
+  EXPECT_EQ(1, cache.disk_cache()->create_count());
+}
+
+// Tests that new request headers causing a vary mismatch are paired with the
+// new response when the server says the old response can be used.
+TEST(HttpCache, GET_ValidateCache_VaryMismatch_UpdateRequestHeader) {
+  MockHttpCache cache;
+
+  // Write to the cache.
+  ScopedMockTransaction transaction(kTypicalGET_Transaction);
+  transaction.request_headers = "Foo: bar\r\n";
+  transaction.response_headers =
+      "Etag: \"foopy\"\n"
+      "Cache-Control: max-age=3600\n"
+      "Vary: Foo\n";
+  RunTransactionTest(cache.http_cache(), transaction);
+
+  // Vary-mismatch validation receives 304.
+  transaction.request_headers = "Foo: none\r\n";
+  transaction.status = "HTTP/1.1 304 Not Modified";
+  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());
+
+  // Make sure that the ActiveEntry is gone.
+  base::MessageLoop::current()->RunUntilIdle();
+
+  // Generate a vary mismatch.
+  transaction.request_headers = "Foo: bar\r\n";
+  RunTransactionTest(cache.http_cache(), transaction);
+
+  EXPECT_EQ(3, cache.network_layer()->transaction_count());
+  EXPECT_EQ(2, cache.disk_cache()->open_count());
+  EXPECT_EQ(1, cache.disk_cache()->create_count());
+}
+
+// Tests that a 304 without vary headers doesn't delete the previouysly stored

[Adam Rice, 2015/03/06 09:49:12]

s/previouysly/previously/

Also in the following test.

[rvargas (doing something else), 2015/03/06 20:44:11]

Done.

+// vary data after a vary match revalidation.
+TEST(HttpCache, GET_ValidateCache_VaryMatch_DontDeleteVary) {
+  MockHttpCache cache;
+
+  // Write to the cache.
+  ScopedMockTransaction transaction(kTypicalGET_Transaction);
+  transaction.request_headers = "Foo: bar\r\n";
+  transaction.response_headers =
+      "Etag: \"foopy\"\n"
+      "Cache-Control: max-age=0\n"
+      "Vary: Foo\n";
+  RunTransactionTest(cache.http_cache(), transaction);
+
+  // Validate the entry and remove the vary field in the response.
+  transaction.status = "HTTP/1.1 304 Not Modified";
+  transaction.response_headers =
+      "Etag: \"foopy\"\n"
+      "Cache-Control: max-age=3600\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());
+
+  // Make sure that the ActiveEntry is gone.
+  base::MessageLoop::current()->RunUntilIdle();
+
+  // Generate a vary mismatch.
+  transaction.request_headers = "Foo: none\r\n";
+  RunTransactionTest(cache.http_cache(), transaction);
+
+  EXPECT_EQ(3, cache.network_layer()->transaction_count());
+  EXPECT_EQ(2, cache.disk_cache()->open_count());
+  EXPECT_EQ(1, cache.disk_cache()->create_count());
+}
+
+// Tests that a 304 without vary headers doesn't delete the previouysly stored
+// vary data after a vary mismatch.
+TEST(HttpCache, GET_ValidateCache_VaryMismatch_DontDeleteVary) {
+  MockHttpCache cache;
+
+  // Write to the cache.
+  ScopedMockTransaction transaction(kTypicalGET_Transaction);
+  transaction.request_headers = "Foo: bar\r\n";
+  transaction.response_headers =
+      "Etag: \"foopy\"\n"
+      "Cache-Control: max-age=3600\n"
+      "Vary: Foo\n";
+  RunTransactionTest(cache.http_cache(), transaction);
+
+  // Vary-mismatch validation receives 304 and no vary header.
+  transaction.request_headers = "Foo: none\r\n";
+  transaction.status = "HTTP/1.1 304 Not Modified";
+  transaction.response_headers =
+      "Etag: \"foopy\"\n"
+      "Cache-Control: max-age=3600\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());
+
+  // Make sure that the ActiveEntry is gone.
+  base::MessageLoop::current()->RunUntilIdle();
+
+  // Generate a vary mismatch.
+  transaction.request_headers = "Foo: bar\r\n";
+  RunTransactionTest(cache.http_cache(), transaction);
+
+  EXPECT_EQ(3, cache.network_layer()->transaction_count());
+  EXPECT_EQ(2, cache.disk_cache()->open_count());
+  EXPECT_EQ(1, cache.disk_cache()->create_count());
+}
+
 static void ETagGet_UnconditionalRequest_Handler(
     const net::HttpRequestInfo* request,
     std::string* response_status,
     std::string* response_headers,
     std::string* response_data) {
   EXPECT_FALSE(
       request->extra_headers.HasHeader(net::HttpRequestHeaders::kIfNoneMatch));
 }
 
 TEST(HttpCache, ETagGET_Http10) {
@@ skipping 5549 matching lines @@
 
   // Here the second transaction proceeds without reading the first body.
   EXPECT_EQ(net::LOAD_STATE_WAITING_FOR_CACHE, second->trans->GetLoadState());
   base::MessageLoop::current()->RunUntilIdle();
   EXPECT_EQ(net::LOAD_STATE_IDLE, second->trans->GetLoadState());
   ASSERT_TRUE(second->trans->GetResponseInfo());
   EXPECT_TRUE(second->trans->GetResponseInfo()->headers->HasHeaderValue(
       "Cache-Control", "no-store"));
   ReadAndVerifyTransaction(second->trans.get(), kSimpleGET_Transaction);
 }