Add attempt count 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 <vector>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/memory/ref_counted_memory.h"
+#include "base/metrics/histogram_macros.h"
 #include "build/build_config.h"
 #include "components/favicon/core/favicon_service.h"
 #include "components/favicon_base/favicon_util.h"
 #include "components/favicon_base/select_favicon_frames.h"
 #include "skia/ext/image_operations.h"
 #include "ui/gfx/codec/png_codec.h"
 #include "ui/gfx/image/image_skia.h"
 #include "ui/gfx/image/image_util.h"
 
 namespace favicon {
@@ skipping 178 matching lines @@
       got_favicon_from_history_(false),
       initial_history_result_expired_or_incomplete_(false),
       redownload_icons_(false),
       icon_types_(FaviconHandler::GetIconTypesFromHandlerType(handler_type)),
       download_largest_icon_(
           handler_type == FaviconDriverObserver::NON_TOUCH_LARGEST ||
           handler_type == FaviconDriverObserver::TOUCH_LARGEST),
       notification_icon_type_(favicon_base::INVALID_ICON),
       service_(service),
       delegate_(delegate),
+      request_attempts_(0),
       current_candidate_index_(0u) {
   DCHECK(delegate_);
 }
 
 FaviconHandler::~FaviconHandler() {
 }
 
 // static
 int FaviconHandler::GetIconTypesFromHandlerType(
     FaviconDriverObserver::NotificationIconType handler_type) {
   switch (handler_type) {
     case FaviconDriverObserver::NON_TOUCH_16_DIP:
     case FaviconDriverObserver::NON_TOUCH_LARGEST:
       return favicon_base::FAVICON;
     case FaviconDriverObserver::TOUCH_LARGEST:
       return favicon_base::TOUCH_ICON | favicon_base::TOUCH_PRECOMPOSED_ICON;
   }
   return 0;
 }
 
 void FaviconHandler::FetchFavicon(const GURL& url) {
   cancelable_task_tracker_.TryCancelAll();
 
   url_ = url;
 
   initial_history_result_expired_or_incomplete_ = false;
   redownload_icons_ = false;
   got_favicon_from_history_ = false;
   download_request_.Cancel();
+  request_attempts_ = 0;
   image_urls_.clear();
   notification_icon_url_ = GURL();
   notification_icon_type_ = favicon_base::INVALID_ICON;
   current_candidate_index_ = 0u;
   best_favicon_candidate_ = FaviconCandidate();
 
   // Request the favicon from the history service. In parallel to this the
   // renderer is going to notify us (well WebContents) when the favicon url is
   // available.
   if (service_) {
@@ skipping 114 matching lines @@
 
   // Ignore FaviconURL::icon_sizes because FaviconURL::icon_sizes is not stored
   // in the history database.
   if (image_urls_.size() == pruned_candidates.size() &&
       std::equal(pruned_candidates.begin(), pruned_candidates.end(),
                  image_urls_.begin(), FaviconURLsEqualIgnoringSizes)) {
     return;
   }
 
   download_request_.Cancel();
+  request_attempts_ = 0;
   image_urls_ = pruned_candidates;
   current_candidate_index_ = 0u;
   best_favicon_candidate_ = FaviconCandidate();
 
   // TODO(davemoore) Should clear on empty url. Currently we ignore it.
   // This appears to be what FF does as well.
   if (current_candidate() && got_favicon_from_history_)
     OnGotInitialHistoryDataAndIconURLCandidates();
 }
 
@@ skipping 81 matching lines @@
           !UpdateFaviconCandidate(image_url, image, score, icon_type);
     }
   }
 
   if (request_next_icon && current_candidate_index_ + 1 < image_urls_.size()) {
     // Process the next candidate.
     ++current_candidate_index_;
     DownloadCurrentCandidateOrAskFaviconService();
   } else {
     // We have either found the ideal candidate or run out of candidates.
+    if (handler_type_ == FaviconDriverObserver::TOUCH_LARGEST) {
+      UMA_HISTOGRAM_COUNTS_100("Favicons.LargeIconDownloadAttempts",

[jkrcal, 2017/03/30 07:53:28]

Can we actually have three histogram per each
FaviconDriverObserver::NotificationIconType?
(can you also have the naming of the histograms consistent with the naming of
the enum?)

[fhorschig, 2017/04/03 09:56:49]

I added a third historgram but it's slightly different:
If we ever add new type to the enum without new metric, we would lose the total
number of attempts. If we have the overall count and counts for Large and Touch
icon downloads, we can calculate the number of attempts for the small favicons.

We could also add a fourth type for small icons, but I don't think it's
important
enough to record duplicate metrics. WDYT?

+                               request_attempts_);
+    } else {
+      UMA_HISTOGRAM_COUNTS_100("Favicons.DownloadAttempts", request_attempts_);
+    }
     if (best_favicon_candidate_.icon_type != favicon_base::INVALID_ICON) {
       // No more icons to request, set the favicon from the candidate.
       SetFavicon(best_favicon_candidate_.image_url,
                  best_favicon_candidate_.image,
                  best_favicon_candidate_.icon_type);
     }
     // Clear download related state.
     current_candidate_index_ = image_urls_.size();
+    request_attempts_ = 0;
     best_favicon_candidate_ = FaviconCandidate();
   }
 }
 
 bool FaviconHandler::HasPendingTasksForTest() {
   return !download_request_.IsCancelled() ||
          cancelable_task_tracker_.HasTrackedTasks();
 }
 
 bool FaviconHandler::ShouldSaveFavicon() {
@@ skipping 97 matching lines @@
   // 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;
     OnDidDownloadFavicon(icon_type, 0, 0, image_url, std::vector<SkBitmap>(),
                          std::vector<gfx::Size>());
     return;
   }
   download_request_.Reset(base::Bind(&FaviconHandler::OnDidDownloadFavicon,
                                      base::Unretained(this), icon_type));
+  ++request_attempts_;
   // 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(icon_type), download_request_.callback());
   DCHECK_NE(download_id, 0);
 }
 
 }  // namespace favicon