Index: components/favicon/core/favicon_handler.cc |
diff --git a/components/favicon/core/favicon_handler.cc b/components/favicon/core/favicon_handler.cc |
index 64c8961ae23ad760d06e826ee6e04bf06b7f1b2d..79f35f0622513060931374b18488cf052049e147 100644 |
--- a/components/favicon/core/favicon_handler.cc |
+++ b/components/favicon/core/favicon_handler.cc |
@@ -12,6 +12,7 @@ |
#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" |
@@ -59,6 +60,25 @@ bool IsValid(const favicon_base::FaviconRawBitmapResult& bitmap_result) { |
return bitmap_result.is_valid(); |
} |
+void RecordAttemptsForHandlerType( |
pkotwicz
2017/04/12 02:48:20
Maybe rename this method to: RecordDownloadAttempt
fhorschig
2017/04/12 08:32:39
Done.
|
+ FaviconDriverObserver::NotificationIconType handler_type, |
+ int attempts) { |
+ switch (handler_type) { |
+ case FaviconDriverObserver::NON_TOUCH_16_DIP: |
+ UMA_HISTOGRAM_COUNTS_100("Favicons.DownloadAttempts.Favicons", attempts); |
+ return; |
+ case FaviconDriverObserver::NON_TOUCH_LARGEST: |
+ UMA_HISTOGRAM_COUNTS_100("Favicons.DownloadAttempts.LargeIcons", |
+ attempts); |
+ return; |
+ case FaviconDriverObserver::TOUCH_LARGEST: |
+ UMA_HISTOGRAM_COUNTS_100("Favicons.DownloadAttempts.TouchIcons", |
+ attempts); |
+ return; |
+ } |
+ NOTREACHED(); |
+} |
+ |
// Returns true if |bitmap_results| is non-empty and: |
// - At least one of the bitmaps in |bitmap_results| is expired |
// OR |
@@ -167,6 +187,7 @@ FaviconHandler::FaviconHandler( |
notification_icon_type_(favicon_base::INVALID_ICON), |
service_(service), |
delegate_(delegate), |
+ num_download_requests_(0), |
current_candidate_index_(0u) { |
DCHECK(delegate_); |
} |
@@ -199,6 +220,7 @@ void FaviconHandler::FetchFavicon(const GURL& url) { |
candidates_.clear(); |
notification_icon_url_ = GURL(); |
notification_icon_type_ = favicon_base::INVALID_ICON; |
+ num_download_requests_ = 0; |
current_candidate_index_ = 0u; |
best_favicon_ = DownloadedFavicon(); |
@@ -311,6 +333,7 @@ void FaviconHandler::OnUpdateFaviconURL( |
download_request_.Cancel(); |
candidates_ = std::move(sorted_candidates); |
+ num_download_requests_ = 0; |
current_candidate_index_ = 0u; |
best_favicon_ = DownloadedFavicon(); |
@@ -399,6 +422,9 @@ void FaviconHandler::OnDidDownloadFavicon( |
++current_candidate_index_; |
DownloadCurrentCandidateOrAskFaviconService(); |
} else { |
+ // OnDidDownloadFavicon can only be called after invoking a request, so |
+ // |num_download_requests_| can never be 0. |
pkotwicz
2017/04/12 02:48:21
Nit: OnDidDownloadFavicon -> OnDidDownloadFavicon(
fhorschig
2017/04/12 08:32:39
Done.
|
+ RecordAttemptsForHandlerType(handler_type_, num_download_requests_); |
// We have either found the ideal candidate or run out of candidates. |
if (best_favicon_.candidate.icon_type != favicon_base::INVALID_ICON) { |
// No more icons to request, set the favicon from the candidate. |
@@ -407,6 +433,7 @@ void FaviconHandler::OnDidDownloadFavicon( |
} |
// Clear download related state. |
current_candidate_index_ = candidates_.size(); |
+ num_download_requests_ = 0; |
best_favicon_ = DownloadedFavicon(); |
} |
} |
@@ -499,6 +526,9 @@ void FaviconHandler::OnFaviconData(const std::vector< |
favicon_bitmap_results); |
if (has_valid_result) { |
+ if (num_download_requests_ > 0) { |
+ RecordAttemptsForHandlerType(handler_type_, current_candidate_index_); |
pkotwicz
2017/04/12 02:48:21
Don't you want to pass |num_download_requests_| he
fhorschig
2017/04/12 08:32:39
Yes, done.
|
+ } |
// There is a valid favicon. Notify any observers. It is useful to notify |
// the observers even if the favicon is expired or incomplete (incorrect |
// size) because temporarily showing the user an expired favicon or |
@@ -531,6 +561,7 @@ void FaviconHandler::ScheduleDownload(const GURL& image_url, |
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 |