Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1311)

Unified Diff: content/browser/loader/resource_dispatcher_host_impl.cc

Issue 25772002: Allows prefetch requests to live beyond the renderer by delaying (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Rebase. Created 7 years, 2 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: content/browser/loader/resource_dispatcher_host_impl.cc
diff --git a/content/browser/loader/resource_dispatcher_host_impl.cc b/content/browser/loader/resource_dispatcher_host_impl.cc
index a6859df7766d121ad3eae10400f07f5aa514637a..657b385d92993f6612524de071949fcb7e24e7d8 100644
--- a/content/browser/loader/resource_dispatcher_host_impl.cc
+++ b/content/browser/loader/resource_dispatcher_host_impl.cc
@@ -127,6 +127,15 @@ const int kUserGestureWindowMs = 3500;
// use. Arbitrarily chosen.
const double kMaxRequestsPerProcessRatio = 0.45;
+bool IsDetachableResourceType(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,
@@ -406,13 +415,16 @@ void ResourceDispatcherHostImpl::CancelRequestsForContext(
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_detached() ||
+ (*i)->is_transferring());
}
#endif
@@ -694,18 +706,19 @@ void ResourceDispatcherHostImpl::DidReceiveRedirect(ResourceLoader* loader,
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 {
- // We should always have an entry in offline_policy_map_ from when
- // this request traversed Begin{Download,SaveFile,Request}.
+ // Unless detached, we should 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();
+ DCHECK(info->is_detached());
}
int render_process_id, render_view_id;
@@ -1073,6 +1086,7 @@ void ResourceDispatcherHostImpl::BeginRequest(
request_data.transition_type,
false, // is download
false, // is stream
+ false, // is detachable
allow_download,
request_data.has_user_gesture,
request_data.referrer_policy,
@@ -1101,6 +1115,8 @@ void ResourceDispatcherHostImpl::BeginRequest(
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);
}
// The RedirectToFileResourceHandler depends on being next in the chain.
@@ -1219,7 +1235,8 @@ ResourceRequestInfoImpl* ResourceDispatcherHostImpl::CreateRequestInfo(
ResourceType::SUB_RESOURCE,
PAGE_TRANSITION_LINK,
download, // is_download
- false, // is_stream
+ false, // is_stream
+ false, // is_detachable
download, // allow_download
false, // has_user_gesture
WebKit::WebReferrerPolicyDefault,
@@ -1315,8 +1332,8 @@ void ResourceDispatcherHostImpl::ResumeDeferredNavigation(
}
// 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);
@@ -1341,14 +1358,15 @@ void ResourceDispatcherHostImpl::CancelRequestsForRoute(int child_id,
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() &&
- !IsTransferredNavigation(id) &&
- (route_id == -1 || route_id == info->GetRouteID())) {
+
+ if (info->is_detachable()) {
+ i->second->Detach();
+ } else if (!info->is_download() && !info->is_stream() &&
+ !IsTransferredNavigation(id) &&
+ (route_id == -1 || route_id == info->GetRouteID())) {
matching_requests.push_back(id);
}
}
« no previous file with comments | « content/browser/loader/resource_dispatcher_host_impl.h ('k') | content/browser/loader/resource_dispatcher_host_unittest.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698