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

Side by Side Diff: components/favicon/core/favicon_handler.cc

Issue 2781093002: Add attempt count metric to FaviconHandler (Closed)
Patch Set: Correct test and restrict histogram 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 unified diff | Download patch
OLDNEW
1 // Copyright (c) 2012 The Chromium Authors. All rights reserved. 1 // Copyright (c) 2012 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_handler.h" 5 #include "components/favicon/core/favicon_handler.h"
6 6
7 #include <algorithm> 7 #include <algorithm>
8 #include <cmath> 8 #include <cmath>
9 #include <utility> 9 #include <utility>
10 #include <vector> 10 #include <vector>
11 11
12 #include "base/bind.h" 12 #include "base/bind.h"
13 #include "base/bind_helpers.h" 13 #include "base/bind_helpers.h"
14 #include "base/memory/ref_counted_memory.h" 14 #include "base/memory/ref_counted_memory.h"
15 #include "base/metrics/histogram_macros.h"
15 #include "build/build_config.h" 16 #include "build/build_config.h"
16 #include "components/favicon/core/favicon_service.h" 17 #include "components/favicon/core/favicon_service.h"
17 #include "components/favicon_base/favicon_util.h" 18 #include "components/favicon_base/favicon_util.h"
18 #include "components/favicon_base/select_favicon_frames.h" 19 #include "components/favicon_base/select_favicon_frames.h"
19 #include "skia/ext/image_operations.h" 20 #include "skia/ext/image_operations.h"
20 #include "ui/gfx/codec/png_codec.h" 21 #include "ui/gfx/codec/png_codec.h"
21 #include "ui/gfx/image/image_skia.h" 22 #include "ui/gfx/image/image_skia.h"
22 #include "ui/gfx/image/image_util.h" 23 #include "ui/gfx/image/image_util.h"
23 24
24 namespace favicon { 25 namespace favicon {
(...skipping 27 matching lines...) Expand all
52 // Return true if |bitmap_result| is expired. 53 // Return true if |bitmap_result| is expired.
53 bool IsExpired(const favicon_base::FaviconRawBitmapResult& bitmap_result) { 54 bool IsExpired(const favicon_base::FaviconRawBitmapResult& bitmap_result) {
54 return bitmap_result.expired; 55 return bitmap_result.expired;
55 } 56 }
56 57
57 // Return true if |bitmap_result| is valid. 58 // Return true if |bitmap_result| is valid.
58 bool IsValid(const favicon_base::FaviconRawBitmapResult& bitmap_result) { 59 bool IsValid(const favicon_base::FaviconRawBitmapResult& bitmap_result) {
59 return bitmap_result.is_valid(); 60 return bitmap_result.is_valid();
60 } 61 }
61 62
63 void RecordDownloadAttemptsForHandlerType(
64 FaviconDriverObserver::NotificationIconType handler_type,
65 int attempts) {
66 // If not at least one attempts was recorded or more than 15 attempts were
67 // registered, something went wrong. Underflows are stored in bucket 0 and
68 // overflows in bucket 16.
69 attempts = std::max(0, std::min(attempts, 16));
70 switch (handler_type) {
71 case FaviconDriverObserver::NON_TOUCH_16_DIP:
72 INTERNAL_HISTOGRAM_SPARSE_SLOWLY("Favicons.DownloadAttempts.Favicons",
Ilya Sherman 2017/04/13 21:17:59 Please use UMA_HISTOGRAM_SPARSE_SLOWLY. The INTER
fhorschig 2017/04/18 15:15:26 Done and thanks!
73 attempts);
74 return;
75 case FaviconDriverObserver::NON_TOUCH_LARGEST:
76 INTERNAL_HISTOGRAM_SPARSE_SLOWLY("Favicons.DownloadAttempts.LargeIcons",
77 attempts);
78 return;
79 case FaviconDriverObserver::TOUCH_LARGEST:
80 INTERNAL_HISTOGRAM_SPARSE_SLOWLY("Favicons.DownloadAttempts.TouchIcons",
81 attempts);
82 return;
83 }
84 NOTREACHED();
85 }
86
62 // Returns true if |bitmap_results| is non-empty and: 87 // Returns true if |bitmap_results| is non-empty and:
63 // - At least one of the bitmaps in |bitmap_results| is expired 88 // - At least one of the bitmaps in |bitmap_results| is expired
64 // OR 89 // OR
65 // - |bitmap_results| is missing favicons for |desired_size_in_dip| and one of 90 // - |bitmap_results| is missing favicons for |desired_size_in_dip| and one of
66 // the scale factors in favicon_base::GetFaviconScales(). 91 // the scale factors in favicon_base::GetFaviconScales().
67 bool HasExpiredOrIncompleteResult( 92 bool HasExpiredOrIncompleteResult(
68 int desired_size_in_dip, 93 int desired_size_in_dip,
69 const std::vector<favicon_base::FaviconRawBitmapResult>& bitmap_results) { 94 const std::vector<favicon_base::FaviconRawBitmapResult>& bitmap_results) {
70 if (bitmap_results.empty()) 95 if (bitmap_results.empty())
71 return false; 96 return false;
(...skipping 88 matching lines...) Expand 10 before | Expand all | Expand 10 after
160 got_favicon_from_history_(false), 185 got_favicon_from_history_(false),
161 initial_history_result_expired_or_incomplete_(false), 186 initial_history_result_expired_or_incomplete_(false),
162 redownload_icons_(false), 187 redownload_icons_(false),
163 icon_types_(FaviconHandler::GetIconTypesFromHandlerType(handler_type)), 188 icon_types_(FaviconHandler::GetIconTypesFromHandlerType(handler_type)),
164 download_largest_icon_( 189 download_largest_icon_(
165 handler_type == FaviconDriverObserver::NON_TOUCH_LARGEST || 190 handler_type == FaviconDriverObserver::NON_TOUCH_LARGEST ||
166 handler_type == FaviconDriverObserver::TOUCH_LARGEST), 191 handler_type == FaviconDriverObserver::TOUCH_LARGEST),
167 notification_icon_type_(favicon_base::INVALID_ICON), 192 notification_icon_type_(favicon_base::INVALID_ICON),
168 service_(service), 193 service_(service),
169 delegate_(delegate), 194 delegate_(delegate),
195 num_download_requests_(0),
170 current_candidate_index_(0u) { 196 current_candidate_index_(0u) {
171 DCHECK(delegate_); 197 DCHECK(delegate_);
172 } 198 }
173 199
174 FaviconHandler::~FaviconHandler() { 200 FaviconHandler::~FaviconHandler() {
175 } 201 }
176 202
177 // static 203 // static
178 int FaviconHandler::GetIconTypesFromHandlerType( 204 int FaviconHandler::GetIconTypesFromHandlerType(
179 FaviconDriverObserver::NotificationIconType handler_type) { 205 FaviconDriverObserver::NotificationIconType handler_type) {
(...skipping 12 matching lines...) Expand all
192 218
193 url_ = url; 219 url_ = url;
194 220
195 initial_history_result_expired_or_incomplete_ = false; 221 initial_history_result_expired_or_incomplete_ = false;
196 redownload_icons_ = false; 222 redownload_icons_ = false;
197 got_favicon_from_history_ = false; 223 got_favicon_from_history_ = false;
198 download_request_.Cancel(); 224 download_request_.Cancel();
199 candidates_.clear(); 225 candidates_.clear();
200 notification_icon_url_ = GURL(); 226 notification_icon_url_ = GURL();
201 notification_icon_type_ = favicon_base::INVALID_ICON; 227 notification_icon_type_ = favicon_base::INVALID_ICON;
228 num_download_requests_ = 0;
202 current_candidate_index_ = 0u; 229 current_candidate_index_ = 0u;
203 best_favicon_ = DownloadedFavicon(); 230 best_favicon_ = DownloadedFavicon();
204 231
205 // Request the favicon from the history service. In parallel to this the 232 // Request the favicon from the history service. In parallel to this the
206 // renderer is going to notify us (well WebContents) when the favicon url is 233 // renderer is going to notify us (well WebContents) when the favicon url is
207 // available. 234 // available.
208 if (service_) { 235 if (service_) {
209 service_->GetFaviconForPageURL( 236 service_->GetFaviconForPageURL(
210 url_, icon_types_, preferred_icon_size(), 237 url_, icon_types_, preferred_icon_size(),
211 base::Bind( 238 base::Bind(
(...skipping 92 matching lines...) Expand 10 before | Expand all | Expand 10 after
304 &FaviconCandidate::CompareScore); 331 &FaviconCandidate::CompareScore);
305 332
306 if (candidates_.size() == sorted_candidates.size() && 333 if (candidates_.size() == sorted_candidates.size() &&
307 std::equal(sorted_candidates.begin(), sorted_candidates.end(), 334 std::equal(sorted_candidates.begin(), sorted_candidates.end(),
308 candidates_.begin())) { 335 candidates_.begin())) {
309 return; 336 return;
310 } 337 }
311 338
312 download_request_.Cancel(); 339 download_request_.Cancel();
313 candidates_ = std::move(sorted_candidates); 340 candidates_ = std::move(sorted_candidates);
341 num_download_requests_ = 0;
314 current_candidate_index_ = 0u; 342 current_candidate_index_ = 0u;
315 best_favicon_ = DownloadedFavicon(); 343 best_favicon_ = DownloadedFavicon();
316 344
317 // TODO(davemoore) Should clear on empty url. Currently we ignore it. 345 // TODO(davemoore) Should clear on empty url. Currently we ignore it.
318 // This appears to be what FF does as well. 346 // This appears to be what FF does as well.
319 if (current_candidate() && got_favicon_from_history_) 347 if (current_candidate() && got_favicon_from_history_)
320 OnGotInitialHistoryDataAndIconURLCandidates(); 348 OnGotInitialHistoryDataAndIconURLCandidates();
321 } 349 }
322 350
323 // static 351 // static
(...skipping 68 matching lines...) Expand 10 before | Expand all | Expand 10 after
392 downloaded_favicon.candidate.score = score; 420 downloaded_favicon.candidate.score = score;
393 request_next_icon = !UpdateFaviconCandidate(downloaded_favicon); 421 request_next_icon = !UpdateFaviconCandidate(downloaded_favicon);
394 } 422 }
395 } 423 }
396 424
397 if (request_next_icon && current_candidate_index_ + 1 < candidates_.size()) { 425 if (request_next_icon && current_candidate_index_ + 1 < candidates_.size()) {
398 // Process the next candidate. 426 // Process the next candidate.
399 ++current_candidate_index_; 427 ++current_candidate_index_;
400 DownloadCurrentCandidateOrAskFaviconService(); 428 DownloadCurrentCandidateOrAskFaviconService();
401 } else { 429 } else {
430 // OnDidDownloadFavicon() can only be called after requesting a download, so
431 // |num_download_requests_| can never be 0.
432 RecordDownloadAttemptsForHandlerType(handler_type_, num_download_requests_);
402 // We have either found the ideal candidate or run out of candidates. 433 // We have either found the ideal candidate or run out of candidates.
403 if (best_favicon_.candidate.icon_type != favicon_base::INVALID_ICON) { 434 if (best_favicon_.candidate.icon_type != favicon_base::INVALID_ICON) {
404 // No more icons to request, set the favicon from the candidate. 435 // No more icons to request, set the favicon from the candidate.
405 SetFavicon(best_favicon_.candidate.icon_url, best_favicon_.image, 436 SetFavicon(best_favicon_.candidate.icon_url, best_favicon_.image,
406 best_favicon_.candidate.icon_type); 437 best_favicon_.candidate.icon_type);
407 } 438 }
408 // Clear download related state. 439 // Clear download related state.
409 current_candidate_index_ = candidates_.size(); 440 current_candidate_index_ = candidates_.size();
441 num_download_requests_ = 0;
410 best_favicon_ = DownloadedFavicon(); 442 best_favicon_ = DownloadedFavicon();
411 } 443 }
412 } 444 }
413 445
414 const std::vector<GURL> FaviconHandler::GetIconURLs() const { 446 const std::vector<GURL> FaviconHandler::GetIconURLs() const {
415 std::vector<GURL> icon_urls; 447 std::vector<GURL> icon_urls;
416 for (const FaviconCandidate& candidate : candidates_) 448 for (const FaviconCandidate& candidate : candidates_)
417 icon_urls.push_back(candidate.icon_url); 449 icon_urls.push_back(candidate.icon_url);
418 return icon_urls; 450 return icon_urls;
419 } 451 }
(...skipping 90 matching lines...) Expand 10 before | Expand all | Expand 10 after
510 (has_results && !DoUrlsAndIconsMatch(current_candidate()->icon_url, 542 (has_results && !DoUrlsAndIconsMatch(current_candidate()->icon_url,
511 current_candidate()->icon_type, 543 current_candidate()->icon_type,
512 favicon_bitmap_results))) { 544 favicon_bitmap_results))) {
513 // The icon URLs have been updated since the favicon data was requested. 545 // The icon URLs have been updated since the favicon data was requested.
514 return; 546 return;
515 } 547 }
516 548
517 if (has_expired_or_incomplete_result) { 549 if (has_expired_or_incomplete_result) {
518 ScheduleDownload(current_candidate()->icon_url, 550 ScheduleDownload(current_candidate()->icon_url,
519 current_candidate()->icon_type); 551 current_candidate()->icon_type);
552 } else if (num_download_requests_ > 0) {
553 RecordDownloadAttemptsForHandlerType(handler_type_, num_download_requests_);
520 } 554 }
521 } 555 }
522 556
523 void FaviconHandler::ScheduleDownload(const GURL& image_url, 557 void FaviconHandler::ScheduleDownload(const GURL& image_url,
524 favicon_base::IconType icon_type) { 558 favicon_base::IconType icon_type) {
525 DCHECK(image_url.is_valid()); 559 DCHECK(image_url.is_valid());
526 // Note that CancelableCallback starts cancelled. 560 // Note that CancelableCallback starts cancelled.
527 DCHECK(download_request_.IsCancelled()) << "More than one ongoing download"; 561 DCHECK(download_request_.IsCancelled()) << "More than one ongoing download";
528 if (service_ && service_->WasUnableToDownloadFavicon(image_url)) { 562 if (service_ && service_->WasUnableToDownloadFavicon(image_url)) {
529 DVLOG(1) << "Skip Failed FavIcon: " << image_url; 563 DVLOG(1) << "Skip Failed FavIcon: " << image_url;
530 OnDidDownloadFavicon(icon_type, 0, 0, image_url, std::vector<SkBitmap>(), 564 OnDidDownloadFavicon(icon_type, 0, 0, image_url, std::vector<SkBitmap>(),
531 std::vector<gfx::Size>()); 565 std::vector<gfx::Size>());
532 return; 566 return;
533 } 567 }
568 ++num_download_requests_;
534 download_request_.Reset(base::Bind(&FaviconHandler::OnDidDownloadFavicon, 569 download_request_.Reset(base::Bind(&FaviconHandler::OnDidDownloadFavicon,
535 base::Unretained(this), icon_type)); 570 base::Unretained(this), icon_type));
536 // A max bitmap size is specified to avoid receiving huge bitmaps in 571 // A max bitmap size is specified to avoid receiving huge bitmaps in
537 // OnDidDownloadFavicon(). See FaviconDriver::StartDownload() 572 // OnDidDownloadFavicon(). See FaviconDriver::StartDownload()
538 // for more details about the max bitmap size. 573 // for more details about the max bitmap size.
539 const int download_id = 574 const int download_id =
540 delegate_->DownloadImage(image_url, GetMaximalIconSize(handler_type_), 575 delegate_->DownloadImage(image_url, GetMaximalIconSize(handler_type_),
541 download_request_.callback()); 576 download_request_.callback());
542 DCHECK_NE(download_id, 0); 577 DCHECK_NE(download_id, 0);
543 } 578 }
544 579
545 } // namespace favicon 580 } // namespace favicon
OLDNEW
« no previous file with comments | « components/favicon/core/favicon_handler.h ('k') | components/favicon/core/favicon_handler_unittest.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698