Fix the reverted patch from CL 11366042 by addressing lifetimes/acces…

…s to ui and database managers.

BUG=159136


Review URL: https://chromiumcodereview.appspot.com/11411105

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

chrome/browser/memory_purger.cc

@@ -13,6 +13,7 @@
 #include "chrome/browser/history/history.h"
 #include "chrome/browser/history/history_service_factory.h"
 #include "chrome/browser/profiles/profile_manager.h"
+#include "chrome/browser/safe_browsing/database_manager.h"
 #include "chrome/browser/safe_browsing/safe_browsing_service.h"
 #include "chrome/browser/ui/browser_list.h"
 #include "chrome/browser/webdata/web_data_service.h"
@@ -74,7 +75,7 @@ void PurgeMemoryIOHelper::PurgeMemoryOnIOThread() {
   }
 
 #if defined(ENABLE_SAFE_BROWSING)
-  safe_browsing_service_->PurgeMemory();
+  safe_browsing_service_->database_manager()->PurgeMemory();
 #endif
 }
 

chrome/browser/prerender/prerender_browsertest.cc

@@ -26,6 +26,7 @@
 #include "chrome/browser/prerender/prerender_manager.h"
 #include "chrome/browser/prerender/prerender_manager_factory.h"
 #include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/safe_browsing/database_manager.h"
 #include "chrome/browser/safe_browsing/safe_browsing_service.h"
 #include "chrome/browser/task_manager/task_manager.h"
 #include "chrome/browser/task_manager/task_manager_browsertest_util.h"
@@ -34,8 +35,8 @@
 #include "chrome/browser/ui/browser_finder.h"
 #include "chrome/browser/ui/browser_tabstrip.h"
 #include "chrome/browser/ui/browser_window.h"
-#include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/browser/ui/tab_contents/tab_contents.h"
+#include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/common/chrome_notification_types.h"
 #include "chrome/common/chrome_paths.h"
 #include "chrome/common/chrome_switches.h"
@@ -487,12 +488,13 @@ class WaitForLoadPrerenderContentsFactory : public PrerenderContents::Factory {
 };
 
 #if defined(ENABLE_SAFE_BROWSING)
