Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1759)

Unified Diff: components/favicon/core/large_icon_service.cc

Issue 2809783003: [LargeIconService] Report size of favicon for fallback style to UMA (Closed)
Patch Set: Ilya's comment #3 Created 3 years, 8 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | components/favicon/core/large_icon_service_unittest.cc » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: components/favicon/core/large_icon_service.cc
diff --git a/components/favicon/core/large_icon_service.cc b/components/favicon/core/large_icon_service.cc
index c202c244ff784f4bb1634ee2b80da8a9e69e19b7..7fcc9dac9dece281091e5386f88445a411ca22c8 100644
--- a/components/favicon/core/large_icon_service.cc
+++ b/components/favicon/core/large_icon_service.cc
@@ -117,10 +117,20 @@ void ProcessIconOnBackgroundThread(
return;
*fallback_icon_style = favicon_base::FallbackIconStyle();
+ int fallback_icon_size = 0;
if (db_result.is_valid()) {
favicon_base::SetDominantColorAsBackground(db_result.bitmap_data,
fallback_icon_style);
+ // The size must be positive, we cap to 128 to avoid the sparse histogram
+ // to explode (having too many different values, server-side). Size 128
+ // already indicates that there is a problem in the code, 128 px _should_ be
+ // enough in all current UI surfaces.
+ fallback_icon_size = db_result.pixel_size.width();
+ DCHECK_GT(fallback_icon_size, 0);
+ fallback_icon_size = std::min(fallback_icon_size, 128);
}
+ UMA_HISTOGRAM_SPARSE_SLOWLY("Favicons.LargeIconService.FallbackSize",
+ fallback_icon_size);
}
// Processes the bitmap data returned from the FaviconService as part of a
« no previous file with comments | « no previous file | components/favicon/core/large_icon_service_unittest.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698