[SafeBrowsing] Add accessors to provide an API providing SB status of current page.

This change adds the ability for callers to get notified when a page gets an SB hit, or access that information
from the ClientSideDetectionHost.

R=mattm@chromium.org

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

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

7 files changed, 115 insertions, 18 deletions

diff --git a/chrome/browser/safe_browsing/client_side_detection_host.cc b/chrome/browser/safe_browsing/client_side_detection_host.cc
index 5e20945..639216e4 100644
--- a/chrome/browser/safe_browsing/client_side_detection_host.cc
+++ b/chrome/browser/safe_browsing/client_side_detection_host.cc
@@ -47,14 +47,6 @@ namespace safe_browsing {
 const int ClientSideDetectionHost::kMaxUrlsPerIP = 20;
 const int ClientSideDetectionHost::kMaxIPsPerBrowse = 200;
 
-namespace {
-
-void EmptyUrlCheckCallback(bool processed) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
-}
-
-}  // namespace
-
 // This class is instantiated each time a new toplevel URL loads, and
 // asynchronously checks whether the phishing classifier should run for this
 // URL.  If so, it notifies the renderer with a StartPhishingDetection IPC.
@@ -256,7 +248,8 @@ ClientSideDetectionHost::ClientSideDetectionHost(WebContents* tab)
       weak_factory_(this),
       unsafe_unique_page_id_(-1),
       malware_killswitch_on_(false),
