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

5 files changed, 64 insertions, 11 deletions

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;