Add DownloadStatus metric to FaviconHandler

components/favicon/core/favicon_handler.cc

 // Copyright (c) 2012 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 "components/favicon/core/favicon_handler.h"
 
 #include <algorithm>
 #include <cmath>
 #include <utility>
 #include <vector>
@@ skipping 364 matching lines @@
     int id,
     int http_status_code,
     const GURL& image_url,
     const std::vector<SkBitmap>& bitmaps,
     const std::vector<gfx::Size>& original_bitmap_sizes) {
   // Mark download as finished.
   download_request_.Cancel();
 
   if (bitmaps.empty() && http_status_code == 404) {
     DVLOG(1) << "Failed to Download Favicon:" << image_url;
+    UMA_HISTOGRAM_COUNTS_100("Favicons.DownloadOutcome",

[Ilya Sherman, 2017/04/12 23:22:13]

Could this be UMA_HISTOGRAM_SPARSE_SLOWLY or UMA_HISTOGRAM_ENUMERATION? 
UMA_HISTOGRAM_COUNTS_100 does not seem appropriate for this purpose.

[fhorschig, 2017/04/13 15:14:05]

Done.

+                             DownloadOutcome::FAILED);
     if (service_)
       service_->UnableToDownloadFavicon(image_url);
   }
 
   bool request_next_icon = true;
   if (!bitmaps.empty()) {
+    UMA_HISTOGRAM_COUNTS_100("Favicons.DownloadOutcome",
+                             DownloadOutcome::SUCCEEDED);

[Ilya Sherman, 2017/04/12 23:22:13]

Please create a small wrapper function for emitting to this histogram, so that
you do not need to repeat the macro invocation several times.  The macro expands
to a fair bit of code, so it's nice to help the compiler avoid doing that
multiple times.

[fhorschig, 2017/04/13 15:14:05]

Done, thanks for the explanation!

     float score = 0.0f;
     gfx::ImageSkia image_skia;
     if (download_largest_icon_) {
       std::vector<size_t> best_indices;
       SelectFaviconFrameIndices(original_bitmap_sizes,
                                 GetDesiredPixelSizes(handler_type_),
                                 &best_indices, &score);
       DCHECK_EQ(1U, best_indices.size());
       image_skia =
           gfx::ImageSkia::CreateFrom1xBitmap(bitmaps[best_indices.front()]);
@@ skipping 149 matching lines @@
   }
 }
 
 void FaviconHandler::ScheduleDownload(const GURL& image_url,
                                       favicon_base::IconType icon_type) {
   DCHECK(image_url.is_valid());
   // Note that CancelableCallback starts cancelled.
   DCHECK(download_request_.IsCancelled()) << "More than one ongoing download";
   if (service_ && service_->WasUnableToDownloadFavicon(image_url)) {
     DVLOG(1) << "Skip Failed FavIcon: " << image_url;
+    UMA_HISTOGRAM_COUNTS_100("Favicons.DownloadOutcome",
+                             DownloadOutcome::SKIPPED);
     OnDidDownloadFavicon(icon_type, 0, 0, image_url, std::vector<SkBitmap>(),
                          std::vector<gfx::Size>());
     return;
   }
   ++num_download_requests_;
   download_request_.Reset(base::Bind(&FaviconHandler::OnDidDownloadFavicon,
                                      base::Unretained(this), icon_type));
   // A max bitmap size is specified to avoid receiving huge bitmaps in
   // OnDidDownloadFavicon(). See FaviconDriver::StartDownload()
   // for more details about the max bitmap size.
   const int download_id =
       delegate_->DownloadImage(image_url, GetMaximalIconSize(handler_type_),
                                download_request_.callback());
   DCHECK_NE(download_id, 0);
 }
 
 }  // namespace favicon