Add metrics on download deep scanning effectiveness split by trigger

The previous metric in this space,
SafeBrowsingBinaryUploadRequest.AdvancedProtectionScanVerdict
was removed by https://crrev.com/c/2505943, so take this opportunity
to re-implement it in a slightly more robust way, based on the trigger
rather than inspection of the Request.

Change-Id: Ib5418d3cba4eadb110503ca48d81aff7fe2e78aa
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2506572Reviewed-by: Mark Pearson <mpearson@chromium.org>
Reviewed-by: Dominique Fauteux-Chapleau <domfc@chromium.org>
Reviewed-by: Xinghui Lu <xinghuilu@chromium.org>
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Commit-Queue: Daniel Rubery <drubery@chromium.org>
Cr-Commit-Position: refs/heads/master@{#823383}

chrome/browser/safe_browsing/download_protection/deep_scanning_request.cc

@@ -7,6 +7,7 @@
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/callback_forward.h"
+#include "base/metrics/histogram_functions.h"
 #include "base/optional.h"
 #include "base/strings/string_number_conversions.h"
 #include "chrome/browser/browser_process.h"
@@ -151,6 +152,17 @@ EventResult GetEventResult(DownloadCheckResult download_result,
   return EventResult::UNKNOWN;
 }
 
+std::string GetTriggerName(DeepScanningRequest::DeepScanTrigger trigger) {
+  switch (trigger) {
+    case DeepScanningRequest::DeepScanTrigger::TRIGGER_UNKNOWN:
+      return "Unknown";
+    case DeepScanningRequest::DeepScanTrigger::TRIGGER_APP_PROMPT:
+      return "AdvancedProtectionPrompt";
+    case DeepScanningRequest::DeepScanTrigger::TRIGGER_POLICY:
+      return "Policy";
+  }
+}
+
 }  // namespace
 
 /* static */
@@ -220,6 +232,8 @@ void DeepScanningRequest::Start() {
     OnScanComplete(BinaryUploadService::Result::UNKNOWN,
                    enterprise_connectors::ContentAnalysisResponse());
   }
