CrossSiteResourceHandler: cancel request if there's no RFH to do

a policy check against.

Also included is a new test which demonstrates the issue. This was based
on CrossSiteTransferTest, which I did some cleanup on.

BUG=538784,268640
TEST=content_browsertests, with and without --enable-browser-side-navigation

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

Cr-Commit-Position: refs/heads/master@{#352413}

5 files changed, 357 insertions, 46 deletions

diff --git a/content/browser/cross_site_transfer_browsertest.cc b/content/browser/cross_site_transfer_browsertest.cc
index 0ce755c..c9d5816 100644
--- a/content/browser/cross_site_transfer_browsertest.cc
+++ b/content/browser/cross_site_transfer_browsertest.cc
@@ -64,11 +64,10 @@ class TrackingResourceDispatcherHostDelegate
         BrowserThread::IO, FROM_HERE,
         base::Bind(
             &TrackingResourceDispatcherHostDelegate::SetTrackedURLOnIOThread,
-            base::Unretained(this),
-            tracked_url));
+            base::Unretained(this), tracked_url, run_loop_->QuitClosure()));
   }
 
-  // Waits until the tracked URL has been requests, and the request for it has
+  // Waits until the tracked URL has been requested, and the request for it has
   // been destroyed.
   bool WaitForTrackedURLAndGetCompleted() {
     CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
@@ -107,10 +106,12 @@ class TrackingResourceDispatcherHostDelegate
     DISALLOW_COPY_AND_ASSIGN(TrackingThrottle);
   };
 
-  void SetTrackedURLOnIOThread(const GURL& tracked_url) {
+  void SetTrackedURLOnIOThread(const GURL& tracked_url,
+                               const base::Closure& run_loop_quit_closure) {
     CHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
     throttle_created_ = false;
     tracked_url_ = tracked_url;
+    run_loop_quit_closure_ = run_loop_quit_closure;
   }
 
   void OnTrackedRequestDestroyed(bool completed) {
@@ -118,20 +119,20 @@ class TrackingResourceDispatcherHostDelegate
     tracked_request_completed_ = completed;
     tracked_url_ = GURL();
 
-    BrowserThread::PostTask(
-        BrowserThread::UI, FROM_HERE, run_loop_->QuitClosure());
+    BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
+                            run_loop_quit_closure_);
   }
 
   // These live on the IO thread.
   GURL tracked_url_;
   bool throttle_created_;
+  base::Closure run_loop_quit_closure_;
 
-  // This is created and destroyed on the UI thread, but stopped on the IO
-  // thread.
+  // This lives on the UI 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_.
+  // Set on the IO thread while |run_loop_| is non-nullptr, read on the UI
+  // thread after deleting run_loop_.
   bool tracked_request_completed_;
 
   DISALLOW_COPY_AND_ASSIGN(TrackingResourceDispatcherHostDelegate);
