Allows prefetch requests to live beyond the renderer by delaying

content/browser/loader/resource_dispatcher_host_impl.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.
 
 // See http://dev.chromium.org/developers/design-documents/multi-process-resource-loading
 
 #include "content/browser/loader/resource_dispatcher_host_impl.h"
 
 #include <set>
 #include <vector>
@@ skipping 14 matching lines @@
 #include "content/browser/appcache/chrome_appcache_service.h"
 #include "content/browser/cert_store_impl.h"
 #include "content/browser/child_process_security_policy_impl.h"
 #include "content/browser/cross_site_request_manager.h"
 #include "content/browser/download/download_resource_handler.h"
 #include "content/browser/download/save_file_manager.h"
 #include "content/browser/download/save_file_resource_handler.h"
 #include "content/browser/fileapi/chrome_blob_storage_context.h"
 #include "content/browser/loader/async_resource_handler.h"
 #include "content/browser/loader/buffered_resource_handler.h"
+#include "content/browser/loader/detached_resource_handler.h"

[mmenke, 2013/10/17 18:33:48]

Remove.

[jkarlin2, 2013/10/24 15:33:11]

Done.

 #include "content/browser/loader/cross_site_resource_handler.h"
 #include "content/browser/loader/power_save_block_resource_throttle.h"
 #include "content/browser/loader/redirect_to_file_resource_handler.h"
 #include "content/browser/loader/resource_message_filter.h"
 #include "content/browser/loader/resource_request_info_impl.h"
 #include "content/browser/loader/stream_resource_handler.h"
 #include "content/browser/loader/sync_resource_handler.h"
 #include "content/browser/loader/throttling_resource_handler.h"
 #include "content/browser/loader/upload_data_stream_builder.h"
 #include "content/browser/plugin_service_impl.h"
@@ skipping 75 matching lines @@
 // net::LOAD_MAYBE_USER_GESTURE load flag. This is a fairly arbitrary
 // guess at how long to expect direct impact from a user gesture, but
 // this should be OK as the load flag is a best-effort thing only,
 // rather than being intended as fully accurate.
 const int kUserGestureWindowMs = 3500;
 
 // Ratio of |max_num_in_flight_requests_| that any one renderer is allowed to
 // use. Arbitrarily chosen.
 const double kMaxRequestsPerProcessRatio = 0.45;
 
