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

net/http/http_cache_transaction.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "net/http/http_cache_transaction.h"
 
 #include "build/build_config.h"
 
 #if defined(OS_POSIX)
 #include <unistd.h>
@@ skipping 1182 matching lines @@
   return OK;
 }
 
 int HttpCache::Transaction::DoAddToEntry() {
   DCHECK(new_entry_);
   cache_pending_ = true;
   next_state_ = STATE_ADD_TO_ENTRY_COMPLETE;
   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) {

[rvargas (doing something else), 2014/04/22 00:44:31]

Why giving up on trying to see if the lock owner is also a byte range request? I
guess it's not that different in practice given that this one is partial_... are
you worried that there is no writer?

[rileya (GONE FROM CHROMIUM), 2014/04/23 17:45:05]

I was thinking that we could be blocked by either a writer or reader(s), so I
wasn't sure what to check here. I was also thinking it doesn't really matter
whether the holder of the lock was itself a range request, as long as this
transaction is and was blocked.

[rvargas (doing something else), 2014/04/23 18:56:54]

yeah, I've been going back and forth about this. It would be nice to limit the
scope as much as we can, but it probably doesn't matter much in practice because
mixed access (normal and byte range) should be uncommon.

+    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;
+    new_entry_->pending_queue.pop_front();

[rvargas (doing something else), 2014/04/22 00:44:31]

cache_->RemovePendingTransaction(this);

[rileya (GONE FROM CHROMIUM), 2014/04/23 17:45:05]

Done.

+    new_entry_ = entry_ = NULL;

[rvargas (doing something else), 2014/04/22 00:44:31]

entry_ should be NULL already.

[rileya (GONE FROM CHROMIUM), 2014/04/23 17:45:05]

Ahh, alright, removed setting it.

+    return OK;
+  }
+  return result;
 }
 
 int HttpCache::Transaction::DoAddToEntryComplete(int result) {
   net_log_.EndEventWithNetErrorCode(NetLog::TYPE_HTTP_CACHE_ADD_TO_ENTRY,
                                     result);
   const TimeDelta entry_lock_wait =
       TimeTicks::Now() - entry_lock_waiting_since_;
   UMA_HISTOGRAM_TIMES("HttpCache.EntryLockWait", entry_lock_wait);
 
   entry_lock_waiting_since_ = TimeTicks();
@@ skipping 1306 matching lines @@
     default:
       NOTREACHED();
   }
 }
 
 void HttpCache::Transaction::OnIOComplete(int result) {
   DoLoop(result);
 }
 
 }  // namespace net