-      malware_report_enabled_(false) {
+      malware_report_enabled_(false),
+      malware_or_phishing_match_(false) {
   DCHECK(tab);
   // Note: csd_service_ and sb_service will be NULL here in testing.
   csd_service_ = g_browser_process->safe_browsing_detection_service();
@@ -298,6 +291,8 @@ bool ClientSideDetectionHost::OnMessageReceived(const IPC::Message& message) {
 void ClientSideDetectionHost::DidNavigateMainFrame(
     const content::LoadCommittedDetails& details,
     const content::FrameNavigateParams& params) {
+  malware_or_phishing_match_ = false;
+
   // TODO(noelutz): move this DCHECK to WebContents and fix all the unit tests
   // that don't call this method on the UI thread.
   // DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
@@ -366,9 +361,18 @@ void ClientSideDetectionHost::OnSafeBrowsingHit(
   unsafe_resource_->callback.Reset();  // Don't do anything stupid.
 }
 
+void ClientSideDetectionHost::OnSafeBrowsingMatch(
+    const SafeBrowsingUIManager::UnsafeResource& resource) {
+  malware_or_phishing_match_ = true;
+}
+
 scoped_refptr<SafeBrowsingDatabaseManager>
 ClientSideDetectionHost::database_manager() {
-    return database_manager_;
+  return database_manager_;
+}
+
+bool ClientSideDetectionHost::DidPageReceiveSafeBrowsingMatch() const {
+  return malware_or_phishing_match_ || DidShowSBInterstitial();
 }
 
 void ClientSideDetectionHost::WebContentsDestroyed(WebContents* tab) {
@@ -463,9 +467,8 @@ void ClientSideDetectionHost::MaybeShowPhishingWarning(GURL phishing_url,
         // We need to stop any pending navigations, otherwise the interstital
         // might not get created properly.
         web_contents()->GetController().DiscardNonCommittedEntries();
-        resource.callback = base::Bind(&EmptyUrlCheckCallback);
-        ui_manager_->DoDisplayBlockingPage(resource);
       }
+      ui_manager_->DoDisplayBlockingPage(resource);
     }
     // If there is true phishing verdict, invalidate weakptr so that no longer
     // consider the malware vedict.
@@ -495,9 +498,8 @@ void ClientSideDetectionHost::MaybeShowMalwareWarning(GURL original_url,
         // We need to stop any pending navigations, otherwise the interstital
         // might not get created properly.
         web_contents()->GetController().DiscardNonCommittedEntries();
-        resource.callback = base::Bind(&EmptyUrlCheckCallback);
-        ui_manager_->DoDisplayBlockingPage(resource);
       }
+      ui_manager_->DoDisplayBlockingPage(resource);
     }
     // If there is true malware verdict, invalidate weakptr so that no longer
     // consider the phishing vedict.
@@ -575,7 +577,7 @@ void ClientSideDetectionHost::Observe(
   }
 }
 
-bool ClientSideDetectionHost::DidShowSBInterstitial() {
+bool ClientSideDetectionHost::DidShowSBInterstitial() const {
   if (unsafe_unique_page_id_ <= 0 || !web_contents()) {
     return false;
   }
diff --git a/chrome/browser/safe_browsing/client_side_detection_host.h b/chrome/browser/safe_browsing/client_side_detection_host.h
index 540d61e..8ad5df5 100644
--- a/chrome/browser/safe_browsing/client_side_detection_host.h
+++ b/chrome/browser/safe_browsing/client_side_detection_host.h
@@ -52,8 +52,17 @@ class ClientSideDetectionHost : public content::WebContentsObserver,
   virtual void OnSafeBrowsingHit(
       const SafeBrowsingUIManager::UnsafeResource& resource) OVERRIDE;
 
+  // Called when the SafeBrowsingService finds a match on the SB lists.
+  // Called on the UI thread. Called even if the resource is whitelisted.
+  virtual void OnSafeBrowsingMatch(
+      const SafeBrowsingUIManager::UnsafeResource& resource) OVERRIDE;
+
   virtual scoped_refptr<SafeBrowsingDatabaseManager> database_manager();
 
+  // Returns whether the current page contains a malware or phishing safe
+  // browsing match.
+  bool DidPageReceiveSafeBrowsingMatch() const;
+
  protected:
   explicit ClientSideDetectionHost(content::WebContents* tab);
 
@@ -107,7 +116,7 @@ class ClientSideDetectionHost : public content::WebContentsObserver,
   // interstitial for the current page.  This is only true if the user has
   // actually clicked through the warning.  This method is called on the UI
   // thread.
-  bool DidShowSBInterstitial();
+  bool DidShowSBInterstitial() const;
 
   // Used for testing.  This function does not take ownership of the service
   // class.
@@ -160,6 +169,10 @@ class ClientSideDetectionHost : public content::WebContentsObserver,
   // This should be accessed from UI thread.
   bool malware_report_enabled_;
 
+  // Set to true if we got a match on malware or phishing for the current
+  // page load. Is reset to false when DidNavigateMainFrame is received.
+  bool malware_or_phishing_match_;
+
   DISALLOW_COPY_AND_ASSIGN(ClientSideDetectionHost);
 };
 
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 ab6ccce..a6add64 100644
--- a/chrome/browser/safe_browsing/client_side_detection_host_unittest.cc
+++ b/chrome/browser/safe_browsing/client_side_detection_host_unittest.cc
@@ -128,10 +128,10 @@ class MockSafeBrowsingUIManager : public SafeBrowsingUIManager {
   // object.
   void InvokeOnBlockingPageComplete(const UrlCheckCallback& callback) {
     DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
-    DCHECK(!callback.is_null());
     // Note: this will delete the client object in the case of the CsdClient
     // implementation.
-    callback.Run(false);
+    if (!callback.is_null())
+      callback.Run(false);
   }
 
  protected:
@@ -326,7 +326,11 @@ class ClientSideDetectionHostTest : public ChromeRenderViewHostTestHarness {
         GetID();
     resource.render_view_id =
         web_contents()->GetRenderViewHost()->GetRoutingID();
+    ASSERT_FALSE(csd_host_->DidPageReceiveSafeBrowsingMatch());
+    csd_host_->OnSafeBrowsingMatch(resource);
+    ASSERT_TRUE(csd_host_->DidPageReceiveSafeBrowsingMatch());
     csd_host_->OnSafeBrowsingHit(resource);
+    ASSERT_TRUE(csd_host_->DidPageReceiveSafeBrowsingMatch());
     resource.callback.Reset();
     ASSERT_TRUE(csd_host_->DidShowSBInterstitial());
     TestUnsafeResourceCopied(resource);
@@ -353,6 +357,7 @@ class ClientSideDetectionHostTest : public ChromeRenderViewHostTestHarness {
     resource.callback = base::Bind(&EmptyUrlCheckCallback);
     resource.render_process_host_id = pending_rvh()->GetProcess()->GetID();
     resource.render_view_id = pending_rvh()->GetRoutingID();
+    csd_host_->OnSafeBrowsingMatch(resource);
     csd_host_->OnSafeBrowsingHit(resource);
     resource.callback.Reset();
 
@@ -360,6 +365,7 @@ class ClientSideDetectionHostTest : public ChromeRenderViewHostTestHarness {
     // a notification that it actually navigated.
     content::WebContentsTester::For(web_contents())->CommitPendingNavigation();
 
+    ASSERT_TRUE(csd_host_->DidPageReceiveSafeBrowsingMatch());
     ASSERT_TRUE(csd_host_->DidShowSBInterstitial());
     TestUnsafeResourceCopied(resource);
   }
diff --git a/chrome/browser/safe_browsing/safe_browsing_service_browsertest.cc b/chrome/browser/safe_browsing/safe_browsing_service_browsertest.cc
index 6aad7cb..1e50be36 100644
--- a/chrome/browser/safe_browsing/safe_browsing_service_browsertest.cc
+++ b/chrome/browser/safe_browsing/safe_browsing_service_browsertest.cc
@@ -320,6 +320,8 @@ class MockObserver : public SafeBrowsingUIManager::Observer {
   virtual ~MockObserver() {}
   MOCK_METHOD1(OnSafeBrowsingHit,
                void(const SafeBrowsingUIManager::UnsafeResource&));
+  MOCK_METHOD1(OnSafeBrowsingMatch,
+               void(const SafeBrowsingUIManager::UnsafeResource&));
 };
 
 MATCHER_P(IsUnsafeResourceFor, url, "") {
@@ -440,6 +442,18 @@ class SafeBrowsingServiceTest : public InProcessBrowserTest {
     sb_service->RefreshState();
   }
 
+  void ProceedAndWhitelist(
+      const SafeBrowsingUIManager::UnsafeResource& resource) {
+    std::vector<SafeBrowsingUIManager::UnsafeResource> resources;
+    resources.push_back(resource);
+    BrowserThread::PostTask(
+        BrowserThread::IO, FROM_HERE,
+        base::Bind(&SafeBrowsingUIManager::OnBlockingPageDone,
+                   g_browser_process->safe_browsing_service()->ui_manager(),
+                   resources, true));
+    WaitForIOThread();
+  }
+
  protected:
   StrictMock<MockObserver> observer_;
 
@@ -483,6 +497,8 @@ IN_PROC_BROWSER_TEST_F(SafeBrowsingServiceTest, Malware) {
   int chunk_id = 0;
   GenUrlFullhashResult(url, safe_browsing_util::kMalwareList, chunk_id,
                        &malware_full_hash);
+  EXPECT_CALL(observer_,
+              OnSafeBrowsingMatch(IsUnsafeResourceFor(url))).Times(1);
   EXPECT_CALL(observer_, OnSafeBrowsingHit(IsUnsafeResourceFor(url))).Times(1);
   SetupResponseForUrl(url, malware_full_hash);
   ui_test_utils::NavigateToURL(browser(), url);
@@ -491,6 +507,42 @@ IN_PROC_BROWSER_TEST_F(SafeBrowsingServiceTest, Malware) {
       ui_manager()->RemoveObserver(&observer_);
 }
 
+IN_PROC_BROWSER_TEST_F(SafeBrowsingServiceTest, MalwareWithWhitelist) {
+  GURL url = test_server()->GetURL(kEmptyPage);
+  g_browser_process->safe_browsing_service()->
+      ui_manager()->AddObserver(&observer_);
+
+  // After adding the url to safebrowsing database and getfullhash result,
+  // we should see the interstitial page.
+  SBFullHashResult malware_full_hash;
+  int chunk_id = 0;
+  GenUrlFullhashResult(url, safe_browsing_util::kMalwareList, chunk_id,
+                       &malware_full_hash);
+  EXPECT_CALL(observer_,
+              OnSafeBrowsingMatch(IsUnsafeResourceFor(url))).Times(1);
+  EXPECT_CALL(observer_, OnSafeBrowsingHit(IsUnsafeResourceFor(url))).Times(1)
+      .WillOnce(testing::Invoke(
+          this, &SafeBrowsingServiceTest::ProceedAndWhitelist));
+  SetupResponseForUrl(url, malware_full_hash);
+
+  ui_test_utils::NavigateToURL(browser(), url);
+  // Mock calls OnBlockingPageDone set to proceed, so the interstitial
+  // is removed.
+  EXPECT_FALSE(ShowingInterstitialPage());
+  Mock::VerifyAndClearExpectations(&observer_);
+
+  // Navigate back to kEmptyPage -- should hit the whitelist, and send a match
+  // call, but no hit call.
+  EXPECT_CALL(observer_,
+              OnSafeBrowsingMatch(IsUnsafeResourceFor(url))).Times(1);
+  EXPECT_CALL(observer_, OnSafeBrowsingHit(IsUnsafeResourceFor(url))).Times(0);
+  ui_test_utils::NavigateToURL(browser(), url);
+  EXPECT_FALSE(ShowingInterstitialPage());
+
+  g_browser_process->safe_browsing_service()->
+      ui_manager()->RemoveObserver(&observer_);
+}
+
 const char kPrefetchMalwarePage[] = "files/safe_browsing/prefetch_malware.html";
 
 // This test confirms that prefetches don't themselves get the
@@ -534,6 +586,8 @@ IN_PROC_BROWSER_TEST_F(SafeBrowsingServiceTest, Prefetch) {
 
   // However, when we navigate to the malware page, we should still get
   // the interstitial.
+  EXPECT_CALL(observer_, OnSafeBrowsingMatch(IsUnsafeResourceFor(malware_url)))
+      .Times(1);
   EXPECT_CALL(observer_, OnSafeBrowsingHit(IsUnsafeResourceFor(malware_url)))
       .Times(1);
   ui_test_utils::NavigateToURL(browser(), malware_url);
diff --git a/chrome/browser/safe_browsing/safe_browsing_tab_observer.h b/chrome/browser/safe_browsing/safe_browsing_tab_observer.h
index e9f563b..1ee2a55 100644
--- a/chrome/browser/safe_browsing/safe_browsing_tab_observer.h
+++ b/chrome/browser/safe_browsing/safe_browsing_tab_observer.h
@@ -23,6 +23,10 @@ class SafeBrowsingTabObserver
  public:
   virtual ~SafeBrowsingTabObserver();
 
+  ClientSideDetectionHost* detection_host() {
+    return safebrowsing_detection_host_.get();
+  }
+
  private:
   explicit SafeBrowsingTabObserver(content::WebContents* web_contents);
   friend class content::WebContentsUserData<SafeBrowsingTabObserver>;
diff --git a/chrome/browser/safe_browsing/ui_manager.cc b/chrome/browser/safe_browsing/ui_manager.cc
index 241930d..d97de29 100644
--- a/chrome/browser/safe_browsing/ui_manager.cc
+++ b/chrome/browser/safe_browsing/ui_manager.cc
@@ -121,6 +121,14 @@ void SafeBrowsingUIManager::OnBlockingPageDone(
 void SafeBrowsingUIManager::DoDisplayBlockingPage(
     const UnsafeResource& resource) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+
+  // Indicate to interested observers that the resource in question matched the
+  // SB filters. If the resource is already whitelisted, OnSafeBrowsingHit
+  // won't be called.
+  if (resource.threat_type != SB_THREAT_TYPE_SAFE) {
+    FOR_EACH_OBSERVER(Observer, observer_list_, OnSafeBrowsingMatch(resource));
+  }
+
   // Check if the user has already ignored our warning for this render_view
   // and domain.
   if (IsWhitelisted(resource)) {
@@ -290,3 +298,4 @@ bool SafeBrowsingUIManager::IsWhitelisted(const UnsafeResource& resource) {
   }
   return false;
 }
+
diff --git a/chrome/browser/safe_browsing/ui_manager.h b/chrome/browser/safe_browsing/ui_manager.h
index 5c896ca..53a3c03 100644
--- a/chrome/browser/safe_browsing/ui_manager.h
+++ b/chrome/browser/safe_browsing/ui_manager.h
@@ -54,6 +54,15 @@ class SafeBrowsingUIManager
   // was found.
   class Observer {
    public:
+    // The |resource| was classified as unsafe by SafeBrowsing.
+    // This method will be called every time an unsafe resource is
+    // loaded, even if it has already been whitelisted by the user.
+    // The |resource| must not be accessed after OnSafeBrowsingHit returns.
+    // This method will be called on the UI thread.
+    virtual void OnSafeBrowsingMatch(const UnsafeResource& resource) = 0;
+
+    // The |resource| was classified as unsafe by SafeBrowsing, and is
+    // not whitelisted.
     // The |resource| must not be accessed after OnSafeBrowsingHit returns.
     // This method will be called on the UI thread.
     virtual void OnSafeBrowsingHit(const UnsafeResource& resource) = 0;