@@ -148,7 +149,7 @@ class NoTransferRequestDelegate : public WebContentsDelegate {
     bool is_transfer =
         (params.transferred_global_request_id != GlobalRequestID());
     if (is_transfer)
-      return NULL;
+      return nullptr;
     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;
@@ -167,16 +168,14 @@ class NoTransferRequestDelegate : public WebContentsDelegate {
 
 class CrossSiteTransferTest : public ContentBrowserTest {
  public:
-  CrossSiteTransferTest() : old_delegate_(NULL) {
-  }
+  CrossSiteTransferTest() : old_delegate_(nullptr) {}
 
   // ContentBrowserTest implementation:
   void SetUpOnMainThread() override {
     BrowserThread::PostTask(
         BrowserThread::IO, FROM_HERE,
-        base::Bind(
-            &CrossSiteTransferTest::InjectResourceDisptcherHostDelegate,
-            base::Unretained(this)));
+        base::Bind(&CrossSiteTransferTest::InjectResourceDispatcherHostDelegate,
+                   base::Unretained(this)));
     host_resolver()->AddRule("*", "127.0.0.1");
     ASSERT_TRUE(embedded_test_server()->InitializeAndWaitUntilReady());
     content::SetupCrossSiteRedirector(embedded_test_server());
@@ -211,7 +210,7 @@ class CrossSiteTransferTest : public ContentBrowserTest {
     IsolateAllSitesForTesting(command_line);
   }
 
-  void InjectResourceDisptcherHostDelegate() {
+  void InjectResourceDispatcherHostDelegate() {
     DCHECK_CURRENTLY_ON(BrowserThread::IO);
     old_delegate_ = ResourceDispatcherHostImpl::Get()->delegate();
     ResourceDispatcherHostImpl::Get()->SetDelegate(&tracking_delegate_);
@@ -220,7 +219,7 @@ class CrossSiteTransferTest : public ContentBrowserTest {
   void RestoreResourceDisptcherHostDelegate() {
     DCHECK_CURRENTLY_ON(BrowserThread::IO);
     ResourceDispatcherHostImpl::Get()->SetDelegate(old_delegate_);
-    old_delegate_ = NULL;
+    old_delegate_ = nullptr;
   }
 
   TrackingResourceDispatcherHostDelegate& tracking_delegate() {
@@ -253,7 +252,7 @@ IN_PROC_BROWSER_TEST_F(CrossSiteTransferTest,
 
   // Navigate to a starting URL, so there is a history entry to replace.
   GURL url1 = embedded_test_server()->GetURL("/site_isolation/blank.html?1");
-  NavigateToURL(shell(), url1);
+  EXPECT_TRUE(NavigateToURL(shell(), url1));
 
   // Force all future navigations to transfer. Note that this includes same-site
   // navigiations which may cause double process swaps (via OpenURL and then via
@@ -272,7 +271,7 @@ IN_PROC_BROWSER_TEST_F(CrossSiteTransferTest,
   NavigateToURLContentInitiated(shell(), url2, true, true);
 
   // There should be one history entry. url2 should have replaced url1.
-  EXPECT_TRUE(controller.GetPendingEntry() == NULL);
+  EXPECT_TRUE(controller.GetPendingEntry() == nullptr);
   EXPECT_EQ(1, controller.GetEntryCount());
   EXPECT_EQ(0, controller.GetCurrentEntryIndex());
   EXPECT_EQ(url2, controller.GetEntryAtIndex(0)->GetURL());
@@ -291,7 +290,7 @@ IN_PROC_BROWSER_TEST_F(CrossSiteTransferTest,
 
   // There should be two history entries. url2 should have replaced url1. url2
   // should not have replaced url3.
-  EXPECT_TRUE(controller.GetPendingEntry() == NULL);
+  EXPECT_TRUE(controller.GetPendingEntry() == nullptr);
   EXPECT_EQ(2, controller.GetEntryCount());
   EXPECT_EQ(1, controller.GetCurrentEntryIndex());
   EXPECT_EQ(url2, controller.GetEntryAtIndex(0)->GetURL());
@@ -312,7 +311,7 @@ IN_PROC_BROWSER_TEST_F(CrossSiteTransferTest,
 
   // Navigate to a starting URL, so there is a history entry to replace.
   GURL url = embedded_test_server()->GetURL("/site_isolation/blank.html?1");
-  NavigateToURL(shell(), url);
+  EXPECT_TRUE(NavigateToURL(shell(), url));
 
   // Force all future navigations to transfer. Note that this includes same-site
   // navigiations which may cause double process swaps (via OpenURL and then via
@@ -326,7 +325,7 @@ IN_PROC_BROWSER_TEST_F(CrossSiteTransferTest,
   NavigateToURLContentInitiated(shell(), url2, true, true);
 
   // There should be one history entry. url2 should have replaced url1.
-  EXPECT_TRUE(controller.GetPendingEntry() == NULL);
+  EXPECT_TRUE(controller.GetPendingEntry() == nullptr);
   EXPECT_EQ(1, controller.GetEntryCount());
   EXPECT_EQ(0, controller.GetCurrentEntryIndex());
   EXPECT_EQ(url2, controller.GetEntryAtIndex(0)->GetURL());
@@ -337,7 +336,7 @@ IN_PROC_BROWSER_TEST_F(CrossSiteTransferTest,
 
   // There should be two history entries. url2 should have replaced url1. url2
   // should not have replaced url3.
-  EXPECT_TRUE(controller.GetPendingEntry() == NULL);
+  EXPECT_TRUE(controller.GetPendingEntry() == nullptr);
   EXPECT_EQ(2, controller.GetEntryCount());
   EXPECT_EQ(1, controller.GetCurrentEntryIndex());
   EXPECT_EQ(url2, controller.GetEntryAtIndex(0)->GetURL());
@@ -353,7 +352,7 @@ IN_PROC_BROWSER_TEST_F(CrossSiteTransferTest,
 
   // Navigate to a starting URL, so there is a history entry to replace.
   GURL url1 = embedded_test_server()->GetURL("/site_isolation/blank.html?1");
-  NavigateToURL(shell(), url1);
+  EXPECT_TRUE(NavigateToURL(shell(), url1));
 
   // Navigate to a page on A.com which redirects to B.com with entry
   // replacement. This will switch processes via OpenURL twice. First to A.com,
@@ -368,7 +367,7 @@ IN_PROC_BROWSER_TEST_F(CrossSiteTransferTest,
   NavigateToURLContentInitiated(shell(), url2a, true, true);
 
   // There should be one history entry. url2b should have replaced url1.
-  EXPECT_TRUE(controller.GetPendingEntry() == NULL);
+  EXPECT_TRUE(controller.GetPendingEntry() == nullptr);
   EXPECT_EQ(1, controller.GetEntryCount());
   EXPECT_EQ(0, controller.GetCurrentEntryIndex());
   EXPECT_EQ(url2b, controller.GetEntryAtIndex(0)->GetURL());
@@ -382,7 +381,7 @@ IN_PROC_BROWSER_TEST_F(CrossSiteTransferTest,
 
   // There should be two history entries. url2b should have replaced url1. url2b
   // should not have replaced url3b.
-  EXPECT_TRUE(controller.GetPendingEntry() == NULL);
+  EXPECT_TRUE(controller.GetPendingEntry() == nullptr);
   EXPECT_EQ(2, controller.GetEntryCount());
   EXPECT_EQ(1, controller.GetCurrentEntryIndex());
   EXPECT_EQ(url2b, controller.GetEntryAtIndex(0)->GetURL());
@@ -397,7 +396,7 @@ IN_PROC_BROWSER_TEST_F(CrossSiteTransferTest, NoLeakOnCrossSiteCancel) {
 
   // Navigate to a starting URL, so there is a history entry to replace.
   GURL url1 = embedded_test_server()->GetURL("/site_isolation/blank.html?1");
-  NavigateToURL(shell(), url1);
+  EXPECT_TRUE(NavigateToURL(shell(), url1));
 
   // Force all future navigations to transfer.
   ShellContentBrowserClient::SetSwapProcessesForRedirect(true);
diff --git a/content/browser/loader/cross_site_resource_handler.cc b/content/browser/loader/cross_site_resource_handler.cc
index 7ffb659..b05ac5d 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;
   }
 }
 
diff --git a/content/browser/loader/cross_site_resource_handler.h b/content/browser/loader/cross_site_resource_handler.h
index b73a5a1..db1dc84 100644
--- a/content/browser/loader/cross_site_resource_handler.h
+++ b/content/browser/loader/cross_site_resource_handler.h
@@ -26,6 +26,12 @@ struct TransitionLayerData;
 // and not a download.
 class CrossSiteResourceHandler : public LayeredResourceHandler {
  public:
+  enum class NavigationDecision {
+    TRANSFER_REQUIRED,
+    USE_EXISTING_RENDERER,
+    CANCEL_REQUEST
+  };
+
   CrossSiteResourceHandler(scoped_ptr<ResourceHandler> next_handler,
                            net::URLRequest* request);
   ~CrossSiteResourceHandler() override;
@@ -63,7 +69,7 @@ class CrossSiteResourceHandler : public LayeredResourceHandler {
   bool OnNormalResponseStarted(ResourceResponse* response,
                                bool* defer);
 
-  void ResumeOrTransfer(bool is_transfer);
+  void ResumeOrTransfer(NavigationDecision decision);
   void ResumeIfDeferred();
 
   // Called when about to defer a request.  Sets |did_defer_| and logs the
diff --git a/content/browser/loader/cross_site_resource_handler_browsertest.cc b/content/browser/loader/cross_site_resource_handler_browsertest.cc
new file mode 100644
index 0000000..4d66c1a
--- /dev/null
+++ b/content/browser/loader/cross_site_resource_handler_browsertest.cc
@@ -0,0 +1,277 @@
+// Copyright 2015 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 "base/callback.h"
+#include "base/command_line.h"
+#include "base/memory/weak_ptr.h"
+#include "base/single_thread_task_runner.h"
+#include "base/thread_task_runner_handle.h"
+#include "content/browser/frame_host/render_frame_host_impl.h"
+#include "content/browser/loader/resource_dispatcher_host_impl.h"
+#include "content/browser/web_contents/web_contents_impl.h"
+#include "content/common/frame_messages.h"
+#include "content/public/browser/resource_dispatcher_host.h"
+#include "content/public/browser/resource_dispatcher_host_delegate.h"
+#include "content/public/browser/resource_throttle.h"
+#include "content/public/browser/web_contents.h"
+#include "content/public/test/browser_test_utils.h"
+#include "content/public/test/content_browser_test.h"
+#include "content/public/test/content_browser_test_utils.h"
+#include "content/public/test/test_utils.h"
+#include "content/shell/browser/shell.h"
+#include "content/shell/browser/shell_resource_dispatcher_host_delegate.h"
+#include "ipc/ipc_security_test_util.h"
+#include "net/dns/mock_host_resolver.h"
+#include "net/test/embedded_test_server/embedded_test_server.h"
+#include "net/url_request/url_request.h"
+
+namespace content {
+
+namespace {
+
+// A ResourceDispatchHostDelegate that uses ResourceThrottles to pause a
+// targeted request temporarily, to run a chunk of test code.
+class TestResourceDispatcherHostDelegate
+    : public ShellResourceDispatcherHostDelegate {
+ public:
+  using RequestDeferredHook = base::Callback<void(const base::Closure& resume)>;
+  TestResourceDispatcherHostDelegate() : throttle_created_(false) {}
+
+  void RequestBeginning(net::URLRequest* request,
+                        ResourceContext* resource_context,
+                        AppCacheService* appcache_service,
+                        ResourceType resource_type,
+                        ScopedVector<ResourceThrottle>* throttles) override {
+    CHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+    ShellResourceDispatcherHostDelegate::RequestBeginning(
+        request, resource_context, appcache_service, resource_type, throttles);
+
+    // If this is a request for the tracked URL, add a throttle to track it.
+    if (request->url() == tracked_url_) {
+      // Expect only a single request for the tracked url.
+      ASSERT_FALSE(throttle_created_);
+      throttle_created_ = true;
+
+      throttles->push_back(
+          new CallbackRunningResourceThrottle(request, this, run_on_start_));
+    }
+  }
+
+  // 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,
+                     const RequestDeferredHook& run_on_start) {
+    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(&TestResourceDispatcherHostDelegate::SetTrackedURLOnIOThread,
+                   base::Unretained(this), tracked_url, run_on_start,
+                   run_loop_->QuitClosure()));
+  }
+
+  // Waits until the tracked URL has been requested, 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:
+  // A ResourceThrottle which defers the request at WillStartRequest time until
+  // a test-supplied callback completes. Notifies |tracker| when the request is
+  // destroyed.
+  class CallbackRunningResourceThrottle : public ResourceThrottle {
+   public:
+    CallbackRunningResourceThrottle(net::URLRequest* request,
+                                    TestResourceDispatcherHostDelegate* tracker,
+                                    const RequestDeferredHook& run_on_start)
+        : request_(request),
+          tracker_(tracker),
+          run_on_start_(run_on_start),
+          weak_factory_(this) {}
+
+    void WillStartRequest(bool* defer) override {
+      *defer = true;
+      base::Closure resume_request_on_io_thread = base::Bind(
+          base::IgnoreResult(&BrowserThread::PostTask), BrowserThread::IO,
+          FROM_HERE, base::Bind(&CallbackRunningResourceThrottle::Resume,
+                                weak_factory_.GetWeakPtr()));
+      BrowserThread::PostTask(
+          BrowserThread::UI, FROM_HERE,
+          base::Bind(run_on_start_, resume_request_on_io_thread));
+    }
+
+    ~CallbackRunningResourceThrottle() override {
+      // 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:
+    const char* GetNameForLogging() const override {
+      return "CallbackRunningResourceThrottle";
+    }
+
+   private:
+    void Resume() { controller()->Resume(); }
+    net::URLRequest* request_;
+    TestResourceDispatcherHostDelegate* tracker_;
+    RequestDeferredHook run_on_start_;
+    base::WeakPtrFactory<CallbackRunningResourceThrottle> weak_factory_;
+
+    DISALLOW_COPY_AND_ASSIGN(CallbackRunningResourceThrottle);
+  };
+
+  void SetTrackedURLOnIOThread(const GURL& tracked_url,
+                               const RequestDeferredHook& run_on_start,
+                               const base::Closure& run_loop_quit_closure) {
+    CHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+    throttle_created_ = false;
+    tracked_url_ = tracked_url;
+    run_on_start_ = run_on_start;
+    run_loop_quit_closure_ = run_loop_quit_closure;
+  }
+
+  void OnTrackedRequestDestroyed(bool completed) {
+    CHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+    tracked_request_completed_ = completed;
+    tracked_url_ = GURL();
+    run_on_start_ = RequestDeferredHook();
+
+    BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
+                            run_loop_quit_closure_);
+  }
+
+  // These live on the IO thread.
+  GURL tracked_url_;
+  bool throttle_created_;
+  base::Closure run_loop_quit_closure_;
+  RequestDeferredHook run_on_start_;
+
+  // This lives on the UI thread.
+  scoped_ptr<base::RunLoop> run_loop_;
+
+  // Set on the IO thread while |run_loop_| is non-nullptr, read on the UI
+  // thread after deleting run_loop_.
+  bool tracked_request_completed_;
+
+  DISALLOW_COPY_AND_ASSIGN(TestResourceDispatcherHostDelegate);
+};
+
+class CrossSiteResourceHandlerTest : public ContentBrowserTest {
+ public:
+  CrossSiteResourceHandlerTest() : old_delegate_(nullptr) {}
+
+  // ContentBrowserTest implementation:
+  void SetUpOnMainThread() override {
+    BrowserThread::PostTask(
+        BrowserThread::IO, FROM_HERE,
+        base::Bind(
+            &CrossSiteResourceHandlerTest::InjectResourceDispatcherHostDelegate,
+            base::Unretained(this)));
+    host_resolver()->AddRule("*", "127.0.0.1");
+    ASSERT_TRUE(embedded_test_server()->InitializeAndWaitUntilReady());
+    content::SetupCrossSiteRedirector(embedded_test_server());
+  }
+
+  void TearDownOnMainThread() override {
+    BrowserThread::PostTask(
+        BrowserThread::IO, FROM_HERE,
+        base::Bind(&CrossSiteResourceHandlerTest::
+                       RestoreResourceDispatcherHostDelegate,
+                   base::Unretained(this)));
+  }
+
+ protected:
+  void SetUpCommandLine(base::CommandLine* command_line) override {
+    IsolateAllSitesForTesting(command_line);
+  }
+
+  void InjectResourceDispatcherHostDelegate() {
+    DCHECK_CURRENTLY_ON(BrowserThread::IO);
+    old_delegate_ = ResourceDispatcherHostImpl::Get()->delegate();
+    ResourceDispatcherHostImpl::Get()->SetDelegate(&tracking_delegate_);
+  }
+
+  void RestoreResourceDispatcherHostDelegate() {
+    DCHECK_CURRENTLY_ON(BrowserThread::IO);
+    ResourceDispatcherHostImpl::Get()->SetDelegate(old_delegate_);
+    old_delegate_ = nullptr;
+  }
+
+  TestResourceDispatcherHostDelegate& tracking_delegate() {
+    return tracking_delegate_;
+  }
+
+ private:
+  TestResourceDispatcherHostDelegate tracking_delegate_;
+  ResourceDispatcherHostDelegate* old_delegate_;
+};
+
+void SimulateMaliciousFrameDetachOnUIThread(int render_process_id,
+                                            int frame_routing_id,
+                                            const base::Closure& done_cb) {
+  RenderFrameHostImpl* rfh =
+      RenderFrameHostImpl::FromID(render_process_id, frame_routing_id);
+  CHECK(rfh);
+
+  // Inject a frame detach message. An attacker-controlled renderer could do
+  // this without also cancelling the pending navigation (as blink would, if you
+  // removed the iframe from the document via js).
+  rfh->OnMessageReceived(FrameHostMsg_Detach(frame_routing_id));
+  done_cb.Run();
+}
+
+}  // namespace
+
+// Regression test for https://crbug.com/538784 -- ensures that one can't
+// sidestep CrossSiteResourceHandler by detaching a frame mid-request.
+IN_PROC_BROWSER_TEST_F(CrossSiteResourceHandlerTest,
+                       NoDeliveryToDetachedFrame) {
+  GURL attacker_page = embedded_test_server()->GetURL(
+      "evil.com", "/cross_site_iframe_factory.html?evil(evil)");
+  EXPECT_TRUE(NavigateToURL(shell(), attacker_page));
+
+  FrameTreeNode* root = static_cast<WebContentsImpl*>(shell()->web_contents())
+                            ->GetFrameTree()
+                            ->root();
+
+  RenderFrameHost* child_frame = root->child_at(0)->current_frame_host();
+
+  // Attacker initiates a navigation to a cross-site document. Under --site-per-
+  // process, these bytes must not be sent to the attacker process.
+  GURL target_resource =
+      embedded_test_server()->GetURL("a.com", "/title1.html");
+
+  // We add a testing hook to simulate the attacker-controlled process sending
+  // FrameHostMsg_Detach before the http response arrives. At the time this test
+  // was written, the resource request had a lifetime separate from the RFH,
+  tracking_delegate().SetTrackedURL(
+      target_resource, base::Bind(&SimulateMaliciousFrameDetachOnUIThread,
+                                  child_frame->GetProcess()->GetID(),
+                                  child_frame->GetRoutingID()));
+  EXPECT_TRUE(ExecuteScript(
+      shell()->web_contents()->GetMainFrame(),
+      base::StringPrintf("document.getElementById('child-0').src='%s'",
+                         target_resource.spec().c_str())));
+
+  // Wait for the scenario to play out. If this returns false, it means the
+  // request did not succeed, which is good in this case.
+  EXPECT_FALSE(tracking_delegate().WaitForTrackedURLAndGetCompleted())
+      << "Request should have been cancelled before reaching the renderer.";
+}
+
+}  // namespace content
diff --git a/content/content_tests.gypi b/content/content_tests.gypi
index e374b9e..b3155e8 100644
--- a/content/content_tests.gypi
+++ b/content/content_tests.gypi
@@ -224,6 +224,7 @@
       'browser/indexed_db/mock_browsertest_indexed_db_class_factory.cc',
       'browser/indexed_db/mock_browsertest_indexed_db_class_factory.h',
       'browser/loader/resource_dispatcher_host_browsertest.cc',
+      'browser/loader/cross_site_resource_handler_browsertest.cc',
       'browser/loader/async_resource_handler_browsertest.cc',
       'browser/manifest/manifest_browsertest.cc',
       'browser/media/android/media_session_browsertest.cc',