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..5446d7bd8207a4483d57825f657fbd6fa0586041 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"
@@ -210,6 +212,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_ms_ = 60000;

[mmenke, 2013/10/24 20:27:21]

Wonder if we should reduce this...  Regardless, this should be #defined as a
constant at the top of the file.  KDetaultDetachableDelayMs or somesuch.

May also want to rename both variable and value to be more precise -
detached_delay_on_cancel_ms_.

[jkarlin2, 2013/10/25 14:10:21]

Done.  Yes, 60 seconds is long.  It's just until we get a feel for what value it
should be, and until we stop blocking subsequent requests for the same resource.
 I've added a comment on this to the new const (but it hasn't been uploaded
yet).

[mmenke, 2013/10/25 14:57:51]

I suggest starting short (Like the current behavior, which deletes it instantly)
and then experimenting with lengthening it, rather than the other way around, as
that has less chance of breaking things badly.  We'll probably want to use a
different timeout for <a ping>.

[jkarlin2, 2013/10/25 18:24:54]

I originally had it at seconds.

cbentzel had recommended high so that:
1) We can find out what a good short measurement is.
2) We are unlikely to wind up fetching twice as it should load in 60 seconds.

You're right in that we make the worst case 60 seconds worse, which is certainly
bad.

I can go either way.  Thoughts from the crowd?


On 2013/10/25 14:57:51, mmenke wrote:

[jkarlin2, 2013/10/25 18:28:21]

I meant to say that I originally had it at 3 seconds.

On 2013/10/25 18:24:54, jkarlin wrote:

 
   request_->set_delegate(this);
   handler_->SetController(this);
@@ -458,6 +461,16 @@ void ResourceLoader::StartRequestInternal() {
   delegate_->DidStartRequest(this);
 }
 
+void ResourceLoader::CancelAfterTimeout() {
+  if (detached_timer_)
+    return;
+
+  detached_timer_.reset(new base::OneShotTimer<ResourceLoader>());
+  detached_timer_->Start(FROM_HERE,
+                         TimeDelta::FromMilliseconds(detachable_delay_ms_),
+                         this, &ResourceLoader::Cancel);
+}
+
 void ResourceLoader::CancelRequestInternal(int error, bool from_renderer) {
   VLOG(1) << "CancelRequestInternal: " << request_->url().spec();
 
@@ -469,6 +482,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()) {
+    CancelAfterTimeout();
+    return;
+  }
+
   // TODO(darin): Perhaps we should really be looking to see if the status is
   // IO_PENDING?
   bool was_pending = request_->is_pending();