CrossSiteResourceHandler: cancel request if the RFH is gone

content/browser/cross_site_transfer_browsertest.cc

 // Copyright (c) 2012 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/command_line.h"
 #include "base/strings/stringprintf.h"
 #include "content/browser/loader/resource_dispatcher_host_impl.h"
 #include "content/public/browser/navigation_entry.h"
 #include "content/public/browser/resource_dispatcher_host_delegate.h"
 #include "content/public/browser/resource_throttle.h"
@@ skipping 46 matching lines @@
 
     // 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));
+            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));
     run_loop_->Run();
     run_loop_.reset();
     return tracked_request_completed_;
   }
 
  private:
   // ResourceThrottle attached to request for the tracked URL.  On destruction,
@@ skipping 18 matching lines @@
       return "TrackingThrottle";
     }
 
    private:
     net::URLRequest* request_;
     TrackingResourceDispatcherHostDelegate* tracker_;
 
     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) {
     CHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
     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);
 };
 
 // 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() {}
 
   WebContents* OpenURLFromTab(WebContents* source,
                               const OpenURLParams& params) override {
     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;
     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 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());
   }
 
   void TearDownOnMainThread() override {
     BrowserThread::PostTask(
         BrowserThread::IO, FROM_HERE,
         base::Bind(
             &CrossSiteTransferTest::RestoreResourceDisptcherHostDelegate,
@@ skipping 14 matching lines @@
     bool result = ExecuteScript(window->web_contents(), script);
     EXPECT_TRUE(result);
     if (should_wait_for_navigation)
       load_observer.Wait();
   }
 
   void SetUpCommandLine(base::CommandLine* command_line) override {
     IsolateAllSitesForTesting(command_line);
   }
 
-  void InjectResourceDisptcherHostDelegate() {
+  void InjectResourceDispatcherHostDelegate() {
     DCHECK_CURRENTLY_ON(BrowserThread::IO);
     old_delegate_ = ResourceDispatcherHostImpl::Get()->delegate();
     ResourceDispatcherHostImpl::Get()->SetDelegate(&tracking_delegate_);
   }
 
   void RestoreResourceDisptcherHostDelegate() {
     DCHECK_CURRENTLY_ON(BrowserThread::IO);
     ResourceDispatcherHostImpl::Get()->SetDelegate(old_delegate_);
-    old_delegate_ = NULL;
+    old_delegate_ = nullptr;
   }
 
   TrackingResourceDispatcherHostDelegate& tracking_delegate() {
     return tracking_delegate_;
   }
 
  private:
   TrackingResourceDispatcherHostDelegate tracking_delegate_;
   ResourceDispatcherHostDelegate* old_delegate_;
 };
@@ skipping 12 matching lines @@
 #endif
 // Tests that the |should_replace_current_entry| flag persists correctly across
 // request transfers that began with a cross-process navigation.
 IN_PROC_BROWSER_TEST_F(CrossSiteTransferTest,
                        MAYBE_ReplaceEntryCrossProcessThenTransfer) {
   const NavigationController& controller =
       shell()->web_contents()->GetController();
 
   // 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
   // transfer). This test intentionally exercises that case.
   ShellContentBrowserClient::SetSwapProcessesForRedirect(true);
 
   // 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 =
       embedded_test_server()->GetURL("A.com", "/site_isolation/blank.html?2");
   // 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_TRUE(controller.GetPendingEntry() == nullptr);
   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
   // OpenURL and then transfer.
   GURL url3 =
       embedded_test_server()->GetURL("B.com", "/site_isolation/blank.html?3");
   // 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.
-  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());
   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
 // request transfers that began with a content-initiated in-process
 // navigation. This test is the same as the test above, except transfering from
 // in-process.
 IN_PROC_BROWSER_TEST_F(CrossSiteTransferTest,
                        ReplaceEntryInProcessThenTranfers) {
   const NavigationController& controller =
       shell()->web_contents()->GetController();
 
   // 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
   // transfer). All navigations in this test are same-site, so it only swaps
   // processes via request transfer.
   ShellContentBrowserClient::SetSwapProcessesForRedirect(true);
 
   // Navigate in-process with entry replacement. It will then be transferred
   // into a new one due to the call above.
   GURL url2 = embedded_test_server()->GetURL("/site_isolation/blank.html?2");
   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());
 
   // Now navigate as before, but without replacement.
   GURL url3 = embedded_test_server()->GetURL("/site_isolation/blank.html?3");
   NavigateToURLContentInitiated(shell(), url3, false, true);
 
   // 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());
   EXPECT_EQ(url3, controller.GetEntryAtIndex(1)->GetURL());
 }
 
 // Tests that the |should_replace_current_entry| flag persists correctly across
 // request transfers that cross processes twice from renderer policy.
 IN_PROC_BROWSER_TEST_F(CrossSiteTransferTest,
                        MAYBE_ReplaceEntryCrossProcessTwice) {
   const NavigationController& controller =
       shell()->web_contents()->GetController();
 
   // 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,
   // and second in response to the server redirect to B.com. The second swap is
   // also renderer-initiated via OpenURL because decidePolicyForNavigation is
   // currently applied on redirects.
   GURL::Replacements replace_host;
   GURL url2b =
       embedded_test_server()->GetURL("B.com", "/site_isolation/blank.html?2");
   GURL url2a = embedded_test_server()->GetURL(
       "A.com", "/cross-site/" + url2b.host() + url2b.PathForRequest());
   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());
 
   // Now repeat without replacement.
   GURL url3b =
       embedded_test_server()->GetURL("B.com", "/site_isolation/blank.html?3");
   GURL url3a = embedded_test_server()->GetURL(
       "A.com", "/cross-site/" + url3b.host() + url3b.PathForRequest());
   NavigateToURLContentInitiated(shell(), url3a, false, true);
 
   // 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());
   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(CrossSiteTransferTest, NoLeakOnCrossSiteCancel) {
   const NavigationController& controller =
       shell()->web_contents()->GetController();
 
   // 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);
 
   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
@@ skipping 14 matching lines @@
   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