Add response code to the success callback of ReportSender

chrome/browser/safe_browsing/notification_image_reporter.cc

 // Copyright 2017 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/notification_image_reporter.h"
 
 #include <cmath>
 #include <vector>
 
 #include "base/bind.h"
@@ skipping 25 matching lines @@
 namespace safe_browsing {
 
 namespace {
 
 const size_t kMaxReportsPerDay = 5;
 const base::Feature kNotificationImageReporterFeature{
     "NotificationImageReporterFeature", base::FEATURE_ENABLED_BY_DEFAULT};
 const char kReportChance[] = "ReportChance";
 const char kDefaultMimeType[] = "image/png";
 
-// Passed to ReportSender::Send as an ErrorCallback, so must take a GURL, but it
-// is unused.
-void LogReportResult(const GURL& url, int net_error) {
+// Passed to ReportSender::Send as an ErrorCallback.
+void LogReportErrorCallback(const GURL& url, int net_error) {
   UMA_HISTOGRAM_SPARSE_SLOWLY("SafeBrowsing.NotificationImageReporter.NetError",
                               net_error);
 }
 
+// Passed to ReportSender::Send as an SuccessCallback.
+void LogReportSuccessCallback(int response_code) {

[eroman, 2017/04/25 21:02:39]

Is it intentional that |response_code| is unused? If you are trying to track
down failures you may want to histogram |response_code / 100|, or a sparse
histogram of the exact HTTP code.

[meacer, 2017/04/25 22:19:07]

I intentionally left it unused here, but I think SafeBrowsing wants to use it
later. nparker, is that correct?

I'm going to use the response code in CertificateReportingService and record the
exact code in a sparse histogram in a separate CL.

+  UMA_HISTOGRAM_SPARSE_SLOWLY("SafeBrowsing.NotificationImageReporter.NetError",
+                              net::OK);
+}
+
 }  // namespace
 
 const char NotificationImageReporter::kReportingUploadUrl[] =
     "https://safebrowsing.googleusercontent.com/safebrowsing/clientreport/"
     "notification-image";
 
 NotificationImageReporter::NotificationImageReporter(
     net::URLRequestContext* request_context)
     : NotificationImageReporter(base::MakeUnique<net::ReportSender>(
           request_context,
@@ skipping 146 matching lines @@
   report.mutable_image()->mutable_dimensions()->set_height(dimensions.height());
   if (dimensions != original_dimensions) {
     report.mutable_image()->mutable_original_dimensions()->set_width(
         original_dimensions.width());
     report.mutable_image()->mutable_original_dimensions()->set_height(
         original_dimensions.height());
   }
 
   std::string serialized_report;
   report.SerializeToString(&serialized_report);
-  report_sender_->Send(
-      GURL(kReportingUploadUrl), "application/octet-stream", serialized_report,
-      base::Bind(&LogReportResult, GURL(kReportingUploadUrl), net::OK),
-      base::Bind(&LogReportResult));
+  report_sender_->Send(GURL(kReportingUploadUrl), "application/octet-stream",
+                       serialized_report, base::Bind(&LogReportSuccessCallback),
+                       base::Bind(&LogReportErrorCallback));
   // TODO(johnme): Consider logging bandwidth and/or duration to UMA.
 }
 
 }  // namespace safe_browsing