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

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

Issue 464593003: Don't swap out the old RenderFrameHost until the new one commits. (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: Rebase past PlzNavigate CL Created 6 years, 4 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/cross_site_resource_handler.cc
diff --git a/content/browser/loader/cross_site_resource_handler.cc b/content/browser/loader/cross_site_resource_handler.cc
index 6efe97d297bf7a6fe288ee82c96a2c64a5c5ca3c..664f34917d21ffc4013e4d0f7d9e1eb06430a57c 100644
--- a/content/browser/loader/cross_site_resource_handler.cc
+++ b/content/browser/loader/cross_site_resource_handler.cc
@@ -11,11 +11,11 @@
#include "base/logging.h"
#include "content/browser/appcache/appcache_interceptor.h"
#include "content/browser/child_process_security_policy_impl.h"
-#include "content/browser/cross_site_request_manager.h"
#include "content/browser/frame_host/cross_site_transferring_request.h"
#include "content/browser/frame_host/render_frame_host_impl.h"
#include "content/browser/loader/resource_dispatcher_host_impl.h"
#include "content/browser/loader/resource_request_info_impl.h"
+#include "content/browser/site_instance_impl.h"
#include "content/browser/transition_request_manager.h"
#include "content/public/browser/browser_thread.h"
#include "content/public/browser/content_browser_client.h"
@@ -40,14 +40,12 @@ struct CrossSiteResponseParams {
CrossSiteResponseParams(
int render_frame_id,
const GlobalRequestID& global_request_id,
- bool is_transfer,
const std::vector<GURL>& transfer_url_chain,
const Referrer& referrer,
PageTransition page_transition,
bool should_replace_current_entry)
: render_frame_id(render_frame_id),
global_request_id(global_request_id),
- is_transfer(is_transfer),
transfer_url_chain(transfer_url_chain),
referrer(referrer),
page_transition(page_transition),
@@ -56,7 +54,6 @@ struct CrossSiteResponseParams {
int render_frame_id;
GlobalRequestID global_request_id;
- bool is_transfer;
std::vector<GURL> transfer_url_chain;
Referrer referrer;
PageTransition page_transition;
@@ -64,11 +61,8 @@ struct CrossSiteResponseParams {
};
void OnCrossSiteResponseHelper(const CrossSiteResponseParams& params) {
- scoped_ptr<CrossSiteTransferringRequest> cross_site_transferring_request;
- if (params.is_transfer) {
- cross_site_transferring_request.reset(new CrossSiteTransferringRequest(
- params.global_request_id));
- }
+ scoped_ptr<CrossSiteTransferringRequest> cross_site_transferring_request(
+ new CrossSiteTransferringRequest(params.global_request_id));
RenderFrameHostImpl* rfh =
RenderFrameHostImpl::FromID(params.global_request_id.child_id,
@@ -95,7 +89,9 @@ void OnDeferredAfterResponseStartedHelper(
rfh->OnDeferredAfterResponseStarted(global_request_id, transition_data);
}
+// Returns whether a transfer is needed by doing a check on the UI thread.
bool CheckNavigationPolicyOnUI(GURL url, int process_id, int render_frame_id) {
+ CHECK(CommandLine::ForCurrentProcess()->HasSwitch(switches::kSitePerProcess));
RenderFrameHostImpl* rfh =
RenderFrameHostImpl::FromID(process_id, render_frame_id);
if (!rfh)
@@ -176,10 +172,11 @@ bool CrossSiteResourceHandler::OnNormalResponseStarted(
ResourceRequestInfoImpl* info = GetRequestInfo();
- // We will need to swap processes if either (1) a redirect that requires a
- // transfer occurred before we got here, or (2) a pending cross-site request
- // was already in progress. Note that a swap may no longer be needed if we
- // transferred back into the original process due to a redirect.
+ // We only need to pause the response if a transfer to a different process is
+ // required. Other cross-process navigations can proceed immediately, since
+ // we run the unload handler at commit time.
+ // Note that a process swap may no longer be necessary if we transferred back
+ // into the original process due to a redirect.
bool should_transfer =
GetContentClient()->browser()->ShouldSwapProcessesForRedirect(
info->GetContext(), request()->original_url(), request()->url());
@@ -196,11 +193,6 @@ bool CrossSiteResourceHandler::OnNormalResponseStarted(
return DeferForNavigationPolicyCheck(info, response, defer);
}
- bool swap_needed =
- should_transfer ||
- CrossSiteRequestManager::GetInstance()->HasPendingCrossSiteRequest(
- info->GetChildID(), info->GetRenderFrameID());
-
// If this is a download, just pass the response through without doing a
// cross-site check. The renderer will see it is a download and abort the
// request.
@@ -216,18 +208,18 @@ bool CrossSiteResourceHandler::OnNormalResponseStarted(
//
// TODO(davidben): Unify IsDownload() and is_stream(). Several places need to
// check for both and remembering about streams is error-prone.
- if (!swap_needed || info->IsDownload() || info->is_stream() ||
+ if (!should_transfer || info->IsDownload() || info->is_stream() ||
(response->head.headers.get() &&
response->head.headers->response_code() == 204)) {
return next_handler_->OnResponseStarted(response, defer);
}
- // Now that we know a swap is needed and we have something to commit, we
- // pause to let the UI thread run the unload handler of the previous page
- // and set up a transfer if needed.
- StartCrossSiteTransition(response, should_transfer);
+ // Now that we know a transfer is needed and we have something to commit, we
+ // pause to let the UI thread set up the transfer.
+ StartCrossSiteTransition(response);
- // Defer loading until after the onunload event handler has run.
+ // Defer loading until after the new renderer process has issued a
+ // corresponding request.
*defer = true;
OnDidDefer();
return true;
@@ -266,7 +258,7 @@ void CrossSiteResourceHandler::ResumeResponseDeferredAtStart(int request_id) {
void CrossSiteResourceHandler::ResumeOrTransfer(bool is_transfer) {
if (is_transfer) {
- StartCrossSiteTransition(response_, is_transfer);
+ StartCrossSiteTransition(response_);
} else {
ResumeResponse();
}
@@ -282,23 +274,9 @@ void CrossSiteResourceHandler::OnResponseCompleted(
const std::string& security_info,
bool* defer) {
if (!in_cross_site_transition_) {
- ResourceRequestInfoImpl* info = GetRequestInfo();
- // If we've already completed the transition, or we're canceling the
- // request, or an error occurred with no cross-process navigation in
- // progress, then we should just pass this through.
- if (has_started_response_ ||
- status.status() != net::URLRequestStatus::FAILED ||
- !CrossSiteRequestManager::GetInstance()->HasPendingCrossSiteRequest(
- info->GetChildID(), info->GetRenderFrameID())) {
- next_handler_->OnResponseCompleted(status, security_info, defer);
- return;
- }
-
- // An error occurred. We should wait now for the cross-process transition,
- // so that the error message (e.g., 404) can be displayed to the user.
- // Also continue with the logic below to remember that we completed
- // during the cross-site transition.
- StartCrossSiteTransition(NULL, false);
+ // If we're not transferring, then we should pass this through.
+ next_handler_->OnResponseCompleted(status, security_info, defer);
+ return;
}
// We have to buffer the call until after the transition completes.
@@ -353,11 +331,9 @@ void CrossSiteResourceHandler::SetLeakRequestsForTesting(
leak_requests_for_testing_ = leak_requests_for_testing;
}
-// Prepare to render the cross-site response in a new RenderFrameHost, by
-// telling the old RenderFrameHost to run its onunload handler.
+// Prepare to transfer the response to a new RenderFrameHost.
void CrossSiteResourceHandler::StartCrossSiteTransition(
- ResourceResponse* response,
- bool should_transfer) {
+ ResourceResponse* response) {
in_cross_site_transition_ = true;
response_ = response;
@@ -377,14 +353,13 @@ void CrossSiteResourceHandler::StartCrossSiteTransition(
std::vector<GURL> transfer_url_chain;
Referrer referrer;
int render_frame_id = info->GetRenderFrameID();
- if (should_transfer) {
- transfer_url_chain = request()->url_chain();
- referrer = Referrer(GURL(request()->referrer()), info->GetReferrerPolicy());
+ transfer_url_chain = request()->url_chain();
+ referrer = Referrer(GURL(request()->referrer()), info->GetReferrerPolicy());
+
+ AppCacheInterceptor::PrepareForCrossSiteTransfer(
+ request(), global_id.child_id);
+ ResourceDispatcherHostImpl::Get()->MarkAsTransferredNavigation(global_id);
- AppCacheInterceptor::PrepareForCrossSiteTransfer(
- request(), global_id.child_id);
- ResourceDispatcherHostImpl::Get()->MarkAsTransferredNavigation(global_id);
- }
BrowserThread::PostTask(
BrowserThread::UI,
FROM_HERE,
@@ -392,7 +367,6 @@ void CrossSiteResourceHandler::StartCrossSiteTransition(
&OnCrossSiteResponseHelper,
CrossSiteResponseParams(render_frame_id,
global_id,
- should_transfer,
transfer_url_chain,
referrer,
info->GetPageTransition(),
« no previous file with comments | « content/browser/loader/cross_site_resource_handler.h ('k') | content/browser/loader/resource_dispatcher_host_impl.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698