Remove calls of PrerenderTracker::TryCancelOnIOThread in SafeBrowsingResourceThrottle and instead PrerenderContents::Destroy directly on the UI thread.

This is part of the work on removing calls to ResourceRequestInfo::GetAssociatedRenderView. The last callers to that method are using it to cancel prerenders if they haven't been used yet, so the checking/cancelling needs to all move to the UI thread.

BUG=304341
R=davidben@chromium.org

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

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

9 files changed, 109 insertions, 109 deletions

diff --git a/chrome/browser/prerender/prerender_contents.cc b/chrome/browser/prerender/prerender_contents.cc
index 714f85c..952e809 100644
--- a/chrome/browser/prerender/prerender_contents.cc
+++ b/chrome/browser/prerender/prerender_contents.cc
@@ -17,6 +17,7 @@
 #include "chrome/browser/prerender/prerender_final_status.h"
 #include "chrome/browser/prerender/prerender_handle.h"
 #include "chrome/browser/prerender/prerender_manager.h"
+#include "chrome/browser/prerender/prerender_manager_factory.h"
 #include "chrome/browser/prerender/prerender_tracker.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser.h"
@@ -237,6 +238,18 @@ PrerenderContents::Factory* PrerenderContents::CreateFactory() {
   return new PrerenderContentsFactoryImpl();
 }
 
