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..34795ef8447d9d96ac82761899b298374b99972b 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( |
+ FaviconDriverObserver::NotificationIconType handler_type, |
+ int attempts) { |
+ // The name of the histogram must not be a variable. |
+ switch (handler_type) { |
+ case FaviconDriverObserver::NON_TOUCH_16_DIP: |
+ UMA_HISTOGRAM_COUNTS_100("Favicons.FaviconDownloadAttempts", attempts); |
+ return; |
+ case FaviconDriverObserver::NON_TOUCH_LARGEST: |
+ UMA_HISTOGRAM_COUNTS_100("Favicons.LargeIconDownloadAttempts", attempts); |
+ return; |
+ case FaviconDriverObserver::TOUCH_LARGEST: |
+ UMA_HISTOGRAM_COUNTS_100("Favicons.TouchIconDownloadAttempts", attempts); |
+ return; |
pkotwicz
2017/04/04 13:04:37
Can we use the same histogram for Favicons.LargeIc
fhorschig
2017/04/10 09:55:50
Done.
fhorschig
2017/04/10 13:59:41
I was informed that we might have a large delay on
|
+ } |
+ NOTREACHED(); |
+ return; |
pkotwicz
2017/04/04 13:04:37
Nit: The return here is redundant
fhorschig
2017/04/10 09:55:50
Gone.
|
+} |
+ |
// 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), |
+ request_attempts_(0), |
current_candidate_index_(0u) { |
DCHECK(delegate_); |
} |
@@ -197,6 +218,7 @@ void FaviconHandler::FetchFavicon(const GURL& url) { |
got_favicon_from_history_ = false; |
download_request_.Cancel(); |
candidates_.clear(); |
+ request_attempts_ = 0; |
notification_icon_url_ = GURL(); |
notification_icon_type_ = favicon_base::INVALID_ICON; |
current_candidate_index_ = 0u; |
@@ -311,6 +333,7 @@ void FaviconHandler::OnUpdateFaviconURL( |
download_request_.Cancel(); |
candidates_ = std::move(sorted_candidates); |
+ request_attempts_ = 0; |
current_candidate_index_ = 0u; |
best_favicon_ = DownloadedFavicon(); |
@@ -399,6 +422,7 @@ void FaviconHandler::OnDidDownloadFavicon( |
++current_candidate_index_; |
DownloadCurrentCandidateOrAskFaviconService(); |
} else { |
+ RecordAttemptsForHandlerType(handler_type_, request_attempts_); |
pkotwicz
2017/04/04 13:04:38
Can you store |current_candidate_index_ + 1| inste
fhorschig
2017/04/10 09:55:50
Done. This captures 404s as attempts as well.
|
// 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 +431,7 @@ void FaviconHandler::OnDidDownloadFavicon( |
} |
// Clear download related state. |
current_candidate_index_ = candidates_.size(); |
+ request_attempts_ = 0; |
best_favicon_ = DownloadedFavicon(); |
} |
} |
@@ -533,6 +558,7 @@ void FaviconHandler::ScheduleDownload(const GURL& image_url, |
} |
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. |