Ignore disk cache for concurrent byte range requests to a single resource, to prevent stalling.

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 c87fa1b7eecba7e2f12fef38c342b24e87325947..112f293d4dacc426fe898db481a499cc87827046 100644
--- a/net/http/http_cache_unittest.cc
+++ b/net/http/http_cache_unittest.cc
@@ -4421,6 +4421,41 @@ TEST(HttpCache, RangeGET_Cancel3) {
   RemoveMockTransaction(&kRangeGET_TransactionOK);
 }
 
+// Tests that we allow multiple simultaneous, non-overlapping transactions to
+// take place on a sparse entry. Currently this is supported with a hack (see
+// comment in Transaction::DoAddToEntry in http_cache_transaction.cc), but a

[rvargas (doing something else), 2014/05/29 19:58:52]

If we expect this test to pass now and with a future (better) implementation
without change, we'll forget about this comment and it will become stale. It's
better to avoid the comment now.

[rileya (GONE FROM CHROMIUM), 2014/05/29 20:15:03]

Good point, removed.

+// future, cleaner solution should still pass this test.
+TEST(HttpCache, RangeGET_MultipleRequests) {
+  MockHttpCache cache;
+
+  // Create a transaction for bytes 0-9.
+  MockHttpRequest request(kRangeGET_TransactionOK);
+  request.load_flags |= net::LOAD_VALIDATE_CACHE;

[rvargas (doing something else), 2014/05/29 19:58:52]

no need for this flag

[rileya (GONE FROM CHROMIUM), 2014/05/29 20:15:03]

Removed.

+  MockTransaction transaction(kRangeGET_TransactionOK);
+  transaction.request_headers = "Range: bytes = 0-9\r\n" EXTRA_HEADER;
+  transaction.data = "rg: 00-09";

[rvargas (doing something else), 2014/05/29 19:58:52]

Should need an extra space at the end (to make it 10 bytes)

[rileya (GONE FROM CHROMIUM), 2014/05/29 20:15:03]

Added.

+  transaction.handler = NULL;

[rvargas (doing something else), 2014/05/29 19:58:52]

why remove the server?

[rileya (GONE FROM CHROMIUM), 2014/05/29 20:15:03]

Another test elsewhere did this so I assumed it had some importance. Removed.

+  AddMockTransaction(&transaction);

[rvargas (doing something else), 2014/05/29 19:58:52]

Should be paired with a RemoveMockTransaction()

[rileya (GONE FROM CHROMIUM), 2014/05/29 20:15:03]

Added.

+
+  net::TestCompletionCallback callback;
+  scoped_ptr<net::HttpTransaction> trans;
+  int rv = cache.http_cache()->CreateTransaction(net::DEFAULT_PRIORITY, &trans);
+  EXPECT_EQ(net::OK, rv);
+  ASSERT_TRUE(trans.get());
+
+  // Start our transaction.
+  trans->Start(&request, callback.callback(), net::BoundNetLog());
+
+  // A second transaction on a different part of the file (the default
+  // kRangeGET_TransactionOK requests 40-49) should not be blocked by
+  // the already pending transaction.
+  AddMockTransaction(&kRangeGET_TransactionOK);

[rvargas (doing something else), 2014/05/29 19:58:52]

This is not needed... transactions are keyed by the URL and that is not
changing.

[rileya (GONE FROM CHROMIUM), 2014/05/29 20:15:03]

Removed.

+  RunTransactionTest(cache.http_cache(), kRangeGET_TransactionOK);
+
+  // Let the first transaction complete.
+  callback.WaitForResult();
+}
+
 // Tests that an invalid range response results in no cached entry.
 TEST(HttpCache, RangeGET_InvalidResponse1) {
   MockHttpCache cache;