Separate pre-classification checks for client-side malware and phishing detection and enable the client-side malware

feature in all channels (not just dev and canary).

BUG=352782

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

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

7 files changed, 554 insertions, 484 deletions

diff --git a/chrome/browser/safe_browsing/browser_feature_extractor.cc b/chrome/browser/safe_browsing/browser_feature_extractor.cc
index 522c3fc..8d0bd73 100644
--- a/chrome/browser/safe_browsing/browser_feature_extractor.cc
+++ b/chrome/browser/safe_browsing/browser_feature_extractor.cc
@@ -262,7 +262,6 @@ void BrowserFeatureExtractor::ExtractMalwareFeatures(
     ClientMalwareRequest* request,
     const MalwareDoneCallback& callback) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  DCHECK_EQ(0U, request->url().find("http:"));
   DCHECK(!callback.is_null());
 
   // Grab the IPs because they might go away before we're done
diff --git a/chrome/browser/safe_browsing/browser_feature_extractor.h b/chrome/browser/safe_browsing/browser_feature_extractor.h
index 82a4e0d..bdad206 100644
--- a/chrome/browser/safe_browsing/browser_feature_extractor.h
+++ b/chrome/browser/safe_browsing/browser_feature_extractor.h
@@ -58,6 +58,9 @@ struct IPUrlInfo {
 typedef std::map<std::string, std::vector<IPUrlInfo> > IPUrlMap;
 
 struct BrowseInfo {
+  // The URL we're currently browsing.
+  GURL url;
+
   // List of IPv4 and IPv6 addresses from which content was requested
   // together with the hosts on it, while browsing to the |url|.
   IPUrlMap ips;
@@ -78,6 +81,9 @@ struct BrowseInfo {
   // The HTTP status code from this navigation.
   int http_status_code;
 
+  // The page ID of the navigation.  This comes from FrameNavigateParams.
+  int32 page_id;
+
   BrowseInfo();
   ~BrowseInfo();
 };
diff --git a/chrome/browser/safe_browsing/client_side_detection_host.cc b/chrome/browser/safe_browsing/client_side_detection_host.cc
index 130f0ca..c2830fc 100644
--- a/chrome/browser/safe_browsing/client_side_detection_host.cc
+++ b/chrome/browser/safe_browsing/client_side_detection_host.cc
@@ -19,8 +19,6 @@
 #include "chrome/browser/safe_browsing/client_side_detection_service.h"
 #include "chrome/browser/safe_browsing/database_manager.h"
 #include "chrome/browser/safe_browsing/safe_browsing_service.h"
-#include "chrome/common/chrome_switches.h"
-#include "chrome/common/chrome_version_info.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/common/safe_browsing/csd.pb.h"
 #include "chrome/common/safe_browsing/safebrowsing_messages.h"
@@ -36,6 +34,7 @@
 #include "content/public/browser/resource_request_details.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/common/frame_navigate_params.h"
+#include "content/public/common/url_constants.h"
 #include "url/gurl.h"
 
 using content::BrowserThread;
@@ -50,28 +49,34 @@ const int ClientSideDetectionHost::kMaxIPsPerBrowse = 200;
 
 const char kSafeBrowsingMatchKey[] = "safe_browsing_match";
 
+typedef base::Callback<void(bool)> ShouldClassifyUrlCallback;
+
 // 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.
-// Objects of this class are ref-counted and will be destroyed once nobody
-// uses it anymore.  If |web_contents|, |csd_service| or |host| go away you need
-// to call Cancel().  We keep the |database_manager| alive in a ref pointer for
-// as long as it takes.
+// asynchronously checks whether the malware and phishing classifiers should run
+// for this URL.  If so, it notifies the host class by calling the provided
+// callback form the UI thread.  Objects of this class are ref-counted and will
+// be destroyed once nobody uses it anymore.  If |web_contents|, |csd_service|
+// or |host| go away you need to call Cancel().  We keep the |database_manager|
+// alive in a ref pointer for as long as it takes.
 class ClientSideDetectionHost::ShouldClassifyUrlRequest
     : public base::RefCountedThreadSafe<
           ClientSideDetectionHost::ShouldClassifyUrlRequest> {
  public:
-  ShouldClassifyUrlRequest(const content::FrameNavigateParams& params,
-                           WebContents* web_contents,
-                           ClientSideDetectionService* csd_service,
-                           SafeBrowsingDatabaseManager* database_manager,
-                           ClientSideDetectionHost* host)
-      : canceled_(false),
-        params_(params),
+  ShouldClassifyUrlRequest(
+      const content::FrameNavigateParams& params,
+      const ShouldClassifyUrlCallback& start_phishing_classification,
+      const ShouldClassifyUrlCallback& start_malware_classification,
+      WebContents* web_contents,
+      ClientSideDetectionService* csd_service,
+      SafeBrowsingDatabaseManager* database_manager,
+      ClientSideDetectionHost* host)
+      : params_(params),
         web_contents_(web_contents),
         csd_service_(csd_service),
         database_manager_(database_manager),
-        host_(host) {
+        host_(host),
+        start_phishing_classification_cb_(start_phishing_classification),
+        start_malware_classification_cb_(start_malware_classification) {
     DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
     DCHECK(web_contents_);
     DCHECK(csd_service_);
@@ -83,7 +88,8 @@ class ClientSideDetectionHost::ShouldClassifyUrlRequest
     DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
     // We start by doing some simple checks that can run on the UI thread.
-    UMA_HISTOGRAM_COUNTS("SBClientPhishing.ClassificationStart", 1);
+    UMA_HISTOGRAM_BOOLEAN("SBClientPhishing.ClassificationStart", 1);
+    UMA_HISTOGRAM_BOOLEAN("SBClientMalware.ClassificationStart", 1);
 
     // Only classify [X]HTML documents.
     if (params_.contents_mime_type != "text/html" &&
@@ -91,50 +97,53 @@ class ClientSideDetectionHost::ShouldClassifyUrlRequest
       VLOG(1) << "Skipping phishing classification for URL: " << params_.url
               << " because it has an unsupported MIME type: "
               << params_.contents_mime_type;
-      UMA_HISTOGRAM_ENUMERATION("SBClientPhishing.PreClassificationCheckFail",
-                                NO_CLASSIFY_UNSUPPORTED_MIME_TYPE,
-                                NO_CLASSIFY_MAX);
-      return;
+      DontClassifyForPhishing(NO_CLASSIFY_UNSUPPORTED_MIME_TYPE);
     }
 
     if (csd_service_->IsPrivateIPAddress(params_.socket_address.host())) {
       VLOG(1) << "Skipping phishing classification for URL: " << params_.url
               << " because of hosting on private IP: "
               << params_.socket_address.host();
-      UMA_HISTOGRAM_ENUMERATION("SBClientPhishing.PreClassificationCheckFail",
-                                NO_CLASSIFY_PRIVATE_IP,
-                                NO_CLASSIFY_MAX);
-      return;
+      DontClassifyForPhishing(NO_CLASSIFY_PRIVATE_IP);
+      DontClassifyForMalware(NO_CLASSIFY_PRIVATE_IP);
     }
 
-    // Don't run the phishing classifier if the tab is incognito.
-    if (web_contents_->GetBrowserContext()->IsOffTheRecord()) {
+    // For phishing we only classify HTTP pages.
+    if (!params_.url.SchemeIs(content::kHttpScheme)) {
       VLOG(1) << "Skipping phishing classification for URL: " << params_.url
-              << " because we're browsing incognito.";
-      UMA_HISTOGRAM_ENUMERATION("SBClientPhishing.PreClassificationCheckFail",
-                                NO_CLASSIFY_OFF_THE_RECORD,
-                                NO_CLASSIFY_MAX);
+              << " because it is not HTTP: "
+              << params_.socket_address.host();
+      DontClassifyForPhishing(NO_CLASSIFY_NOT_HTTP_URL);
+    }
 
-      return;
+    // Don't run any classifier if the tab is incognito.
+    if (web_contents_->GetBrowserContext()->IsOffTheRecord()) {
+      VLOG(1) << "Skipping phishing and malware classification for URL: "
+              << params_.url << " because we're browsing incognito.";
+      DontClassifyForPhishing(NO_CLASSIFY_OFF_THE_RECORD);
+      DontClassifyForMalware(NO_CLASSIFY_OFF_THE_RECORD);
     }
 
     // We lookup the csd-whitelist before we lookup the cache because
     // a URL may have recently been whitelisted.  If the URL matches
-    // the csd-whitelist we won't start classification.  The
+    // the csd-whitelist we won't start phishing classification.  The
     // csd-whitelist check has to be done on the IO thread because it
     // uses the SafeBrowsing service class.
-    BrowserThread::PostTask(
-        BrowserThread::IO,
-        FROM_HERE,
-        base::Bind(&ShouldClassifyUrlRequest::CheckCsdWhitelist,
-                   this, params_.url));
+    if (ShouldClassifyForPhishing() || ShouldClassifyForMalware()) {
+      BrowserThread::PostTask(
+          BrowserThread::IO,
+          FROM_HERE,
+          base::Bind(&ShouldClassifyUrlRequest::CheckSafeBrowsingDatabase,
+                     this, params_.url));
+    }
   }
 
   void Cancel() {
-    canceled_ = true;
+    DontClassifyForPhishing(NO_CLASSIFY_CANCEL);
+    DontClassifyForMalware(NO_CLASSIFY_CANCEL);
     // Just to make sure we don't do anything stupid we reset all these
     // pointers except for the safebrowsing service class which may be
-    // accessed by CheckCsdWhitelist().
+    // accessed by CheckSafeBrowsingDatabase().
     web_contents_ = NULL;
     csd_service_ = NULL;
     host_ = NULL;
@@ -152,6 +161,11 @@ class ClientSideDetectionHost::ShouldClassifyUrlRequest
     NO_CLASSIFY_MATCH_CSD_WHITELIST,
     NO_CLASSIFY_TOO_MANY_REPORTS,
     NO_CLASSIFY_UNSUPPORTED_MIME_TYPE,
+    NO_CLASSIFY_NO_DATABASE_MANAGER,
+    NO_CLASSIFY_KILLSWITCH,
+    NO_CLASSIFY_CANCEL,
+    NO_CLASSIFY_RESULT_FROM_CACHE,
+    NO_CLASSIFY_NOT_HTTP_URL,
 
     NO_CLASSIFY_MAX  // Always add new values before this one.
   };
@@ -159,75 +173,120 @@ class ClientSideDetectionHost::ShouldClassifyUrlRequest
   // The destructor can be called either from the UI or the IO thread.
   virtual ~ShouldClassifyUrlRequest() { }
 
-  void CheckCsdWhitelist(const GURL& url) {
-    DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
-    if (!database_manager_.get() ||
-        database_manager_->MatchCsdWhitelistUrl(url)) {
-      // We're done.  There is no point in going back to the UI thread.
-      VLOG(1) << "Skipping phishing classification for URL: " << url
-              << " because it matches the csd whitelist";
+  bool ShouldClassifyForPhishing() const {
+    DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+    return !start_phishing_classification_cb_.is_null();
+  }
+
+  bool ShouldClassifyForMalware() const {
+    DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+    return !start_malware_classification_cb_.is_null();
+  }
+
+  void DontClassifyForPhishing(PreClassificationCheckFailures reason) {
+    DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+    if (ShouldClassifyForPhishing()) {
+      // Track the first reason why we stopped classifying for phishing.
       UMA_HISTOGRAM_ENUMERATION("SBClientPhishing.PreClassificationCheckFail",
-                                NO_CLASSIFY_MATCH_CSD_WHITELIST,
-                                NO_CLASSIFY_MAX);
-      return;
+                                reason, NO_CLASSIFY_MAX);
+      DVLOG(2) << "Failed phishing pre-classification checks.  Reason: "
+               << reason;
+      start_phishing_classification_cb_.Run(false);
     }
+    start_phishing_classification_cb_.Reset();
+  }
 
-    bool malware_killswitch_on = database_manager_->IsMalwareKillSwitchOn();
+  void DontClassifyForMalware(PreClassificationCheckFailures reason) {
+    DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+    if (ShouldClassifyForMalware()) {
+      // Track the first reason why we stopped classifying for malware.
+      UMA_HISTOGRAM_ENUMERATION("SBClientMalware.PreClassificationCheckFail",
+                                reason, NO_CLASSIFY_MAX);
+      DVLOG(2) << "Failed malware pre-classification checks.  Reason: "
+               << reason;
+      start_malware_classification_cb_.Run(false);
+    }
+    start_malware_classification_cb_.Reset();
+  }
 
+  void CheckSafeBrowsingDatabase(const GURL& url) {
+    DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+    // We don't want to call the classification callbacks from the IO
+    // thread so we simply pass the results of this method to CheckCache()
+    // which is called on the UI thread;
+    PreClassificationCheckFailures phishing_reason = NO_CLASSIFY_MAX;
+    PreClassificationCheckFailures malware_reason = NO_CLASSIFY_MAX;
+    if (!database_manager_.get()) {
+      // We cannot check the Safe Browsing whitelists so we stop here
+      // for safety.
+      malware_reason = phishing_reason = NO_CLASSIFY_NO_DATABASE_MANAGER;
+    } else {
+      if (database_manager_->MatchCsdWhitelistUrl(url)) {
+        VLOG(1) << "Skipping phishing classification for URL: " << url
+                << " because it matches the csd whitelist";
+        phishing_reason = NO_CLASSIFY_MATCH_CSD_WHITELIST;
+      }
+      if (database_manager_->IsMalwareKillSwitchOn()) {
+        malware_reason = NO_CLASSIFY_KILLSWITCH;
+      }
+    }
     BrowserThread::PostTask(
         BrowserThread::UI,
         FROM_HERE,
-        base::Bind(&ShouldClassifyUrlRequest::CheckCache, this,
-                   malware_killswitch_on));
+        base::Bind(&ShouldClassifyUrlRequest::CheckCache,
+                   this,
+                   phishing_reason,
+                   malware_reason));
   }
 
-  void CheckCache(bool malware_killswitch_on) {
+  void CheckCache(PreClassificationCheckFailures phishing_reason,
+                  PreClassificationCheckFailures malware_reason) {
     DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-    if (canceled_) {
-      return;
+    if (phishing_reason != NO_CLASSIFY_MAX)
+      DontClassifyForPhishing(phishing_reason);
+    if (malware_reason != NO_CLASSIFY_MAX)
+      DontClassifyForMalware(malware_reason);
+    if (!ShouldClassifyForMalware() && !ShouldClassifyForPhishing()) {
+      return;  // No point in doing anything else.
     }
-
-    host_->SetMalwareKillSwitch(malware_killswitch_on);
-    // If result is cached, we don't want to run classification again
+    // If result is cached, we don't want to run classification again.
+    // In that case we're just trying to show the warning.
     bool is_phishing;
     if (csd_service_->GetValidCachedResult(params_.url, &is_phishing)) {
       VLOG(1) << "Satisfying request for " << params_.url << " from cache";
-      UMA_HISTOGRAM_COUNTS("SBClientPhishing.RequestSatisfiedFromCache", 1);
+      UMA_HISTOGRAM_BOOLEAN("SBClientPhishing.RequestSatisfiedFromCache", 1);
       // Since we are already on the UI thread, this is safe.
       host_->MaybeShowPhishingWarning(params_.url, is_phishing);
-      return;
+      DontClassifyForPhishing(NO_CLASSIFY_RESULT_FROM_CACHE);
     }
 
     // We want to limit the number of requests, though we will ignore the
     // limit for urls in the cache.  We don't want to start classifying
     // too many pages as phishing, but for those that we already think are
-    // phishing we want to give ourselves a chance to fix false positives.
+    // phishing we want to send a request to the server to give ourselves
+    // a chance to fix misclassifications.
     if (csd_service_->IsInCache(params_.url)) {
       VLOG(1) << "Reporting limit skipped for " << params_.url
               << " as it was in the cache.";
-      UMA_HISTOGRAM_COUNTS("SBClientPhishing.ReportLimitSkipped", 1);
+      UMA_HISTOGRAM_BOOLEAN("SBClientPhishing.ReportLimitSkipped", 1);
     } else if (csd_service_->OverPhishingReportLimit()) {
       VLOG(1) << "Too many report phishing requests sent recently, "
               << "not running classification for " << params_.url;
-      UMA_HISTOGRAM_ENUMERATION("SBClientPhishing.PreClassificationCheckFail",
-                                NO_CLASSIFY_TOO_MANY_REPORTS,
-                                NO_CLASSIFY_MAX);
-      return;
+      DontClassifyForPhishing(NO_CLASSIFY_TOO_MANY_REPORTS);
+    }
+    if (csd_service_->OverMalwareReportLimit()) {
+      DontClassifyForMalware(NO_CLASSIFY_TOO_MANY_REPORTS);
     }
 
     // Everything checks out, so start classification.
     // |web_contents_| is safe to call as we will be destructed
     // before it is.
-    VLOG(1) << "Instruct renderer to start phishing detection for URL: "
-            << params_.url;
-    content::RenderViewHost* rvh = web_contents_->GetRenderViewHost();
-    rvh->Send(new SafeBrowsingMsg_StartPhishingDetection(
-        rvh->GetRoutingID(), params_.url));
+    if (ShouldClassifyForPhishing())
+      start_phishing_classification_cb_.Run(true);
+    if (ShouldClassifyForMalware())
+      start_malware_classification_cb_.Run(true);
   }
 
-  // No need to protect |canceled_| with a lock because it is only read and
-  // written by the UI thread.
-  bool canceled_;
   content::FrameNavigateParams params_;
   WebContents* web_contents_;
   ClientSideDetectionService* csd_service_;
@@ -236,6 +295,9 @@ class ClientSideDetectionHost::ShouldClassifyUrlRequest
   scoped_refptr<SafeBrowsingDatabaseManager> database_manager_;
   ClientSideDetectionHost* host_;
 
+  ShouldClassifyUrlCallback start_phishing_classification_cb_;
+  ShouldClassifyUrlCallback start_malware_classification_cb_;
+
   DISALLOW_COPY_AND_ASSIGN(ShouldClassifyUrlRequest);
 };
 
@@ -248,10 +310,12 @@ ClientSideDetectionHost* ClientSideDetectionHost::Create(
 ClientSideDetectionHost::ClientSideDetectionHost(WebContents* tab)
     : content::WebContentsObserver(tab),
       csd_service_(NULL),
+      classification_request_(NULL),
+      should_extract_malware_features_(true),
+      should_classify_for_malware_(false),
+      onload_complete_(false),
       weak_factory_(this),
-      unsafe_unique_page_id_(-1),
-      malware_killswitch_on_(false),
-      malware_report_enabled_(false) {
+      unsafe_unique_page_id_(-1) {
   DCHECK(tab);
   // Note: csd_service_ and sb_service will be NULL here in testing.
   csd_service_ = g_browser_process->safe_browsing_detection_service();
@@ -266,13 +330,6 @@ ClientSideDetectionHost::ClientSideDetectionHost(WebContents* tab)
     database_manager_ = sb_service->database_manager();
     ui_manager_->AddObserver(this);
   }
-
-  // Only enable the malware bad IP matching and report feature for canary
-  // and dev channel.
-  chrome::VersionInfo::Channel channel = chrome::VersionInfo::GetChannel();
-  malware_report_enabled_ = (
-      channel == chrome::VersionInfo::CHANNEL_DEV ||
-      channel == chrome::VersionInfo::CHANNEL_CANARY);
 }
 
 ClientSideDetectionHost::~ClientSideDetectionHost() {
@@ -302,6 +359,10 @@ void ClientSideDetectionHost::DidNavigateMainFrame(
     // begin a new classification.
     return;
   }
+  // Cancel any pending classification request.
+  if (classification_request_.get()) {
+    classification_request_->Cancel();
+  }
   // If we navigate away and there currently is a pending phishing
   // report request we have to cancel it to make sure we don't display
   // an interstitial for the wrong page.  Note that this won't cancel
@@ -312,11 +373,6 @@ void ClientSideDetectionHost::DidNavigateMainFrame(
   if (!csd_service_) {
     return;
   }
-
-  // Cancel any pending classification request.
-  if (classification_request_.get()) {
-    classification_request_->Cancel();
-  }
   browse_info_.reset(new BrowseInfo);
 
   // Store redirect chain information.
@@ -324,14 +380,25 @@ void ClientSideDetectionHost::DidNavigateMainFrame(
     cur_host_ = params.url.host();
     cur_host_redirects_ = params.redirects;
   }
+  browse_info_->url = params.url;
   browse_info_->host_redirects = cur_host_redirects_;
   browse_info_->url_redirects = params.redirects;
   browse_info_->referrer = params.referrer.url;
   browse_info_->http_status_code = details.http_status_code;
+  browse_info_->page_id = params.page_id;
 
-  // Notify the renderer if it should classify this URL.
+  should_extract_malware_features_ = true;
+  should_classify_for_malware_ = false;
+  onload_complete_ = false;
+
+  // Check whether we can cassify the current URL for phishing or malware.
   classification_request_ = new ShouldClassifyUrlRequest(
-      params, web_contents(), csd_service_, database_manager_.get(), this);
+      params,
+      base::Bind(&ClientSideDetectionHost::OnPhishingPreClassificationDone,
+                 weak_factory_.GetWeakPtr()),
+      base::Bind(&ClientSideDetectionHost::OnMalwarePreClassificationDone,
+                 weak_factory_.GetWeakPtr()),
+      web_contents(), csd_service_, database_manager_.get(), this);
   classification_request_->Start();
 }
 
@@ -414,6 +481,70 @@ void ClientSideDetectionHost::WebContentsDestroyed(WebContents* tab) {
   feature_extractor_.reset();
 }
 
+void ClientSideDetectionHost::OnPhishingPreClassificationDone(
+    bool should_classify) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  if (browse_info_.get() && should_classify) {
+    VLOG(1) << "Instruct renderer to start phishing detection for URL: "
+            << browse_info_->url;
+    content::RenderViewHost* rvh = web_contents()->GetRenderViewHost();
+    rvh->Send(new SafeBrowsingMsg_StartPhishingDetection(
+        rvh->GetRoutingID(), browse_info_->url));
+  }
+}
+
+void ClientSideDetectionHost::OnMalwarePreClassificationDone(
+    bool should_classify) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  // If classification checks failed we should stop extracting malware features.
+  DVLOG(2) << "Malware pre-classification checks done. Should classify: "
+           << should_classify;
+  should_extract_malware_features_ = should_classify;
+  should_classify_for_malware_ = should_classify;
+  MaybeStartMalwareFeatureExtraction();
+}
+
+void ClientSideDetectionHost::DocumentOnLoadCompletedInMainFrame(
+    int32 page_id) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  if (!csd_service_ || !browse_info_.get())
+    return;
+  DVLOG(2) << "Main frame onload hander called.";
+  if (browse_info_->page_id != page_id) {
+    // Something weird is happening here.  The BrowseInfo page ID
+    // should always be the same as the most recent load.
+    UMA_HISTOGRAM_BOOLEAN("SBClientMalware.UnexpectedPageId", 1);
+    return;
+  }
+  onload_complete_ = true;
+  MaybeStartMalwareFeatureExtraction();
+}
+
+void ClientSideDetectionHost::MaybeStartMalwareFeatureExtraction() {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  if (csd_service_ && browse_info_.get() &&
+      should_classify_for_malware_ &&
+      onload_complete_) {
+    scoped_ptr<ClientMalwareRequest> malware_request(
+        new ClientMalwareRequest);
+    // Start browser-side malware feature extraction.  Once we're done it will
+    // send the malware client verdict request.
+    malware_request->set_url(browse_info_->url.spec());
+    const GURL& referrer = browse_info_->referrer;
+    if (referrer.SchemeIs("http")) {  // Only send http urls.
+      malware_request->set_referrer_url(referrer.spec());
+    }
+    // This function doesn't expect browse_info_ to stay around after this
+    // function returns.
+    feature_extractor_->ExtractMalwareFeatures(
+        browse_info_.get(),
+        malware_request.release(),
+        base::Bind(&ClientSideDetectionHost::MalwareFeatureExtractionDone,
+                   weak_factory_.GetWeakPtr()));
+    should_classify_for_malware_ = false;
+  }
+}
+
 void ClientSideDetectionHost::OnPhishingDetectionDone(
     const std::string& verdict_str) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
@@ -421,40 +552,15 @@ void ClientSideDetectionHost::OnPhishingDetectionDone(
   // this method is called.  The renderer should not start phishing detection
   // if there isn't any service class in the browser.
   DCHECK(csd_service_);
-  // There shouldn't be any pending requests because we revoke them everytime
-  // we navigate away.
-  DCHECK(!weak_factory_.HasWeakPtrs());
   DCHECK(browse_info_.get());
 
   // We parse the protocol buffer here.  If we're unable to parse it we won't
   // send the verdict further.
   scoped_ptr<ClientPhishingRequest> verdict(new ClientPhishingRequest);
   if (csd_service_ &&
-      !weak_factory_.HasWeakPtrs() &&
       browse_info_.get() &&
       verdict->ParseFromString(verdict_str) &&
       verdict->IsInitialized()) {
-    // We do the malware IP matching and request sending if the feature
-    // is enabled.
-    if (malware_report_enabled_ && !MalwareKillSwitchIsOn()) {
-      scoped_ptr<ClientMalwareRequest> malware_verdict(
-          new ClientMalwareRequest);
-      // Start browser-side malware feature extraction.  Once we're done it will
-      // send the malware client verdict request.
-      malware_verdict->set_url(verdict->url());
-      const GURL& referrer = browse_info_->referrer;
-      if (referrer.SchemeIs("http")) {  // Only send http urls.
-          malware_verdict->set_referrer_url(referrer.spec());
-      }
-      // This function doesn't expect browse_info_ to stay around after this
-      // function returns.
-      feature_extractor_->ExtractMalwareFeatures(
-          browse_info_.get(),
-          malware_verdict.release(),
-          base::Bind(&ClientSideDetectionHost::MalwareFeatureExtractionDone,
-                     weak_factory_.GetWeakPtr()));
-    }
-
     // We only send phishing verdict to the server if the verdict is phishing or
     // if a SafeBrowsing interstitial was already shown for this site.  E.g., a
     // malware or phishing interstitial was shown but the user clicked
@@ -472,14 +578,13 @@ void ClientSideDetectionHost::OnPhishingDetectionDone(
                      weak_factory_.GetWeakPtr()));
     }
   }
-  browse_info_.reset();
 }
 
 void ClientSideDetectionHost::MaybeShowPhishingWarning(GURL phishing_url,
                                                        bool is_phishing) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  VLOG(2) << "Received server phishing verdict for URL:" << phishing_url
-          << " is_phishing:" << is_phishing;
+  DVLOG(2) << "Received server phishing verdict for URL:" << phishing_url
+           << " is_phishing:" << is_phishing;
   if (is_phishing) {
     DCHECK(web_contents());
     if (ui_manager_.get()) {
@@ -509,8 +614,8 @@ void ClientSideDetectionHost::MaybeShowMalwareWarning(GURL original_url,
                                                       GURL malware_url,
                                                       bool is_malware) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  VLOG(2) << "Received server malawre IP verdict for URL:" << malware_url
-          << " is_malware:" << is_malware;
+  DVLOG(2) << "Received server malawre IP verdict for URL:" << malware_url
+           << " is_malware:" << is_malware;
   if (is_malware && malware_url.is_valid() && original_url.is_valid()) {
     DCHECK(web_contents());
     if (ui_manager_.get()) {
@@ -540,8 +645,8 @@ void ClientSideDetectionHost::FeatureExtractionDone(
     bool success,
     ClientPhishingRequest* request) {
   DCHECK(request);
-  VLOG(2) << "Feature extraction done (success:" << success << ") for URL: "
-          << request->url() << ". Start sending client phishing request.";
+  DVLOG(2) << "Feature extraction done (success:" << success << ") for URL: "
+           << request->url() << ". Start sending client phishing request.";
   ClientSideDetectionService::ClientReportPhishingRequestCallback callback;
   // If the client-side verdict isn't phishing we don't care about the server
   // response because we aren't going to display a warning.
@@ -559,8 +664,8 @@ void ClientSideDetectionHost::MalwareFeatureExtractionDone(
     bool feature_extraction_success,
     scoped_ptr<ClientMalwareRequest> request) {
   DCHECK(request.get());
-  VLOG(2) << "Malware Feature extraction done for URL: " << request->url()
-          << ", with badip url count:" << request->bad_ip_url_info_size();
+  DVLOG(2) << "Malware Feature extraction done for URL: " << request->url()
+           << ", with badip url count:" << request->bad_ip_url_info_size();
 
   // Send ping if there is matching features.
   if (feature_extraction_success && request->bad_ip_url_info_size() > 0) {
@@ -601,15 +706,13 @@ void ClientSideDetectionHost::Observe(
   DCHECK_EQ(type, content::NOTIFICATION_RESOURCE_RESPONSE_STARTED);
   const ResourceRequestDetails* req = content::Details<ResourceRequestDetails>(
       details).ptr();
-  if (req && browse_info_.get() && malware_report_enabled_ &&
-      !MalwareKillSwitchIsOn()) {
-    if (req->url.is_valid()) {
-      UpdateIPUrlMap(req->socket_address.host() /* ip */,
-                     req->url.spec()  /* url */,
-                     req->method,
-                     req->referrer,
-                     req->resource_type);
-    }
+  if (req && browse_info_.get() &&
+      should_extract_malware_features_ && req->url.is_valid()) {
+    UpdateIPUrlMap(req->socket_address.host() /* ip */,
+                   req->url.spec()  /* url */,
+                   req->method,
+                   req->referrer,
+                   req->resource_type);
   }
 }
 
@@ -640,14 +743,4 @@ void ClientSideDetectionHost::set_safe_browsing_managers(
   database_manager_ = database_manager;
 }
 
-bool ClientSideDetectionHost::MalwareKillSwitchIsOn() {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  return malware_killswitch_on_;
-}
-
-void ClientSideDetectionHost::SetMalwareKillSwitch(bool killswitch_on) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  malware_killswitch_on_ = killswitch_on;
-}
-
 }  // namespace safe_browsing
diff --git a/chrome/browser/safe_browsing/client_side_detection_host.h b/chrome/browser/safe_browsing/client_side_detection_host.h
index c5cf95a..1492f13 100644
--- a/chrome/browser/safe_browsing/client_side_detection_host.h
+++ b/chrome/browser/safe_browsing/client_side_detection_host.h
@@ -79,6 +79,11 @@ class ClientSideDetectionHost : public content::WebContentsObserver,
   class ShouldClassifyUrlRequest;
   friend class ShouldClassifyUrlRequest;
 
+  // These methods are called when pre-classification checks are done for
+  // the phishing and malware clasifiers.
+  void OnPhishingPreClassificationDone(bool should_classify);
+  void OnMalwarePreClassificationDone(bool should_classify);
+
   // Verdict is an encoded ClientPhishingRequest protocol message.
   void OnPhishingDetectionDone(const std::string& verdict);
 
@@ -98,10 +103,14 @@ class ClientSideDetectionHost : public content::WebContentsObserver,
   // the UI thread.
   void FeatureExtractionDone(bool success, ClientPhishingRequest* request);
 
+  // Start malware classification once the onload handler was called and
+  // malware pre-classification checks are done and passed.
+  void MaybeStartMalwareFeatureExtraction();
+
   // Function to be called when the browser malware feature extractor is done.
   // Called on the UI thread.
-  void MalwareFeatureExtractionDone(bool success,
-                                    scoped_ptr<ClientMalwareRequest> request);
+  void MalwareFeatureExtractionDone(
+      bool success, scoped_ptr<ClientMalwareRequest> request);
 
   // Update the entries in browse_info_->ips map.
   void UpdateIPUrlMap(const std::string& ip,
@@ -116,6 +125,10 @@ class ClientSideDetectionHost : public content::WebContentsObserver,
                        const content::NotificationSource& source,
                        const content::NotificationDetails& details) OVERRIDE;
 
+  // Inherited from WebContentsObserver.  This is called once the onload handler
+  // is called.
+  virtual void DocumentOnLoadCompletedInMainFrame(int32 page_id) OVERRIDE;
+
   // Returns true if the user has seen a regular SafeBrowsing
   // 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
@@ -126,10 +139,6 @@ class ClientSideDetectionHost : public content::WebContentsObserver,
   // class.
   void set_client_side_detection_service(ClientSideDetectionService* service);
 
-  // Get/Set malware_killswitch_on_ value. These methods called on UI thread.
-  bool MalwareKillSwitchIsOn();
-  void SetMalwareKillSwitch(bool killswitch_on);
-
   // This pointer may be NULL if client-side phishing detection is disabled.
   ClientSideDetectionService* csd_service_;
   // These pointers may be NULL if SafeBrowsing is disabled.
@@ -158,6 +167,10 @@ class ClientSideDetectionHost : public content::WebContentsObserver,
   // Max number of urls we report for each malware IP.
   static const int kMaxUrlsPerIP;
 
+  bool should_extract_malware_features_;
+  bool should_classify_for_malware_;
+  bool onload_complete_;
+
   base::WeakPtrFactory<ClientSideDetectionHost> weak_factory_;
 
   // Unique page ID of the most recent unsafe site that was loaded in this tab
@@ -165,14 +178,6 @@ class ClientSideDetectionHost : public content::WebContentsObserver,
   int unsafe_unique_page_id_;
   scoped_ptr<SafeBrowsingUIManager::UnsafeResource> unsafe_resource_;
 
-  // Whether the malware IP matching feature killswitch is on.
-  // This should be accessed from UI thread.
-  bool malware_killswitch_on_;
-
-  // Whether the malware bad ip matching and report feature is enabled.
-  // This should be accessed from UI thread.
-  bool malware_report_enabled_;
-
   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 3a66f8a..784182b 100644
--- a/chrome/browser/safe_browsing/client_side_detection_host_unittest.cc
+++ b/chrome/browser/safe_browsing/client_side_detection_host_unittest.cc
@@ -123,6 +123,7 @@ class MockClientSideDetectionService : public ClientSideDetectionService {
   MOCK_METHOD2(GetValidCachedResult, bool(const GURL&, bool*));
   MOCK_METHOD1(IsInCache, bool(const GURL&));
   MOCK_METHOD0(OverPhishingReportLimit, bool());
+  MOCK_METHOD0(OverMalwareReportLimit, bool());
 
  private:
   DISALLOW_COPY_AND_ASSIGN(MockClientSideDetectionService);
@@ -159,6 +160,7 @@ class MockSafeBrowsingDatabaseManager : public SafeBrowsingDatabaseManager {
 
   MOCK_METHOD1(MatchCsdWhitelistUrl, bool(const GURL&));
   MOCK_METHOD1(MatchMalwareIP, bool(const std::string& ip_address));
+  MOCK_METHOD0(IsMalwareKillSwitchOn, bool());
 
  protected:
   virtual ~MockSafeBrowsingDatabaseManager() {}
@@ -219,10 +221,6 @@ class ClientSideDetectionHostTest : public ChromeRenderViewHostTestHarness {
     // We need to create this here since we don't call
     // DidNavigateMainFramePostCommit in this test.
     csd_host_->browse_info_.reset(new BrowseInfo);
-
-    // By default this is set to false. Turn it on as if we are in canary or
-    // dev channel
-    csd_host_->malware_report_enabled_ = true;
   }
 
   virtual void TearDown() {
@@ -248,6 +246,10 @@ class ClientSideDetectionHostTest : public ChromeRenderViewHostTestHarness {
     csd_host_->OnPhishingDetectionDone(verdict_str);
   }
 
+  void DocumentOnLoadCompletedInMainFrame(int32 page_id) {
+    csd_host_->DocumentOnLoadCompletedInMainFrame(page_id);
+  }
+
   void UpdateIPUrlMap(const std::string& ip, const std::string& host) {
     csd_host_->UpdateIPUrlMap(ip, host, "", "", ResourceType::OBJECT);
   }
@@ -260,9 +262,11 @@ class ClientSideDetectionHostTest : public ChromeRenderViewHostTestHarness {
                                      const bool* is_private,
                                      const bool* is_incognito,
                                      const bool* match_csd_whitelist,
+                                     const bool* malware_killswitch,
                                      const bool* get_valid_cached_result,
                                      const bool* is_in_cache,
-                                     const bool* over_report_limit) {
+                                     const bool* over_phishing_report_limit,
+                                     const bool* over_malware_report_limit) {
     if (is_private) {
       EXPECT_CALL(*csd_service_, IsPrivateIPAddress(_))
           .WillOnce(Return(*is_private));
@@ -275,6 +279,10 @@ class ClientSideDetectionHostTest : public ChromeRenderViewHostTestHarness {
       EXPECT_CALL(*database_manager_.get(), MatchCsdWhitelistUrl(url))
           .WillOnce(Return(*match_csd_whitelist));
     }
+    if (malware_killswitch) {
+      EXPECT_CALL(*database_manager_.get(), IsMalwareKillSwitchOn())
+          .WillRepeatedly(Return(*malware_killswitch));
+    }
     if (get_valid_cached_result) {
       EXPECT_CALL(*csd_service_, GetValidCachedResult(url, NotNull()))
           .WillOnce(DoAll(SetArgumentPointee<1>(true),
@@ -283,9 +291,13 @@ class ClientSideDetectionHostTest : public ChromeRenderViewHostTestHarness {
     if (is_in_cache) {
       EXPECT_CALL(*csd_service_, IsInCache(url)).WillOnce(Return(*is_in_cache));
     }
-    if (over_report_limit) {
+    if (over_phishing_report_limit) {
       EXPECT_CALL(*csd_service_, OverPhishingReportLimit())
-          .WillOnce(Return(*over_report_limit));
+          .WillOnce(Return(*over_phishing_report_limit));
+    }
+    if (over_malware_report_limit) {
+      EXPECT_CALL(*csd_service_, OverMalwareReportLimit())
+          .WillOnce(Return(*over_malware_report_limit));
     }
   }
 
@@ -310,6 +322,25 @@ class ClientSideDetectionHostTest : public ChromeRenderViewHostTestHarness {
     csd_host_->browse_info_->referrer = referrer;
   }
 
+  void ExpectShouldClassifyForMalwareResult(bool should_classify) {
+    EXPECT_EQ(should_classify, csd_host_->should_classify_for_malware_);
+  }
+
+  void ExpectStartPhishingDetection(const GURL* url) {
+    const IPC::Message* msg = process()->sink().GetFirstMessageMatching(
+        SafeBrowsingMsg_StartPhishingDetection::ID);
+    if (url) {
+      ASSERT_TRUE(msg);
+      Tuple1<GURL> actual_url;
+      SafeBrowsingMsg_StartPhishingDetection::Read(msg, &actual_url);
+      EXPECT_EQ(*url, actual_url.a);
+      EXPECT_EQ(rvh()->GetRoutingID(), msg->routing_id());
+      process()->sink().ClearMessages();
+    } else {
+      ASSERT_FALSE(msg);
+    }
+  }
+
   void TestUnsafeResourceCopied(const UnsafeResource& resource) {
     ASSERT_TRUE(csd_host_->unsafe_resource_.get());
     // Test that the resource from OnSafeBrowsingHit notification was copied
@@ -450,13 +481,9 @@ TEST_F(ClientSideDetectionHostTest, OnPhishingDetectionDoneNotPhishing) {
   verdict.set_client_score(1.0f);
   verdict.set_is_phishing(true);
 
-  ClientMalwareRequest malware_verdict;
-  malware_verdict.set_url(verdict.url());
   EXPECT_CALL(*mock_extractor, ExtractFeatures(_, _, _))
       .WillOnce(DoAll(DeleteArg<1>(),
                       InvokeCallbackArgument<2>(true, &verdict)));
-  EXPECT_CALL(*mock_extractor, ExtractMalwareFeatures(_, _, _))
-      .WillOnce(InvokeMalwareCallback(&malware_verdict));
   EXPECT_CALL(*csd_service_,
               SendClientReportPhishingRequest(
                   Pointee(PartiallyEqualVerdict(verdict)), _))
@@ -494,14 +521,6 @@ TEST_F(ClientSideDetectionHostTest, OnPhishingDetectionDoneDisabled) {
               SendClientReportPhishingRequest(
                   Pointee(PartiallyEqualVerdict(verdict)), _))
       .WillOnce(SaveArg<1>(&cb));
-
-  ClientMalwareRequest malware_verdict;
-  malware_verdict.set_url(verdict.url());
-  EXPECT_CALL(*mock_extractor, ExtractMalwareFeatures(_, _, _))
-      .WillOnce(InvokeMalwareCallback(&malware_verdict));
-  EXPECT_CALL(*csd_service_,
-              SendClientReportMalwareRequest(_, _)).Times(0);
-
   OnPhishingDetectionDone(verdict.SerializeAsString());
   EXPECT_TRUE(Mock::VerifyAndClear(csd_host_.get()));
   ASSERT_FALSE(cb.is_null());
@@ -529,14 +548,9 @@ TEST_F(ClientSideDetectionHostTest, OnPhishingDetectionDoneShowInterstitial) {
   verdict.set_client_score(1.0f);
   verdict.set_is_phishing(true);
 
-  ClientMalwareRequest malware_verdict;
-  malware_verdict.set_url(verdict.url());
-
   EXPECT_CALL(*mock_extractor, ExtractFeatures(_, _, _))
       .WillOnce(DoAll(DeleteArg<1>(),
                       InvokeCallbackArgument<2>(true, &verdict)));
-  EXPECT_CALL(*mock_extractor, ExtractMalwareFeatures(_, _, _))
-      .WillOnce(InvokeMalwareCallback(&malware_verdict));
   EXPECT_CALL(*csd_service_,
               SendClientReportPhishingRequest(
                   Pointee(PartiallyEqualVerdict(verdict)), _))
@@ -589,14 +603,9 @@ TEST_F(ClientSideDetectionHostTest, OnPhishingDetectionDoneMultiplePings) {
   verdict.set_client_score(1.0f);
   verdict.set_is_phishing(true);
 
-  ClientMalwareRequest malware_verdict;
-  malware_verdict.set_url(verdict.url());
-
   EXPECT_CALL(*mock_extractor, ExtractFeatures(_, _, _))
       .WillOnce(DoAll(DeleteArg<1>(),
                       InvokeCallbackArgument<2>(true, &verdict)));
-  EXPECT_CALL(*mock_extractor, ExtractMalwareFeatures(_, _, _))
-      .WillOnce(InvokeMalwareCallback(&malware_verdict));
   EXPECT_CALL(*csd_service_,
               SendClientReportPhishingRequest(
                   Pointee(PartiallyEqualVerdict(verdict)), _))
@@ -613,7 +622,7 @@ TEST_F(ClientSideDetectionHostTest, OnPhishingDetectionDoneMultiplePings) {
                                                   csd_host_.get()));
   GURL other_phishing_url("http://other_phishing_url.com/bla");
   ExpectPreClassificationChecks(other_phishing_url, &kFalse, &kFalse, &kFalse,
-                                &kFalse, &kFalse, &kFalse);
+                                &kFalse, &kFalse, &kFalse, &kFalse, &kFalse);
   // We navigate away.  The callback cb should be revoked.
   NavigateAndCommit(other_phishing_url);
   // Wait for the pre-classification checks to finish for other_phishing_url.
@@ -679,12 +688,7 @@ TEST_F(ClientSideDetectionHostTest,
   verdict.set_client_score(0.1f);
   verdict.set_is_phishing(false);
 
-  ClientMalwareRequest malware_verdict;
-  malware_verdict.set_url(verdict.url());
-
   EXPECT_CALL(*mock_extractor, ExtractFeatures(_, _, _)).Times(0);
-  EXPECT_CALL(*mock_extractor, ExtractMalwareFeatures(_, _, _))
-      .WillOnce(InvokeMalwareCallback(&malware_verdict));
   OnPhishingDetectionDone(verdict.SerializeAsString());
   EXPECT_TRUE(Mock::VerifyAndClear(mock_extractor));
 }
@@ -701,7 +705,7 @@ TEST_F(ClientSideDetectionHostTest,
 
   // First we have to navigate to the URL to set the unique page ID.
   ExpectPreClassificationChecks(url, &kFalse, &kFalse, &kFalse, &kFalse,
-                                &kFalse, &kFalse);
+                                &kFalse, &kFalse, &kFalse, &kFalse);
   NavigateAndCommit(url);
   WaitAndCheckPreClassificationChecks();
   SetUnsafeSubResourceForCurrent();
@@ -727,7 +731,8 @@ TEST_F(ClientSideDetectionHostTest,
   // Do an initial navigation to a safe host.
   GURL start_url("http://safe.example.com/");
   ExpectPreClassificationChecks(
-      start_url, &kFalse, &kFalse, &kFalse, &kFalse, &kFalse, &kFalse);
+      start_url, &kFalse, &kFalse, &kFalse, &kFalse, &kFalse, &kFalse, &kFalse,
+      &kFalse);
   NavigateAndCommit(start_url);
   WaitAndCheckPreClassificationChecks();
 
@@ -740,7 +745,7 @@ TEST_F(ClientSideDetectionHostTest,
   verdict.set_is_phishing(false);
 
   ExpectPreClassificationChecks(url, &kFalse, &kFalse, &kFalse, &kFalse,
-                                &kFalse, &kFalse);
+                                &kFalse, &kFalse, &kFalse, &kFalse);
   NavigateWithSBHitAndCommit(url);
   WaitAndCheckPreClassificationChecks();
 
@@ -756,11 +761,72 @@ TEST_F(ClientSideDetectionHostTest,
   EXPECT_TRUE(Mock::VerifyAndClear(csd_host_.get()));
 
   ExpectPreClassificationChecks(start_url, &kFalse, &kFalse, &kFalse, &kFalse,
-                                &kFalse, &kFalse);
+                                &kFalse, &kFalse, &kFalse, &kFalse);
   NavigateWithoutSBHitAndCommit(start_url);
   WaitAndCheckPreClassificationChecks();
 }
 
+TEST_F(ClientSideDetectionHostTest,
+       DocumentOnLoadCompletedInMainFrameShowMalwareInterstitial) {
+  // Case 9: client thinks the page match malware IP and so does the server.
+  // We show an sub-resource malware interstitial.
+  MockBrowserFeatureExtractor* mock_extractor =
+      new StrictMock<MockBrowserFeatureExtractor>(
+          web_contents(),
+          csd_host_.get());
+  SetFeatureExtractor(mock_extractor);  // The host class takes ownership.
+
+  GURL malware_landing_url("http://malware.com/");
+  GURL malware_ip_url("http://badip.com");
+  ClientMalwareRequest malware_verdict;
+  malware_verdict.set_url("http://malware.com/");
+  ClientMalwareRequest::UrlInfo* badipurl =
+      malware_verdict.add_bad_ip_url_info();
+  badipurl->set_ip("1.2.3.4");
+  badipurl->set_url("http://badip.com");
+
+  ExpectPreClassificationChecks(GURL(malware_verdict.url()), &kFalse, &kFalse,
+                                &kFalse, &kFalse, &kFalse, &kFalse, &kFalse,
+                                &kFalse);
+  NavigateAndCommit(GURL(malware_verdict.url()));
+  WaitAndCheckPreClassificationChecks();
+
+  ClientSideDetectionService::ClientReportMalwareRequestCallback cb;
+  EXPECT_CALL(*mock_extractor, ExtractMalwareFeatures(_, _, _))
+      .WillOnce(InvokeMalwareCallback(&malware_verdict));
+  EXPECT_CALL(*csd_service_,
+              SendClientReportMalwareRequest(
+                  Pointee(PartiallyEqualMalwareVerdict(malware_verdict)), _))
+      .WillOnce(DoAll(DeleteArg<0>(), SaveArg<1>(&cb)));
+  DocumentOnLoadCompletedInMainFrame(GetBrowseInfo()->page_id);
+  EXPECT_TRUE(Mock::VerifyAndClear(csd_host_.get()));
+  EXPECT_TRUE(Mock::VerifyAndClear(csd_service_.get()));
+  ASSERT_FALSE(cb.is_null());
+
+  UnsafeResource resource;
+  EXPECT_CALL(*ui_manager_.get(), DisplayBlockingPage(_))
+      .WillOnce(SaveArg<0>(&resource));
+  cb.Run(malware_landing_url, malware_ip_url, true);
+
+  base::RunLoop().RunUntilIdle();
+  EXPECT_TRUE(Mock::VerifyAndClear(ui_manager_.get()));
+  EXPECT_EQ(malware_ip_url, resource.url);
+  EXPECT_EQ(malware_landing_url, resource.original_url);
+  EXPECT_TRUE(resource.is_subresource);
+  EXPECT_EQ(SB_THREAT_TYPE_CLIENT_SIDE_MALWARE_URL, resource.threat_type);
+  EXPECT_EQ(web_contents()->GetRenderProcessHost()->GetID(),
+            resource.render_process_host_id);
+  EXPECT_EQ(web_contents()->GetRenderViewHost()->GetRoutingID(),
+            resource.render_view_id);
+
+  // Make sure the client object will be deleted.
+  BrowserThread::PostTask(
+      BrowserThread::IO,
+      FROM_HERE,
+      base::Bind(&MockSafeBrowsingUIManager::InvokeOnBlockingPageComplete,
+                 ui_manager_, resource.callback));
+}
+
 TEST_F(ClientSideDetectionHostTest, UpdateIPUrlMap) {
   BrowseInfo* browse_info = GetBrowseInfo();
 
@@ -819,180 +885,6 @@ TEST_F(ClientSideDetectionHostTest, UpdateIPUrlMap) {
                   browse_info->ips["100.100.100.256"]);
 }
 
-TEST_F(ClientSideDetectionHostTest,
-       OnPhishingDetectionDoneVerdictNotPhishingNotMalwareIP) {
-  // Case 7: renderer sends a verdict string that isn't phishing and not matches
-  // malware bad IP list
-  MockBrowserFeatureExtractor* mock_extractor =
-      new StrictMock<MockBrowserFeatureExtractor>(
-          web_contents(),
-          csd_host_.get());
-  SetFeatureExtractor(mock_extractor);  // The host class takes ownership.
-
-  ClientPhishingRequest verdict;
-  verdict.set_url("http://not-phishing.com/");
-  verdict.set_client_score(0.1f);
-  verdict.set_is_phishing(false);
-
-  ClientMalwareRequest malware_verdict;
-  malware_verdict.set_url(verdict.url());
-
-  // That is a special case.  If there were no IP matches or if feature
-  // extraction failed the callback will delete the malware_verdict.
-  EXPECT_CALL(*mock_extractor, ExtractMalwareFeatures(_, _, _))
-      .WillOnce(InvokeMalwareCallback(&malware_verdict));
-  EXPECT_CALL(*csd_service_,
-              SendClientReportMalwareRequest(_, _)).Times(0);
-  EXPECT_CALL(*mock_extractor, ExtractFeatures(_, _, _)).Times(0);
-
-  OnPhishingDetectionDone(verdict.SerializeAsString());
-  EXPECT_TRUE(Mock::VerifyAndClear(mock_extractor));
-}
-
-TEST_F(ClientSideDetectionHostTest,
-       OnPhishingDetectionDoneVerdictNotPhishingButMalwareIP) {
-  // Case 8: renderer sends a verdict string that isn't phishing but matches
-  // malware bad IP list
-  MockBrowserFeatureExtractor* mock_extractor =
-      new StrictMock<MockBrowserFeatureExtractor>(
-          web_contents(),
-          csd_host_.get());
-  SetFeatureExtractor(mock_extractor);  // The host class takes ownership.
-
-  ClientPhishingRequest verdict;
-  verdict.set_url("http://not-phishing.com/");
-  verdict.set_client_score(0.1f);
-  verdict.set_is_phishing(false);
-
-  ClientMalwareRequest malware_verdict;
-  malware_verdict.set_url(verdict.url());
-  malware_verdict.set_referrer_url("http://referrer.com/");
-  ClientMalwareRequest::UrlInfo* badipurl =
-      malware_verdict.add_bad_ip_url_info();
-  badipurl->set_ip("1.2.3.4");
-  badipurl->set_url("badip.com");
-
-  EXPECT_CALL(*mock_extractor, ExtractMalwareFeatures(_, _, _))
-      .WillOnce(InvokeMalwareCallback(&malware_verdict));
-  EXPECT_CALL(*csd_service_,
-              SendClientReportMalwareRequest(
-                  Pointee(PartiallyEqualMalwareVerdict(malware_verdict)), _))
-      .WillOnce(DeleteArg<0>());
-  EXPECT_CALL(*mock_extractor, ExtractFeatures(_, _, _)).Times(0);
-
-  SetReferrer(GURL("http://referrer.com/"));
-  OnPhishingDetectionDone(verdict.SerializeAsString());
-  EXPECT_TRUE(Mock::VerifyAndClear(mock_extractor));
-}
-
-TEST_F(ClientSideDetectionHostTest,
-       OnPhishingDetectionDoneVerdictPhishingAndMalwareIP) {
-  // Case 9: renderer sends a verdict string that is phishing and matches
-  // malware bad IP list
-  MockBrowserFeatureExtractor* mock_extractor =
-      new StrictMock<MockBrowserFeatureExtractor>(
-          web_contents(),
-          csd_host_.get());
-  SetFeatureExtractor(mock_extractor);  // The host class takes ownership.
-
-  ClientSideDetectionService::ClientReportPhishingRequestCallback cb;
-  ClientPhishingRequest verdict;
-  verdict.set_url("http://not-phishing.com/");
-  verdict.set_client_score(0.1f);
-  verdict.set_is_phishing(true);
-
-  ClientMalwareRequest malware_verdict;
-  malware_verdict.set_url(verdict.url());
-  ClientMalwareRequest::UrlInfo* badipurl =
-      malware_verdict.add_bad_ip_url_info();
-  badipurl->set_ip("1.2.3.4");
-  badipurl->set_url("badip.com");
-
-  EXPECT_CALL(*mock_extractor, ExtractMalwareFeatures(_, _, _))
-      .WillOnce(InvokeMalwareCallback(&malware_verdict));
-  EXPECT_CALL(*csd_service_,
-              SendClientReportMalwareRequest(
-                  Pointee(PartiallyEqualMalwareVerdict(malware_verdict)), _))
-      .WillOnce(DeleteArg<0>());
-
-  EXPECT_CALL(*mock_extractor, ExtractFeatures(_, _, _))
-      .WillOnce(DoAll(DeleteArg<1>(),
-                      InvokeCallbackArgument<2>(true, &verdict)));
-
-  EXPECT_CALL(*csd_service_,
-              SendClientReportPhishingRequest(
-                  Pointee(PartiallyEqualVerdict(verdict)), _))
-      .WillOnce(SaveArg<1>(&cb));
-
-  // Referrer url using https won't be set and sent out.
-  SetReferrer(GURL("https://referrer.com/"));
-  OnPhishingDetectionDone(verdict.SerializeAsString());
-  EXPECT_TRUE(Mock::VerifyAndClear(mock_extractor));
-  EXPECT_TRUE(Mock::VerifyAndClear(csd_host_.get()));
-  EXPECT_TRUE(Mock::VerifyAndClear(csd_service_.get()));
-  ASSERT_FALSE(cb.is_null());
-}
-
-TEST_F(ClientSideDetectionHostTest,
-       OnPhishingDetectionDoneShowMalwareInterstitial) {
-  // Case 10: client thinks the page match malware IP and so does the server.
-  // We show an sub-resource malware interstitial.
-  MockBrowserFeatureExtractor* mock_extractor =
-      new StrictMock<MockBrowserFeatureExtractor>(
-          web_contents(),
-          csd_host_.get());
-  SetFeatureExtractor(mock_extractor);  // The host class takes ownership.
-
-  ClientPhishingRequest verdict;
-  verdict.set_url("http://not-phishing.com/");
-  verdict.set_client_score(0.1f);
-  verdict.set_is_phishing(false);
-
-  ClientSideDetectionService::ClientReportMalwareRequestCallback cb;
-  GURL malware_landing_url("http://malware.com/");
-  GURL malware_ip_url("http://badip.com");
-  ClientMalwareRequest malware_verdict;
-  malware_verdict.set_url("http://malware.com/");
-  ClientMalwareRequest::UrlInfo* badipurl =
-      malware_verdict.add_bad_ip_url_info();
-  badipurl->set_ip("1.2.3.4");
-  badipurl->set_url("http://badip.com");
-
-  EXPECT_CALL(*mock_extractor, ExtractMalwareFeatures(_, _, _))
-      .WillOnce(InvokeMalwareCallback(&malware_verdict));
-  EXPECT_CALL(*csd_service_,
-              SendClientReportMalwareRequest(
-                  Pointee(PartiallyEqualMalwareVerdict(malware_verdict)), _))
-      .WillOnce(DoAll(DeleteArg<0>(), SaveArg<1>(&cb)));
-  OnPhishingDetectionDone(verdict.SerializeAsString());
-  EXPECT_TRUE(Mock::VerifyAndClear(csd_host_.get()));
-  EXPECT_TRUE(Mock::VerifyAndClear(csd_service_.get()));
-  ASSERT_FALSE(cb.is_null());
-
-  UnsafeResource resource;
-  EXPECT_CALL(*ui_manager_.get(), DisplayBlockingPage(_))
-      .WillOnce(SaveArg<0>(&resource));
-  cb.Run(malware_landing_url, malware_ip_url, true);
-
-  base::RunLoop().RunUntilIdle();
-  EXPECT_TRUE(Mock::VerifyAndClear(ui_manager_.get()));
-  EXPECT_EQ(malware_ip_url, resource.url);
-  EXPECT_EQ(malware_landing_url, resource.original_url);
-  EXPECT_TRUE(resource.is_subresource);
-  EXPECT_EQ(SB_THREAT_TYPE_CLIENT_SIDE_MALWARE_URL, resource.threat_type);
-  EXPECT_EQ(web_contents()->GetRenderProcessHost()->GetID(),
-            resource.render_process_host_id);
-  EXPECT_EQ(web_contents()->GetRenderViewHost()->GetRoutingID(),
-            resource.render_view_id);
-
-  // Make sure the client object will be deleted.
-  BrowserThread::PostTask(
-      BrowserThread::IO,
-      FROM_HERE,
-      base::Bind(&MockSafeBrowsingUIManager::InvokeOnBlockingPageComplete,
-                 ui_manager_, resource.callback));
-}
-
 TEST_F(ClientSideDetectionHostTest, NavigationCancelsShouldClassifyUrl) {
   // Test that canceling pending should classify requests works as expected.
 
@@ -1006,10 +898,10 @@ TEST_F(ClientSideDetectionHostTest, NavigationCancelsShouldClassifyUrl) {
   EXPECT_CALL(*csd_service_, IsPrivateIPAddress(_))
       .WillOnce(Return(false))
       .WillOnce(Return(false));
-  ExpectPreClassificationChecks(first_url, NULL, &kFalse, &kFalse, NULL,
-                                NULL, NULL);
+  ExpectPreClassificationChecks(first_url, NULL, &kFalse, &kFalse, &kFalse,
+                                NULL, NULL, NULL, NULL);
   ExpectPreClassificationChecks(second_url, NULL, &kFalse, &kFalse, &kFalse,
-                                &kFalse, &kFalse);
+                                &kFalse, &kFalse, &kFalse, &kFalse);
 
   NavigateAndCommit(first_url);
   // Don't flush the message loop, as we want to navigate to a different
@@ -1018,156 +910,243 @@ TEST_F(ClientSideDetectionHostTest, NavigationCancelsShouldClassifyUrl) {
   WaitAndCheckPreClassificationChecks();
 }
 
-TEST_F(ClientSideDetectionHostTest, ShouldClassifyUrl) {
+TEST_F(ClientSideDetectionHostTest, TestPreClassificationCheckPass) {
   // Navigate the tab to a page.  We should see a StartPhishingDetection IPC.
   GURL url("http://host.com/");
   ExpectPreClassificationChecks(url, &kFalse, &kFalse, &kFalse, &kFalse,
-                                &kFalse, &kFalse);
+                                &kFalse, &kFalse, &kFalse, &kFalse);
   NavigateAndCommit(url);
   WaitAndCheckPreClassificationChecks();
 
-  const IPC::Message* msg = process()->sink().GetFirstMessageMatching(
-      SafeBrowsingMsg_StartPhishingDetection::ID);
-  ASSERT_TRUE(msg);
-  Tuple1<GURL> actual_url;
-  SafeBrowsingMsg_StartPhishingDetection::Read(msg, &actual_url);
-  EXPECT_EQ(url, actual_url.a);
-  EXPECT_EQ(rvh()->GetRoutingID(), msg->routing_id());
-  process()->sink().ClearMessages();
+  ExpectStartPhishingDetection(&url);
+  ExpectShouldClassifyForMalwareResult(true);
+}
+
+TEST_F(ClientSideDetectionHostTest,
+       TestPreClassificationCheckInPageNavigation) {
+  GURL url("http://host.com/");
+  ExpectPreClassificationChecks(url, &kFalse, &kFalse, &kFalse, &kFalse,
+                                &kFalse, &kFalse, &kFalse, &kFalse);
+  NavigateAndCommit(url);
+  WaitAndCheckPreClassificationChecks();
+
+  ExpectStartPhishingDetection(&url);
+  ExpectShouldClassifyForMalwareResult(true);
 
   // Now try an in-page navigation.  This should not trigger an IPC.
   EXPECT_CALL(*csd_service_, IsPrivateIPAddress(_)).Times(0);
-  url = GURL("http://host.com/#foo");
-  ExpectPreClassificationChecks(url, NULL, NULL, NULL, NULL, NULL, NULL);
-  NavigateAndCommit(url);
+  GURL inpage("http://host.com/#foo");
+  ExpectPreClassificationChecks(inpage, NULL, NULL, NULL, NULL, NULL, NULL,
+                                NULL, NULL);
+  NavigateAndCommit(inpage);
   WaitAndCheckPreClassificationChecks();
 
-  msg = process()->sink().GetFirstMessageMatching(
-      SafeBrowsingMsg_StartPhishingDetection::ID);
-  ASSERT_FALSE(msg);
+  ExpectStartPhishingDetection(NULL);
+  ExpectShouldClassifyForMalwareResult(true);
+}
 
+TEST_F(ClientSideDetectionHostTest, TestPreClassificationCheckXHTML) {
   // Check that XHTML is supported, in addition to the default HTML type.
-  // Note: for this test to work correctly, the new URL must be on the
-  // same domain as the previous URL, otherwise it will create a new
-  // RenderViewHost that won't have the mime type set.
-  url = GURL("http://host.com/xhtml");
+  GURL url("http://host.com/xhtml");
   rvh_tester()->SetContentsMimeType("application/xhtml+xml");
   ExpectPreClassificationChecks(url, &kFalse, &kFalse, &kFalse, &kFalse,
-                                &kFalse, &kFalse);
+                                &kFalse, &kFalse, &kFalse, &kFalse);
   NavigateAndCommit(url);
   WaitAndCheckPreClassificationChecks();
-  msg = process()->sink().GetFirstMessageMatching(
-      SafeBrowsingMsg_StartPhishingDetection::ID);
-  ASSERT_TRUE(msg);
-  SafeBrowsingMsg_StartPhishingDetection::Read(msg, &actual_url);
-  EXPECT_EQ(url, actual_url.a);
-  EXPECT_EQ(rvh()->GetRoutingID(), msg->routing_id());
-  process()->sink().ClearMessages();
-
-  // Navigate to a new host, which should cause another IPC.
-  url = GURL("http://host2.com/");
-  ExpectPreClassificationChecks(url, &kFalse, &kFalse, &kFalse, &kFalse,
-                                &kFalse, &kFalse);
-  NavigateAndCommit(url);
+
+  ExpectStartPhishingDetection(&url);
+  ExpectShouldClassifyForMalwareResult(true);
+}
+
+TEST_F(ClientSideDetectionHostTest, TestPreClassificationCheckTwoNavigations) {
+  // Navigate to two hosts, which should cause two IPCs.
+  GURL url1("http://host1.com/");
+  ExpectPreClassificationChecks(url1, &kFalse, &kFalse, &kFalse, &kFalse,
+                                &kFalse, &kFalse, &kFalse, &kFalse);
+  NavigateAndCommit(url1);
   WaitAndCheckPreClassificationChecks();
-  msg = process()->sink().GetFirstMessageMatching(
-      SafeBrowsingMsg_StartPhishingDetection::ID);
-  ASSERT_TRUE(msg);
-  SafeBrowsingMsg_StartPhishingDetection::Read(msg, &actual_url);
-  EXPECT_EQ(url, actual_url.a);
-  EXPECT_EQ(rvh()->GetRoutingID(), msg->routing_id());
-  process()->sink().ClearMessages();
 
-  // If the mime type is not one that we support, no IPC should be triggered.
+  ExpectStartPhishingDetection(&url1);
+  ExpectShouldClassifyForMalwareResult(true);
+
+  GURL url2("http://host2.com/");
+  ExpectPreClassificationChecks(url2, &kFalse, &kFalse, &kFalse, &kFalse,
+                                &kFalse, &kFalse, &kFalse, &kFalse);
+  NavigateAndCommit(url2);
+  WaitAndCheckPreClassificationChecks();
+
+  ExpectStartPhishingDetection(&url2);
+  ExpectShouldClassifyForMalwareResult(true);
+}
+
+TEST_F(ClientSideDetectionHostTest, TestPreClassificationCheckMimeType) {
+  // If the mime type is not one that we support, no IPC should be triggered
+  // but all pre-classification checks should run because we might classify
+  // other mime types for malware.
   // Note: for this test to work correctly, the new URL must be on the
   // same domain as the previous URL, otherwise it will create a new
   // RenderViewHost that won't have the mime type set.
-  url = GURL("http://host2.com/image.jpg");
+  GURL url("http://host2.com/image.jpg");
   rvh_tester()->SetContentsMimeType("image/jpeg");
-  ExpectPreClassificationChecks(url, NULL, NULL, NULL, NULL, NULL, NULL);
+  ExpectPreClassificationChecks(url, &kFalse, &kFalse, &kFalse, &kFalse,
+                                &kFalse, &kFalse,&kFalse, &kFalse);
   NavigateAndCommit(url);
   WaitAndCheckPreClassificationChecks();
-  msg = process()->sink().GetFirstMessageMatching(
-      SafeBrowsingMsg_StartPhishingDetection::ID);
-  ASSERT_FALSE(msg);
 
+  ExpectStartPhishingDetection(NULL);
+  ExpectShouldClassifyForMalwareResult(true);
+}
+
+TEST_F(ClientSideDetectionHostTest,
+       TestPreClassificationCheckPrivateIpAddress) {
   // If IsPrivateIPAddress returns true, no IPC should be triggered.
-  url = GURL("http://host3.com/");
-  ExpectPreClassificationChecks(url, &kTrue, NULL, NULL, NULL, NULL, NULL);
+  GURL url("http://host3.com/");
+  ExpectPreClassificationChecks(url, &kTrue, &kFalse, NULL, NULL, NULL, NULL,
+                                NULL, NULL);
   NavigateAndCommit(url);
   WaitAndCheckPreClassificationChecks();
-  msg = process()->sink().GetFirstMessageMatching(
+  const IPC::Message* msg = process()->sink().GetFirstMessageMatching(
       SafeBrowsingMsg_StartPhishingDetection::ID);
   ASSERT_FALSE(msg);
+  ExpectShouldClassifyForMalwareResult(false);
+}
 
+TEST_F(ClientSideDetectionHostTest, TestPreClassificationCheckIncognito) {
   // If the tab is incognito there should be no IPC.  Also, we shouldn't
   // even check the csd-whitelist.
-  url = GURL("http://host4.com/");
-  ExpectPreClassificationChecks(url, &kFalse, &kTrue, NULL, NULL, NULL, NULL);
+  GURL url("http://host4.com/");
+  ExpectPreClassificationChecks(url, &kFalse, &kTrue, NULL, NULL, NULL, NULL,
+                                NULL, NULL);
   NavigateAndCommit(url);
   WaitAndCheckPreClassificationChecks();
-  msg = process()->sink().GetFirstMessageMatching(
-      SafeBrowsingMsg_StartPhishingDetection::ID);
-  ASSERT_FALSE(msg);
 
-  // If the URL is on the csd whitelist, no IPC should be triggered.
-  url = GURL("http://host5.com/");
-  ExpectPreClassificationChecks(url, &kFalse, &kFalse, &kTrue, NULL, NULL,
-                                NULL);
+  ExpectStartPhishingDetection(NULL);
+  ExpectShouldClassifyForMalwareResult(false);
+}
+
+TEST_F(ClientSideDetectionHostTest, TestPreClassificationCheckCsdWhitelist) {
+  // If the URL is on the csd whitelist no phishing IPC should be sent
+  // but we should classify the URL for malware.
+  GURL url("http://host5.com/");
+  ExpectPreClassificationChecks(url, &kFalse, &kFalse, &kTrue, &kFalse, &kFalse,
+                                &kFalse, &kFalse, &kFalse);
   NavigateAndCommit(url);
   WaitAndCheckPreClassificationChecks();
-  msg = process()->sink().GetFirstMessageMatching(
-      SafeBrowsingMsg_StartPhishingDetection::ID);
-  ASSERT_FALSE(msg);
 
+  ExpectStartPhishingDetection(NULL);
+  ExpectShouldClassifyForMalwareResult(true);
+}
+
+TEST_F(ClientSideDetectionHostTest,
+       TestPreClassificationCheckMalwareKillSwitch) {
+  // If the malware killswitch is on we shouldn't classify the page for malware.
+  GURL url("http://host5.com/kill-switch");
+  ExpectPreClassificationChecks(url, &kFalse, &kFalse, &kFalse, &kTrue, &kFalse,
+                                &kFalse, &kFalse, &kFalse);
+  NavigateAndCommit(url);
+  WaitAndCheckPreClassificationChecks();
+
+  ExpectStartPhishingDetection(&url);
+  ExpectShouldClassifyForMalwareResult(false);
+}
+
+TEST_F(ClientSideDetectionHostTest,
+       TestPreClassificationCheckKillswitchAndCsdWhitelist) {
+  // If both the malware kill-swtich is on and the URL is on the csd whitelist,
+  // we will leave pre-classification checks early.
+  GURL url("http://host5.com/kill-switch-and-whitelisted");
+  ExpectPreClassificationChecks(url, &kFalse, &kFalse, &kTrue, &kTrue, NULL,
+                                NULL, NULL, NULL);
+  NavigateAndCommit(url);
+  WaitAndCheckPreClassificationChecks();
+
+  ExpectStartPhishingDetection(NULL);
+  ExpectShouldClassifyForMalwareResult(false);
+}
+
+TEST_F(ClientSideDetectionHostTest, TestPreClassificationCheckInvalidCache) {
   // If item is in the cache but it isn't valid, we will classify regardless
   // of whether we are over the reporting limit.
-  url = GURL("http://host6.com/");
-  ExpectPreClassificationChecks(url, &kFalse, &kFalse, &kFalse, &kFalse, &kTrue,
-                                NULL);
+  GURL url("http://host6.com/");
+  ExpectPreClassificationChecks(url, &kFalse, &kFalse, &kFalse, &kFalse,
+                                &kFalse, &kTrue, NULL, &kFalse);
+
   NavigateAndCommit(url);
   WaitAndCheckPreClassificationChecks();
-  msg = process()->sink().GetFirstMessageMatching(
-      SafeBrowsingMsg_StartPhishingDetection::ID);
-  ASSERT_TRUE(msg);
-  SafeBrowsingMsg_StartPhishingDetection::Read(msg, &actual_url);
-  EXPECT_EQ(url, actual_url.a);
-  EXPECT_EQ(rvh()->GetRoutingID(), msg->routing_id());
-  process()->sink().ClearMessages();
 
+  ExpectStartPhishingDetection(&url);
+  ExpectShouldClassifyForMalwareResult(true);
+}
+
+TEST_F(ClientSideDetectionHostTest,
+       TestPreClassificationCheckOverPhishingReportingLimit) {
   // If the url isn't in the cache and we are over the reporting limit, we
   // don't do classification.
-  url = GURL("http://host7.com/");
+  GURL url("http://host7.com/");
   ExpectPreClassificationChecks(url, &kFalse, &kFalse, &kFalse, &kFalse,
-                                &kFalse, &kTrue);
+                                &kFalse, &kFalse, &kTrue, &kFalse);
   NavigateAndCommit(url);
   WaitAndCheckPreClassificationChecks();
-  msg = process()->sink().GetFirstMessageMatching(
-      SafeBrowsingMsg_StartPhishingDetection::ID);
-  ASSERT_FALSE(msg);
 
+  ExpectStartPhishingDetection(NULL);
+  ExpectShouldClassifyForMalwareResult(true);
+}
+
+TEST_F(ClientSideDetectionHostTest,
+       TestPreClassificationCheckOverMalwareReportingLimit) {
+  GURL url("http://host.com/");
+  ExpectPreClassificationChecks(url, &kFalse, &kFalse, &kFalse, &kFalse,
+                                &kFalse, &kFalse, &kFalse, &kTrue);
+  NavigateAndCommit(url);
+  WaitAndCheckPreClassificationChecks();
+
+  ExpectStartPhishingDetection(&url);
+  ExpectShouldClassifyForMalwareResult(false);
+}
+
+TEST_F(ClientSideDetectionHostTest,
+       TestPreClassificationCheckOverBothReportingLimits) {
+  GURL url("http://host.com/");
+  ExpectPreClassificationChecks(url, &kFalse, &kFalse, &kFalse, &kFalse,
+                                &kFalse, &kFalse, &kTrue, &kTrue);
+  NavigateAndCommit(url);
+  WaitAndCheckPreClassificationChecks();
+
+  ExpectStartPhishingDetection(NULL);
+  ExpectShouldClassifyForMalwareResult(false);
+}
+
+TEST_F(ClientSideDetectionHostTest, TestPreClassificationCheckHttpsUrl) {
+  GURL url("https://host.com/");
+  ExpectPreClassificationChecks(url, &kFalse, &kFalse, &kFalse, &kFalse,
+                                &kFalse, &kFalse, &kFalse, &kFalse);
+  NavigateAndCommit(url);
+  WaitAndCheckPreClassificationChecks();
+
+  ExpectStartPhishingDetection(NULL);
+  ExpectShouldClassifyForMalwareResult(true);
+}
+
+TEST_F(ClientSideDetectionHostTest, TestPreClassificationCheckValidCached) {
   // If result is cached, we will try and display the blocking page directly
   // with no start classification message.
-  url = GURL("http://host8.com/");
-  ExpectPreClassificationChecks(url, &kFalse, &kFalse, &kFalse, &kTrue, NULL,
-                                NULL);
+  GURL url("http://host8.com/");
+  ExpectPreClassificationChecks(url, &kFalse, &kFalse, &kFalse, &kFalse, &kTrue,
+                                &kFalse, &kFalse, &kFalse);
 
   UnsafeResource resource;
   EXPECT_CALL(*ui_manager_.get(), DisplayBlockingPage(_))
       .WillOnce(SaveArg<0>(&resource));
 
   NavigateAndCommit(url);
-  // Wait for CheckCsdWhitelist and CheckCache() to be called.
-  base::RunLoop().RunUntilIdle();
-  // Now we check that all expected functions were indeed called on the two
-  // service objects.
-  EXPECT_TRUE(Mock::VerifyAndClear(csd_host_.get()));
-  EXPECT_TRUE(Mock::VerifyAndClear(ui_manager_.get()));
+  WaitAndCheckPreClassificationChecks();
   EXPECT_EQ(url, resource.url);
   EXPECT_EQ(url, resource.original_url);
-  resource.callback.Reset();
-  msg = process()->sink().GetFirstMessageMatching(
-      SafeBrowsingMsg_StartPhishingDetection::ID);
-  ASSERT_FALSE(msg);
+
+  ExpectStartPhishingDetection(NULL);
+
+  // Showing a phishing warning will invalidate all the weak pointers which
+  // means we will not extract malware features.
+  ExpectShouldClassifyForMalwareResult(false);
 }
 }  // namespace safe_browsing
diff --git a/chrome/browser/safe_browsing/client_side_detection_service.cc b/chrome/browser/safe_browsing/client_side_detection_service.cc
index d63e45c..d045bd9 100644
--- a/chrome/browser/safe_browsing/client_side_detection_service.cc
+++ b/chrome/browser/safe_browsing/client_side_detection_service.cc
@@ -368,15 +368,6 @@ void ClientSideDetectionService::StartClientReportMalwareRequest(
     return;
   }
 
-  if (OverMalwareReportLimit()) {
-    UpdateEnumUMAHistogram(REPORT_HIT_LIMIT);
-    DVLOG(1) << "Too many malware report requests sent recently."
-             << "Skip sending malware report for " << GURL(request->url());
-    if (!callback.is_null())
-      callback.Run(GURL(request->url()), GURL(request->url()), false);
-    return;
-  }
-
   std::string request_data;
   if (!request->SerializeToString(&request_data)) {
     UpdateEnumUMAHistogram(REPORT_FAILED_SERIALIZATION);
diff --git a/chrome/browser/safe_browsing/client_side_detection_service_unittest.cc b/chrome/browser/safe_browsing/client_side_detection_service_unittest.cc
index b5f64d5..5762847 100644
--- a/chrome/browser/safe_browsing/client_side_detection_service_unittest.cc
+++ b/chrome/browser/safe_browsing/client_side_detection_service_unittest.cc
@@ -472,25 +472,22 @@ TEST_F(ClientSideDetectionServiceTest, SendClientReportMalwareRequest) {
                                  net::URLRequestStatus::FAILED);
   EXPECT_FALSE(SendClientReportMalwareRequest(url));
 
-  // server blacklist decision is false, and response is succesful
+  // Server blacklist decision is false, and response is successful
   response.set_blacklist(false);
   SetClientReportMalwareResponse(response.SerializeAsString(), net::HTTP_OK,
                                  net::URLRequestStatus::SUCCESS);
   EXPECT_FALSE(SendClientReportMalwareRequest(url));
 
-  // Check that we have recorded all 4 requests within the correct time range.
+  // Check that we have recorded all 5 requests within the correct time range.
   base::Time after = base::Time::Now();
   std::queue<base::Time>& report_times = GetMalwareReportTimes();
-  EXPECT_EQ(4U, report_times.size());
+  EXPECT_EQ(5U, report_times.size());
 
-  // Another normal behavior will fail because of the limit is hit
-  response.set_blacklist(true);
-  SetClientReportMalwareResponse(response.SerializeAsString(), net::HTTP_OK,
-                                 net::URLRequestStatus::SUCCESS);
-  EXPECT_FALSE(SendClientReportMalwareRequest(url));
+  // Check that the malware report limit was reached.
+  EXPECT_TRUE(csd_service_->OverMalwareReportLimit());
 
   report_times = GetMalwareReportTimes();
-  EXPECT_EQ(4U, report_times.size());
+  EXPECT_EQ(5U, report_times.size());
   while (!report_times.empty()) {
     base::Time time = report_times.back();
     report_times.pop();