When cross-site navigations are cancelled, delete the

ResourceLoader that would have been trasferred to the
new renderer. These were being leaked, and would keep
a lock on disk cache entries, making a page impossible
to visit after a cancelled cross site navigation to it.

BUG=341134

Review URL: https://codereview.chromium.org/143183009

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@251561 0039d316-1c4b-4281-b951-d872f2087c98

15 files changed, 483 insertions, 69 deletions

diff --git a/content/browser/frame_host/render_frame_host_manager.cc b/content/browser/frame_host/render_frame_host_manager.cc
index ef68d60..c871e04 100644
--- a/content/browser/frame_host/render_frame_host_manager.cc
+++ b/content/browser/frame_host/render_frame_host_manager.cc
@@ -18,6 +18,7 @@
 #include "content/browser/frame_host/navigation_entry_impl.h"
 #include "content/browser/frame_host/render_frame_host_factory.h"
 #include "content/browser/frame_host/render_frame_host_impl.h"
+#include "content/browser/renderer_host/cross_site_transferring_request.h"
 #include "content/browser/renderer_host/render_process_host_impl.h"
 #include "content/browser/renderer_host/render_view_host_factory.h"
 #include "content/browser/renderer_host/render_view_host_impl.h"
@@ -37,20 +38,16 @@
 
 namespace content {
 
-RenderFrameHostManager::PendingNavigationParams::PendingNavigationParams()
-    : is_transfer(false), frame_id(-1), should_replace_current_entry(false) {
-}
-
 RenderFrameHostManager::PendingNavigationParams::PendingNavigationParams(
     const GlobalRequestID& global_request_id,
-    bool is_transfer,
+    scoped_ptr<CrossSiteTransferringRequest> cross_site_transferring_request,
     const std::vector<GURL>& transfer_url_chain,
     Referrer referrer,
     PageTransition page_transition,
     int64 frame_id,
     bool should_replace_current_entry)
     : global_request_id(global_request_id),
-      is_transfer(is_transfer),
+      cross_site_transferring_request(cross_site_transferring_request.Pass()),
       transfer_url_chain(transfer_url_chain),
       referrer(referrer),
       page_transition(page_transition),
@@ -207,6 +204,15 @@ RenderFrameHostImpl* RenderFrameHostManager::Navigate(
     }
   }
 
+  // If entry includes the request ID of a request that is being transferred,
+  // the destination render frame will take ownership, so release ownership of
+  // the request.
+  if (pending_nav_params_ &&
+      pending_nav_params_->global_request_id ==
+          entry.transferred_global_request_id()) {
+    pending_nav_params_->cross_site_transferring_request->ReleaseRequest();
+  }
+
   return dest_render_frame_host;
 }
 