+// static
+PrerenderContents* PrerenderContents::FromWebContents(
+    content::WebContents* web_contents) {
+  if (!web_contents)
+    return NULL;
+  PrerenderManager* prerender_manager = PrerenderManagerFactory::GetForProfile(
+      Profile::FromBrowserContext(web_contents->GetBrowserContext()));
+  if (!prerender_manager)
+    return NULL;
+  return prerender_manager->GetPrerenderContents(web_contents);
+}
+
 void PrerenderContents::StartPrerendering(
     int creator_child_id,
     const gfx::Size& size,
diff --git a/chrome/browser/prerender/prerender_contents.h b/chrome/browser/prerender/prerender_contents.h
index 25cc3e1..e17d4509 100644
--- a/chrome/browser/prerender/prerender_contents.h
+++ b/chrome/browser/prerender/prerender_contents.h
@@ -148,6 +148,11 @@ class PrerenderContents : public content::NotificationObserver,
 
   static Factory* CreateFactory();
 
+  // Returns a PrerenderContents from the given web_contents, if it's used for
+  // prerendering. Otherwise returns NULL. Handles a NULL input for
+  // convenience.
+  static PrerenderContents* FromWebContents(content::WebContents* web_contents);
+
   // Start rendering the contents in the prerendered state. If
   // |is_control_group| is true, this will go through some of the mechanics of
   // starting a prerender, without actually creating the RenderView.
diff --git a/chrome/browser/prerender/prerender_resource_throttle.cc b/chrome/browser/prerender/prerender_resource_throttle.cc
index 304879f..b9fe1d3 100644
--- a/chrome/browser/prerender/prerender_resource_throttle.cc
+++ b/chrome/browser/prerender/prerender_resource_throttle.cc
@@ -6,10 +6,8 @@
 
 #include "chrome/browser/prerender/prerender_final_status.h"
 #include "chrome/browser/prerender/prerender_manager.h"
-#include "chrome/browser/prerender/prerender_manager_factory.h"
 #include "chrome/browser/prerender/prerender_tracker.h"
 #include "chrome/browser/prerender/prerender_util.h"
-#include "chrome/browser/profiles/profile.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/resource_controller.h"
@@ -180,16 +178,7 @@ PrerenderContents* PrerenderResourceThrottle::PrerenderContentsFromRenderFrame(
       render_process_id, render_frame_id);
   content::WebContents* web_contents =
       content::WebContents::FromRenderFrameHost(rfh);
-  if (!web_contents)
-    return NULL;
-
-  PrerenderManager* prerender_manager =
-      PrerenderManagerFactory::GetForProfile(
-          Profile::FromBrowserContext(web_contents->GetBrowserContext()));
-  if (!prerender_manager)
-    return NULL;
-
-  return prerender_manager->GetPrerenderContents(web_contents);
+  return PrerenderContents::FromWebContents(web_contents);
 }
 
 void PrerenderResourceThrottle::AddResourceThrottle() {
diff --git a/chrome/browser/renderer_host/safe_browsing_resource_throttle.cc b/chrome/browser/renderer_host/safe_browsing_resource_throttle.cc
index e5cebb1..c6923ee 100644
--- a/chrome/browser/renderer_host/safe_browsing_resource_throttle.cc
+++ b/chrome/browser/renderer_host/safe_browsing_resource_throttle.cc
@@ -6,11 +6,14 @@
 
 #include "base/logging.h"
 #include "chrome/browser/browser_process.h"
-#include "chrome/browser/prerender/prerender_tracker.h"
+#include "chrome/browser/prerender/prerender_contents.h"
 #include "chrome/browser/renderer_host/chrome_url_request_user_data.h"
 #include "chrome/browser/safe_browsing/safe_browsing_service.h"
+#include "content/public/browser/browser_thread.h"
+#include "content/public/browser/render_view_host.h"
 #include "content/public/browser/resource_controller.h"
 #include "content/public/browser/resource_request_info.h"
+#include "content/public/browser/web_contents.h"
 #include "net/base/load_flags.h"
 #include "net/url_request/url_request.h"
 
@@ -94,54 +97,72 @@ void SafeBrowsingResourceThrottle::OnCheckBrowseUrlResult(
 
     // Continue the request.
     ResumeRequest();
-  } else {
-    bool should_show_blocking_page = true;
-    if (request_->load_flags() & net::LOAD_PREFETCH) {
-      // Don't prefetch resources that fail safe browsing, disallow
-      // them.
-      controller()->Cancel();
-      should_show_blocking_page = false;
-    } else {
-      ChromeURLRequestUserData* user_data =
-          ChromeURLRequestUserData::Get(request_);
-      if (user_data && user_data->is_prerender()) {
-        const content::ResourceRequestInfo* info =
-            content::ResourceRequestInfo::ForRequest(request_);
-        prerender::PrerenderTracker* prerender_tracker = g_browser_process->
-            prerender_tracker();
-        if (prerender_tracker->TryCancelOnIOThread(
-                info->GetChildID(),
-                info->GetRouteID(),
-                prerender::FINAL_STATUS_SAFE_BROWSING)) {
-          controller()->Cancel();
-          should_show_blocking_page = false;
-        }
-      }
-    }
-    if (should_show_blocking_page)
-      StartDisplayingBlockingPage(url, threat_type);
+    return;
+  }
+
+  if (request_->load_flags() & net::LOAD_PREFETCH) {
+    // Don't prefetch resources that fail safe browsing, disallow
+    // them.
+    controller()->Cancel();
+    return;
   }
+
+  const content::ResourceRequestInfo* info =
+      content::ResourceRequestInfo::ForRequest(request_);
+
+  SafeBrowsingUIManager::UnsafeResource resource;
+  resource.url = url;
+  resource.original_url = request_->original_url();
+  resource.redirect_urls = redirect_urls_;
+  resource.is_subresource = is_subresource_;
+  resource.threat_type = threat_type;
+  resource.callback = base::Bind(
+      &SafeBrowsingResourceThrottle::OnBlockingPageComplete, AsWeakPtr());
+  resource.render_process_host_id = info->GetChildID();
+  resource.render_view_id =  info->GetRouteID();
+
+  state_ = STATE_DISPLAYING_BLOCKING_PAGE;
+
+  content::BrowserThread::PostTask(
+      content::BrowserThread::UI,
+      FROM_HERE,
+      base::Bind(&SafeBrowsingResourceThrottle::StartDisplayingBlockingPage,
+                 AsWeakPtr(), ui_manager_, resource));
 }
 
 void SafeBrowsingResourceThrottle::StartDisplayingBlockingPage(
-    const GURL& url, SBThreatType threat_type) {
-  CHECK(state_ == STATE_NONE);
-  CHECK(defer_state_ != DEFERRED_NONE);
+    const base::WeakPtr<SafeBrowsingResourceThrottle>& throttle,
+    scoped_refptr<SafeBrowsingUIManager> ui_manager,
+    const SafeBrowsingUIManager::UnsafeResource& resource) {
+  bool should_show_blocking_page = true;
+
+  content::RenderViewHost* rvh = content::RenderViewHost::FromID(
+      resource.render_process_host_id, resource.render_view_id);
+  if (rvh) {
+    content::WebContents* web_contents =
+        content::WebContents::FromRenderViewHost(rvh);
+    prerender::PrerenderContents* prerender_contents =
+        prerender::PrerenderContents::FromWebContents(web_contents);
+    if (prerender_contents) {
+      prerender_contents->Destroy(prerender::FINAL_STATUS_SAFE_BROWSING);
+      should_show_blocking_page = false;
+    }
 
-  state_ = STATE_DISPLAYING_BLOCKING_PAGE;
+    if (should_show_blocking_page)  {
+      ui_manager->DisplayBlockingPage(resource);
+      return;
+    }
+  }
 
-  const content::ResourceRequestInfo* info =
-      content::ResourceRequestInfo::ForRequest(request_);
-  ui_manager_->DisplayBlockingPage(
-      url,
-      request_->original_url(),
-      redirect_urls_,
-      is_subresource_,
-      threat_type,
-      base::Bind(
-          &SafeBrowsingResourceThrottle::OnBlockingPageComplete, AsWeakPtr()),
-      info->GetChildID(),
-      info->GetRouteID());
+  // Tab is gone or it's being prerendered.
+  content::BrowserThread::PostTask(
+      content::BrowserThread::IO,
+      FROM_HERE,
+      base::Bind(&SafeBrowsingResourceThrottle::Cancel, throttle));
+}
+
+void SafeBrowsingResourceThrottle::Cancel() {
+  controller()->Cancel();
 }
 
 // SafeBrowsingService::UrlCheckCallback implementation, called on the IO
