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

Side by Side 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 unified diff | Download patch
OLDNEW
1 // Copyright 2015 The Chromium Authors. All rights reserved. 1 // Copyright 2015 The Chromium Authors. All rights reserved.
2 // Use of this source code is governed by a BSD-style license that can be 2 // Use of this source code is governed by a BSD-style license that can be
3 // found in the LICENSE file. 3 // found in the LICENSE file.
4 4
5 #include "components/favicon/core/favicon_driver_impl.h" 5 #include "components/favicon/core/favicon_driver_impl.h"
6 6
7 #include "base/logging.h" 7 #include "base/logging.h"
8 #include "base/metrics/histogram_macros.h"
8 #include "base/strings/string_util.h" 9 #include "base/strings/string_util.h"
9 #include "build/build_config.h" 10 #include "build/build_config.h"
10 #include "components/bookmarks/browser/bookmark_model.h" 11 #include "components/bookmarks/browser/bookmark_model.h"
11 #include "components/favicon/core/favicon_driver_observer.h" 12 #include "components/favicon/core/favicon_driver_observer.h"
12 #include "components/favicon/core/favicon_handler.h" 13 #include "components/favicon/core/favicon_handler.h"
13 #include "components/favicon/core/favicon_service.h" 14 #include "components/favicon/core/favicon_service.h"
15 #include "components/favicon/core/favicon_url.h"
14 #include "components/history/core/browser/history_service.h" 16 #include "components/history/core/browser/history_service.h"
15 17
16 namespace favicon { 18 namespace favicon {
17 namespace { 19 namespace {
18 20
19 #if defined(OS_ANDROID) || defined(OS_IOS) 21 #if defined(OS_ANDROID) || defined(OS_IOS)
20 const bool kEnableTouchIcon = true; 22 const bool kEnableTouchIcon = true;
21 #else 23 #else
22 const bool kEnableTouchIcon = false; 24 const bool kEnableTouchIcon = false;
23 #endif 25 #endif
24 26
27 void RecordCandidateMetrics(const std::vector<FaviconURL>& candidates) {
28 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.
29 size_t with_defined_touch_icons = 0;
30 size_t with_defined_sizes = 0;
31 for (const auto& candidate : candidates) {
32 if (!candidate.icon_sizes.empty()) {
33 with_defined_sizes++;
34 }
35 if (candidate.icon_type &
36 (favicon_base::IconType::TOUCH_ICON |
37 favicon_base::IconType::TOUCH_PRECOMPOSED_ICON)) {
38 with_defined_touch_icons++;
39 }
40 }
41 UMA_HISTOGRAM_COUNTS_100("Favicons.CandidatesWithDefinedSizesCount",
42 with_defined_sizes);
43 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.
44 with_defined_touch_icons);
45 }
46
25 } // namespace 47 } // namespace
26 48
27 FaviconDriverImpl::FaviconDriverImpl(FaviconService* favicon_service, 49 FaviconDriverImpl::FaviconDriverImpl(FaviconService* favicon_service,
28 history::HistoryService* history_service, 50 history::HistoryService* history_service,
29 bookmarks::BookmarkModel* bookmark_model) 51 bookmarks::BookmarkModel* bookmark_model)
30 : favicon_service_(favicon_service), 52 : favicon_service_(favicon_service),
31 history_service_(history_service), 53 history_service_(history_service),
32 bookmark_model_(bookmark_model) { 54 bookmark_model_(bookmark_model) {
33 favicon_handler_.reset(new FaviconHandler( 55 favicon_handler_.reset(new FaviconHandler(
34 favicon_service_, this, kEnableTouchIcon 56 favicon_service_, this, kEnableTouchIcon
(...skipping 56 matching lines...) Expand 10 before | Expand all | Expand 10 after
91 favicon_service_->SetFaviconOutOfDateForPage(url); 113 favicon_service_->SetFaviconOutOfDateForPage(url);
92 if (force_reload) 114 if (force_reload)
93 favicon_service_->ClearUnableToDownloadFavicons(); 115 favicon_service_->ClearUnableToDownloadFavicons();
94 } 116 }
95 } 117 }
96 118
97 void FaviconDriverImpl::OnUpdateFaviconURL( 119 void FaviconDriverImpl::OnUpdateFaviconURL(
98 const GURL& page_url, 120 const GURL& page_url,
99 const std::vector<FaviconURL>& candidates) { 121 const std::vector<FaviconURL>& candidates) {
100 DCHECK(!candidates.empty()); 122 DCHECK(!candidates.empty());
123 RecordCandidateMetrics(candidates);
101 favicon_handler_->OnUpdateFaviconURL(page_url, candidates); 124 favicon_handler_->OnUpdateFaviconURL(page_url, candidates);
102 if (touch_icon_handler_.get()) 125 if (touch_icon_handler_.get())
103 touch_icon_handler_->OnUpdateFaviconURL(page_url, candidates); 126 touch_icon_handler_->OnUpdateFaviconURL(page_url, candidates);
104 } 127 }
105 128
106 } // namespace favicon 129 } // namespace favicon
OLDNEW

Powered by Google App Engine
This is Rietveld 408576698