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

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

Issue 1384113002: CrossSiteResourceHandler: cancel request if the RFH is gone (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@no_isolate_apps
Patch Set: Add EXPECT_TRUE's Created 5 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/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 7ffb659744a03ca22070a0c390e2fb18d7b86367..b05ac5d8e4f13255d7839e98d88f8312a9af8dd1 100644
--- a/content/browser/loader/cross_site_resource_handler.cc
+++ b/content/browser/loader/cross_site_resource_handler.cc
@@ -86,16 +86,18 @@ void OnCrossSiteResponseHelper(const CrossSiteResponseParams& params) {
}
}
-// Returns whether a transfer is needed by doing a check on the UI thread.
-bool CheckNavigationPolicyOnUI(GURL real_url,
- int process_id,
- int render_frame_id) {
- CHECK(SiteIsolationPolicy::AreCrossProcessFramesPossible());
- RenderFrameHostImpl* rfh =
- RenderFrameHostImpl::FromID(process_id, render_frame_id);
- if (!rfh)
- return false;
-
+// Determines whether a navigation to |dest_url| may be completed using an
+// existing RenderFrameHost, or whether transferring to a new RenderFrameHost
+// backed by a different render process is required. This is a security policy
+// check determined by the current site isolation mode, and must be done
+// before the resource at |dest_url| is delivered to |rfh|.
+//
+// When this function returns true for a subframe, an out-of-process iframe
+// must be created.
+//
+// TODO(nick): Move this function to RFHM.
+bool IsRendererTransferNeededForNavigation(RenderFrameHostImpl* rfh,
+ const GURL& dest_url) {
// A transfer is not needed if the current SiteInstance doesn't yet have a
// site. This is the case for tests that use NavigateToURL.
if (!rfh->GetSiteInstance()->HasSite())
@@ -108,14 +110,14 @@ bool CheckNavigationPolicyOnUI(GURL real_url,
return false;
GURL effective_url = SiteInstanceImpl::GetEffectiveURL(
- rfh->GetSiteInstance()->GetBrowserContext(), real_url);
+ rfh->GetSiteInstance()->GetBrowserContext(), dest_url);
// TODO(nasko, nick): These following --site-per-process checks are
// overly simplistic. Update them to match all the cases
// considered by RenderFrameHostManager::DetermineSiteInstanceForURL.
if (SiteInstance::IsSameWebSite(rfh->GetSiteInstance()->GetBrowserContext(),
rfh->GetSiteInstance()->GetSiteURL(),
- real_url)) {
+ dest_url)) {
return false; // The same site, no transition needed.
}
@@ -125,6 +127,25 @@ bool CheckNavigationPolicyOnUI(GURL real_url,
SiteIsolationPolicy::DoesSiteRequireDedicatedProcess(effective_url);
}
+// Returns whether a transfer is needed by doing a check on the UI thread.
+CrossSiteResourceHandler::NavigationDecision
+CheckNavigationPolicyOnUI(GURL real_url, int process_id, int render_frame_id) {
+ CHECK(SiteIsolationPolicy::AreCrossProcessFramesPossible());
+ RenderFrameHostImpl* rfh =
+ RenderFrameHostImpl::FromID(process_id, render_frame_id);
+
+ // Without a valid RFH against which to check, we must cancel the request,
+ // to prevent the resource at |url| from being delivered to a potentially
+ // unsuitable renderer process.
+ if (!rfh)
+ return CrossSiteResourceHandler::NavigationDecision::CANCEL_REQUEST;
+
+ if (IsRendererTransferNeededForNavigation(rfh, real_url))
+ return CrossSiteResourceHandler::NavigationDecision::TRANSFER_REQUIRED;
+ else
+ return CrossSiteResourceHandler::NavigationDecision::USE_EXISTING_RENDERER;
+}
+
} // namespace
CrossSiteResourceHandler::CrossSiteResourceHandler(
@@ -236,11 +257,18 @@ bool CrossSiteResourceHandler::OnNormalResponseStarted(
return next_handler_->OnResponseStarted(response, defer);
}
-void CrossSiteResourceHandler::ResumeOrTransfer(bool is_transfer) {
- if (is_transfer) {
- StartCrossSiteTransition(response_.get());
- } else {
- ResumeResponse();
+void CrossSiteResourceHandler::ResumeOrTransfer(NavigationDecision decision) {
+ switch (decision) {
+ case NavigationDecision::CANCEL_REQUEST:
+ // TODO(nick): What kind of cleanup do we need here?
+ controller()->Cancel();
+ break;
+ case NavigationDecision::USE_EXISTING_RENDERER:
+ ResumeResponse();
+ break;
+ case NavigationDecision::TRANSFER_REQUIRED:
+ StartCrossSiteTransition(response_.get());
+ break;
}
}

Powered by Google App Engine
This is Rietveld 408576698