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

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

Issue 2710953002: Add metrics for quantity and type of Favicon candidates (Closed)
Patch Set: Use UMA_HISTOGRAM_COUNTS_100 and update histograms description Created 3 years, 10 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
Index: components/favicon/core/favicon_driver_impl.cc
diff --git a/components/favicon/core/favicon_driver_impl.cc b/components/favicon/core/favicon_driver_impl.cc
index 2aecafd35a1afc42dd4fdb252f426db2eec242dc..4a38ec3a161a693ea3b10dd9bd61573caded2d88 100644
--- a/components/favicon/core/favicon_driver_impl.cc
+++ b/components/favicon/core/favicon_driver_impl.cc
@@ -5,12 +5,14 @@
#include "components/favicon/core/favicon_driver_impl.h"
#include "base/logging.h"
+#include "base/metrics/histogram_macros.h"
#include "base/strings/string_util.h"
#include "build/build_config.h"
#include "components/bookmarks/browser/bookmark_model.h"
#include "components/favicon/core/favicon_driver_observer.h"
#include "components/favicon/core/favicon_handler.h"
#include "components/favicon/core/favicon_service.h"
+#include "components/favicon/core/favicon_url.h"
#include "components/history/core/browser/history_service.h"
namespace favicon {
@@ -22,6 +24,26 @@ const bool kEnableTouchIcon = true;
const bool kEnableTouchIcon = false;
#endif
+void RecordCandidateMetrics(const std::vector<FaviconURL>& candidates) {
+ UMA_HISTOGRAM_COUNTS_100("Favicons.CandidatesCount", candidates.size());
pkotwicz 2017/02/27 18:54:57 Nit: Put this metric together with the other metri
fhorschig 2017/03/01 20:56:01 Done.
+ size_t with_defined_touch_icons = 0;
+ size_t with_defined_sizes = 0;
+ for (const auto& candidate : candidates) {
+ if (!candidate.icon_sizes.empty()) {
+ with_defined_sizes++;
+ }
+ if (candidate.icon_type &
+ (favicon_base::IconType::TOUCH_ICON |
+ favicon_base::IconType::TOUCH_PRECOMPOSED_ICON)) {
+ with_defined_touch_icons++;
+ }
+ }
+ UMA_HISTOGRAM_COUNTS_100("Favicons.CandidatesWithDefinedSizesCount",
+ with_defined_sizes);
+ UMA_HISTOGRAM_COUNTS_100("Favicons.CandidatesWithLargeIconsCount",
pkotwicz 2017/02/27 18:54:57 Nit: Maybe rename to CandidatesWithTouchIconsCount
fhorschig 2017/03/01 20:56:01 Done.
+ with_defined_touch_icons);
+}
+
} // namespace
FaviconDriverImpl::FaviconDriverImpl(FaviconService* favicon_service,
@@ -98,6 +120,7 @@ void FaviconDriverImpl::OnUpdateFaviconURL(
const GURL& page_url,
const std::vector<FaviconURL>& candidates) {
DCHECK(!candidates.empty());
+ RecordCandidateMetrics(candidates);
favicon_handler_->OnUpdateFaviconURL(page_url, candidates);
if (touch_icon_handler_.get())
touch_icon_handler_->OnUpdateFaviconURL(page_url, candidates);

Powered by Google App Engine
This is Rietveld 408576698