Make unauthorized users finish request with special result.

FinishRequest() is called with UNAUTHORIZED, which lets the user
upload/download the file but doesn't record UMA metrics.

Bug: 1028133
Change-Id: I66405a78e0d33997f1a12b322ca5a8bb5250ada0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1947362Reviewed-by: Daniel Rubery <drubery@chromium.org>
Commit-Queue: Dominique Fauteux-Chapleau <domfc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#721537}

chrome/browser/safe_browsing/cloud_content_scanning/binary_upload_service.cc

@@ -75,8 +75,12 @@ void BinaryUploadService::MaybeUploadForDeepScanningCallback(
     std::unique_ptr<BinaryUploadService::Request> request,
     bool authorized) {
   // Ignore the request if the browser cannot upload data.
-  if (!authorized)
+  if (!authorized) {
+    // TODO(crbug/1028133): Add extra logic to handle UX for non-authorized
+    // users.
+    request->FinishRequest(Result::UNAUTHORIZED, DeepScanningClientResponse());
     return;
+  }
   UploadForDeepScanning(std::move(request));
 }
 

chrome/browser/safe_browsing/cloud_content_scanning/binary_upload_service.h

@@ -65,7 +65,10 @@ class BinaryUploadService {
     // The BinaryUploadService failed to get an InstanceID token.
     FAILED_TO_GET_TOKEN = 5,
 
-    kMaxValue = FAILED_TO_GET_TOKEN,
+    // The user is unauthorized to make the request.
+    UNAUTHORIZED = 6,
+
+    kMaxValue = UNAUTHORIZED,
   };
 
   // Callbacks used to pass along the results of scanning. The response protos

chrome/browser/safe_browsing/cloud_content_scanning/binary_upload_service_unittest.cc

@@ -379,13 +379,17 @@ TEST_F(BinaryUploadServiceTest, OnUnauthorized) {
   simulated_response.mutable_malware_scan_verdict();
   ExpectNetworkResponse(true, simulated_response);
 
+  EXPECT_EQ(scanning_result, BinaryUploadService::Result::UNKNOWN);
+
   UploadForDeepScanning(std::move(request), /*authorized=*/false);
 
-  EXPECT_EQ(scanning_result, BinaryUploadService::Result::UNKNOWN);
+  // The result is set synchronously on unauthorized requests, so it is
+  // UNAUTHORIZED before and after waiting.
+  EXPECT_EQ(scanning_result, BinaryUploadService::Result::UNAUTHORIZED);
 
   content::RunAllTasksUntilIdle();
 
-  EXPECT_EQ(scanning_result, BinaryUploadService::Result::UNKNOWN);
+  EXPECT_EQ(scanning_result, BinaryUploadService::Result::UNAUTHORIZED);
 }
 
 TEST_F(BinaryUploadServiceTest, OnGetSynchronousResponse) {

chrome/browser/safe_browsing/cloud_content_scanning/deep_scanning_dialog_delegate.cc

@@ -480,9 +480,9 @@ void DeepScanningDialogDelegate::CompleteFileRequestCallback(
                      MalwareDeepScanningVerdict::MALWARE;
   }
 
-  bool file_complies =
-      (result == BinaryUploadService::Result::SUCCESS) && dlp_ok && malware_ok;
-
+  bool file_complies = (result == BinaryUploadService::Result::SUCCESS ||
+                        result == BinaryUploadService::Result::UNAUTHORIZED) &&
+                       dlp_ok && malware_ok;
   result_.paths_results[index] = file_complies;
 
   ++file_result_count_;

chrome/browser/safe_browsing/cloud_content_scanning/deep_scanning_utils.cc

@@ -138,6 +138,9 @@ void RecordDeepScanMetrics(DeepScanAccessPoint access_point,
     case BinaryUploadService::Result::UNKNOWN:
       result_value = "Unknown";
       break;
+    case BinaryUploadService::Result::UNAUTHORIZED:
+      // Don't record UMA metrics for this result.
+      return;
   }
 
   // Update |success| so non-SUCCESS results don't log the bytes/sec metric.

chrome/browser/safe_browsing/cloud_content_scanning/deep_scanning_utils_unittest.cc

@@ -20,7 +20,8 @@ constexpr BinaryUploadService::Result kAllBinaryUploadServiceResults[]{
     BinaryUploadService::Result::UPLOAD_FAILURE,
     BinaryUploadService::Result::TIMEOUT,
     BinaryUploadService::Result::FILE_TOO_LARGE,
-    BinaryUploadService::Result::FAILED_TO_GET_TOKEN};
+    BinaryUploadService::Result::FAILED_TO_GET_TOKEN,
+    BinaryUploadService::Result::UNAUTHORIZED};
 
 constexpr int64_t kTotalBytes = 1000;
 
@@ -53,6 +54,8 @@ std::string ResultToString(const BinaryUploadService::Result& result,
     case BinaryUploadService::Result::UNKNOWN:
       result_value = "Unknown";
       break;
+    case BinaryUploadService::Result::UNAUTHORIZED:
+      return "";
   }
   return result_value;
 }
