Mostly integrate new malware IP blacklist with the csd client. When

chrome/browser/safe_browsing/browser_feature_extractor.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/safe_browsing/browser_feature_extractor.h"
 
 #include <map>
 #include <utility>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/format_macros.h"
 #include "base/stl_util.h"
 #include "base/strings/stringprintf.h"
 #include "base/time/time.h"
 #include "chrome/browser/common/cancelable_request.h"
 #include "chrome/browser/history/history_service.h"
 #include "chrome/browser/history/history_service_factory.h"
 #include "chrome/browser/history/history_types.h"
 #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/client_side_detection_host.h"
+#include "chrome/browser/safe_browsing/database_manager.h"
 #include "chrome/common/safe_browsing/csd.pb.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/navigation_controller.h"
 #include "content/public/browser/navigation_entry.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/common/page_transition_types.h"
 #include "url/gurl.h"
 
 using content::BrowserThread;
 using content::NavigationController;
 using content::NavigationEntry;
 using content::WebContents;
 
 namespace safe_browsing {
 
-const int BrowserFeatureExtractor::kMaxMalwareIPPerRequest = 5;
+namespace {
+
+const int kMaxMalwareIPPerRequest = 5;
+
+void FilterBenignIpsOnIOThread(
+    scoped_refptr<SafeBrowsingDatabaseManager> database_manager,
+    IPUrlMap* ips) {

[mattm, 2013/10/31 05:26:40]

could add  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));

[noé, 2013/10/31 20:39:58]

Done.

+  std::vector<std::string> to_delete;
+  for (IPUrlMap::const_iterator it = ips->begin();
+       it != ips->end(); ++it) {
+    if (!database_manager.get() ||
+        !database_manager->MatchMalwareIP(it->first)) {
+      to_delete.push_back(it->first);

[mattm, 2013/10/31 05:26:40]

It should be possible to do this without the second loop.. if you make a copy of
the iterator, then advance it, and then use the copy of the old location to do
the erase.

[noé, 2013/10/31 20:39:58]

Actually, we don't even need a copy iterator.  Done.

+    }
+  }
+  for (std::vector<std::string>::const_iterator it = to_delete.begin();
+       it != to_delete.end(); ++it) {
+    ips->erase(*it);
+  }
+}
+}  // namespace
 
 BrowseInfo::BrowseInfo() : http_status_code(0) {}
 
 BrowseInfo::~BrowseInfo() {}
 
 static void AddFeature(const std::string& feature_name,
                        double feature_value,
                        ClientPhishingRequest* request) {
   DCHECK(request);
   ClientPhishingRequest::Feature* feature =
@@ skipping 76 matching lines @@
                                   features::kRedirect,
                                   i,
                                   printable_redirect.c_str()),
                1.0,
                request);
   }
 }
 
 BrowserFeatureExtractor::BrowserFeatureExtractor(
     WebContents* tab,
-    ClientSideDetectionService* service)
+    ClientSideDetectionHost* host)
     : tab_(tab),
-      service_(service),
+      host_(host),
       weak_factory_(this) {
   DCHECK(tab);
 }
 
 BrowserFeatureExtractor::~BrowserFeatureExtractor() {
   weak_factory_.InvalidateWeakPtrs();
   // Delete all the pending extractions (delete callback and request objects).
   STLDeleteContainerPairFirstPointers(pending_extractions_.begin(),
                                       pending_extractions_.end());
 
@@ skipping 14 matching lines @@
 }
 
 void BrowserFeatureExtractor::ExtractFeatures(const BrowseInfo* info,
                                               ClientPhishingRequest* request,
                                               const DoneCallback& callback) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   DCHECK(request);
   DCHECK(info);
   DCHECK_EQ(0U, request->url().find("http:"));
   DCHECK(!callback.is_null());
-  if (callback.is_null()) {
-    DLOG(ERROR) << "ExtractFeatures called without a callback object";
-    return;
-  }
-
   // Extract features pertaining to this navigation.
   const NavigationController& controller = tab_->GetController();
   int url_index = -1;
   int first_host_index = -1;
 
   GURL request_url(request->url());
   int index = controller.GetCurrentEntryIndex();
   // The url that we are extracting features for should already be commited.
   DCHECK_NE(index, -1);
   for (; index >= 0; index--) {
@@ skipping 36 matching lines @@
 
   ExtractBrowseInfoFeatures(*info, request);
   pending_extractions_[request] = callback;
   base::MessageLoop::current()->PostTask(
       FROM_HERE,
       base::Bind(&BrowserFeatureExtractor::StartExtractFeatures,
                  weak_factory_.GetWeakPtr(), request, callback));
 }
 
 void BrowserFeatureExtractor::ExtractMalwareFeatures(
-    const BrowseInfo* info,
-    ClientMalwareRequest* request) {
+    BrowseInfo* info,
+    ClientMalwareRequest* request,
+    const MalwareDoneCallback& callback) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  DCHECK(request);
-  DCHECK(info);
   DCHECK_EQ(0U, request->url().find("http:"));
