Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(136)

Unified Diff: net/http/http_cache_unittest.cc

Issue 2774603003: Doom and create new entry when validation is not a match (Closed)
Patch Set: Added a new state + tests Created 3 years, 8 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« net/http/http_cache_transaction.cc ('K') | « net/http/http_cache_transaction.cc ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: net/http/http_cache_unittest.cc
diff --git a/net/http/http_cache_unittest.cc b/net/http/http_cache_unittest.cc
index 5303a7515f462c0c2f2c1411e4226eac7c3c8238..5f12f80244a64bcd558973d4108b72fdc49d5abc 100644
--- a/net/http/http_cache_unittest.cc
+++ b/net/http/http_cache_unittest.cc
@@ -1417,7 +1417,7 @@ TEST(HttpCache, SimpleGET_ManyReaders) {
}
// Parallel validation results in 200.
-TEST(HttpCache, SimpleGET_ParallelValidationNoMatch) {
+TEST(HttpCache, SimpleGET_ParallelValidationNoMatchCacheCreateRace) {
MockHttpCache cache;
MockHttpRequest request(kSimpleGET_Transaction);
@@ -1472,6 +1472,10 @@ TEST(HttpCache, SimpleGET_ParallelValidationNoMatch) {
EXPECT_EQ(5, cache.network_layer()->transaction_count());
EXPECT_EQ(0, cache.disk_cache()->open_count());
+
+ // Note only 3 entries got created instead of 5 since every other transaction
+ // received a ERR_CACHE_CREATE_FAILURE due to racing with another transaction
+ // to create the entry.
EXPECT_EQ(3, cache.disk_cache()->create_count());
for (int i = 0; i < kNumTransactions; ++i) {
@@ -1480,6 +1484,71 @@ TEST(HttpCache, SimpleGET_ParallelValidationNoMatch) {
}
}
+// Parallel validation results in 200. Similar to above except here there are
+// only 2 transactions so no cache create race happens.
+TEST(HttpCache, SimpleGET_ParallelValidationNoMatch) {
+ MockHttpCache cache;
+
+ MockHttpRequest request(kSimpleGET_Transaction);
+ request.load_flags |= LOAD_VALIDATE_CACHE;
+
+ std::vector<Context*> context_list;
+ const int kNumTransactions = 2;
+
+ for (int i = 0; i < kNumTransactions; ++i) {
+ context_list.push_back(new Context());
+ Context* c = context_list[i];
+
+ c->result = cache.CreateTransaction(&c->trans);
+ ASSERT_THAT(c->result, IsOk());
+ EXPECT_EQ(LOAD_STATE_IDLE, c->trans->GetLoadState());
+
+ c->result =
+ c->trans->Start(&request, c->callback.callback(), NetLogWithSource());
+ }
+
+ // All requests are waiting for the active entry.
+ for (int i = 0; i < kNumTransactions; ++i) {
+ Context* c = context_list[i];
+ EXPECT_EQ(LOAD_STATE_WAITING_FOR_CACHE, c->trans->GetLoadState());
+ }
+
+ // Allow all requests to move from the Create queue to the active entry.
+ base::RunLoop().RunUntilIdle();
+
+ // The first request should be a writer at this point, and the subsequent
+ // requests should have passed the validation phase and created their own
+ // entries since none of them matched the headers of the earlier one.
+ EXPECT_TRUE(cache.IsWriterPresent(kSimpleGET_Transaction.url));
+
+ EXPECT_EQ(2, cache.network_layer()->transaction_count());
+ EXPECT_EQ(0, cache.disk_cache()->open_count());
+ EXPECT_EQ(2, cache.disk_cache()->create_count());
+
+ // All requests depend on the writer, and the writer is between Start and
+ // Read, i.e. idle.
+ for (int i = 0; i < kNumTransactions; ++i) {
+ Context* c = context_list[i];
+ EXPECT_EQ(LOAD_STATE_IDLE, c->trans->GetLoadState());
+ }
+
+ for (int i = 0; i < kNumTransactions; ++i) {
+ Context* c = context_list[i];
+ if (c->result == ERR_IO_PENDING)
+ c->result = c->callback.WaitForResult();
+ ReadAndVerifyTransaction(c->trans.get(), kSimpleGET_Transaction);
+ }
+
+ EXPECT_EQ(2, cache.network_layer()->transaction_count());
+ EXPECT_EQ(0, cache.disk_cache()->open_count());
+ EXPECT_EQ(2, cache.disk_cache()->create_count());
+
+ for (int i = 0; i < kNumTransactions; ++i) {
+ Context* c = context_list[i];
+ delete c;
+ }
+}
+
// Tests that a GET followed by a DELETE results in DELETE immediately starting
// the headers phase and the entry is doomed.
TEST(HttpCache, SimpleGET_ParallelValidationDelete) {
@@ -1784,10 +1853,6 @@ TEST(HttpCache, SimpleGET_ParallelValidationCancelWriter) {
c->trans->ResumeNetworkStart();
base::RunLoop().RunUntilIdle();
- // Headers transaction would have doomed the new entry created.
- EXPECT_TRUE(
- cache.disk_cache()->IsDiskEntryDoomed(kSimpleGET_Transaction.url));
-
// Complete the rest of the transactions.
for (int i = 1; i < kNumTransactions; ++i) {
Context* c = context_list[i];
@@ -1796,7 +1861,10 @@ TEST(HttpCache, SimpleGET_ParallelValidationCancelWriter) {
EXPECT_EQ(4, cache.network_layer()->transaction_count());
EXPECT_EQ(0, cache.disk_cache()->open_count());
- EXPECT_EQ(2, cache.disk_cache()->create_count());
+
+ // Headers transaction would have doomed the new entry created and created
+ // another new entry.
+ EXPECT_EQ(3, cache.disk_cache()->create_count());
for (int i = 1; i < kNumTransactions; ++i) {
Context* c = context_list[i];
« net/http/http_cache_transaction.cc ('K') | « net/http/http_cache_transaction.cc ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698