+
+  base::UmaHistogramEnumeration("SBClientDownload.DeepScanTrigger", trigger_);
 }
 
 void DeepScanningRequest::PrepareRequest(BinaryUploadService::Request* request,
@@ -251,6 +265,9 @@ void DeepScanningRequest::OnScanComplete(
   DownloadCheckResult download_result = DownloadCheckResult::UNKNOWN;
   if (result == BinaryUploadService::Result::SUCCESS) {
     ResponseToDownloadCheckResult(response, &download_result);
+    base::UmaHistogramEnumeration(
+        "SBClientDownload.MalwareDeepScanResult." + GetTriggerName(trigger_),
+        download_result);
   } else if (trigger_ == DeepScanTrigger::TRIGGER_APP_PROMPT &&
              MaybeShowDeepScanFailureModalDialog(
                  base::BindOnce(&DeepScanningRequest::Start,

chrome/browser/safe_browsing/download_protection/deep_scanning_request.h

@@ -24,16 +24,20 @@ class DownloadProtectionService;
 class DeepScanningRequest : public download::DownloadItem::Observer {
  public:
   // Enum representing the trigger of the scan request.
+  // These values are persisted to logs. Entries should not be renumbered and
+  // numeric values should never be reused.
   enum class DeepScanTrigger {
     // The trigger is unknown.
-    TRIGGER_UNKNOWN,
+    TRIGGER_UNKNOWN = 0,
 
     // The trigger is the prompt in the download shelf, shown for Advanced
     // Protection users.
-    TRIGGER_APP_PROMPT,
+    TRIGGER_APP_PROMPT = 1,
 
     // The trigger is the enterprise policy.
-    TRIGGER_POLICY,
+    TRIGGER_POLICY = 2,
+
+    kMaxValue = TRIGGER_POLICY,
   };
 
   // Checks the current policies to determine whether files must be uploaded by

chrome/browser/safe_browsing/download_protection/download_protection_util.h

@@ -14,22 +14,25 @@
 
 namespace safe_browsing {
 
+// These values are persisted to logs. Entries should not be renumbered and
+// numeric values should never be reused.
 enum class DownloadCheckResult {
-  UNKNOWN,
-  SAFE,
-  DANGEROUS,
-  UNCOMMON,
-  DANGEROUS_HOST,
-  POTENTIALLY_UNWANTED,
-  WHITELISTED_BY_POLICY,
-  ASYNC_SCANNING,
-  BLOCKED_PASSWORD_PROTECTED,
-  BLOCKED_TOO_LARGE,
-  SENSITIVE_CONTENT_WARNING,
-  SENSITIVE_CONTENT_BLOCK,
-  DEEP_SCANNED_SAFE,
-  PROMPT_FOR_SCANNING,
-  BLOCKED_UNSUPPORTED_FILE_TYPE,
+  UNKNOWN = 0,
+  SAFE = 1,
+  DANGEROUS = 2,
+  UNCOMMON = 3,
+  DANGEROUS_HOST = 4,
+  POTENTIALLY_UNWANTED = 5,
+  WHITELISTED_BY_POLICY = 6,
+  ASYNC_SCANNING = 7,
+  BLOCKED_PASSWORD_PROTECTED = 8,
+  BLOCKED_TOO_LARGE = 9,
+  SENSITIVE_CONTENT_WARNING = 10,
+  SENSITIVE_CONTENT_BLOCK = 11,
+  DEEP_SCANNED_SAFE = 12,
+  PROMPT_FOR_SCANNING = 13,
+  BLOCKED_UNSUPPORTED_FILE_TYPE = 14,
+  kMaxValue = BLOCKED_UNSUPPORTED_FILE_TYPE,
 };
 
 // Enum to keep track why a particular download verdict was chosen.

tools/metrics/histograms/enums.xml

@@ -65165,6 +65165,12 @@ https://www.dmtf.org/sites/default/files/standards/documents/DSP0134_2.7.1.pdf
   <int value="3" label="Skipped: Unsupported scheme"/>
 </enum>
 
+<enum name="SBDeepScanTriggers">
+  <int value="0" label="Unknown"/>
+  <int value="1" label="Advanced Protection Prompt"/>
+  <int value="2" label="Enterprise policy"/>
+</enum>
+
 <enum name="SBDownloadFeedbackUploadResult">
   <int value="0" label="UPLOAD_SUCCESS"/>
   <int value="1" label="UPLOAD_CANCELLED"/>

tools/metrics/histograms/histograms_xml/sb_client/histograms.xml

@@ -47,6 +47,16 @@ reviews. Googlers can read more about this at go/gwsq-gerrit.
   </summary>
 </histogram>
 
+<histogram name="SBClientDownload.DeepScanTrigger" enum="SBDeepScanTriggers"
+    expires_after="2021-04-28">
+  <owner>drubery@chromium.org</owner>
+  <owner>chrome-safebrowsing-alerts@google.com</owner>
+  <summary>
+    Records the reason each file was uploaded to Safe Browsing for deep
+    scanning. This is logged on every deep scan.
+  </summary>
+</histogram>
+
 <histogram name="SBClientDownload.DmgFileSuccessByType"
     enum="SBClientDownloadExtensions" expires_after="M90">
   <owner>drubery@chromium.org</owner>
@@ -179,6 +189,24 @@ reviews. Googlers can read more about this at go/gwsq-gerrit.
   </summary>
 </histogram>
 
+<histogram name="SBClientDownload.MalwareDeepScanResult.{trigger}"
+    enum="SBClientDownloadCheckResult" expires_after="2021-04-28">
+  <owner>drubery@chromium.org</owner>
+  <owner>chrome-safebrowsing-alerts@google.com</owner>
+  <summary>
+    Records the result of a malware deep scan, split by the reason the file was
+    uploaded for scanning. This is logged only for succesful scans. The overall
+    rate of successful scans is logged in
+    SafeBrowsingBinaryUploadRequest.MalwareResult.
+  </summary>
+  <token key="trigger">
+    <variant name="AdvancedProtectionPrompt"
+        summary="Advanced Protection user selected 'Scan'"/>
+    <variant name="Policy" summary="Triggered by enterprise policy"/>
+    <variant name="Unknown" summary="Unknown trigger"/>
+  </token>
+</histogram>
+
 <histogram name="SBClientDownload.ZipFileSuccess" enum="BooleanSuccess"
     expires_after="M83">
   <owner>drubery@chromium.org</owner>