+bool IsDetachableResourceType(const ResourceType::Type type) {
+  switch (type) {
+    case ResourceType::PREFETCH:
+      return true;
+    default:
+      return false;
+  }
+}
+
 // Aborts a request before an URLRequest has actually been created.
 void AbortRequestBeforeItStarts(ResourceMessageFilter* filter,
                                 IPC::Message* sync_result,
                                 int request_id) {
   if (sync_result) {
     SyncLoadResult result;
     result.error_code = net::ERR_ABORTED;
     ResourceHostMsg_SyncLoad::WriteReplyParams(sync_result, result);
     filter->Send(sync_result);
   } else {
@@ skipping 259 matching lines @@
       delete loaders;
     } else {
       ++i;
     }
   }
 
 #ifndef NDEBUG
   for (LoaderList::iterator i = loaders_to_cancel.begin();
        i != loaders_to_cancel.end(); ++i) {
     // There is no strict requirement that this be the case, but currently
-    // downloads, streams  and transferred requests are the only requests that
-    // aren't cancelled when the associated processes go away. It may be OK for
-    // this invariant to change in the future, but if this assertion fires
-    // without the invariant changing, then it's indicative of a leak.
-    DCHECK((*i)->GetRequestInfo()->is_download() ||
-           (*i)->GetRequestInfo()->is_stream() ||
-           (*i)->is_transferring());
+    // downloads, streams, detachable requests, and transferred requests are the
+    // only requests that aren't cancelled when the associated processes go
+    // away. It may be OK for this invariant to change in the future, but if
+    // this assertion fires without the invariant changing, then it's indicative
+    // of a leak.
+    DCHECK(
+        (*i)->GetRequestInfo()->is_download() ||
+        (*i)->GetRequestInfo()->is_stream() ||
+        (*i)->GetRequestInfo()->is_detachable() ||

[mmenke, 2013/10/17 18:33:48]

Wait...what's stopping us from putting detachable requests in this list?

[jkarlin2, 2013/10/24 15:33:11]

I don't understand, they are in the list?

On 2013/10/17 18:33:48, mmenke wrote:

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

Sorry...What's stopping requests from being in the loaders_to_cancel list?

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

At this point the renderers (and their attached requests) for the context should
have already disappeared, so all that we're left with to clean up are detached
requests, like downloads and detachable resources.

On 2013/10/24 20:27:20, mmenke wrote:

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

Ah, right...I was misreading the code (I thought we were DCHECKing if this
wasn't true, not if it was).

+        (*i)->is_transferring());
   }
 #endif
 
   loaders_to_cancel.clear();
 
   // Validate that no more requests for this context were added.
   for (LoaderMap::const_iterator i = pending_loaders_.begin();
        i != pending_loaders_.end(); ++i) {
     // http://crbug.com/90971
     CHECK_NE(i->second->GetRequestInfo()->GetContext(), context);
@@ skipping 261 matching lines @@
       new_url));
   BrowserThread::PostTask(
       BrowserThread::UI, FROM_HERE,
       base::Bind(
           &NotifyRedirectOnUI,
           render_process_id, render_view_id, base::Passed(&detail)));
 }
 
 void ResourceDispatcherHostImpl::DidReceiveResponse(ResourceLoader* loader) {
   ResourceRequestInfoImpl* info = loader->GetRequestInfo();
+
   // There should be an entry in the map created when we dispatched the
-  // request.
+  // request unless it's been detached and the renderer has died.
   OfflineMap::iterator policy_it(
       offline_policy_map_.find(info->GetGlobalRoutingID()));
   if (offline_policy_map_.end() != policy_it) {
     policy_it->second->UpdateStateForSuccessfullyStartedRequest(
         loader->request()->response_info());
-  } else {
+  } else if (!info->is_detachable()) {
     // We should always have an entry in offline_policy_map_ from when
     // this request traversed Begin{Download,SaveFile,Request}.
     // TODO(rdsmith): This isn't currently true; see http://crbug.com/241176.
     NOTREACHED();
   }
 
   int render_process_id, render_view_id;
   if (!info->GetAssociatedRenderView(&render_process_id, &render_view_id))
     return;
 
@@ skipping 351 matching lines @@
           request_data.origin_pid,
           request_id,
           request_data.is_main_frame,
           request_data.frame_id,
           request_data.parent_is_main_frame,
           request_data.parent_frame_id,
           request_data.resource_type,
           request_data.transition_type,
           false,  // is download
           false,  // is stream
+          false,  // is detachable
           allow_download,
           request_data.has_user_gesture,
           request_data.referrer_policy,
           resource_context,
           filter_->GetWeakPtr(),
           !is_sync_load);
   extra_info->AssociateWithRequest(request);  // Request takes ownership.
 
   if (request->url().SchemeIs(chrome::kBlobScheme)) {
     // Hang on to a reference to ensure the blob is not released prior
     // to the job being started.
     webkit_blob::BlobProtocolHandler::SetRequestedBlobDataHandle(
         request,
         filter_->blob_storage_context()->context()->
             GetBlobDataFromPublicURL(request->url()));
   }
 
   // Have the appcache associate its extra info with the request.
   appcache::AppCacheInterceptor::SetExtraRequestInfo(
       request, filter_->appcache_service(), child_id,
       request_data.appcache_host_id, request_data.resource_type);
 
   // Construct the IPC resource handler.
   scoped_ptr<ResourceHandler> handler;
   if (sync_result) {
     handler.reset(new SyncResourceHandler(request, sync_result, this));
   } else {
     handler.reset(new AsyncResourceHandler(request, this));
+    if (IsDetachableResourceType(request_data.resource_type))
+      extra_info->set_is_detachable(true);
   }
 
+

[mmenke, 2013/10/17 18:33:48]

nit:  Remove extra line break.

[jkarlin2, 2013/10/24 15:33:11]