diff --git a/chrome/browser/renderer_host/safe_browsing_resource_throttle.h b/chrome/browser/renderer_host/safe_browsing_resource_throttle.h
index 45483c1..5246f99 100644
--- a/chrome/browser/renderer_host/safe_browsing_resource_throttle.h
+++ b/chrome/browser/renderer_host/safe_browsing_resource_throttle.h
@@ -90,8 +90,16 @@ class SafeBrowsingResourceThrottle
   // StartCheckingUrl()) has taken longer than kCheckUrlTimeoutMs.
   void OnCheckUrlTimeout();
 
-  // Starts displaying the safe browsing interstitial page.
-  void StartDisplayingBlockingPage(const GURL& url, SBThreatType threat_type);
+  // Starts displaying the safe browsing interstitial page if it's not
+  // prerendering. Called on the UI thread.
+  static void StartDisplayingBlockingPage(
+      const base::WeakPtr<SafeBrowsingResourceThrottle>& throttle,
+      scoped_refptr<SafeBrowsingUIManager> ui_manager,
+      const SafeBrowsingUIManager::UnsafeResource& resource);
+
+  // Called on the IO thread if the request turned out to be for a prerendered
+  // page.
+  void Cancel();
 
   // Resumes the request, by continuing the deferred action (either starting the
   // request, or following a redirect).
diff --git a/chrome/browser/safe_browsing/client_side_detection_host.cc b/chrome/browser/safe_browsing/client_side_detection_host.cc
index aeff3d6..130f0ca 100644
--- a/chrome/browser/safe_browsing/client_side_detection_host.cc
+++ b/chrome/browser/safe_browsing/client_side_detection_host.cc
@@ -497,7 +497,7 @@ void ClientSideDetectionHost::MaybeShowPhishingWarning(GURL phishing_url,
         // might not get created properly.
         web_contents()->GetController().DiscardNonCommittedEntries();
       }
-      ui_manager_->DoDisplayBlockingPage(resource);
+      ui_manager_->DisplayBlockingPage(resource);
     }
     // If there is true phishing verdict, invalidate weakptr so that no longer
     // consider the malware vedict.
@@ -528,7 +528,7 @@ void ClientSideDetectionHost::MaybeShowMalwareWarning(GURL original_url,
         // might not get created properly.
         web_contents()->GetController().DiscardNonCommittedEntries();
       }
-      ui_manager_->DoDisplayBlockingPage(resource);
+      ui_manager_->DisplayBlockingPage(resource);
     }
     // If there is true malware verdict, invalidate weakptr so that no longer
     // consider the phishing vedict.