-// A SafeBrowingService implementation that returns a fixed result for a given
-// URL.
-class FakeSafeBrowsingService :  public SafeBrowsingService {
+// A SafeBrowsingDatabaseManager implementation that returns a fixed result for
+// a given URL.
+class FakeSafeBrowsingDatabaseManager :  public SafeBrowsingDatabaseManager {
  public:
-  FakeSafeBrowsingService() :
-      threat_type_(SB_THREAT_TYPE_SAFE) {}
+  explicit FakeSafeBrowsingDatabaseManager(SafeBrowsingService* service)
+      : SafeBrowsingDatabaseManager(service),
+        threat_type_(SB_THREAT_TYPE_SAFE) { }
 
   // Called on the IO thread to check if the given url is safe or not.  If we
   // can synchronously determine that the url is safe, CheckUrl returns true.
@@ -509,8 +511,8 @@ class FakeSafeBrowsingService :  public SafeBrowsingService {
 
     BrowserThread::PostTask(
         BrowserThread::IO, FROM_HERE,
-        base::Bind(&FakeSafeBrowsingService::OnCheckBrowseURLDone, this, gurl,
-                   client));
+        base::Bind(&FakeSafeBrowsingDatabaseManager::OnCheckBrowseURLDone,
+                   this, gurl, client));
     return false;
   }
 
@@ -520,10 +522,10 @@ class FakeSafeBrowsingService :  public SafeBrowsingService {
   }
 
  private:
-  virtual ~FakeSafeBrowsingService() {}
+  virtual ~FakeSafeBrowsingDatabaseManager() {}
 
   void OnCheckBrowseURLDone(const GURL& gurl, Client* client) {
-    SafeBrowsingService::SafeBrowsingCheck check;
+    SafeBrowsingDatabaseManager::SafeBrowsingCheck check;
     check.urls.push_back(gurl);
     check.client = client;
     check.threat_type = threat_type_;
@@ -532,6 +534,31 @@ class FakeSafeBrowsingService :  public SafeBrowsingService {
 
   GURL url_;
   SBThreatType threat_type_;
+  DISALLOW_COPY_AND_ASSIGN(FakeSafeBrowsingDatabaseManager);
+};
+
+class FakeSafeBrowsingService : public SafeBrowsingService {
+ public:
+  FakeSafeBrowsingService() { }
+
+  // Returned pointer has the same lifespan as the database_manager_ refcounted
+  // object.
+  FakeSafeBrowsingDatabaseManager* fake_database_manager() {
+    return fake_database_manager_;
+  }
+
+ protected:
+  virtual ~FakeSafeBrowsingService() { }
+
+  virtual SafeBrowsingDatabaseManager* CreateDatabaseManager() {
+    fake_database_manager_ = new FakeSafeBrowsingDatabaseManager(this);
+    return fake_database_manager_;
+  }
+
+ private:
+  FakeSafeBrowsingDatabaseManager* fake_database_manager_;
+
+  DISALLOW_COPY_AND_ASSIGN(FakeSafeBrowsingService);
 };
 
 // Factory that creates FakeSafeBrowsingService instances.
@@ -879,8 +906,9 @@ class PrerenderBrowserTest : virtual public InProcessBrowserTest {
   }
 
 #if defined(ENABLE_SAFE_BROWSING)
-  FakeSafeBrowsingService* GetSafeBrowsingService() {
-    return safe_browsing_factory_->most_recent_service();
+  FakeSafeBrowsingDatabaseManager* GetFakeSafeBrowsingDatabaseManager() {
+    return safe_browsing_factory_->most_recent_service()->
+        fake_database_manager();
   }
 #endif
 
@@ -2130,7 +2158,7 @@ IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest, PrerenderSSLClientCertIframe) {
 // interstitial.
 IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest, PrerenderSafeBrowsingTopLevel) {
   GURL url = test_server()->GetURL("files/prerender/prerender_page.html");
-  GetSafeBrowsingService()->SetThreatTypeForUrl(
+  GetFakeSafeBrowsingDatabaseManager()->SetThreatTypeForUrl(
       url, SB_THREAT_TYPE_URL_MALWARE);
   PrerenderTestURL("files/prerender/prerender_page.html",
                    FINAL_STATUS_SAFE_BROWSING, 1);
@@ -2140,7 +2168,7 @@ IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest, PrerenderSafeBrowsingTopLevel) {
 IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest,
                        PrerenderSafeBrowsingServerRedirect) {
   GURL url = test_server()->GetURL("files/prerender/prerender_page.html");
-  GetSafeBrowsingService()->SetThreatTypeForUrl(
+  GetFakeSafeBrowsingDatabaseManager()->SetThreatTypeForUrl(
       url, SB_THREAT_TYPE_URL_MALWARE);
   PrerenderTestURL(CreateServerRedirect("files/prerender/prerender_page.html"),
                    FINAL_STATUS_SAFE_BROWSING,
@@ -2151,7 +2179,7 @@ IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest,
 IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest,
                        PrerenderSafeBrowsingClientRedirect) {
   GURL url = test_server()->GetURL("files/prerender/prerender_page.html");
-  GetSafeBrowsingService()->SetThreatTypeForUrl(
+  GetFakeSafeBrowsingDatabaseManager()->SetThreatTypeForUrl(
       url, SB_THREAT_TYPE_URL_MALWARE);
   PrerenderTestURL(CreateClientRedirect("files/prerender/prerender_page.html"),
                    FINAL_STATUS_SAFE_BROWSING,
@@ -2161,7 +2189,7 @@ IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest,
 // Ensures that we do not prerender pages which have a malware subresource.
 IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest, PrerenderSafeBrowsingSubresource) {
   GURL image_url = test_server()->GetURL("files/prerender/image.jpeg");
-  GetSafeBrowsingService()->SetThreatTypeForUrl(
+  GetFakeSafeBrowsingDatabaseManager()->SetThreatTypeForUrl(
       image_url, SB_THREAT_TYPE_URL_MALWARE);
   std::vector<net::TestServer::StringPair> replacement_text;
   replacement_text.push_back(
@@ -2180,7 +2208,7 @@ IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest, PrerenderSafeBrowsingSubresource) {
 IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest, PrerenderSafeBrowsingIframe) {
   GURL iframe_url = test_server()->GetURL(
       "files/prerender/prerender_embedded_content.html");
-  GetSafeBrowsingService()->SetThreatTypeForUrl(
+  GetFakeSafeBrowsingDatabaseManager()->SetThreatTypeForUrl(
       iframe_url, SB_THREAT_TYPE_URL_MALWARE);
   std::vector<net::TestServer::StringPair> replacement_text;
   replacement_text.push_back(

chrome/browser/renderer_host/safe_browsing_resource_throttle.cc

@@ -9,6 +9,7 @@
 #include "chrome/browser/prerender/prerender_tracker.h"
 #include "chrome/browser/renderer_host/chrome_url_request_user_data.h"
 #include "chrome/browser/renderer_host/safe_browsing_resource_throttle_factory.h"
+#include "chrome/browser/safe_browsing/safe_browsing_service.h"
 #include "content/public/browser/resource_controller.h"
 #include "net/base/load_flags.h"
 #include "net/url_request/url_request.h"
@@ -44,14 +45,15 @@ SafeBrowsingResourceThrottle::SafeBrowsingResourceThrottle(
       threat_type_(SB_THREAT_TYPE_SAFE),
       render_process_host_id_(render_process_host_id),
       render_view_id_(render_view_id),
-      safe_browsing_(safe_browsing),
+      database_manager_(safe_browsing->database_manager()),
+      ui_manager_(safe_browsing->ui_manager()),
       request_(request),
       is_subresource_(is_subresource) {
 }
 
 SafeBrowsingResourceThrottle::~SafeBrowsingResourceThrottle() {
   if (state_ == STATE_CHECKING_URL)
-    safe_browsing_->CancelCheck(this);
+    database_manager_->CancelCheck(this);
 }
 
 void SafeBrowsingResourceThrottle::WillStartRequest(bool* defer) {
@@ -100,8 +102,7 @@ void SafeBrowsingResourceThrottle::OnCheckBrowseUrlResult(
 
   if (threat_type == SB_THREAT_TYPE_SAFE) {
     // Log how much time the safe browsing check cost us.
-    safe_browsing_->LogPauseDelay(
-        base::TimeTicks::Now() - url_check_start_time_);
+    ui_manager_->LogPauseDelay(base::TimeTicks::Now() - url_check_start_time_);
 
     // Continue the request.
     ResumeRequest();
@@ -139,7 +140,7 @@ void SafeBrowsingResourceThrottle::StartDisplayingBlockingPage(
 
   state_ = STATE_DISPLAYING_BLOCKING_PAGE;
 
-  safe_browsing_->DisplayBlockingPage(
+  ui_manager_->DisplayBlockingPage(
       url,
       request_->original_url(),
       redirect_urls_,
@@ -168,10 +169,10 @@ void SafeBrowsingResourceThrottle::OnBlockingPageComplete(bool proceed) {
 
 bool SafeBrowsingResourceThrottle::CheckUrl(const GURL& url) {
   CHECK(state_ == STATE_NONE);
-  bool succeeded_synchronously = safe_browsing_->CheckBrowseUrl(url, this);
+  bool succeeded_synchronously = database_manager_->CheckBrowseUrl(url, this);
   if (succeeded_synchronously) {
     threat_type_ = SB_THREAT_TYPE_SAFE;
-    safe_browsing_->LogPauseDelay(base::TimeDelta());  // No delay.
+    ui_manager_->LogPauseDelay(base::TimeDelta());  // No delay.
     return true;
   }
 
@@ -193,7 +194,7 @@ void SafeBrowsingResourceThrottle::OnCheckUrlTimeout() {
   CHECK(state_ == STATE_CHECKING_URL);
   CHECK(defer_state_ != DEFERRED_NONE);
 
-  safe_browsing_->CancelCheck(this);
+  database_manager_->CancelCheck(this);
   OnCheckBrowseUrlResult(url_being_checked_, SB_THREAT_TYPE_SAFE);
 }
 

chrome/browser/renderer_host/safe_browsing_resource_throttle.h

@@ -11,7 +11,8 @@
 #include "base/memory/ref_counted.h"
 #include "base/time.h"
 #include "base/timer.h"
-#include "chrome/browser/safe_browsing/safe_browsing_service.h"
+#include "chrome/browser/safe_browsing/database_manager.h"
+#include "chrome/browser/safe_browsing/ui_manager.h"
 #include "content/public/browser/resource_throttle.h"
 
 class ResourceDispatcherHost;
@@ -44,7 +45,7 @@ class URLRequest;
 // resumed.
 class SafeBrowsingResourceThrottle
     : public content::ResourceThrottle,
-      public SafeBrowsingService::Client,
+      public SafeBrowsingDatabaseManager::Client,
       public base::SupportsWeakPtr<SafeBrowsingResourceThrottle> {
  public:
   SafeBrowsingResourceThrottle(const net::URLRequest* request,
@@ -57,7 +58,7 @@ class SafeBrowsingResourceThrottle
   virtual void WillStartRequest(bool* defer) OVERRIDE;
   virtual void WillRedirectRequest(const GURL& new_url, bool* defer) OVERRIDE;
 
-  // SafeBrowsingService::Client implementation (called on IO thread):
+  // SafeBrowsingDabaseManager::Client implementation (called on IO thread):
   virtual void OnCheckBrowseUrlResult(
       const GURL& url, SBThreatType result) OVERRIDE;
 
@@ -117,7 +118,8 @@ class SafeBrowsingResourceThrottle
 
   int render_process_host_id_;
   int render_view_id_;
-  scoped_refptr<SafeBrowsingService> safe_browsing_;
+  scoped_refptr<SafeBrowsingDatabaseManager> database_manager_;
+  scoped_refptr<SafeBrowsingUIManager> ui_manager_;
   const net::URLRequest* request_;
   bool is_subresource_;
 

chrome/browser/safe_browsing/browser_feature_extractor.h

@@ -24,6 +24,7 @@
 #include "chrome/browser/common/cancelable_request.h"
 #include "chrome/browser/history/history_types.h"
 #include "chrome/browser/safe_browsing/safe_browsing_service.h"
+#include "chrome/browser/safe_browsing/ui_manager.h"
 #include "googleurl/src/gurl.h"
 
 class HistoryService;
@@ -43,7 +44,7 @@ struct BrowseInfo {
 
   // If a SafeBrowsing interstitial was shown for the current URL
   // this will contain the UnsafeResource struct for that URL.
-  scoped_ptr<SafeBrowsingService::UnsafeResource> unsafe_resource;
+  scoped_ptr<SafeBrowsingUIManager::UnsafeResource> unsafe_resource;
 
   // List of redirects that lead to the first page on the current host and
   // the current url respectively. These may be the same if the current url

chrome/browser/safe_browsing/browser_feature_extractor_unittest.cc

@@ -18,6 +18,7 @@
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/safe_browsing/browser_features.h"
 #include "chrome/browser/safe_browsing/client_side_detection_service.h"
+#include "chrome/browser/safe_browsing/ui_manager.h"
 #include "chrome/common/safe_browsing/csd.pb.h"
 #include "chrome/test/base/chrome_render_view_host_test_harness.h"
 #include "chrome/test/base/testing_profile.h"
@@ -500,7 +501,8 @@ TEST_F(BrowserFeatureExtractorTest, SafeBrowsingFeatures) {
   request.set_url("http://www.foo.com/malware.html");
   request.set_client_score(0.5);
 
-  browse_info_->unsafe_resource.reset(new SafeBrowsingService::UnsafeResource);
+  browse_info_->unsafe_resource.reset(
+      new SafeBrowsingUIManager::UnsafeResource);
   browse_info_->unsafe_resource->url = GURL("http://www.malware.com/");
   browse_info_->unsafe_resource->original_url = GURL("http://www.good.com/");
   browse_info_->unsafe_resource->is_subresource = true;