-  // get the IPs and urls that match the malware blacklisted IP list.
-  if (service_) {
-    int matched_bad_ips = 0;
-    for (IPUrlMap::const_iterator it = info->ips.begin();
-         it != info->ips.end(); ++it) {
-      if (service_->IsBadIpAddress(it->first)) {
-        AddMalwareFeature(features::kBadIpFetch + it->first,
-                          it->second, 1.0, request);
-        ++matched_bad_ips;
-        // Limit the number of matched bad IPs in one request to control
-        // the request's size
-        if (matched_bad_ips >= kMaxMalwareIPPerRequest) {
-          return;
-        }
-      }
-    }
-  }
+  DCHECK(!callback.is_null());
+
+  // Copy the IPs because they might go away before we're done

[mattm, 2013/10/31 05:26:40]

s/Copy/Grab/ (or something)

[noé, 2013/10/31 20:39:58]

Done.

+  // checking them against the IP blacklist on the IO thread.
+  scoped_ptr<IPUrlMap> ips(new IPUrlMap);
+  ips->swap(info->ips);
+
+  IPUrlMap* ips_ptr = ips.get();
+
+  // The API doesn't take a scoped_ptr because the API gets mocked and we
+  // cannot mock an API that takes scoped_ptr as arguments.
+  scoped_ptr<ClientMalwareRequest> req(request);
+
+  // IP blacklist lookups have to happen on the IO thread.
+  BrowserThread::PostTaskAndReply(
+      BrowserThread::IO,
+      FROM_HERE,
+      base::Bind(&FilterBenignIpsOnIOThread,
+                 host_->database_manager(),
+                 ips_ptr),
+      base::Bind(&BrowserFeatureExtractor::FinishExtractMalwareFeatures,
+                 weak_factory_.GetWeakPtr(),
+                 base::Passed(&ips), callback, base::Passed(&req)));
 }
 
 void BrowserFeatureExtractor::ExtractBrowseInfoFeatures(
     const BrowseInfo& info,
     ClientPhishingRequest* request) {
-  if (service_) {
-    for (IPUrlMap::const_iterator it = info.ips.begin();
-         it != info.ips.end(); ++it) {
-      if (service_->IsBadIpAddress(it->first)) {
-        AddFeature(features::kBadIpFetch + it->first, 1.0, request);
-      }
-    }
-  }
   if (info.unsafe_resource.get()) {
     // A SafeBrowsing interstitial was shown for the current URL.
     AddFeature(features::kSafeBrowsingMaliciousUrl +
                info.unsafe_resource->url.spec(),
                1.0,
                request);
     AddFeature(features::kSafeBrowsingOriginalUrl +
                info.unsafe_resource->original_url.spec(),
                1.0,
                request);
@@ skipping 209 matching lines @@
     *history = HistoryServiceFactory::GetForProfile(profile,
                                                     Profile::EXPLICIT_ACCESS);
     if (*history) {
       return true;
     }
   }
   VLOG(2) << "Unable to query history.  No history service available.";
   return false;
 }
 
+void BrowserFeatureExtractor::FinishExtractMalwareFeatures(
+    scoped_ptr<IPUrlMap> bad_ips,
+    MalwareDoneCallback callback,
+    scoped_ptr<ClientMalwareRequest> request) {

[mattm, 2013/10/31 05:26:40]

could add DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));

[noé, 2013/10/31 20:39:58]

Done.

+  int matched_bad_ips = 0;
+  for (IPUrlMap::const_iterator it = bad_ips->begin();
+       it != bad_ips->end(); ++it) {
+    AddMalwareFeature(features::kBadIpFetch + it->first,
+                      it->second, 1.0, request.get());
+    ++matched_bad_ips;
+    // Limit the number of matched bad IPs in one request to control
+    // the request's size
+    if (matched_bad_ips >= kMaxMalwareIPPerRequest) {
+      break;
+    }
+  }
+  callback.Run(true, request.Pass());
+}
+
 }  // namespace safe_browsing