Http Cache: Update vary data after a 304.

net/http/http_cache_unittest.cc

Index: net/http/http_cache_unittest.cc
diff --git a/net/http/http_cache_unittest.cc b/net/http/http_cache_unittest.cc
index 189e952ec6b2eb157546d9297195b4f77263a0ed..6f10f8cff1276eeec288f1056d008e3db68a6bf6 100644
--- a/net/http/http_cache_unittest.cc
+++ b/net/http/http_cache_unittest.cc
@@ -2108,7 +2108,7 @@ void RevalidationServer::Handler(const net::HttpRequestInfo* request,
 }
 
 // Tests revalidation after a vary match.
-TEST(HttpCache, SimpleGET_LoadValidateCache_VaryMatch) {
+TEST(HttpCache, GET_ValidateCache_VaryMatch) {
   MockHttpCache cache;
 
   // Write to the cache.
@@ -2141,7 +2141,7 @@ TEST(HttpCache, SimpleGET_LoadValidateCache_VaryMatch) {
 }
 
 // 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.
@@ -2175,7 +2175,7 @@ TEST(HttpCache, SimpleGET_LoadValidateCache_VaryMismatch) {
 }
 
 // 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.
@@ -2207,6 +2207,154 @@ TEST(HttpCache, SimpleGET_LoadDontValidateCache_VaryMismatch) {
   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,