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

net/http/http_cache_transaction.cc

Index: net/http/http_cache_transaction.cc
diff --git a/net/http/http_cache_transaction.cc b/net/http/http_cache_transaction.cc
index 648199063a7e0f032f885a0467296a01ece8b035..a0446f98930df0a3c71c24bd493dc303fc78a6bd 100644
--- a/net/http/http_cache_transaction.cc
+++ b/net/http/http_cache_transaction.cc
@@ -1200,7 +1200,35 @@ int HttpCache::Transaction::DoAddToEntry() {
   net_log_.BeginEvent(NetLog::TYPE_HTTP_CACHE_ADD_TO_ENTRY);
   DCHECK(entry_lock_waiting_since_.is_null());
   entry_lock_waiting_since_ = TimeTicks::Now();
-  return cache_->AddTransactionToEntry(new_entry_, this);
+
+  int result = cache_->AddTransactionToEntry(new_entry_, this);
+
+  // If we're a range request and we're blocked by the reader/writer lock, then
+  // we'll switch to pass-through mode, this eliminates a long-running issue
+  // (crbug.com/31014) where two of the same video resource could not be played
+  // back simultaneously due to one locking the cache entry until the entire
+  // video was downloaded.
+  //
+  // This is not an ideal solution, as we are now ignoring the cache entirely
+  // for all range requests to a resource beyond the first. This is however a
+  // much more succinct solution than the alternatives, which would require
+  // somewhat significant changes to the http caching logic.
+  // TODO(rileya): Support simultaneous transactions on sparse cache entries,
+  // and remove this hack (crbug.com/362215).
+  if (partial_ && result == net::ERR_IO_PENDING && (new_entry_->writer ||
+      (mode() & Transaction::WRITE && !new_entry_->readers.empty()))) {

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

You mentioned that this fixed some small issues. My reaction would be to remove
the extra check because it seems to be just duplicating the logic that returns
io_pending from AddTransactionToEntry... except in the case of a race where the
cache is in the process of moving from one transaction to the next. If that is
causing issues, it seems too subtle to handle it by duplicating some logic here
and it would be better to ask the cache about it. What was the issue?

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

It was indeed a problem of racing when moving between transactions in the
pending queue, when there weren't actually any readers/writers to block the
transaction (this caused several net_unittests to fail). How would you recommend
handling this without duplicating this logic? 

[rvargas (doing something else), 2014/05/29 21:03:50]

It doesn't seem reasonable (possible?) to ask transactions to be aware of the
state of the cache (ok, ActiveEntry but still) before calling
RemovePendingTransaction. That says to me that the cache must be prepared to
receive that call at any time, and it should handle the case where the
transaction that is going away is the only transaction tied to the entry.

Reading HttpCache::OnProcessPendingQueue it looks like it does everything as
intended... it just deletes the entry if there's no transaction to deal with. So
I guess I would have to see exactly what is failing and why to answer you.

Is it just that unit tests that expect requests to be queued are seeing requests
escaping the lock? If that's the case then the issue would not be
will_process_pending_queue but the fact that the workaround lets regular (not
byte range related) requests trigger the escape.

+    entry_lock_waiting_since_ = TimeTicks();
+    net_log_.EndEvent(NetLog::TYPE_HTTP_CACHE_ADD_TO_ENTRY);
+    mode_ = NONE;
+    partial_->RestoreHeaders(&custom_request_->extra_headers);
+    partial_.reset();
+    next_state_ = STATE_SEND_REQUEST;
+    cache_pending_ = false;
+    cache_->RemovePendingTransaction(this);
+    new_entry_ = NULL;
+    return OK;
+  }
+  return result;
 }
 
 int HttpCache::Transaction::DoAddToEntryComplete(int result) {