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

Unified Diff: net/http/http_cache_unittest.cc

Issue 2886483002: Adds a new class HttpCache::Writers for multiple cache transactions reading from the network. (Closed)
Patch Set: Feedback addressed. Created 3 years, 5 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
Index: net/http/http_cache_unittest.cc
diff --git a/net/http/http_cache_unittest.cc b/net/http/http_cache_unittest.cc
index b01427a7a6164b85c0ede119fb4c1da12d1bc7d7..05dcf9061149533851730dcbdd2aed064a91a723 100644
--- a/net/http/http_cache_unittest.cc
+++ b/net/http/http_cache_unittest.cc
@@ -218,7 +218,7 @@ void RunTransactionTestBase(HttpCache* cache,
rv = trans->Start(&request, callback.callback(), net_log);
if (rv == ERR_IO_PENDING)
rv = callback.WaitForResult();
- ASSERT_EQ(trans_info.return_code, rv);
+ ASSERT_EQ(trans_info.start_return_code, rv);
if (OK != rv)
return;
@@ -1076,7 +1076,7 @@ TEST(HttpCache, SimpleGET_CacheSignal_Failure) {
RemoveMockTransaction(&transaction);
// Network failure with error; should fail but have was_cached set.
- transaction.return_code = ERR_FAILED;
+ transaction.start_return_code = ERR_FAILED;
AddMockTransaction(&transaction);
MockHttpRequest request(transaction);
@@ -1887,11 +1887,9 @@ TEST(HttpCache, SimpleGET_ParallelValidationNoMatch) {
// entries since none of them matched the headers of the earlier one.
EXPECT_TRUE(cache.IsWriterPresent(kSimpleGET_Transaction.url));
- // Note that there are only 3 entries created and not 5 since every other
- // transaction would have gone to the network.
EXPECT_EQ(5, cache.network_layer()->transaction_count());
EXPECT_EQ(0, cache.disk_cache()->open_count());
- EXPECT_EQ(3, cache.disk_cache()->create_count());
+ EXPECT_EQ(5, cache.disk_cache()->create_count());
// All requests depend on the writer, and the writer is between Start and
// Read, i.e. idle.
@@ -1907,7 +1905,72 @@ TEST(HttpCache, SimpleGET_ParallelValidationNoMatch) {
EXPECT_EQ(5, cache.network_layer()->transaction_count());
EXPECT_EQ(0, cache.disk_cache()->open_count());
- EXPECT_EQ(3, cache.disk_cache()->create_count());
+ EXPECT_EQ(5, cache.disk_cache()->create_count());
+}
+
+// Parallel validation results in 200 for 1 transaction and validation matches
+// for subsequent transactions.
+TEST(HttpCache, SimpleGET_ParallelValidationNoMatch1) {
+ MockHttpCache cache;
+ MockHttpRequest request(kSimpleGET_Transaction);
+
+ MockTransaction transaction(kSimpleGET_Transaction);
+ transaction.load_flags |= LOAD_VALIDATE_CACHE;
+ MockHttpRequest validate_request(transaction);
+ std::vector<std::unique_ptr<Context>> context_list;
+ const int kNumTransactions = 5;
+ for (int i = 0; i < kNumTransactions; ++i) {
+ context_list.push_back(base::MakeUnique<Context>());
+ auto& c = context_list[i];
+ c->result = cache.CreateTransaction(&c->trans);
+ ASSERT_THAT(c->result, IsOk());
+ EXPECT_EQ(LOAD_STATE_IDLE, c->trans->GetLoadState());
+
+ MockHttpRequest* this_request = &request;
+ if (i == 1)
+ this_request = &validate_request;
+
+ c->result = c->trans->Start(this_request, c->callback.callback(),
+ NetLogWithSource());
+ }
+
+ // All requests are waiting for the active entry.
+ for (auto& context : context_list) {
+ EXPECT_EQ(LOAD_STATE_WAITING_FOR_CACHE, context->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 second
+ // request should have passed the validation phase and created a new
+ // entry. Rest of them should be added to the new entry.
+ EXPECT_TRUE(cache.IsWriterPresent(kSimpleGET_Transaction.url));
+ EXPECT_EQ(3, cache.GetCountDoneHeadersQueue(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 (auto& context : context_list) {
+ EXPECT_EQ(LOAD_STATE_IDLE, context->trans->GetLoadState());
+ }
+
+ EXPECT_EQ(1, cache.disk_cache()->doomed_count());
+
+ for (size_t i = 0; i < context_list.size(); i++) {
+ if (context_list[i]->result == ERR_IO_PENDING)
+ context_list[i]->result = context_list[i]->callback.WaitForResult();
+
+ ReadAndVerifyTransaction(context_list[i]->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());
}
// Tests that a GET followed by a DELETE results in DELETE immediately starting
@@ -2156,7 +2219,7 @@ TEST(HttpCache, SimpleGET_ParallelValidationCancelReader) {
// The pending transactions will be added to a new entry.
base::RunLoop().RunUntilIdle();
- EXPECT_EQ(1, cache.GetCountDoneHeadersQueue(kSimpleGET_Transaction.url));
+ EXPECT_EQ(2, cache.GetCountDoneHeadersQueue(kSimpleGET_Transaction.url));
EXPECT_TRUE(cache.IsWriterPresent(kSimpleGET_Transaction.url));
// Complete the rest of the transactions.
@@ -2165,7 +2228,7 @@ TEST(HttpCache, SimpleGET_ParallelValidationCancelReader) {
ReadAndVerifyTransaction(c->trans.get(), kSimpleGET_Transaction);
}
- EXPECT_EQ(3, cache.network_layer()->transaction_count());
+ EXPECT_EQ(2, cache.network_layer()->transaction_count());
EXPECT_EQ(0, cache.disk_cache()->open_count());
EXPECT_EQ(2, cache.disk_cache()->create_count());
}
@@ -2230,10 +2293,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 (auto& context : context_list) {
if (!context)
@@ -2243,7 +2302,7 @@ 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());
+ EXPECT_EQ(3, cache.disk_cache()->create_count());
}
// Tests when a writer is destroyed mid-read and entry is marked as truncated,
@@ -2372,10 +2431,6 @@ TEST(HttpCache, SimpleGET_ParallelValidationStopCaching) {
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 (auto& context : context_list) {
if (!context)
@@ -2385,7 +2440,7 @@ TEST(HttpCache, SimpleGET_ParallelValidationStopCaching) {
EXPECT_EQ(4, cache.network_layer()->transaction_count());
EXPECT_EQ(0, cache.disk_cache()->open_count());
- EXPECT_EQ(2, cache.disk_cache()->create_count());
+ EXPECT_EQ(3, cache.disk_cache()->create_count());
}
// Tests that a transaction is currently in headers phase and is destroyed
@@ -2679,7 +2734,7 @@ TEST(HttpCache, FastNoStoreGET_DoneWithPending) {
EXPECT_EQ(3, cache.network_layer()->transaction_count());
EXPECT_EQ(0, cache.disk_cache()->open_count());
- EXPECT_EQ(2, cache.disk_cache()->create_count());
+ EXPECT_EQ(3, cache.disk_cache()->create_count());
// Now, make sure that the second request asks for the entry not to be stored.
request_handler.set_no_store(true);
@@ -2694,7 +2749,7 @@ TEST(HttpCache, FastNoStoreGET_DoneWithPending) {
EXPECT_EQ(3, cache.network_layer()->transaction_count());
EXPECT_EQ(0, cache.disk_cache()->open_count());
- EXPECT_EQ(2, cache.disk_cache()->create_count());
+ EXPECT_EQ(3, cache.disk_cache()->create_count());
RemoveMockTransaction(&kFastNoStoreGET_Transaction);
}
@@ -4458,7 +4513,7 @@ TEST(HttpCache, SimpleHEAD_WithRanges) {
transaction.method = "HEAD";
transaction.request_headers = "Range: bytes = 0-4\r\n";
transaction.load_flags |= LOAD_ONLY_FROM_CACHE | LOAD_SKIP_CACHE_VALIDATION;
- transaction.return_code = ERR_CACHE_MISS;
+ transaction.start_return_code = ERR_CACHE_MISS;
RunTransactionTest(cache.http_cache(), transaction);
EXPECT_EQ(0, cache.disk_cache()->open_count());
@@ -4624,7 +4679,7 @@ TEST(HttpCache, SimpleHEAD_InvalidatesEntry) {
// Load from the cache.
transaction.method = "GET";
transaction.load_flags |= LOAD_ONLY_FROM_CACHE | LOAD_SKIP_CACHE_VALIDATION;
- transaction.return_code = ERR_CACHE_MISS;
+ transaction.start_return_code = ERR_CACHE_MISS;
RunTransactionTest(cache.http_cache(), transaction);
RemoveMockTransaction(&transaction);
@@ -4901,7 +4956,7 @@ TEST(HttpCache, SimpleGET_DontInvalidateOnFailure) {
// Fail the network request.
MockTransaction transaction(kSimpleGET_Transaction);
- transaction.return_code = ERR_FAILED;
+ transaction.start_return_code = ERR_FAILED;
transaction.load_flags |= LOAD_VALIDATE_CACHE;
AddMockTransaction(&transaction);
@@ -4910,7 +4965,7 @@ TEST(HttpCache, SimpleGET_DontInvalidateOnFailure) {
RemoveMockTransaction(&transaction);
transaction.load_flags = LOAD_ONLY_FROM_CACHE | LOAD_SKIP_CACHE_VALIDATION;
- transaction.return_code = OK;
+ transaction.start_return_code = OK;
AddMockTransaction(&transaction);
RunTransactionTest(cache.http_cache(), transaction);
@@ -7985,12 +8040,12 @@ TEST(HttpCache, SkipVaryCheck) {
// The request should fail.
transaction.load_flags = LOAD_ONLY_FROM_CACHE;
transaction.request_headers = "accept-encoding: foo\r\n";
- transaction.return_code = ERR_CACHE_MISS;
+ transaction.start_return_code = ERR_CACHE_MISS;
RunTransactionTest(cache.http_cache(), transaction);
// Change the load flags to ignore vary checks, the request should now hit.
transaction.load_flags = LOAD_ONLY_FROM_CACHE | LOAD_SKIP_VARY_CHECK;
- transaction.return_code = OK;
+ transaction.start_return_code = OK;
RunTransactionTest(cache.http_cache(), transaction);
}
@@ -8016,7 +8071,7 @@ TEST(HttpCache, ValidLoadOnlyFromCache) {
// If the cache entry is checked for validitiy, it should fail.
transaction.load_flags = LOAD_ONLY_FROM_CACHE;
- transaction.return_code = ERR_CACHE_MISS;
+ transaction.start_return_code = ERR_CACHE_MISS;
RunTransactionTest(cache.http_cache(), transaction);
}
@@ -8032,7 +8087,7 @@ TEST(HttpCache, InvalidLoadFlagCombination) {
// DevTools relies on this combination of flags for "disable cache" mode
// when a resource is only supposed to be loaded from cache.
transaction.load_flags = LOAD_ONLY_FROM_CACHE | LOAD_BYPASS_CACHE;
- transaction.return_code = ERR_CACHE_MISS;
+ transaction.start_return_code = ERR_CACHE_MISS;
RunTransactionTest(cache.http_cache(), transaction);
}

Powered by Google App Engine
This is Rietveld 408576698