Bypass http cache for concurrent range requests.

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 1364 matching lines @@
   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();
   int rv = cache_->AddTransactionToEntry(new_entry_, this);
   if (rv == ERR_IO_PENDING) {
     if (bypass_lock_for_test_) {
       OnAddToEntryTimeout(entry_lock_waiting_since_);
     } else {
-      const int kTimeoutSeconds = 20;
+      // Immediately timeout and bypass the cache if we're a range request and
+      // we're blocked by the reader/writer lock. Doing so eliminates a long
+      // running issue, http://crbug.com/31014, where two of the same media
+      // resources could not be played back simultaneously due to one locking
+      // the cache entry until the entire video was downloaded.
+      //
+      // Bypassing the cache is not ideal, 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.
+      const int kTimeoutSeconds = partial_ ? 0 : 20;

[rvargas (doing something else), 2014/08/22 21:06:13]

What happens if this is (partial_ && new_entry_->writer &&
new_entry_->writer->range_requested_) instead?

[DaleCurtis, 2014/08/22 21:25:52]

Passes all the test the previous patch did. Do you want me to switch to that?

[rvargas (doing something else), 2014/08/22 21:44:40]

If all test pages etc behave the same, yes, I would prefer to make sure that the
first request was also for a range.

[DaleCurtis, 2014/08/22 21:45:41]

Done.

       base::MessageLoop::current()->PostDelayedTask(
           FROM_HERE,
           base::Bind(&HttpCache::Transaction::OnAddToEntryTimeout,

[scherkus (not reviewing), 2014/08/22 21:01:51]

what is the end-user visible effect caused by timing out http cache
transactions?

[rvargas (doing something else), 2014/08/22 21:10:25]

The transaction is not timed out, just waiting on the lock. The effect is that
the transaction will ignore the cache, so the response will not update the
cached entry in any way.... which may lead to consistency issues because a
future request may end up returning things "from the past" from the point of
view of the renderer.

[DaleCurtis, 2014/08/22 21:25:52]

Nothing from a casual user perspective, even for developers nothing obvious
happens in the inspector view. To my untrained eye, there's nothing obvious even
in net-internals.

                      weak_factory_.GetWeakPtr(), entry_lock_waiting_since_),
           TimeDelta::FromSeconds(kTimeoutSeconds));
     }
   }
   return rv;
 }
 
 int HttpCache::Transaction::DoAddToEntryComplete(int result) {
   net_log_.EndEventWithNetErrorCode(NetLog::TYPE_HTTP_CACHE_ADD_TO_ENTRY,
                                     result);
@@ skipping 1463 matching lines @@
     default:
       NOTREACHED();
   }
 }
 
 void HttpCache::Transaction::OnIOComplete(int result) {
   DoLoop(result);
 }
 
 }  // namespace net