Done.

   // The RedirectToFileResourceHandler depends on being next in the chain.
   if (request_data.download_to_file) {
     handler.reset(
         new RedirectToFileResourceHandler(handler.Pass(), request, this));
   }
 
   // Install a CrossSiteResourceHandler for all main frame requests.  This will
   // let us check whether a transfer is required and pause for the unload
   // handler either if so or if a cross-process navigation is already under way.
  if (request_data.resource_type == ResourceType::MAIN_FRAME &&
@@ skipping 97 matching lines @@
       0,
       request_id_,
       false,     // is_main_frame
       -1,        // frame_id
       false,     // parent_is_main_frame
       -1,        // parent_frame_id
       ResourceType::SUB_RESOURCE,
       PAGE_TRANSITION_LINK,
       download,  // is_download
       false,  // is_stream
+      false,  // is_detachable
       download,  // allow_download
       false,     // has_user_gesture
       WebKit::WebReferrerPolicyDefault,
       context,
       base::WeakPtr<ResourceMessageFilter>(),  // filter
       true);     // is_async
 }
 
 void ResourceDispatcherHostImpl::OnRenderViewHostCreated(
     int child_id,
@@ skipping 75 matching lines @@
   ResourceLoader* loader = GetLoader(id);
   if (loader) {
     // The response we were meant to resume could have already been canceled.
     ResourceRequestInfoImpl* info = loader->GetRequestInfo();
     if (info->cross_site_handler())
       info->cross_site_handler()->ResumeResponse();
   }
 }
 
 // The object died, so cancel and detach all requests associated with it except
-// for downloads, which belong to the browser process even if initiated via a
-// renderer.
+// for downloads and detachable resources, which belong to the browser process
+// even if initiated via a renderer.
 void ResourceDispatcherHostImpl::CancelRequestsForProcess(int child_id) {
   CancelRequestsForRoute(child_id, -1 /* cancel all */);
   registered_temp_files_.erase(child_id);
 }
 
 void ResourceDispatcherHostImpl::CancelRequestsForRoute(int child_id,
                                                         int route_id) {
   // Since pending_requests_ is a map, we first build up a list of all of the
   // matching requests to be cancelled, and then we cancel them.  Since there
   // may be more than one request to cancel, we cannot simply hold onto the map
   // iterators found in the first loop.
 
   // Find the global ID of all matching elements.
   bool any_requests_transferring = false;
   std::vector<GlobalRequestID> matching_requests;
   for (LoaderMap::const_iterator i = pending_loaders_.begin();
        i != pending_loaders_.end(); ++i) {
     if (i->first.child_id != child_id)
       continue;
 
     ResourceRequestInfoImpl* info = i->second->GetRequestInfo();
 
     GlobalRequestID id(child_id, i->first.request_id);
     DCHECK(id == i->first);
-
-    // Don't cancel navigations that are transferring to another process,
-    // since they belong to another process now.
+    // Don't cancel navigations that are expected to live beyond this process.
     if (IsTransferredNavigation(id))
       any_requests_transferring = true;
-    if (!info->is_download() && !info->is_stream() &&
+    if (!info->is_download() && !info->is_stream() && !info->is_detachable() &&

[mmenke, 2013/10/17 18:33:48]

We aren't calling into DelayCancelForDetachable here, are we?  So we don't start
the timer, and don't listen to the command line flag?  Think we need more
testing of this.

[jkarlin2, 2013/10/24 15:33:11]

Done. Great catch. Fixed and tests added.

         !IsTransferredNavigation(id) &&
         (route_id == -1 || route_id == info->GetRouteID())) {
       matching_requests.push_back(id);
     }
   }
 
   // Remove matches.
   for (size_t i = 0; i < matching_requests.size(); ++i) {
     LoaderMap::iterator iter = pending_loaders_.find(matching_requests[i]);
     // Although every matching request was in pending_requests_ when we built
@@ skipping 552 matching lines @@
   if ((load_flags & net::LOAD_REPORT_RAW_HEADERS)
       && !policy->CanReadRawCookies(child_id)) {
     VLOG(1) << "Denied unauthorized request for raw headers";
     load_flags &= ~net::LOAD_REPORT_RAW_HEADERS;
   }
 
   return load_flags;
 }
 
 }  // namespace content