@@ -279,7 +285,7 @@ void RenderFrameHostManager::SwappedOut(RenderViewHost* render_view_host) {
   // TODO(creis): The blank swapped out page is visible during this time, but
   // we can shorten this by delivering the response directly, rather than
   // forcing an identical request to be made.
-  if (pending_nav_params_->is_transfer) {
+  if (pending_nav_params_->cross_site_transferring_request) {
     // Treat the last URL in the chain as the destination and the remainder as
     // the redirect chain.
     CHECK(pending_nav_params_->transfer_url_chain.size());
@@ -328,7 +334,7 @@ void RenderFrameHostManager::SwappedOutFrame(
   // TODO(creis): The blank swapped out page is visible during this time, but
   // we can shorten this by delivering the response directly, rather than
   // forcing an identical request to be made.
-  if (pending_nav_params_->is_transfer) {
+  if (pending_nav_params_->cross_site_transferring_request) {
     // Treat the last URL in the chain as the destination and the remainder as
     // the redirect chain.
     CHECK(pending_nav_params_->transfer_url_chain.size());
@@ -480,7 +486,7 @@ void RenderFrameHostManager::ShouldClosePage(
 void RenderFrameHostManager::OnCrossSiteResponse(
     RenderViewHost* pending_render_view_host,
     const GlobalRequestID& global_request_id,
-    bool is_transfer,
+    scoped_ptr<CrossSiteTransferringRequest> cross_site_transferring_request,
     const std::vector<GURL>& transfer_url_chain,
     const Referrer& referrer,
     PageTransition page_transition,
@@ -496,10 +502,12 @@ void RenderFrameHostManager::OnCrossSiteResponse(
   // here, but currently we pass information for a transfer if
   // ShouldSwapProcessesForRedirect returned true in the network stack.
   // In that case, we should set up a transfer after the unload handler runs.
-  // If is_transfer is false, we will just run the unload handler and resume.
+  // If |cross_site_transferring_request| is NULL, we will just run the unload
+  // handler and resume.
   pending_nav_params_.reset(new PendingNavigationParams(
-      global_request_id, is_transfer, transfer_url_chain, referrer,
-      page_transition, frame_id, should_replace_current_entry));
+      global_request_id, cross_site_transferring_request.Pass(),
+      transfer_url_chain, referrer, page_transition, frame_id,
+      should_replace_current_entry));
 
   // Run the unload handler of the current page.
   SwapOutOldPage();
diff --git a/content/browser/frame_host/render_frame_host_manager.h b/content/browser/frame_host/render_frame_host_manager.h
index 5e23c58..768532a8 100644
--- a/content/browser/frame_host/render_frame_host_manager.h
+++ b/content/browser/frame_host/render_frame_host_manager.h
@@ -21,6 +21,7 @@
 namespace content {
 class BrowserContext;
 class CrossProcessFrameConnector;
+class CrossSiteTransferringRequest;
 class InterstitialPageImpl;
 class FrameTreeNode;
 class NavigationControllerImpl;
@@ -239,7 +240,7 @@ class CONTENT_EXPORT RenderFrameHostManager
   virtual void OnCrossSiteResponse(
       RenderViewHost* pending_render_view_host,
       const GlobalRequestID& global_request_id,
-      bool is_transfer,
+      scoped_ptr<CrossSiteTransferringRequest> cross_site_transferring_request,
       const std::vector<GURL>& transfer_url_chain,
       const Referrer& referrer,
       PageTransition page_transition,
@@ -280,44 +281,46 @@ class CONTENT_EXPORT RenderFrameHostManager
 
   // Tracks information about a navigation while a cross-process transition is
   // in progress, in case we need to transfer it to a new RenderFrameHost.
+  // When a request is being transferred, deleting the PendingNavigationParams,
+  // and thus |cross_site_transferring_request|, will cancel the request being
+  // transferred, unless its ReleaseRequest method has been called.
   struct PendingNavigationParams {
-    PendingNavigationParams();
-    PendingNavigationParams(const GlobalRequestID& global_request_id,
-                            bool is_transfer,
-                            const std::vector<GURL>& transfer_url,
-                            Referrer referrer,
-                            PageTransition page_transition,
-                            int64 frame_id,
-                            bool should_replace_current_entry);
+    PendingNavigationParams(
+        const GlobalRequestID& global_request_id,
+        scoped_ptr<CrossSiteTransferringRequest>
+            cross_site_transferring_request,
+        const std::vector<GURL>& transfer_url,
+        Referrer referrer,
+        PageTransition page_transition,
+        int64 frame_id,
+        bool should_replace_current_entry);
     ~PendingNavigationParams();
 
     // The child ID and request ID for the pending navigation.  Present whether
-    // |is_transfer| is true or false.
+    // |request_transfer| is NULL or not.
     GlobalRequestID global_request_id;
 
-    // Whether this pending navigation needs to be transferred to another
-    // process than the one it was going to commit in.  If so, the
-    // |transfer_url|, |referrer|, and |frame_id| parameters will be set.
-    bool is_transfer;
+    // If a pending request needs to be transferred to another process, this
+    // owns the request until it's transferred to the new process, so it will be
+    // cleaned up if the navigation is cancelled.  Otherwise, this is NULL.
+    scoped_ptr<CrossSiteTransferringRequest> cross_site_transferring_request;
 
-    // If |is_transfer|, this is the URL chain of the request.  The first entry
-    // is the original request URL, and the last entry is the destination URL to
-    // request in the new process.
+    // If |request_transfer| is non-NULL, the values below are all set.
+
+    // The first entry is the original request URL, and the last entry is the
+    // destination URL to request in the new process.
     std::vector<GURL> transfer_url_chain;
 
-    // If |is_transfer|, this is the referrer to use for the request in the new
-    // process.
+    // This is the referrer to use for the request in the new process.
     Referrer referrer;
 
-    // If |is_transfer|, this is the transition type for the original
-    // navigation.
+    // This is the transition type for the original navigation.
     PageTransition page_transition;
 
-    // If |is_transfer|, this is the frame ID to use in RequestTransferURL.
+    // This is the frame ID to use in RequestTransferURL.
     int64 frame_id;
 
-    // If |is_transfer|, this is whether the navigation should replace the
-    // current history entry.
+    // This is whether the navigation should replace the current history entry.
     bool should_replace_current_entry;
   };
 
diff --git a/content/browser/frame_host/render_frame_host_manager_unittest.cc b/content/browser/frame_host/render_frame_host_manager_unittest.cc
index 40eea0b..2a82834 100644
--- a/content/browser/frame_host/render_frame_host_manager_unittest.cc
+++ b/content/browser/frame_host/render_frame_host_manager_unittest.cc
@@ -7,6 +7,7 @@
 #include "content/browser/frame_host/navigation_entry_impl.h"
 #include "content/browser/frame_host/navigator.h"
 #include "content/browser/frame_host/render_frame_host_manager.h"
+#include "content/browser/renderer_host/cross_site_transferring_request.h"
 #include "content/browser/site_instance_impl.h"
 #include "content/browser/webui/web_ui_controller_factory_registry.h"
 #include "content/common/frame_messages.h"
@@ -1394,8 +1395,8 @@ TEST_F(RenderFrameHostManagerTest,
   std::vector<GURL> url_chain;
   url_chain.push_back(GURL());
   contents()->GetRenderManagerForTesting()->OnCrossSiteResponse(
-      rvh2, GlobalRequestID(0, 0), false, url_chain, Referrer(),
-      PAGE_TRANSITION_TYPED, 1, false);
+      rvh2, GlobalRequestID(0, 0), scoped_ptr<CrossSiteTransferringRequest>(),
+      url_chain, Referrer(), PAGE_TRANSITION_TYPED, 1, false);
   EXPECT_TRUE(contents()->cross_navigation_pending());
   EXPECT_EQ(RenderViewHostImpl::STATE_WAITING_FOR_UNLOAD_ACK,
             rvh1->rvh_state());
@@ -1447,8 +1448,8 @@ TEST_F(RenderFrameHostManagerTest,
   std::vector<GURL> url_chain;
   url_chain.push_back(GURL());
   contents()->GetRenderManagerForTesting()->OnCrossSiteResponse(
-      rvh2, GlobalRequestID(0, 0), false, url_chain, Referrer(),
-      PAGE_TRANSITION_TYPED, 1, false);
+      rvh2, GlobalRequestID(0, 0), scoped_ptr<CrossSiteTransferringRequest>(),
+      url_chain, Referrer(), PAGE_TRANSITION_TYPED, 1, false);
   EXPECT_TRUE(contents()->cross_navigation_pending());
   EXPECT_EQ(RenderViewHostImpl::STATE_WAITING_FOR_UNLOAD_ACK,
             rvh1->rvh_state());
@@ -1495,8 +1496,8 @@ TEST_F(RenderFrameHostManagerTest,
   std::vector<GURL> url_chain;
   url_chain.push_back(GURL());
   contents()->GetRenderManagerForTesting()->OnCrossSiteResponse(
-      rvh2, GlobalRequestID(0, 0), false, url_chain, Referrer(),
-      PAGE_TRANSITION_TYPED, 1, false);
+      rvh2, GlobalRequestID(0, 0), scoped_ptr<CrossSiteTransferringRequest>(),
+      url_chain, Referrer(), PAGE_TRANSITION_TYPED, 1, false);
   EXPECT_TRUE(contents()->cross_navigation_pending());
   EXPECT_EQ(RenderViewHostImpl::STATE_WAITING_FOR_UNLOAD_ACK,
             rvh1->rvh_state());
@@ -1548,8 +1549,8 @@ TEST_F(RenderFrameHostManagerTest,
   std::vector<GURL> url_chain;
   url_chain.push_back(GURL());
   contents()->GetRenderManagerForTesting()->OnCrossSiteResponse(
-      rvh2, GlobalRequestID(0, 0), false, url_chain, Referrer(),
-      PAGE_TRANSITION_TYPED, 1, false);
+      rvh2, GlobalRequestID(0, 0), scoped_ptr<CrossSiteTransferringRequest>(),
+      url_chain, Referrer(), PAGE_TRANSITION_TYPED, 1, false);
   EXPECT_TRUE(contents()->cross_navigation_pending());
   EXPECT_EQ(RenderViewHostImpl::STATE_WAITING_FOR_UNLOAD_ACK,
             rvh1->rvh_state());
diff --git a/content/browser/loader/cross_site_resource_handler.cc b/content/browser/loader/cross_site_resource_handler.cc
index 75009a29..2ed007a 100644
--- a/content/browser/loader/cross_site_resource_handler.cc
+++ b/content/browser/loader/cross_site_resource_handler.cc
@@ -13,6 +13,7 @@
 #include "content/browser/cross_site_request_manager.h"
 #include "content/browser/loader/resource_dispatcher_host_impl.h"
 #include "content/browser/loader/resource_request_info_impl.h"
+#include "content/browser/renderer_host/cross_site_transferring_request.h"
 #include "content/browser/renderer_host/render_view_host_delegate.h"
 #include "content/browser/renderer_host/render_view_host_impl.h"
 #include "content/public/browser/browser_thread.h"
@@ -30,17 +31,20 @@ namespace content {
 
 namespace {
 
+bool leak_requests_for_testing_ = false;
+
 // The parameters to OnCrossSiteResponseHelper exceed the number of arguments
 // base::Bind supports.
 struct CrossSiteResponseParams {
-  CrossSiteResponseParams(int render_view_id,
-                          const GlobalRequestID& global_request_id,
-                          bool is_transfer,
-                          const std::vector<GURL>& transfer_url_chain,
-                          const Referrer& referrer,
-                          PageTransition page_transition,
-                          int64 frame_id,
-                          bool should_replace_current_entry)
+  CrossSiteResponseParams(
+      int render_view_id,
+      const GlobalRequestID& global_request_id,
+      bool is_transfer,
+      const std::vector<GURL>& transfer_url_chain,
+      const Referrer& referrer,
+      PageTransition page_transition,
+      int64 frame_id,
+      bool should_replace_current_entry)
       : render_view_id(render_view_id),
         global_request_id(global_request_id),
         is_transfer(is_transfer),
@@ -62,15 +66,25 @@ 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));
+  }
+
   RenderViewHostImpl* rvh =
       RenderViewHostImpl::FromID(params.global_request_id.child_id,
                                  params.render_view_id);
   if (rvh) {
     rvh->OnCrossSiteResponse(
-        params.global_request_id, params.is_transfer,
+        params.global_request_id, cross_site_transferring_request.Pass(),
         params.transfer_url_chain, params.referrer,
         params.page_transition, params.frame_id,
         params.should_replace_current_entry);
+  } else if (leak_requests_for_testing_ && cross_site_transferring_request) {
+    // Some unit tests expect requests to be leaked in this case, so they can
+    // pass them along manually.
+    cross_site_transferring_request->ReleaseRequest();
   }
 }
 
@@ -83,8 +97,7 @@ CrossSiteResourceHandler::CrossSiteResourceHandler(
       has_started_response_(false),
       in_cross_site_transition_(false),
       completed_during_transition_(false),
-      did_defer_(false),
-      completed_status_() {
+      did_defer_(false) {
 }
 
 CrossSiteResourceHandler::~CrossSiteResourceHandler() {
@@ -263,6 +276,12 @@ void CrossSiteResourceHandler::ResumeResponse() {
   }
 }
 
+// static
+void CrossSiteResourceHandler::SetLeakRequestsForTesting(
+    bool leak_requests_for_testing) {
+  leak_requests_for_testing_ = leak_requests_for_testing;
+}
+
 // Prepare to render the cross-site response in a new RenderViewHost, by
 // telling the old RenderViewHost to run its onunload handler.
 void CrossSiteResourceHandler::StartCrossSiteTransition(
diff --git a/content/browser/loader/cross_site_resource_handler.h b/content/browser/loader/cross_site_resource_handler.h
index e26c1d5..9f487a6 100644
--- a/content/browser/loader/cross_site_resource_handler.h
+++ b/content/browser/loader/cross_site_resource_handler.h
@@ -7,6 +7,7 @@
 
 #include "base/memory/ref_counted.h"
 #include "content/browser/loader/layered_resource_handler.h"
+#include "content/common/content_export.h"
 #include "net/url_request/url_request_status.h"
 
 namespace net {
@@ -46,6 +47,11 @@ class CrossSiteResourceHandler : public LayeredResourceHandler {
   // WebContentsImpl to swap in the new renderer and destroy the old one.
   void ResumeResponse();
 
+  // When set to true, requests are leaked when they can't be passed to a
+  // RenderViewHost, for unit tests.
+  CONTENT_EXPORT static void SetLeakRequestsForTesting(
+      bool leak_requests_for_testing);
+
  private:
   // Prepare to render the cross-site response in a new RenderViewHost, by
   // telling the old RenderViewHost to run its onunload handler.
diff --git a/content/browser/loader/resource_dispatcher_host_impl.cc b/content/browser/loader/resource_dispatcher_host_impl.cc
index 04772e1..653b2db 100644
--- a/content/browser/loader/resource_dispatcher_host_impl.cc
+++ b/content/browser/loader/resource_dispatcher_host_impl.cc
@@ -1383,6 +1383,13 @@ void ResourceDispatcherHostImpl::MarkAsTransferredNavigation(
   GetLoader(id)->MarkAsTransferring();
 }
 
+void ResourceDispatcherHostImpl::CancelTransferringNavigation(
+    const GlobalRequestID& id) {
+  // Request should still exist and be in the middle of a transfer.
+  DCHECK(IsTransferredNavigation(id));
+  RemovePendingRequest(id.child_id, id.request_id);
+}
+
 void ResourceDispatcherHostImpl::ResumeDeferredNavigation(
     const GlobalRequestID& id) {
   ResourceLoader* loader = GetLoader(id);
diff --git a/content/browser/loader/resource_dispatcher_host_impl.h b/content/browser/loader/resource_dispatcher_host_impl.h
index d85a24f..8a8bdc3 100644
--- a/content/browser/loader/resource_dispatcher_host_impl.h
+++ b/content/browser/loader/resource_dispatcher_host_impl.h
@@ -130,6 +130,10 @@ class CONTENT_EXPORT ResourceDispatcherHostImpl
   // redirected cross-site and needs to be resumed by a new render view.
   void MarkAsTransferredNavigation(const GlobalRequestID& id);
 
+  // Cancels a request previously marked as being transferred, for use when a
+  // navigation was cancelled.
+  void CancelTransferringNavigation(const GlobalRequestID& id);
+
   // Resumes the request without transferring it to a new render view.
   void ResumeDeferredNavigation(const GlobalRequestID& id);
 
diff --git a/content/browser/loader/resource_dispatcher_host_unittest.cc b/content/browser/loader/resource_dispatcher_host_unittest.cc
index 88b4f94..ad7da47 100644
--- a/content/browser/loader/resource_dispatcher_host_unittest.cc
+++ b/content/browser/loader/resource_dispatcher_host_unittest.cc
@@ -14,6 +14,7 @@
 #include "base/strings/string_split.h"
 #include "content/browser/browser_thread_impl.h"
 #include "content/browser/child_process_security_policy_impl.h"
+#include "content/browser/loader/cross_site_resource_handler.h"
 #include "content/browser/loader/detachable_resource_handler.h"
 #include "content/browser/loader/resource_dispatcher_host_impl.h"
 #include "content/browser/loader/resource_loader.h"
@@ -2171,6 +2172,10 @@ TEST_F(ResourceDispatcherHostTest, CancelRequestsForContextTransferred) {
 // Test transferred navigations with text/html, which doesn't trigger any
 // content sniffing.
 TEST_F(ResourceDispatcherHostTest, TransferNavigationHtml) {
+  // This test expects the cross site request to be leaked, so it can transfer
+  // the request directly.
+  CrossSiteResourceHandler::SetLeakRequestsForTesting(true);
+
   EXPECT_EQ(0, host_.pending_requests());
 
   int render_view_id = 0;
@@ -2243,6 +2248,10 @@ TEST_F(ResourceDispatcherHostTest, TransferNavigationHtml) {
 // BufferedResourceHandler to buffer the response to sniff the content
 // before the transfer occurs.
 TEST_F(ResourceDispatcherHostTest, TransferNavigationText) {
+  // This test expects the cross site request to be leaked, so it can transfer
+  // the request directly.
+  CrossSiteResourceHandler::SetLeakRequestsForTesting(true);
+
   EXPECT_EQ(0, host_.pending_requests());
 
   int render_view_id = 0;
@@ -2314,6 +2323,10 @@ TEST_F(ResourceDispatcherHostTest, TransferNavigationText) {
 }
 
 TEST_F(ResourceDispatcherHostTest, TransferNavigationWithProcessCrash) {
+  // This test expects the cross site request to be leaked, so it can transfer
+  // the request directly.
+  CrossSiteResourceHandler::SetLeakRequestsForTesting(true);
+
   EXPECT_EQ(0, host_.pending_requests());
 
   int render_view_id = 0;
@@ -2404,6 +2417,10 @@ TEST_F(ResourceDispatcherHostTest, TransferNavigationWithProcessCrash) {
 }
 
 TEST_F(ResourceDispatcherHostTest, TransferNavigationWithTwoRedirects) {
+  // This test expects the cross site request to be leaked, so it can transfer
+  // the request directly.
+  CrossSiteResourceHandler::SetLeakRequestsForTesting(true);
+
   EXPECT_EQ(0, host_.pending_requests());
 
   int render_view_id = 0;
diff --git a/content/browser/renderer_host/cross_site_transferring_request.cc b/content/browser/renderer_host/cross_site_transferring_request.cc
new file mode 100644
index 0000000..47b684d
--- /dev/null
+++ b/content/browser/renderer_host/cross_site_transferring_request.cc
@@ -0,0 +1,46 @@
+// Copyright 2014 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.
+
+#include "content/browser/renderer_host/cross_site_transferring_request.h"
+
+#include "base/logging.h"
+#include "content/browser/loader/cross_site_resource_handler.h"
+#include "content/browser/loader/resource_dispatcher_host_impl.h"
+#include "content/public/browser/browser_thread.h"
+
+namespace content {
+
+namespace {
+
+void CancelRequestOnIOThread(GlobalRequestID global_request_id) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+
+  ResourceDispatcherHostImpl::Get()->CancelTransferringNavigation(
+      global_request_id);
+}
+
+}  // namespace
+
+CrossSiteTransferringRequest::CrossSiteTransferringRequest(
+    GlobalRequestID global_request_id)
+    : global_request_id_(global_request_id) {
+  DCHECK(global_request_id_ != GlobalRequestID());
+}
+
+CrossSiteTransferringRequest::~CrossSiteTransferringRequest() {
+  if (global_request_id_ == GlobalRequestID())
+    return;
+  BrowserThread::PostTask(
+      BrowserThread::IO,
+      FROM_HERE,
+      base::Bind(&CancelRequestOnIOThread, global_request_id_));
+}
+
+void CrossSiteTransferringRequest::ReleaseRequest() {
+  DCHECK_NE(-1, global_request_id_.child_id);
+  DCHECK_NE(-1, global_request_id_.request_id);
+  global_request_id_ = GlobalRequestID();
+}
+
+}  // namespace content
diff --git a/content/browser/renderer_host/cross_site_transferring_request.h b/content/browser/renderer_host/cross_site_transferring_request.h
new file mode 100644
index 0000000..99ca9de
--- /dev/null
+++ b/content/browser/renderer_host/cross_site_transferring_request.h
@@ -0,0 +1,40 @@
+// Copyright 2014 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.
+
+#ifndef CONTENT_BROWSER_RENDERER_HOST_CROSS_SITE_TRANSFERRING_REQUEST_H_
+#define CONTENT_BROWSER_RENDERER_HOST_CROSS_SITE_TRANSFERRING_REQUEST_H_
+
+#include "base/basictypes.h"
+#include "content/common/content_export.h"
+#include "content/public/browser/global_request_id.h"
+
+namespace content {
+
+class CrossSiteResourceHandler;
+
+// A UI thread object that owns a request being transferred.  Deleting the
+// object without releasing the request will delete the underlying URLRequest.
+// This is needed to clean up the URLRequest when a cross site navigation is
+// cancelled.
+class CONTENT_EXPORT CrossSiteTransferringRequest {
+ public:
+  explicit CrossSiteTransferringRequest(GlobalRequestID global_request_id);
+  ~CrossSiteTransferringRequest();
+
+  // Relinquishes ownership of the request, so another process can take
+  // control of it.
+  void ReleaseRequest();
+
+ private:
+  // No need for a weak pointer here - nothing should have ownership of the
+  // cross site request until after |this| is deleted, or ReleaseRequest is
+  // called.
+  GlobalRequestID global_request_id_;
+
+  DISALLOW_COPY_AND_ASSIGN(CrossSiteTransferringRequest);
+};
+
+}  // namespace content
+
+#endif  // CONTENT_BROWSER_RENDERER_HOST_CROSS_SITE_TRANSFERRING_REQUEST_H_
diff --git a/content/browser/renderer_host/render_view_host_delegate.h b/content/browser/renderer_host/render_view_host_delegate.h
index bd56228..7b9a64c 100644
--- a/content/browser/renderer_host/render_view_host_delegate.h
+++ b/content/browser/renderer_host/render_view_host_delegate.h
@@ -46,6 +46,7 @@ class Size;
 namespace content {
 
 class BrowserContext;
+class CrossSiteTransferringRequest;
 class FrameTree;
 class PageState;
 class RenderViewHost;
@@ -93,10 +94,13 @@ class CONTENT_EXPORT RenderViewHostDelegate {
     // The |pending_render_view_host| is ready to commit a page.  The delegate
     // should ensure that the old RenderViewHost runs its unload handler first
     // and determine whether a RenderViewHost transfer is needed.
+    // |cross_site_transferring_request| is NULL if a request is not being
+    // transferred between renderers.
     virtual void OnCrossSiteResponse(
         RenderViewHost* pending_render_view_host,
         const GlobalRequestID& global_request_id,
-        bool is_transfer,
+        scoped_ptr<CrossSiteTransferringRequest>
+            cross_site_transferring_request,
         const std::vector<GURL>& transfer_url_chain,
         const Referrer& referrer,
         PageTransition page_transition,
diff --git a/content/browser/renderer_host/render_view_host_impl.cc b/content/browser/renderer_host/render_view_host_impl.cc
index d0879ab..2210d13 100644
--- a/content/browser/renderer_host/render_view_host_impl.cc
+++ b/content/browser/renderer_host/render_view_host_impl.cc
@@ -34,6 +34,7 @@
 #include "content/browser/gpu/gpu_surface_tracker.h"
 #include "content/browser/host_zoom_map_impl.h"
 #include "content/browser/loader/resource_dispatcher_host_impl.h"
+#include "content/browser/renderer_host/cross_site_transferring_request.h"
 #include "content/browser/renderer_host/dip_util.h"
 #include "content/browser/renderer_host/input/timeout_monitor.h"
 #include "content/browser/renderer_host/media/audio_renderer_host.h"
@@ -711,7 +712,7 @@ void RenderViewHostImpl::FirePageBeforeUnload(bool for_cross_site_transition) {
 
 void RenderViewHostImpl::OnCrossSiteResponse(
     const GlobalRequestID& global_request_id,
-    bool is_transfer,
+    scoped_ptr<CrossSiteTransferringRequest> cross_site_transferring_request,
     const std::vector<GURL>& transfer_url_chain,
     const Referrer& referrer,
     PageTransition page_transition,
@@ -727,7 +728,8 @@ void RenderViewHostImpl::OnCrossSiteResponse(
   // but today the frame_id is -1 for the main frame.
   RenderViewHostDelegate::RendererManagement* manager = node ?
       node->render_manager() : delegate_->GetRendererManagementDelegate();
-  manager->OnCrossSiteResponse(this, global_request_id, is_transfer,
+  manager->OnCrossSiteResponse(this, global_request_id,
+                               cross_site_transferring_request.Pass(),
                                transfer_url_chain, referrer, page_transition,
                                frame_id, should_replace_current_entry);
 }
diff --git a/content/browser/renderer_host/render_view_host_impl.h b/content/browser/renderer_host/render_view_host_impl.h
index 8fc9d16..5c7e534 100644
--- a/content/browser/renderer_host/render_view_host_impl.h
+++ b/content/browser/renderer_host/render_view_host_impl.h
@@ -60,6 +60,7 @@ namespace content {
 
 class BrowserMediaPlayerManager;
 class ChildProcessSecurityPolicyImpl;
+class CrossSiteTransferringRequest;
 class PageState;
 class RenderWidgetHostDelegate;
 class SessionStorageNamespace;
@@ -325,7 +326,7 @@ class CONTENT_EXPORT RenderViewHostImpl
   // needed.
   void OnCrossSiteResponse(
       const GlobalRequestID& global_request_id,
-      bool is_transfer,
+      scoped_ptr<CrossSiteTransferringRequest> cross_site_transferring_request,
       const std::vector<GURL>& transfer_url_chain,
       const Referrer& referrer,
       PageTransition page_transition,
diff --git a/content/browser/site_per_process_browsertest.cc b/content/browser/site_per_process_browsertest.cc
index e536cf6..f2e09e1 100644
--- a/content/browser/site_per_process_browsertest.cc
+++ b/content/browser/site_per_process_browsertest.cc
@@ -6,12 +6,15 @@
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
 #include "content/browser/frame_host/frame_tree.h"
+#include "content/browser/loader/resource_dispatcher_host_impl.h"
 #include "content/browser/renderer_host/render_view_host_impl.h"
 #include "content/browser/web_contents/web_contents_impl.h"
 #include "content/public/browser/navigation_entry.h"
 #include "content/public/browser/notification_observer.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/notification_types.h"
+#include "content/public/browser/resource_dispatcher_host_delegate.h"
+#include "content/public/browser/resource_throttle.h"
 #include "content/public/browser/web_contents_observer.h"
 #include "content/public/common/content_switches.h"
 #include "content/public/common/url_constants.h"
@@ -20,10 +23,13 @@
 #include "content/public/test/test_utils.h"
 #include "content/shell/browser/shell.h"
 #include "content/shell/browser/shell_content_browser_client.h"
+#include "content/shell/browser/shell_resource_dispatcher_host_delegate.h"
 #include "content/test/content_browser_test.h"
 #include "content/test/content_browser_test_utils.h"
 #include "net/base/escape.h"
 #include "net/dns/mock_host_resolver.h"
+#include "net/url_request/url_request.h"
+#include "net/url_request/url_request_status.h"
 
 namespace content {
 
@@ -159,7 +165,172 @@ void RedirectNotificationObserver::Observe(
   running_ = false;
 }
 
+// Tracks a single request for a specified URL, and allows waiting until the
+// request is destroyed, and then inspecting whether it completed successfully.
+class TrackingResourceDispatcherHostDelegate
+    : public ShellResourceDispatcherHostDelegate {
+ public:
+  TrackingResourceDispatcherHostDelegate() : throttle_created_(false) {
+  }
+
+  virtual void RequestBeginning(
+      net::URLRequest* request,
+      ResourceContext* resource_context,
+      appcache::AppCacheService* appcache_service,
+      ResourceType::Type resource_type,
+      int child_id,
+      int route_id,
+      ScopedVector<ResourceThrottle>* throttles) OVERRIDE {
+    CHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+    ShellResourceDispatcherHostDelegate::RequestBeginning(
+        request, resource_context, appcache_service, resource_type, child_id,
+        route_id, throttles);
+    // Expect only a single request for the tracked url.
+    ASSERT_FALSE(throttle_created_);
+    // If this is a request for the tracked URL, add a throttle to track it.
+    if (request->url() == tracked_url_)
+      throttles->push_back(new TrackingThrottle(request, this));
+  }
+
+  // Starts tracking a URL.  The request for previously tracked URL, if any,
+  // must have been made and deleted before calling this function.
+  void SetTrackedURL(const GURL& tracked_url) {
+    CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+    // Should not currently be tracking any URL.
+    ASSERT_FALSE(run_loop_);
+
+    // Create a RunLoop that will be stopped once the request for the tracked
+    // URL has been destroyed, to allow tracking the URL while also waiting for
+    // other events.
+    run_loop_.reset(new base::RunLoop());
+
+    BrowserThread::PostTask(
+        BrowserThread::IO, FROM_HERE,
+        base::Bind(
+            &TrackingResourceDispatcherHostDelegate::SetTrackedURLOnIOThread,
+            base::Unretained(this),
+            tracked_url));
+  }
+
+  // Waits until the tracked URL has been requests, and the request for it has
+  // been destroyed.
+  bool WaitForTrackedURLAndGetCompleted() {
+    CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+    run_loop_->Run();
+    run_loop_.reset();
+    return tracked_request_completed_;
+  }
+
+ private:
+  // ResourceThrottle attached to request for the tracked URL.  On destruction,
+  // passes the final URLRequestStatus back to the delegate.
+  class TrackingThrottle : public ResourceThrottle {
+   public:
+    TrackingThrottle(net::URLRequest* request,
+                     TrackingResourceDispatcherHostDelegate* tracker)
+        : request_(request), tracker_(tracker) {
+    }
+
+    virtual ~TrackingThrottle() {
+      // If the request is deleted without being cancelled, its status will
+      // indicate it succeeded, so have to check if the request is still pending
+      // as well.
+      tracker_->OnTrackedRequestDestroyed(
+          !request_->is_pending() && request_->status().is_success());
+    }
+
+    // ResourceThrottle implementation:
+    virtual const char* GetNameForLogging() const OVERRIDE {
+      return "TrackingThrottle";
+    }
+
+   private:
+    net::URLRequest* request_;
+    TrackingResourceDispatcherHostDelegate* tracker_;
+
+    DISALLOW_COPY_AND_ASSIGN(TrackingThrottle);
+  };
+
+  void SetTrackedURLOnIOThread(const GURL& tracked_url) {
+    CHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+    throttle_created_ = false;
+    tracked_url_ = tracked_url;
+  }
+
+  void OnTrackedRequestDestroyed(bool completed) {
+    CHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+    tracked_request_completed_ = completed;
+    tracked_url_ = GURL();
+
+    BrowserThread::PostTask(
+        BrowserThread::UI, FROM_HERE, run_loop_->QuitClosure());
+  }
+
+  // These live on the IO thread.
+  GURL tracked_url_;
+  bool throttle_created_;
+
+  // This is created and destroyed on the UI thread, but stopped on the IO
+  // thread.
+  scoped_ptr<base::RunLoop> run_loop_;
+
+  // Set on the IO thread while |run_loop_| is non-NULL, read on the UI thread
+  // after deleting run_loop_.
+  bool tracked_request_completed_;
+
+  DISALLOW_COPY_AND_ASSIGN(TrackingResourceDispatcherHostDelegate);
+};
+
+// WebContentsDelegate that fails to open a URL when there's a request that
+// needs to be transferred between renderers.
+class NoTransferRequestDelegate : public WebContentsDelegate {
+ public:
+  NoTransferRequestDelegate() {}
+
+  virtual WebContents* OpenURLFromTab(WebContents* source,
+                                      const OpenURLParams& params) OVERRIDE {
+    bool is_transfer =
+        (params.transferred_global_request_id != GlobalRequestID());
+    if (is_transfer)
+      return NULL;
+    NavigationController::LoadURLParams load_url_params(params.url);
+    load_url_params.referrer = params.referrer;
+    load_url_params.frame_tree_node_id = params.frame_tree_node_id;
+    load_url_params.transition_type = params.transition;
+    load_url_params.extra_headers = params.extra_headers;
+    load_url_params.should_replace_current_entry =
+        params.should_replace_current_entry;
+    load_url_params.is_renderer_initiated = true;
+    source->GetController().LoadURLWithParams(load_url_params);
+    return source;
+  }
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(NoTransferRequestDelegate);
+};
+
 class SitePerProcessBrowserTest : public ContentBrowserTest {
+ public:
+  SitePerProcessBrowserTest() : old_delegate_(NULL) {
+  }
+
+  // ContentBrowserTest implementation:
+  virtual void SetUpOnMainThread() OVERRIDE {
+    BrowserThread::PostTask(
+        BrowserThread::IO, FROM_HERE,
+        base::Bind(
+            &SitePerProcessBrowserTest::InjectResourceDisptcherHostDelegate,
+            base::Unretained(this)));
+  }
+
+  virtual void TearDownOnMainThread() OVERRIDE {
+    BrowserThread::PostTask(
+        BrowserThread::IO, FROM_HERE,
+        base::Bind(
+            &SitePerProcessBrowserTest::RestoreResourceDisptcherHostDelegate,
+            base::Unretained(this)));
+  }
+
  protected:
   // Start at a data URL so each extra navigation creates a navigation entry.
   // (The first navigation will silently be classified as AUTO_SUBFRAME.)
@@ -195,7 +366,8 @@ class SitePerProcessBrowserTest : public ContentBrowserTest {
 
   void NavigateToURLContentInitiated(Shell* window,
                                      const GURL& url,
-                                     bool should_replace_current_entry) {
+                                     bool should_replace_current_entry,
+                                     bool should_wait_for_navigation) {
     std::string script;
     if (should_replace_current_entry)
       script = base::StringPrintf("location.replace('%s')", url.spec().c_str());
@@ -204,7 +376,8 @@ class SitePerProcessBrowserTest : public ContentBrowserTest {
     TestNavigationObserver load_observer(shell()->web_contents(), 1);
     bool result = ExecuteScript(window->web_contents(), script);
     EXPECT_TRUE(result);
-    load_observer.Wait();
+    if (should_wait_for_navigation)
+      load_observer.Wait();
   }
 
   virtual void SetUpCommandLine(CommandLine* command_line) OVERRIDE {
@@ -213,6 +386,26 @@ class SitePerProcessBrowserTest : public ContentBrowserTest {
     // TODO(creis): Remove this when GTK is no longer a supported platform.
     command_line->AppendSwitch(switches::kForceCompositingMode);
   }
+
+  void InjectResourceDisptcherHostDelegate() {
+    DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+    old_delegate_ = ResourceDispatcherHostImpl::Get()->delegate();
+    ResourceDispatcherHostImpl::Get()->SetDelegate(&tracking_delegate_);
+  }
+
+  void RestoreResourceDisptcherHostDelegate() {
+    DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+    ResourceDispatcherHostImpl::Get()->SetDelegate(old_delegate_);
+    old_delegate_ = NULL;
+  }
+
+  TrackingResourceDispatcherHostDelegate& tracking_delegate() {
+    return tracking_delegate_;
+  }
+
+ private:
+  TrackingResourceDispatcherHostDelegate tracking_delegate_;
+  ResourceDispatcherHostDelegate* old_delegate_;
 };
 
 // Ensure that we can complete a cross-process subframe navigation.
@@ -560,13 +753,18 @@ IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest,
   GURL url2 = test_server()->GetURL("files/site_isolation/blank.html?2");
   replace_host.SetHostStr(a_com);
   url2 = url2.ReplaceComponents(replace_host);
-  NavigateToURLContentInitiated(shell(), url2, true);
+  // Used to make sure the request for url2 succeeds, and there was only one of
+  // them.
+  tracking_delegate().SetTrackedURL(url2);
+  NavigateToURLContentInitiated(shell(), url2, true, true);
 
   // There should be one history entry. url2 should have replaced url1.
   EXPECT_TRUE(controller.GetPendingEntry() == NULL);
   EXPECT_EQ(1, controller.GetEntryCount());
   EXPECT_EQ(0, controller.GetCurrentEntryIndex());
   EXPECT_EQ(url2, controller.GetEntryAtIndex(0)->GetURL());
+  // Make sure the request succeeded.
+  EXPECT_TRUE(tracking_delegate().WaitForTrackedURLAndGetCompleted());
 
   // Now navigate as before to a page on B.com, but normally (without
   // replacement). This will still perform a double process-swap as above, via
@@ -574,7 +772,10 @@ IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest,
   GURL url3 = test_server()->GetURL("files/site_isolation/blank.html?3");
   replace_host.SetHostStr(b_com);
   url3 = url3.ReplaceComponents(replace_host);
-  NavigateToURLContentInitiated(shell(), url3, false);
+  // Used to make sure the request for url3 succeeds, and there was only one of
+  // them.
+  tracking_delegate().SetTrackedURL(url3);
+  NavigateToURLContentInitiated(shell(), url3, false, true);
 
   // There should be two history entries. url2 should have replaced url1. url2
   // should not have replaced url3.
@@ -583,6 +784,9 @@ IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest,
   EXPECT_EQ(1, controller.GetCurrentEntryIndex());
   EXPECT_EQ(url2, controller.GetEntryAtIndex(0)->GetURL());
   EXPECT_EQ(url3, controller.GetEntryAtIndex(1)->GetURL());
+
+  // Make sure the request succeeded.
+  EXPECT_TRUE(tracking_delegate().WaitForTrackedURLAndGetCompleted());
 }
 
 // Tests that the |should_replace_current_entry| flag persists correctly across
@@ -608,7 +812,7 @@ IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest,
   // Navigate in-process with entry replacement. It will then be transferred
   // into a new one due to the call above.
   GURL url2 = test_server()->GetURL("files/site_isolation/blank.html?2");
-  NavigateToURLContentInitiated(shell(), url2, true);
+  NavigateToURLContentInitiated(shell(), url2, true, true);
 
   // There should be one history entry. url2 should have replaced url1.
   EXPECT_TRUE(controller.GetPendingEntry() == NULL);
@@ -618,7 +822,7 @@ IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest,
 
   // Now navigate as before, but without replacement.
   GURL url3 = test_server()->GetURL("files/site_isolation/blank.html?3");
-  NavigateToURLContentInitiated(shell(), url3, false);
+  NavigateToURLContentInitiated(shell(), url3, false, true);
 
   // There should be two history entries. url2 should have replaced url1. url2
   // should not have replaced url3.
@@ -659,7 +863,7 @@ IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest,
       "server-redirect?" + net::EscapeQueryParamValue(url2b.spec(), false));
   replace_host.SetHostStr(a_com);
   url2a = url2a.ReplaceComponents(replace_host);
-  NavigateToURLContentInitiated(shell(), url2a, true);
+  NavigateToURLContentInitiated(shell(), url2a, true, true);
 
   // There should be one history entry. url2b should have replaced url1.
   EXPECT_TRUE(controller.GetPendingEntry() == NULL);
@@ -675,7 +879,7 @@ IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest,
       "server-redirect?" + net::EscapeQueryParamValue(url3b.spec(), false));
   replace_host.SetHostStr(a_com);
   url3a = url3a.ReplaceComponents(replace_host);
-  NavigateToURLContentInitiated(shell(), url3a, false);
+  NavigateToURLContentInitiated(shell(), url3a, false, true);
 
   // There should be two history entries. url2b should have replaced url1. url2b
   // should not have replaced url3b.
@@ -686,4 +890,54 @@ IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest,
   EXPECT_EQ(url3b, controller.GetEntryAtIndex(1)->GetURL());
 }
 
+// Tests that the request is destroyed when a cross process navigation is
+// cancelled.
+IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest, NoLeakOnCrossSiteCancel) {
+  const NavigationController& controller =
+      shell()->web_contents()->GetController();
+  host_resolver()->AddRule("*", "127.0.0.1");
+  ASSERT_TRUE(test_server()->Start());
+
+  // These must all stay in scope with replace_host.
+  GURL::Replacements replace_host;
+  std::string a_com("A.com");
+  std::string b_com("B.com");
+
+  // Navigate to a starting URL, so there is a history entry to replace.
+  GURL url1 = test_server()->GetURL("files/site_isolation/blank.html?1");
+  NavigateToURL(shell(), url1);
+
+  // Force all future navigations to transfer.
+  ShellContentBrowserClient::SetSwapProcessesForRedirect(true);
+
+  NoTransferRequestDelegate no_transfer_request_delegate;
+  WebContentsDelegate* old_delegate = shell()->web_contents()->GetDelegate();
+  shell()->web_contents()->SetDelegate(&no_transfer_request_delegate);
+
+  // Navigate to a page on A.com with entry replacement. This navigation is
+  // cross-site, so the renderer will send it to the browser via OpenURL to give
+  // to a new process. It will then be transferred into yet another process due
+  // to the call above.
+  GURL url2 = test_server()->GetURL("files/site_isolation/blank.html?2");
+  replace_host.SetHostStr(a_com);
+  url2 = url2.ReplaceComponents(replace_host);
+  // Used to make sure the second request is cancelled, and there is only one
+  // request for url2.
+  tracking_delegate().SetTrackedURL(url2);
+
+  // Don't wait for the navigation to complete, since that never happens in
+  // this case.
+  NavigateToURLContentInitiated(shell(), url2, false, false);
+
+  // There should be one history entry, with url1.
+  EXPECT_EQ(1, controller.GetEntryCount());
+  EXPECT_EQ(0, controller.GetCurrentEntryIndex());
+  EXPECT_EQ(url1, controller.GetEntryAtIndex(0)->GetURL());
+
+  // Make sure the request for url2 did not complete.
+  EXPECT_FALSE(tracking_delegate().WaitForTrackedURLAndGetCompleted());
+
+  shell()->web_contents()->SetDelegate(old_delegate);
+}
+
 }  // namespace content
diff --git a/content/browser/web_contents/web_contents_impl_unittest.cc b/content/browser/web_contents/web_contents_impl_unittest.cc
index 92a2f31..e04b441 100644
--- a/content/browser/web_contents/web_contents_impl_unittest.cc
+++ b/content/browser/web_contents/web_contents_impl_unittest.cc
@@ -6,6 +6,7 @@
 #include "base/strings/utf_string_conversions.h"
 #include "content/browser/frame_host/interstitial_page_impl.h"
 #include "content/browser/frame_host/navigation_entry_impl.h"
+#include "content/browser/renderer_host/cross_site_transferring_request.h"
 #include "content/browser/renderer_host/render_view_host_impl.h"
 #include "content/browser/site_instance_impl.h"
 #include "content/browser/webui/web_ui_controller_factory_registry.h"
@@ -1088,8 +1089,9 @@ TEST_F(WebContentsImplTest, CrossSiteCantPreemptAfterUnload) {
   std::vector<GURL> url_chain;
   url_chain.push_back(GURL());
   contents()->GetRenderManagerForTesting()->OnCrossSiteResponse(
-      pending_rvh, GlobalRequestID(0, 0), false, url_chain, Referrer(),
-      PAGE_TRANSITION_TYPED, 1, false);
+      pending_rvh, GlobalRequestID(0, 0),
+      scoped_ptr<CrossSiteTransferringRequest>(), url_chain,
+      Referrer(), PAGE_TRANSITION_TYPED, 1, false);
 
   // Suppose the original renderer navigates now, while the unload request is in
   // flight.  We should ignore it, wait for the unload ack, and let the pending