Sample 1% url whitelisted PPAPI downloads to ping safe browsing server

chrome/browser/safe_browsing/download_protection_service.cc

Index: chrome/browser/safe_browsing/download_protection_service.cc
diff --git a/chrome/browser/safe_browsing/download_protection_service.cc b/chrome/browser/safe_browsing/download_protection_service.cc
index 2381394d23b89fff2dd471b930f8ddf92471a992..806186b0d71bceabb73eb3197b862de1391df231 100644
--- a/chrome/browser/safe_browsing/download_protection_service.cc
+++ b/chrome/browser/safe_browsing/download_protection_service.cc
@@ -76,6 +76,18 @@ namespace {
 static const int64_t kDownloadRequestTimeoutMs = 7000;
 // We sample 1% of whitelisted downloads to still send out download pings.
 static const double kWhitelistDownloadSampleRate = 0.01;
+
+enum WhitelistType {
+  NO_WHITELIST_MATCH,
+  URL_WHITELIST,
+  SIGNATURE_WHITELIST,
+  WHITELIST_TYPE_MAX
+};
+
+static void RecordCountOfWhitelistedDownload(WhitelistType type) {
+  UMA_HISTOGRAM_ENUMERATION("SBClientDownload.CheckWhitelistResult", type,
+                            WHITELIST_TYPE_MAX);
+}
 }  // namespace
 
 namespace safe_browsing {
@@ -753,20 +765,7 @@ class DownloadProtectionService::CheckClientDownloadRequest
   }
 #endif  // defined(OS_MACOSX)
 
-  enum WhitelistType {
-    NO_WHITELIST_MATCH,
-    URL_WHITELIST,
-    SIGNATURE_WHITELIST,
-    WHITELIST_TYPE_MAX
-  };
-
-  static void RecordCountOfWhitelistedDownload(WhitelistType type) {
-    UMA_HISTOGRAM_ENUMERATION("SBClientDownload.CheckWhitelistResult",
-                              type,
-                              WHITELIST_TYPE_MAX);
-  }
-
-  virtual bool ShouldSampleWhitelistedDownload() {
+  bool ShouldSampleWhitelistedDownload() {
     // We currently sample 1% whitelisted downloads from users who opted
     // in extended reporting and are not in incognito mode.
     return service_ && is_extended_reporting_ && !is_incognito_ &&
@@ -1193,6 +1192,8 @@ class DownloadProtectionService::PPAPIDownloadRequest
       const GURL& requestor_url,
       const base::FilePath& default_file_path,
       const std::vector<base::FilePath::StringType>& alternate_extensions,
+      bool is_extended_reporting,
+      bool is_incognito,
       const CheckDownloadCallback& callback,
       DownloadProtectionService* service,
       scoped_refptr<SafeBrowsingDatabaseManager> database_manager)
@@ -1205,6 +1206,9 @@ class DownloadProtectionService::PPAPIDownloadRequest
         start_time_(base::TimeTicks::Now()),
         supported_path_(
             GetSupportedFilePath(default_file_path, alternate_extensions)),
+        is_extended_reporting_(is_extended_reporting),
+        is_incognito_(is_incognito),
+        sample_url_whitelist_(false),
         weakptr_factory_(this) {}
 
   ~PPAPIDownloadRequest() override {
@@ -1255,6 +1259,13 @@ class DownloadProtectionService::PPAPIDownloadRequest
   }
 
  private:
+  bool ShouldSampleWhitelistedDownload() {
+    // We currently sample 1% whitelisted downloads from users who opted
+    // in extended reporting and are not in incognito mode.
+    return service_ && !is_incognito_ && is_extended_reporting_ &&
+           base::RandDouble() < service_->whitelist_sample_rate();
+  }
+
   // Whitelist checking needs to the done on the IO thread.
   static void CheckWhitelistsOnIOThread(
       const GURL& requestor_url,
@@ -1275,10 +1286,13 @@ class DownloadProtectionService::PPAPIDownloadRequest
   void WhitelistCheckComplete(bool was_on_whitelist) {
     DVLOG(2) << __FUNCTION__ << " was_on_whitelist:" << was_on_whitelist;
     if (was_on_whitelist) {
-      // TODO(asanka): Should sample whitelisted downloads based on
-      // service_->whitelist_sample_rate(). http://crbug.com/610924
-      Finish(RequestOutcome::WHITELIST_HIT, SAFE);
-      return;
+      if (ShouldSampleWhitelistedDownload()) {
+        RecordCountOfWhitelistedDownload(URL_WHITELIST);
+        sample_url_whitelist_ = true;
+      } else {
+        Finish(RequestOutcome::WHITELIST_HIT, SAFE);
+        return;
+      }
     }
 
     // Not on whitelist, so we are going to check with the SafeBrowsing
@@ -1299,6 +1313,10 @@ class DownloadProtectionService::PPAPIDownloadRequest
     request.set_file_basename(supported_path_.BaseName().AsUTF8Unsafe());
     request.set_length(0);
     request.mutable_digests()->set_md5(std::string());
+    request.set_skipped_url_whitelist(sample_url_whitelist_);
+    // Download protection does not check certificate whitelist for PPAPI
+    // downloads.
+    request.set_skipped_certificate_whitelist(false);
     for (const auto& alternate_extension : alternate_extensions_) {
       if (alternate_extension.empty())
         continue;
@@ -1449,6 +1467,10 @@ class DownloadProtectionService::PPAPIDownloadRequest
   // ping.
   const base::FilePath supported_path_;
 
+  bool is_extended_reporting_;
+  bool is_incognito_;
+  bool sample_url_whitelist_;
+
   base::WeakPtrFactory<PPAPIDownloadRequest> weakptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(PPAPIDownloadRequest);
@@ -1554,12 +1576,14 @@ void DownloadProtectionService::CheckPPAPIDownloadRequest(
     const GURL& requestor_url,
     const base::FilePath& default_file_path,
     const std::vector<base::FilePath::StringType>& alternate_extensions,
+    bool is_extended_reporting,
+    bool is_incognito,
     const CheckDownloadCallback& callback) {
   DVLOG(1) << __FUNCTION__ << " url:" << requestor_url
            << " default_file_path:" << default_file_path.value();
   std::unique_ptr<PPAPIDownloadRequest> request(new PPAPIDownloadRequest(
-      requestor_url, default_file_path, alternate_extensions, callback, this,
-      database_manager_));
+      requestor_url, default_file_path, alternate_extensions,
+      is_extended_reporting, is_incognito, callback, this, database_manager_));
   PPAPIDownloadRequest* request_copy = request.get();
   auto insertion_result = ppapi_download_requests_.insert(
       std::make_pair(request_copy, std::move(request)));