@@ -98,25 +101,32 @@ INSTANTIATE_TEST_SUITE_P(
 TEST_P(DeepScanningUtilsUMATest, SuccessfulScanVerdicts) {
   RecordDeepScanMetrics(access_point(), kDuration, kTotalBytes, result(),
                         DeepScanningClientResponse());
-  // We expect at least 2 histograms (<access-point>.Duration and
-  // <access-point>.<result>.Duration), but only expect a third histogram in the
-  // success case (bytes/seconds).
-  uint64_t expected_histograms = success() ? 3u : 2u;
-  EXPECT_EQ(
-      expected_histograms,
-      histograms().GetTotalCountsForPrefix("SafeBrowsing.DeepScan.").size());
-  if (success()) {
-    histograms().ExpectUniqueSample(
-        "SafeBrowsing.DeepScan." + access_point_string() + ".BytesPerSeconds",
-        kTotalBytes / kDuration.InSeconds(), 1);
+
+  if (result() == BinaryUploadService::Result::UNAUTHORIZED) {
+    EXPECT_EQ(
+        0u,
+        histograms().GetTotalCountsForPrefix("SafeBrowsing.DeepScan.").size());
+  } else {
+    // We expect at least 2 histograms (<access-point>.Duration and
+    // <access-point>.<result>.Duration), but only expect a third histogram in
+    // the success case (bytes/seconds).
+    uint64_t expected_histograms = success() ? 3u : 2u;
+    EXPECT_EQ(
+        expected_histograms,
+        histograms().GetTotalCountsForPrefix("SafeBrowsing.DeepScan.").size());
+    if (success()) {
+      histograms().ExpectUniqueSample(
+          "SafeBrowsing.DeepScan." + access_point_string() + ".BytesPerSeconds",
+          kTotalBytes / kDuration.InSeconds(), 1);
+    }
+    histograms().ExpectTimeBucketCount(
+        "SafeBrowsing.DeepScan." + access_point_string() + ".Duration",
+        kDuration, 1);
+    histograms().ExpectTimeBucketCount("SafeBrowsing.DeepScan." +
+                                           access_point_string() + "." +
+                                           result_value(true) + ".Duration",
+                                       kDuration, 1);
   }
-  histograms().ExpectTimeBucketCount(
-      "SafeBrowsing.DeepScan." + access_point_string() + ".Duration", kDuration,
-      1);
-  histograms().ExpectTimeBucketCount("SafeBrowsing.DeepScan." +
-                                         access_point_string() + "." +
-                                         result_value(true) + ".Duration",
-                                     kDuration, 1);
 }
 
 TEST_P(DeepScanningUtilsUMATest, UnsuccessfulDlpScanVerdict) {
@@ -128,16 +138,22 @@ TEST_P(DeepScanningUtilsUMATest, UnsuccessfulDlpScanVerdict) {
   RecordDeepScanMetrics(access_point(), kDuration, kTotalBytes, result(),
                         response);
 
-  EXPECT_EQ(
-      2u,
-      histograms().GetTotalCountsForPrefix("SafeBrowsing.DeepScan.").size());
-  histograms().ExpectTimeBucketCount(
-      "SafeBrowsing.DeepScan." + access_point_string() + ".Duration", kDuration,
-      1);
-  histograms().ExpectTimeBucketCount("SafeBrowsing.DeepScan." +
-                                         access_point_string() + "." +
-                                         result_value(false) + ".Duration",
-                                     kDuration, 1);
+  if (result() == BinaryUploadService::Result::UNAUTHORIZED) {
+    EXPECT_EQ(
+        0u,
+        histograms().GetTotalCountsForPrefix("SafeBrowsing.DeepScan.").size());
+  } else {
+    EXPECT_EQ(
+        2u,
+        histograms().GetTotalCountsForPrefix("SafeBrowsing.DeepScan.").size());
+    histograms().ExpectTimeBucketCount(
+        "SafeBrowsing.DeepScan." + access_point_string() + ".Duration",
+        kDuration, 1);
+    histograms().ExpectTimeBucketCount("SafeBrowsing.DeepScan." +
+                                           access_point_string() + "." +
+                                           result_value(false) + ".Duration",
+                                       kDuration, 1);
+  }
 }
 
 TEST_P(DeepScanningUtilsUMATest, UnsuccessfulMalwareScanVerdict) {
@@ -149,16 +165,22 @@ TEST_P(DeepScanningUtilsUMATest, UnsuccessfulMalwareScanVerdict) {
   RecordDeepScanMetrics(access_point(), kDuration, kTotalBytes, result(),
                         response);
 
-  EXPECT_EQ(
-      2u,
-      histograms().GetTotalCountsForPrefix("SafeBrowsing.DeepScan.").size());
-  histograms().ExpectTimeBucketCount(
-      "SafeBrowsing.DeepScan." + access_point_string() + ".Duration", kDuration,
-      1);
-  histograms().ExpectTimeBucketCount("SafeBrowsing.DeepScan." +
-                                         access_point_string() + "." +
-                                         result_value(false) + ".Duration",
-                                     kDuration, 1);
+  if (result() == BinaryUploadService::Result::UNAUTHORIZED) {
+    EXPECT_EQ(
+        0u,
+        histograms().GetTotalCountsForPrefix("SafeBrowsing.DeepScan.").size());
+  } else {
+    EXPECT_EQ(
+        2u,
+        histograms().GetTotalCountsForPrefix("SafeBrowsing.DeepScan.").size());
+    histograms().ExpectTimeBucketCount(
+        "SafeBrowsing.DeepScan." + access_point_string() + ".Duration",
+        kDuration, 1);
+    histograms().ExpectTimeBucketCount("SafeBrowsing.DeepScan." +
+                                           access_point_string() + "." +
+                                           result_value(false) + ".Duration",
+                                       kDuration, 1);
+  }
 }
 
 TEST_P(DeepScanningUtilsUMATest, BypassScanVerdict) {