Allows prefetch requests to live beyond the renderer by delaying

content/browser/loader/resource_loader.cc

Index: content/browser/loader/resource_loader.cc
diff --git a/content/browser/loader/resource_loader.cc b/content/browser/loader/resource_loader.cc
index 88c9c857d55e21cb6b8017de3f08821b32cf8964..a0ae1c6158e9192213b07c222bf6be2935def761 100644
--- a/content/browser/loader/resource_loader.cc
+++ b/content/browser/loader/resource_loader.cc
@@ -7,7 +7,9 @@
 #include "base/command_line.h"
 #include "base/message_loop/message_loop.h"
 #include "base/metrics/histogram.h"
+#include "base/strings/string_number_conversions.h"
 #include "base/time/time.h"
+#include "base/timer/timer.h"
 #include "content/browser/child_process_security_policy_impl.h"
 #include "content/browser/loader/cross_site_resource_handler.h"
 #include "content/browser/loader/resource_loader_delegate.h"
@@ -36,6 +38,13 @@ using base::TimeTicks;
 namespace content {
 namespace {
 
+// TODO(jkarlin): The value is high to reduce the chance of the detachable
+// request timing out, forcing a blocked second request to open a new connection
+// and start over. Reduce this value once we have a better idea of what it
+// should be and once we stop blocking multiple simultaneous requests for the
+// same resource (see bugs 46104 and 31014).

[James Simonsen, 2013/10/28 21:16:40]

I think the worst case scenario here is that the other renderer is hung. The
request will stay open, but the buffer will always be full and it'll stop
reading. This request will then hang for the full duration.

The real way out here is fixing the underlying problem and allow the cache entry
to complete even if the renderer isn't reading. It might be a perf win,
especially if the renderer is busy.

[jkarlin2, 2013/10/29 11:48:59]

I agree that the second reader should be able to drive the cache entry if the
first is hung/throttled.  I also think that the second request should be able to
read what's available from the cache and use it if it's cacheable.  davidben and
I are starting work on this problem.

In the meanwhile, do you prefer the 60 second value or a short one (like 3
seconds)?

On 2013/10/28 21:16:40, James Simonsen wrote:

+const int kDefaultDetachableDelayOnCancelMs = 60000;
+
 void PopulateResourceResponse(net::URLRequest* request,
                               ResourceResponse* response) {
   response->head.error_code = request->status().error();
@@ -210,6 +219,7 @@ void ResourceLoader::Init(scoped_ptr<net::URLRequest> request,
   waiting_for_upload_progress_ack_ = false;
   is_transferring_ = false;
   client_cert_store_ = client_cert_store.Pass();
+  detachable_delay_on_cancel_ms_ = kDefaultDetachableDelayOnCancelMs;
 
   request_->set_delegate(this);
   handler_->SetController(this);
@@ -458,6 +468,18 @@ void ResourceLoader::StartRequestInternal() {
   delegate_->DidStartRequest(this);
 }
 
+void ResourceLoader::Detach() {
+  ResourceRequestInfoImpl* info = GetRequestInfo();
+
+  if (info->is_detached())
+    return;
+  info->set_detached();
+  detached_timer_.reset(new base::OneShotTimer<ResourceLoader>());
+  detached_timer_->Start(
+      FROM_HERE, TimeDelta::FromMilliseconds(detachable_delay_on_cancel_ms_),
+      this, &ResourceLoader::Cancel);
+}
+
 void ResourceLoader::CancelRequestInternal(int error, bool from_renderer) {
   VLOG(1) << "CancelRequestInternal: " << request_->url().spec();
 
@@ -469,6 +491,11 @@ void ResourceLoader::CancelRequestInternal(int error, bool from_renderer) {
   if (from_renderer && (info->is_download() || info->is_stream()))
     return;
 
+  if (from_renderer && info->is_detachable()) {
+    Detach();
+    return;
+  }
+
   // TODO(darin): Perhaps we should really be looking to see if the status is
   // IO_PENDING?
   bool was_pending = request_->is_pending();