Separate SBClientDownload.SignedOrWhitelistedDownload metric into two buckets,

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 71c81a77018e278f0e573d4c045737ab2a929b3a..d36151774b486408987fd8a2885d70c2660c787d 100644
--- a/chrome/browser/safe_browsing/download_protection_service.cc
+++ b/chrome/browser/safe_browsing/download_protection_service.cc
@@ -669,8 +669,17 @@ class DownloadProtectionService::CheckClientDownloadRequest
   }
 #endif  // defined(OS_MACOSX)
 
-  static void RecordCountOfSignedOrWhitelistedDownload() {
-    UMA_HISTOGRAM_COUNTS("SBClientDownload.SignedOrWhitelistedDownload", 1);
+  enum WhitelistType {
+    NO_WHITELIST_MATCH,
+    URL_WHITELIST,
+    SIGNATURE_WHITELIST,
+    WHITELIST_TYPE_MAX
+  };
+
+  static void RecordCountOfWhitelistedDownload(WhitelistType type) {
+    UMA_HISTOGRAM_ENUMERATION("SBClientDownload.WhitelistedDownload",
+                              type,
+                              WHITELIST_TYPE_MAX);
   }
 
   void CheckWhitelists() {
@@ -685,7 +694,7 @@ class DownloadProtectionService::CheckClientDownloadRequest
     // TODO(asanka): This may acquire a lock on the SB DB on the IO thread.
     if (url.is_valid() && database_manager_->MatchDownloadWhitelistUrl(url)) {
       DVLOG(2) << url << " is on the download whitelist.";
-      RecordCountOfSignedOrWhitelistedDownload();
+      RecordCountOfWhitelistedDownload(URL_WHITELIST);
       // TODO(grt): Continue processing without uploading so that
       // ClientDownloadRequest callbacks can be run even for this type of safe
       // download.
@@ -694,10 +703,10 @@ class DownloadProtectionService::CheckClientDownloadRequest
     }
 
     if (signature_info_.trusted()) {
-      RecordCountOfSignedOrWhitelistedDownload();
       for (int i = 0; i < signature_info_.certificate_chain_size(); ++i) {
         if (CertificateChainIsWhitelisted(
                 signature_info_.certificate_chain(i))) {
+          RecordCountOfWhitelistedDownload(SIGNATURE_WHITELIST);
           // TODO(grt): Continue processing without uploading so that
           // ClientDownloadRequest callbacks can be run even for this type of
           // safe download.
@@ -707,17 +716,19 @@ class DownloadProtectionService::CheckClientDownloadRequest
       }
     }
 
+    RecordCountOfWhitelistedDownload(NO_WHITELIST_MATCH);

[mattm, 2016/03/21 20:57:19]

I think this is a little unclear. There are downloads which don't get to this
point in the process and thus don't get recorded in this histogram, so it should
define more precisely what it means in histograms.xml.

[Jialiu Lin, 2016/03/21 21:26:48]

metrics description updated.

+
     if (!pingback_enabled_) {
       PostFinishTask(UNKNOWN, REASON_PING_DISABLED);
       return;
     }
 
-  // The URLFetcher is owned by the UI thread, so post a message to
-  // start the pingback.
-  BrowserThread::PostTask(
-      BrowserThread::UI,
-      FROM_HERE,
-      base::Bind(&CheckClientDownloadRequest::GetTabRedirects, this));
+    // The URLFetcher is owned by the UI thread, so post a message to
+    // start the pingback.
+    BrowserThread::PostTask(
+        BrowserThread::UI,
+        FROM_HERE,
+        base::Bind(&CheckClientDownloadRequest::GetTabRedirects, this));
   }
 
   void GetTabRedirects() {