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

Unified Diff: content/browser/renderer_host/resource_dispatcher_host.cc

Issue 8669014: Fix a bug where redirect chain gets lost on process swap. (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: Created 9 years, 1 month 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/renderer_host/resource_dispatcher_host.cc
diff --git a/content/browser/renderer_host/resource_dispatcher_host.cc b/content/browser/renderer_host/resource_dispatcher_host.cc
index 46d2cc2abd3442ae08c956f9d31d8608da39f35c..19eea1c081af494836ad45f1fdbd7afc5d08e729 100644
--- a/content/browser/renderer_host/resource_dispatcher_host.cc
+++ b/content/browser/renderer_host/resource_dispatcher_host.cc
@@ -452,6 +452,11 @@ void ResourceDispatcherHost::OnSyncLoad(
sync_result->routing_id());
}
+// TODO(mpcomplete): hack
+static GURL g_deferred_url;
+static int g_deferred_child_id;
+static int g_deferred_request_id;
+
void ResourceDispatcherHost::BeginRequest(
int request_id,
const ResourceHostMsg_Request& request_data,
@@ -466,6 +471,31 @@ void ResourceDispatcherHost::BeginRequest(
base::strlcpy(url_buf, request_data.url.spec().c_str(), arraysize(url_buf));
base::debug::Alias(url_buf);
+ // TODO(mpcomplete): Step 3. (See steps 1-2 below).
+ // If the request that's coming in is a cross-process transferring, then we
+ // want to reuse and resume the old request rather than start a new one.
+ // I'm not entirely sure how much of the BeginRequest code should run. We
+ // definitely want to create new ResourceHandlers and ExtraInfo, since those
+ // are tied to the process.
+ // Also, I'm not clear what should happen if any of the early returns are hit
+ // before we actually resume the request via FollowDeferredRedirect. I think
+ // we're then left with an orphaned request that will never be deleted. We
+ // probably want to cancel it in those cases.
+ // It's probably best to split apart BeginRequest into 2 methods, 1 for
+ // regular requests and one for transferred ones - probably with some shared
+ // code, but not all.
+ net::URLRequest* deferred_request = NULL;
+ if (!g_deferred_url.is_empty() && request_data.url == g_deferred_url) {
+ g_deferred_url = GURL();
+ GlobalRequestID old_id(g_deferred_child_id, g_deferred_request_id);
+ PendingRequestList::iterator i = pending_requests_.find(old_id);
+ DCHECK(i != pending_requests_.end());
+ deferred_request = i->second;
+ ResourceDispatcherHostRequestInfo* info = InfoForRequest(deferred_request);
+
+ pending_requests_.erase(old_id);
+ }
+
const content::ResourceContext& resource_context =
filter_->resource_context();
@@ -517,43 +547,49 @@ void ResourceDispatcherHost::BeginRequest(
}
// Construct the request.
- net::URLRequest* request = new net::URLRequest(request_data.url, this);
- request->set_method(request_data.method);
- request->set_first_party_for_cookies(request_data.first_party_for_cookies);
- request->set_referrer(referrer.spec());
- net::HttpRequestHeaders headers;
- headers.AddHeadersFromString(request_data.headers);
- request->SetExtraRequestHeaders(headers);
-
- int load_flags = request_data.load_flags;
- // Although EV status is irrelevant to sub-frames and sub-resources, we have
- // to perform EV certificate verification on all resources because an HTTP
- // keep-alive connection created to load a sub-frame or a sub-resource could
- // be reused to load a main frame.
- load_flags |= net::LOAD_VERIFY_EV_CERT;
- if (request_data.resource_type == ResourceType::MAIN_FRAME) {
- load_flags |= net::LOAD_MAIN_FRAME;
- } else if (request_data.resource_type == ResourceType::SUB_FRAME) {
- load_flags |= net::LOAD_SUB_FRAME;
- } else if (request_data.resource_type == ResourceType::PREFETCH) {
- load_flags |= (net::LOAD_PREFETCH | net::LOAD_DO_NOT_PROMPT_FOR_LOGIN);
- } else if (request_data.resource_type == ResourceType::FAVICON) {
- load_flags |= net::LOAD_DO_NOT_PROMPT_FOR_LOGIN;
- }
-
- if (sync_result)
- load_flags |= net::LOAD_IGNORE_LIMITS;
-
- // Raw headers are sensitive, as they inclide Cookie/Set-Cookie, so only
- // allow requesting them if requestor has ReadRawCookies permission.
- if ((load_flags & net::LOAD_REPORT_RAW_HEADERS)
- && !ChildProcessSecurityPolicy::GetInstance()->
- CanReadRawCookies(child_id)) {
- VLOG(1) << "Denied unathorized request for raw headers";
- load_flags &= ~net::LOAD_REPORT_RAW_HEADERS;
- }
-
- request->set_load_flags(load_flags);
+ net::URLRequest* request;
+ if (deferred_request) {
+ request = deferred_request;
+ } else {
+ request = new net::URLRequest(request_data.url, this);
+ request->set_method(request_data.method);
+ request->set_first_party_for_cookies(request_data.first_party_for_cookies);
+ request->set_referrer(referrer.spec());
+ net::HttpRequestHeaders headers;
+ headers.AddHeadersFromString(request_data.headers);
+ request->SetExtraRequestHeaders(headers);
+
+ int load_flags = request_data.load_flags;
+ // Although EV status is irrelevant to sub-frames and sub-resources, we have
+ // to perform EV certificate verification on all resources because an HTTP
+ // keep-alive connection created to load a sub-frame or a sub-resource could
+ // be reused to load a main frame.
+ load_flags |= net::LOAD_VERIFY_EV_CERT;
+ if (request_data.resource_type == ResourceType::MAIN_FRAME) {
+ load_flags |= net::LOAD_MAIN_FRAME;
+ } else if (request_data.resource_type == ResourceType::SUB_FRAME) {
+ load_flags |= net::LOAD_SUB_FRAME;
+ } else if (request_data.resource_type == ResourceType::PREFETCH) {
+ load_flags |= (net::LOAD_PREFETCH | net::LOAD_DO_NOT_PROMPT_FOR_LOGIN);
+ } else if (request_data.resource_type == ResourceType::FAVICON) {
+ load_flags |= net::LOAD_DO_NOT_PROMPT_FOR_LOGIN;
+ }
+
+ if (sync_result)
+ load_flags |= net::LOAD_IGNORE_LIMITS;
+
+ // Raw headers are sensitive, as they inclide Cookie/Set-Cookie, so only
+ // allow requesting them if requestor has ReadRawCookies permission.
+ if ((load_flags & net::LOAD_REPORT_RAW_HEADERS)
+ && !ChildProcessSecurityPolicy::GetInstance()->
+ CanReadRawCookies(child_id)) {
+ VLOG(1) << "Denied unathorized request for raw headers";
+ load_flags &= ~net::LOAD_REPORT_RAW_HEADERS;
+ }
+
+ request->set_load_flags(load_flags);
+ }
+
request->set_context(
filter_->GetURLRequestContext(request_data.resource_type));
request->set_priority(DetermineRequestPriority(request_data.resource_type));
@@ -626,7 +662,13 @@ void ResourceDispatcherHost::BeginRequest(
request, resource_context.appcache_service(), child_id,
request_data.appcache_host_id, request_data.resource_type);
- BeginRequestInternal(request);
+ if (deferred_request) {
+ GlobalRequestID global_id(extra_info->child_id(), extra_info->request_id());
+ pending_requests_[global_id] = request;
+ request->FollowDeferredRedirect();
+ } else {
+ BeginRequestInternal(request);
+ }
}
void ResourceDispatcherHost::OnReleaseDownloadedFile(int request_id) {
@@ -1185,6 +1227,13 @@ void ResourceDispatcherHost::RemovePendingRequest(
const PendingRequestList::iterator& iter) {
ResourceDispatcherHostRequestInfo* info = InfoForRequest(iter->second);
+ // This is called via CancelRequestsForRoute to remove all requests for a
+ // closing tab. We don't want to remove requests that are being transferred
+ // to a new tab.
+ if (info->child_id() == g_deferred_child_id &&
+ info->request_id() == g_deferred_request_id)
+ return;
+
// Remove the memory credit that we added when pushing the request onto
// the pending list.
IncrementOutstandingRequestsMemoryCost(-1 * info->memory_cost(),
@@ -1205,8 +1254,12 @@ void ResourceDispatcherHost::RemovePendingRequest(
update_load_states_timer_.Stop();
}
+// TODO(mpcomplete): ILLEGAL! remove this
+#include "chrome/browser/profiles/profile_io_data.h"
+#include "chrome/browser/extensions/extension_info_map.h"
// net::URLRequest::Delegate ---------------------------------------------------
+
void ResourceDispatcherHost::OnReceivedRedirect(net::URLRequest* request,
const GURL& new_url,
bool* defer_redirect) {
@@ -1237,6 +1290,50 @@ void ResourceDispatcherHost::OnReceivedRedirect(net::URLRequest* request,
RemovePendingRequest(info->child_id(), info->request_id());
return;
}
+
+ // If we're redirecting into an extension process, we want to switch
+ // processes. We do this by cancelling the redirect and reissuing the request,
+ // so that the navigation controller properly assigns the right process to
+ // host the new URL.
+ const content::ResourceContext& resource_context = *info->context();
+ ProfileIOData* io_data =
+ reinterpret_cast<ProfileIOData*>(resource_context.GetUserData(NULL));
+
+ // TODO(mpcomplete): Step 1.
+ // This code should move into a resource handler instead. See the call to
+ // delegate_->RequestBeginning below - the resource handler should live in
+ // chrome/ so that it can reference extensions.
+ if (io_data->GetExtensionInfoMap()->extensions().GetByURL(new_url) !=
+ io_data->GetExtensionInfoMap()->extensions().GetByURL(request->url())) {
+ int render_process_id, render_view_id;
+ if (RenderViewForRequest(request, &render_process_id, &render_view_id)) {
+ // TODO(mpcomplete): Step 2.
+ // This is uber hacky to short-circuit all the plumbing gunk we need to
+ // do. We'll have to thread the child_id/request_id pair through the
+ // navigation calls from browser to renderer and back again:
+ // 1. new RVHD method that TabContents implements, like TransferNavigation
+ // that issues a new navigation in a new renderer but tells it the ID
+ // pair. Use NavigationController::LoadURL to load the request.
+ // 2. thread the ID pair through
+ // [browser] NavigationEntry,
+ // [browser->renderer] ViewMsg_Navigate,
+ // [renderer] NavigationState -> RequestExtraData,
+ // [renderer->browser] ResourceHostMsg_RequestResource
+ // 3. In ResourceDispatcherHost::BeginRequest (which handles the message
+ // above), check if it is a transferred navigation. If so, proceed as I do
+ // below when g_deferred_url is non-empty.
+
+ g_deferred_url = new_url;
+ g_deferred_child_id = render_process_id;
+ g_deferred_request_id = info->request_id();
+ CallRenderViewHostDelegate(
+ render_process_id, render_view_id,
+ &RenderViewHostDelegate::RequestOpenURL,
+ new_url, GURL(request->referrer()), CURRENT_TAB, info->frame_id_);
+ *defer_redirect = true;
+ return;
+ }
+ }
scoped_refptr<ResourceResponse> response(new ResourceResponse);
PopulateResourceResponse(request, response);
@@ -1417,6 +1514,14 @@ bool ResourceDispatcherHost::CompleteResponseStarted(net::URLRequest* request) {
void ResourceDispatcherHost::CancelRequest(int child_id,
int request_id,
bool from_renderer) {
+ if (from_renderer &&
+ child_id == g_deferred_child_id &&
+ request_id == g_deferred_request_id) {
+ // When the old renderer dies, it sends a message to us to cancel its
+ // requests.
+ return;
+ }
+
PendingRequestList::iterator i = pending_requests_.find(
GlobalRequestID(child_id, request_id));
if (i == pending_requests_.end()) {
@@ -1480,7 +1585,7 @@ int ResourceDispatcherHost::IncrementOutstandingRequestsMemoryCost(
new_cost += cost;
CHECK(new_cost >= 0);
if (new_cost == 0)
- outstanding_requests_memory_cost_map_.erase(prev_entry);
+ outstanding_requests_memory_cost_map_.erase(child_id);
else
outstanding_requests_memory_cost_map_[child_id] = new_cost;

Powered by Google App Engine
This is Rietveld 408576698