diff --git a/chrome/browser/safe_browsing/client_side_detection_host_unittest.cc b/chrome/browser/safe_browsing/client_side_detection_host_unittest.cc
index 3fea44f..3a66f8a 100644
--- a/chrome/browser/safe_browsing/client_side_detection_host_unittest.cc
+++ b/chrome/browser/safe_browsing/client_side_detection_host_unittest.cc
@@ -133,7 +133,7 @@ class MockSafeBrowsingUIManager : public SafeBrowsingUIManager {
   explicit MockSafeBrowsingUIManager(SafeBrowsingService* service)
       : SafeBrowsingUIManager(service) { }
 
-  MOCK_METHOD1(DoDisplayBlockingPage, void(const UnsafeResource& resource));
+  MOCK_METHOD1(DisplayBlockingPage, void(const UnsafeResource& resource));
 
   // Helper function which calls OnBlockingPageComplete for this client
   // object.
@@ -465,8 +465,8 @@ TEST_F(ClientSideDetectionHostTest, OnPhishingDetectionDoneNotPhishing) {
   EXPECT_TRUE(Mock::VerifyAndClear(csd_host_.get()));
   ASSERT_FALSE(cb.is_null());
 
-  // Make sure DoDisplayBlockingPage is not going to be called.
-  EXPECT_CALL(*ui_manager_.get(), DoDisplayBlockingPage(_)).Times(0);
+  // Make sure DisplayBlockingPage is not going to be called.
+  EXPECT_CALL(*ui_manager_.get(), DisplayBlockingPage(_)).Times(0);
   cb.Run(GURL(verdict.url()), false);
   base::RunLoop().RunUntilIdle();
   EXPECT_TRUE(Mock::VerifyAndClear(ui_manager_.get()));
@@ -506,8 +506,8 @@ TEST_F(ClientSideDetectionHostTest, OnPhishingDetectionDoneDisabled) {
   EXPECT_TRUE(Mock::VerifyAndClear(csd_host_.get()));
   ASSERT_FALSE(cb.is_null());
 
-  // Make sure DoDisplayBlockingPage is not going to be called.
-  EXPECT_CALL(*ui_manager_.get(), DoDisplayBlockingPage(_)).Times(0);
+  // Make sure DisplayBlockingPage is not going to be called.
+  EXPECT_CALL(*ui_manager_.get(), DisplayBlockingPage(_)).Times(0);
   cb.Run(GURL(verdict.url()), false);
   base::RunLoop().RunUntilIdle();
   EXPECT_TRUE(Mock::VerifyAndClear(ui_manager_.get()));
@@ -547,7 +547,7 @@ TEST_F(ClientSideDetectionHostTest, OnPhishingDetectionDoneShowInterstitial) {
   ASSERT_FALSE(cb.is_null());
 
   UnsafeResource resource;
-  EXPECT_CALL(*ui_manager_.get(), DoDisplayBlockingPage(_))
+  EXPECT_CALL(*ui_manager_.get(), DisplayBlockingPage(_))
       .WillOnce(SaveArg<0>(&resource));
   cb.Run(phishing_url, true);
 
@@ -640,7 +640,7 @@ TEST_F(ClientSideDetectionHostTest, OnPhishingDetectionDoneMultiplePings) {
   // We expect that the interstitial is shown for the second phishing URL and
   // not for the first phishing URL.
   UnsafeResource resource;
-  EXPECT_CALL(*ui_manager_.get(), DoDisplayBlockingPage(_))
+  EXPECT_CALL(*ui_manager_.get(), DisplayBlockingPage(_))
       .WillOnce(SaveArg<0>(&resource));
 
   cb.Run(phishing_url, true);  // Should have no effect.
@@ -970,7 +970,7 @@ TEST_F(ClientSideDetectionHostTest,
   ASSERT_FALSE(cb.is_null());
 
   UnsafeResource resource;
-  EXPECT_CALL(*ui_manager_.get(), DoDisplayBlockingPage(_))
+  EXPECT_CALL(*ui_manager_.get(), DisplayBlockingPage(_))
       .WillOnce(SaveArg<0>(&resource));
   cb.Run(malware_landing_url, malware_ip_url, true);
 
@@ -1153,7 +1153,7 @@ TEST_F(ClientSideDetectionHostTest, ShouldClassifyUrl) {
                                 NULL);
 
   UnsafeResource resource;
-  EXPECT_CALL(*ui_manager_.get(), DoDisplayBlockingPage(_))
+  EXPECT_CALL(*ui_manager_.get(), DisplayBlockingPage(_))
       .WillOnce(SaveArg<0>(&resource));
 
   NavigateAndCommit(url);
diff --git a/chrome/browser/safe_browsing/ui_manager.cc b/chrome/browser/safe_browsing/ui_manager.cc
index d97de29..611e95b 100644
--- a/chrome/browser/safe_browsing/ui_manager.cc
+++ b/chrome/browser/safe_browsing/ui_manager.cc
@@ -73,36 +73,10 @@ bool SafeBrowsingUIManager::CanReportStats() const {
   return metrics && metrics->reporting_active();
 }
 
-void SafeBrowsingUIManager::DisplayBlockingPage(
-    const GURL& url,
-    const GURL& original_url,
-    const std::vector<GURL>& redirect_urls,
-    bool is_subresource,
-    SBThreatType threat_type,
-    const UrlCheckCallback& callback,
-    int render_process_host_id,
-    int render_view_id) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
-  UnsafeResource resource;
-  resource.url = url;
-  resource.original_url = original_url;
-  resource.redirect_urls = redirect_urls;
-  resource.is_subresource = is_subresource;
-  resource.threat_type = threat_type;
-  resource.callback = callback;
-  resource.render_process_host_id = render_process_host_id;
-  resource.render_view_id = render_view_id;
-
-  // The blocking page must be created from the UI thread.
-  BrowserThread::PostTask(
-      BrowserThread::UI, FROM_HERE,
-      base::Bind(&SafeBrowsingUIManager::DoDisplayBlockingPage, this,
-                 resource));
-}
-
 void SafeBrowsingUIManager::OnBlockingPageDone(
     const std::vector<UnsafeResource>& resources,
     bool proceed) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
   for (std::vector<UnsafeResource>::const_iterator iter = resources.begin();
        iter != resources.end(); ++iter) {
     const UnsafeResource& resource = *iter;
@@ -118,7 +92,7 @@ void SafeBrowsingUIManager::OnBlockingPageDone(
   }
 }
 
-void SafeBrowsingUIManager::DoDisplayBlockingPage(
+void SafeBrowsingUIManager::DisplayBlockingPage(
     const UnsafeResource& resource) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
diff --git a/chrome/browser/safe_browsing/ui_manager.h b/chrome/browser/safe_browsing/ui_manager.h
index 53a3c03..81638e9 100644
--- a/chrome/browser/safe_browsing/ui_manager.h
+++ b/chrome/browser/safe_browsing/ui_manager.h
@@ -45,7 +45,7 @@ class SafeBrowsingUIManager
     std::vector<GURL> redirect_urls;
     bool is_subresource;
     SBThreatType threat_type;
-    UrlCheckCallback callback;
+    UrlCheckCallback callback;  // This is called back on the IO thread.
     int render_process_host_id;
     int render_view_id;
   };
@@ -87,22 +87,12 @@ class SafeBrowsingUIManager
   // could be reported.
   virtual bool CanReportStats() const;
 
-  // Called on the IO thread to display an interstitial page.
+  // Called on the UI thread to display an interstitial page.
   // |url| is the url of the resource that matches a safe browsing list.
   // If the request contained a chain of redirects, |url| is the last url
   // in the chain, and |original_url| is the first one (the root of the
   // chain). Otherwise, |original_url| = |url|.
-  void DisplayBlockingPage(const GURL& url,
-                           const GURL& original_url,
-                           const std::vector<GURL>& redirect_urls,
-                           bool is_subresource,
-                           SBThreatType threat_type,
-                           const UrlCheckCallback& callback,
-                           int render_process_host_id,
-                           int render_view_id);
-
-  // Same as above but gets invoked on the UI thread.
-  virtual void DoDisplayBlockingPage(const UnsafeResource& resource);
+  virtual void DisplayBlockingPage(const UnsafeResource& resource);
 
   // Returns true if we already displayed an interstitial for that resource.
   // Called on the UI thread.