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

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

Issue 2799273002: Add support to process favicons from Web Manifests (Closed)
Patch Set: Addressed comments, improved tests. 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>
(...skipping 209 matching lines...) Expand 10 before | Expand all | Expand 10 after
220 } 220 }
221 221
222 void FaviconHandler::FetchFavicon(const GURL& url) { 222 void FaviconHandler::FetchFavicon(const GURL& url) {
223 cancelable_task_tracker_.TryCancelAll(); 223 cancelable_task_tracker_.TryCancelAll();
224 224
225 url_ = url; 225 url_ = url;
226 226
227 initial_history_result_expired_or_incomplete_ = false; 227 initial_history_result_expired_or_incomplete_ = false;
228 redownload_icons_ = false; 228 redownload_icons_ = false;
229 got_favicon_from_history_ = false; 229 got_favicon_from_history_ = false;
230 download_request_.Cancel(); 230 manifest_download_request_.Cancel();
231 image_download_request_.Cancel();
232 manifest_url_.reset();
231 candidates_.clear(); 233 candidates_.clear();
232 notification_icon_url_ = GURL(); 234 notification_icon_url_ = GURL();
233 notification_icon_type_ = favicon_base::INVALID_ICON; 235 notification_icon_type_ = favicon_base::INVALID_ICON;
234 num_download_requests_ = 0; 236 num_download_requests_ = 0;
235 current_candidate_index_ = 0u; 237 current_candidate_index_ = 0u;
236 best_favicon_ = DownloadedFavicon(); 238 best_favicon_ = DownloadedFavicon();
237 239
238 // Request the favicon from the history service. In parallel to this the 240 // Request the favicon from the history service. In parallel to this the
239 // renderer is going to notify us (well WebContents) when the favicon url is 241 // renderer is going to notify us (well WebContents) when the favicon url is
240 // available. 242 // available.
(...skipping 55 matching lines...) Expand 10 before | Expand all | Expand 10 after
296 favicon_base::IconType icon_type = favicon_bitmap_results[0].icon_type; 298 favicon_base::IconType icon_type = favicon_bitmap_results[0].icon_type;
297 NotifyFaviconUpdated(icon_url, icon_type, resized_image); 299 NotifyFaviconUpdated(icon_url, icon_type, resized_image);
298 } 300 }
299 301
300 void FaviconHandler::NotifyFaviconUpdated(const GURL& icon_url, 302 void FaviconHandler::NotifyFaviconUpdated(const GURL& icon_url,
301 favicon_base::IconType icon_type, 303 favicon_base::IconType icon_type,
302 const gfx::Image& image) { 304 const gfx::Image& image) {
303 if (image.IsEmpty()) 305 if (image.IsEmpty())
304 return; 306 return;
305 307
308 // If a manifest is being processed, it's URL overrides the icon URL.
309 const GURL& notification_icon_url = manifest_url_.value_or(icon_url);
310
306 gfx::Image image_with_adjusted_colorspace = image; 311 gfx::Image image_with_adjusted_colorspace = image;
307 favicon_base::SetFaviconColorSpace(&image_with_adjusted_colorspace); 312 favicon_base::SetFaviconColorSpace(&image_with_adjusted_colorspace);
308 313
309 delegate_->OnFaviconUpdated(url_, handler_type_, icon_url, 314 delegate_->OnFaviconUpdated(url_, handler_type_, notification_icon_url,
310 icon_url != notification_icon_url_, 315 notification_icon_url != notification_icon_url_,
311 image_with_adjusted_colorspace); 316 image_with_adjusted_colorspace);
312 317
313 notification_icon_url_ = icon_url; 318 notification_icon_url_ = notification_icon_url;
314 notification_icon_type_ = icon_type; 319 notification_icon_type_ = icon_type;
315 } 320 }
316 321
317 void FaviconHandler::OnUpdateFaviconURL( 322 void FaviconHandler::OnUpdateCandidates(
318 const GURL& page_url, 323 const GURL& page_url,
319 const std::vector<FaviconURL>& candidates) { 324 const std::vector<FaviconURL>& candidates,
325 const base::Optional<GURL>& manifest_url) {
326 DCHECK_EQ(page_url, url_);
pkotwicz 2017/04/24 14:42:15 Can you please double check that the DCHECK_EQ() i
320 if (page_url != url_) 327 if (page_url != url_)
321 return; 328 return;
322 329
330 // |candidates| or |manifest_url| could have been modified via Javascript.
331 if (manifest_url_ && manifest_url_ == manifest_url)
332 return;
333
334 manifest_url_ = manifest_url;
pkotwicz 2017/04/24 14:42:15 Would this code work correctly if the manifest URL
mastiz 2017/04/27 13:54:50 Fixed this by splitting into two CancelableTaskTra
335 non_manifest_original_candidates_ = candidates;
336
337 // Check if the manifest previously returned a 404 (or a 200 but contained no
338 // icons), and ignore the manifest URL if that's the case.
339 if (manifest_url_ && service_->WasUnableToDownloadFavicon(*manifest_url_)) {
340 DVLOG(1) << "Skip failed Manifest: " << *manifest_url_;
341 manifest_url_.reset();
342 }
343
344 if (manifest_url_) {
345 // See if there is a cached favicon for the manifest.
346 GetFaviconAndUpdateMappingsUnlessIncognito(
347 /*icon_url=*/*manifest_url_, favicon_base::FAVICON,
348 base::Bind(&FaviconHandler::OnFaviconDataForManifestFromFaviconService,
349 base::Unretained(this)));
350 } else {
351 // If no manifest available, proceed with the regular candidates only.
352 OnGotFinalIconURLCandidates(candidates);
353 }
354 }
355
356 void FaviconHandler::OnFaviconDataForManifestFromFaviconService(
357 const std::vector<favicon_base::FaviconRawBitmapResult>&
358 favicon_bitmap_results) {
359 bool has_valid_result = HasValidResult(favicon_bitmap_results);
360 bool has_expired_or_incomplete_result =
361 !has_valid_result || HasExpiredOrIncompleteResult(preferred_icon_size(),
362 favicon_bitmap_results);
363
364 if (has_valid_result) {
365 // There is a valid favicon. Notify any observers. It is useful to notify
366 // the observers even if the favicon is expired or incomplete (incorrect
367 // size) because temporarily showing the user an expired favicon or
368 // streched favicon is preferable to showing the user the default favicon.
369 NotifyFaviconUpdated(favicon_bitmap_results);
370 }
371
372 if (has_expired_or_incomplete_result) {
373 manifest_download_request_.Reset(base::Bind(
374 &FaviconHandler::OnDidDownloadManifest, base::Unretained(this)));
375 delegate_->DownloadManifest(*manifest_url_,
376 manifest_download_request_.callback());
377 }
378 }
379
380 void FaviconHandler::OnDidDownloadManifest(
381 int status_code,
382 const std::vector<FaviconURL>& candidates) {
383 // Mark manifest download as finished.
384 manifest_download_request_.Cancel();
385
386 if (!candidates.empty()) {
387 OnGotFinalIconURLCandidates(candidates);
388 return;
389 }
390
391 // If either the downloading of the manifest failed, OR the manifest contains
392 // no icons, proceed with the list of icons listed in the HTML.
393 DVLOG(1) << "Could not fetch Manifest icons from " << *manifest_url_
394 << ", falling back to inlined ones, which are "
395 << non_manifest_original_candidates_.size();
396
397 if (status_code == 404 || status_code == 200)
398 service_->UnableToDownloadFavicon(*manifest_url_);
399
400 manifest_url_.reset();
401 OnGotFinalIconURLCandidates(non_manifest_original_candidates_);
402 }
403
404 void FaviconHandler::OnGotFinalIconURLCandidates(
405 const std::vector<FaviconURL>& candidates) {
323 std::vector<FaviconCandidate> sorted_candidates; 406 std::vector<FaviconCandidate> sorted_candidates;
324 const std::vector<int> desired_pixel_sizes = 407 const std::vector<int> desired_pixel_sizes =
325 GetDesiredPixelSizes(handler_type_); 408 GetDesiredPixelSizes(handler_type_);
326 for (const FaviconURL& candidate : candidates) { 409 for (const FaviconURL& candidate : candidates) {
327 if (!candidate.icon_url.is_empty() && (candidate.icon_type & icon_types_)) { 410 if (!candidate.icon_url.is_empty() && (candidate.icon_type & icon_types_)) {
328 sorted_candidates.push_back( 411 sorted_candidates.push_back(
329 FaviconCandidate::FromFaviconURL(candidate, desired_pixel_sizes)); 412 FaviconCandidate::FromFaviconURL(candidate, desired_pixel_sizes));
330 } 413 }
331 } 414 }
332 415
333 std::stable_sort(sorted_candidates.begin(), sorted_candidates.end(), 416 std::stable_sort(sorted_candidates.begin(), sorted_candidates.end(),
334 &FaviconCandidate::CompareScore); 417 &FaviconCandidate::CompareScore);
335 418
336 if (candidates_.size() == sorted_candidates.size() && 419 if (candidates_.size() == sorted_candidates.size() &&
337 std::equal(sorted_candidates.begin(), sorted_candidates.end(), 420 std::equal(sorted_candidates.begin(), sorted_candidates.end(),
338 candidates_.begin())) { 421 candidates_.begin())) {
339 return; 422 return;
340 } 423 }
341 424
342 download_request_.Cancel(); 425 image_download_request_.Cancel();
343 candidates_ = std::move(sorted_candidates); 426 candidates_ = std::move(sorted_candidates);
344 num_download_requests_ = 0; 427 num_download_requests_ = 0;
345 current_candidate_index_ = 0u; 428 current_candidate_index_ = 0u;
346 best_favicon_ = DownloadedFavicon(); 429 best_favicon_ = DownloadedFavicon();
347 430
348 // TODO(davemoore) Should clear on empty url. Currently we ignore it. 431 // TODO(davemoore) Should clear on empty url. Currently we ignore it.
349 // This appears to be what FF does as well. 432 // This appears to be what FF does as well.
350 if (current_candidate() && got_favicon_from_history_) 433 if (current_candidate() && got_favicon_from_history_)
351 OnGotInitialHistoryDataAndIconURLCandidates(); 434 OnGotInitialHistoryDataAndIconURLCandidates();
352 } 435 }
(...skipping 26 matching lines...) Expand all
379 } 462 }
380 463
381 void FaviconHandler::OnDidDownloadFavicon( 464 void FaviconHandler::OnDidDownloadFavicon(
382 favicon_base::IconType icon_type, 465 favicon_base::IconType icon_type,
383 int id, 466 int id,
384 int http_status_code, 467 int http_status_code,
385 const GURL& image_url, 468 const GURL& image_url,
386 const std::vector<SkBitmap>& bitmaps, 469 const std::vector<SkBitmap>& bitmaps,
387 const std::vector<gfx::Size>& original_bitmap_sizes) { 470 const std::vector<gfx::Size>& original_bitmap_sizes) {
388 // Mark download as finished. 471 // Mark download as finished.
389 download_request_.Cancel(); 472 image_download_request_.Cancel();
390 473
391 if (bitmaps.empty() && http_status_code == 404) { 474 if (bitmaps.empty() && http_status_code == 404) {
392 DVLOG(1) << "Failed to Download Favicon:" << image_url; 475 DVLOG(1) << "Failed to Download Favicon:" << image_url;
393 RecordDownloadOutcome(DownloadOutcome::FAILED); 476 RecordDownloadOutcome(DownloadOutcome::FAILED);
394 service_->UnableToDownloadFavicon(image_url); 477 service_->UnableToDownloadFavicon(image_url);
395 } 478 }
396 479
397 bool request_next_icon = true; 480 bool request_next_icon = true;
398 if (!bitmaps.empty()) { 481 if (!bitmaps.empty()) {
399 RecordDownloadOutcome(DownloadOutcome::SUCCEEDED); 482 RecordDownloadOutcome(DownloadOutcome::SUCCEEDED);
(...skipping 29 matching lines...) Expand all
429 if (request_next_icon && current_candidate_index_ + 1 < candidates_.size()) { 512 if (request_next_icon && current_candidate_index_ + 1 < candidates_.size()) {
430 // Process the next candidate. 513 // Process the next candidate.
431 ++current_candidate_index_; 514 ++current_candidate_index_;
432 DownloadCurrentCandidateOrAskFaviconService(); 515 DownloadCurrentCandidateOrAskFaviconService();
433 } else { 516 } else {
434 // OnDidDownloadFavicon() can only be called after requesting a download, so 517 // OnDidDownloadFavicon() can only be called after requesting a download, so
435 // |num_download_requests_| can never be 0. 518 // |num_download_requests_| can never be 0.
436 RecordDownloadAttemptsForHandlerType(handler_type_, num_download_requests_); 519 RecordDownloadAttemptsForHandlerType(handler_type_, num_download_requests_);
437 // We have either found the ideal candidate or run out of candidates. 520 // We have either found the ideal candidate or run out of candidates.
438 if (best_favicon_.candidate.icon_type != favicon_base::INVALID_ICON) { 521 if (best_favicon_.candidate.icon_type != favicon_base::INVALID_ICON) {
439 // No more icons to request, set the favicon from the candidate. 522 // No more icons to request, set the favicon from the candidate. Note that
440 SetFavicon(best_favicon_.candidate.icon_url, best_favicon_.image, 523 // manifest URLs override icon URLs if available.
441 best_favicon_.candidate.icon_type); 524 SetFavicon(manifest_url_.value_or(best_favicon_.candidate.icon_url),
525 best_favicon_.image, best_favicon_.candidate.icon_type);
442 } 526 }
443 // Clear download related state. 527 // Clear download related state.
444 current_candidate_index_ = candidates_.size(); 528 current_candidate_index_ = candidates_.size();
445 num_download_requests_ = 0; 529 num_download_requests_ = 0;
446 best_favicon_ = DownloadedFavicon(); 530 best_favicon_ = DownloadedFavicon();
447 } 531 }
448 } 532 }
449 533
450 const std::vector<GURL> FaviconHandler::GetIconURLs() const { 534 const std::vector<GURL> FaviconHandler::GetIconURLs() const {
451 std::vector<GURL> icon_urls; 535 std::vector<GURL> icon_urls;
452 for (const FaviconCandidate& candidate : candidates_) 536 for (const FaviconCandidate& candidate : candidates_)
453 icon_urls.push_back(candidate.icon_url); 537 icon_urls.push_back(candidate.icon_url);
454 return icon_urls; 538 return icon_urls;
455 } 539 }
456 540
457 bool FaviconHandler::HasPendingTasksForTest() { 541 bool FaviconHandler::HasPendingTasksForTest() {
458 return !download_request_.IsCancelled() || 542 return !manifest_download_request_.IsCancelled() ||
543 !image_download_request_.IsCancelled() ||
459 cancelable_task_tracker_.HasTrackedTasks(); 544 cancelable_task_tracker_.HasTrackedTasks();
460 } 545 }
461 546
462 bool FaviconHandler::ShouldSaveFavicon() { 547 bool FaviconHandler::ShouldSaveFavicon() {
463 if (!delegate_->IsOffTheRecord()) 548 if (!delegate_->IsOffTheRecord())
464 return true; 549 return true;
465 550
466 // Always save favicon if the page is bookmarked. 551 // Always save favicon if the page is bookmarked.
467 return delegate_->IsBookmarked(url_); 552 return delegate_->IsBookmarked(url_);
468 } 553 }
469 554
470 void FaviconHandler::OnFaviconDataForInitialURLFromFaviconService( 555 void FaviconHandler::OnFaviconDataForInitialURLFromFaviconService(
471 const std::vector<favicon_base::FaviconRawBitmapResult>& 556 const std::vector<favicon_base::FaviconRawBitmapResult>&
472 favicon_bitmap_results) { 557 favicon_bitmap_results) {
473 got_favicon_from_history_ = true; 558 got_favicon_from_history_ = true;
474 bool has_valid_result = HasValidResult(favicon_bitmap_results); 559 bool has_valid_result = HasValidResult(favicon_bitmap_results);
475 initial_history_result_expired_or_incomplete_ = 560 initial_history_result_expired_or_incomplete_ =
476 !has_valid_result || 561 !has_valid_result ||
477 HasExpiredOrIncompleteResult(preferred_icon_size(), 562 HasExpiredOrIncompleteResult(preferred_icon_size(),
478 favicon_bitmap_results); 563 favicon_bitmap_results);
479 redownload_icons_ = initial_history_result_expired_or_incomplete_ && 564 redownload_icons_ = initial_history_result_expired_or_incomplete_ &&
480 !favicon_bitmap_results.empty(); 565 !favicon_bitmap_results.empty();
481 566
482 if (has_valid_result && (!current_candidate() || 567 if (has_valid_result) {
483 DoUrlsAndIconsMatch(current_candidate()->icon_url, 568 if (!current_candidate() ||
484 current_candidate()->icon_type, 569 DoUrlsAndIconsMatch(current_candidate()->icon_url,
485 favicon_bitmap_results))) { 570 current_candidate()->icon_type,
486 // The db knows the favicon (although it may be out of date) and the entry 571 favicon_bitmap_results) ||
487 // doesn't have an icon. Set the favicon now, and if the favicon turns out 572 (manifest_url_ &&
488 // to be expired (or the wrong url) we'll fetch later on. This way the 573 DoUrlsAndIconsMatch(*manifest_url_, favicon_base::FAVICON,
489 // user doesn't see a flash of the default favicon. 574 favicon_bitmap_results))) {
490 NotifyFaviconUpdated(favicon_bitmap_results); 575 // The db knows the favicon (although it may be out of date) and the entry
576 // doesn't have an icon. Set the favicon now, and if the favicon turns out
577 // to be expired (or the wrong url) we'll fetch later on. This way the
578 // user doesn't see a flash of the default favicon.
579 NotifyFaviconUpdated(favicon_bitmap_results);
580 }
491 } 581 }
492 582
493 if (current_candidate()) 583 if (current_candidate())
494 OnGotInitialHistoryDataAndIconURLCandidates(); 584 OnGotInitialHistoryDataAndIconURLCandidates();
495 } 585 }
496 586
497 void FaviconHandler::DownloadCurrentCandidateOrAskFaviconService() { 587 void FaviconHandler::DownloadCurrentCandidateOrAskFaviconService() {
498 GURL icon_url = current_candidate()->icon_url; 588 const GURL icon_url = current_candidate()->icon_url;
499 favicon_base::IconType icon_type = current_candidate()->icon_type; 589 const favicon_base::IconType icon_type = current_candidate()->icon_type;
500 590 // If the icons listed in a manifest are being processed, skip the cache
501 if (redownload_icons_) { 591 // lookup for |icon_url| since the manifest's URL is used for caching anyway,
592 // and this lookup has happened earlier.
593 if (redownload_icons_ || manifest_url_) {
502 // We have the mapping, but the favicon is out of date. Download it now. 594 // We have the mapping, but the favicon is out of date. Download it now.
503 ScheduleDownload(icon_url, icon_type); 595 ScheduleFaviconDownload(icon_url, icon_type);
504 } else { 596 } else {
505 // We don't know the favicon, but we may have previously downloaded the 597 GetFaviconAndUpdateMappingsUnlessIncognito(
506 // favicon for another page that shares the same favicon. Ask for the 598 icon_url, icon_type,
507 // favicon given the favicon URL. 599 base::Bind(&FaviconHandler::OnFaviconData, base::Unretained(this)));
508 if (delegate_->IsOffTheRecord()) {
509 service_->GetFavicon(
510 icon_url, icon_type, preferred_icon_size(),
511 base::Bind(&FaviconHandler::OnFaviconData, base::Unretained(this)),
512 &cancelable_task_tracker_);
513 } else {
514 // Ask the history service for the icon. This does two things:
515 // 1. Attempts to fetch the favicon data from the database.
516 // 2. If the favicon exists in the database, this updates the database to
517 // include the mapping between the page url and the favicon url.
518 // This is asynchronous. The history service will call back when done.
519 // TODO(pkotwicz): pass in all of |image_urls_| to
520 // UpdateFaviconMappingsAndFetch().
521 service_->UpdateFaviconMappingsAndFetch(
522 url_, {icon_url}, icon_type, preferred_icon_size(),
523 base::Bind(&FaviconHandler::OnFaviconData, base::Unretained(this)),
524 &cancelable_task_tracker_);
525 }
526 } 600 }
527 } 601 }
528 602
603 void FaviconHandler::GetFaviconAndUpdateMappingsUnlessIncognito(
604 const GURL& icon_url,
605 favicon_base::IconType icon_type,
606 const favicon_base::FaviconResultsCallback& callback) {
607 // We don't know the favicon, but we may have previously downloaded the
608 // favicon for another page that shares the same favicon. Ask for the
609 // favicon given the favicon URL.
610 if (delegate_->IsOffTheRecord()) {
611 service_->GetFavicon(icon_url, icon_type, preferred_icon_size(), callback,
612 &cancelable_task_tracker_);
613 } else {
614 // Ask the history service for the icon. This does two things:
615 // 1. Attempts to fetch the favicon data from the database.
616 // 2. If the favicon exists in the database, this updates the database to
617 // include the mapping between the page url and the favicon url.
618 // This is asynchronous. The history service will call back when done.
619 // TODO(pkotwicz): pass in all of |image_urls_| to
620 // UpdateFaviconMappingsAndFetch().
621 service_->UpdateFaviconMappingsAndFetch(url_, {icon_url}, icon_type,
622 preferred_icon_size(), callback,
623 &cancelable_task_tracker_);
624 }
625 }
626
529 void FaviconHandler::OnFaviconData(const std::vector< 627 void FaviconHandler::OnFaviconData(const std::vector<
530 favicon_base::FaviconRawBitmapResult>& favicon_bitmap_results) { 628 favicon_base::FaviconRawBitmapResult>& favicon_bitmap_results) {
531 bool has_results = !favicon_bitmap_results.empty(); 629 bool has_results = !favicon_bitmap_results.empty();
532 bool has_valid_result = HasValidResult(favicon_bitmap_results); 630 bool has_valid_result = HasValidResult(favicon_bitmap_results);
533 bool has_expired_or_incomplete_result = 631 bool has_expired_or_incomplete_result =
534 !has_valid_result || HasExpiredOrIncompleteResult(preferred_icon_size(), 632 !has_valid_result || HasExpiredOrIncompleteResult(preferred_icon_size(),
535 favicon_bitmap_results); 633 favicon_bitmap_results);
536 634
537 if (has_valid_result) { 635 if (has_valid_result) {
538 // There is a valid favicon. Notify any observers. It is useful to notify 636 // There is a valid favicon. Notify any observers. It is useful to notify
539 // the observers even if the favicon is expired or incomplete (incorrect 637 // the observers even if the favicon is expired or incomplete (incorrect
540 // size) because temporarily showing the user an expired favicon or 638 // size) because temporarily showing the user an expired favicon or
541 // streched favicon is preferable to showing the user the default favicon. 639 // streched favicon is preferable to showing the user the default favicon.
542 NotifyFaviconUpdated(favicon_bitmap_results); 640 NotifyFaviconUpdated(favicon_bitmap_results);
543 } 641 }
544 642
545 if (!current_candidate() || 643 if (!current_candidate() ||
546 (has_results && !DoUrlsAndIconsMatch(current_candidate()->icon_url, 644 (has_results && !DoUrlsAndIconsMatch(current_candidate()->icon_url,
547 current_candidate()->icon_type, 645 current_candidate()->icon_type,
548 favicon_bitmap_results))) { 646 favicon_bitmap_results))) {
549 // The icon URLs have been updated since the favicon data was requested. 647 // The icon URLs have been updated since the favicon data was requested.
550 return; 648 return;
551 } 649 }
552 650
553 if (has_expired_or_incomplete_result) { 651 if (has_expired_or_incomplete_result) {
554 ScheduleDownload(current_candidate()->icon_url, 652 ScheduleFaviconDownload(current_candidate()->icon_url,
555 current_candidate()->icon_type); 653 current_candidate()->icon_type);
556 } else if (num_download_requests_ > 0) { 654 } else if (num_download_requests_ > 0) {
557 RecordDownloadAttemptsForHandlerType(handler_type_, num_download_requests_); 655 RecordDownloadAttemptsForHandlerType(handler_type_, num_download_requests_);
558 } 656 }
559 } 657 }
560 658
561 void FaviconHandler::ScheduleDownload(const GURL& image_url, 659 void FaviconHandler::ScheduleFaviconDownload(const GURL& image_url,
562 favicon_base::IconType icon_type) { 660 favicon_base::IconType icon_type) {
563 DCHECK(image_url.is_valid()); 661 DCHECK(image_url.is_valid());
564 // Note that CancelableCallback starts cancelled. 662 // Note that CancelableCallback starts cancelled.
565 DCHECK(download_request_.IsCancelled()) << "More than one ongoing download"; 663 DCHECK(image_download_request_.IsCancelled())
664 << "More than one ongoing download";
566 if (service_->WasUnableToDownloadFavicon(image_url)) { 665 if (service_->WasUnableToDownloadFavicon(image_url)) {
567 DVLOG(1) << "Skip Failed FavIcon: " << image_url; 666 DVLOG(1) << "Skip Failed FavIcon: " << image_url;
568 RecordDownloadOutcome(DownloadOutcome::SKIPPED); 667 RecordDownloadOutcome(DownloadOutcome::SKIPPED);
569 OnDidDownloadFavicon(icon_type, 0, 0, image_url, std::vector<SkBitmap>(), 668 OnDidDownloadFavicon(icon_type, 0, 0, image_url, std::vector<SkBitmap>(),
570 std::vector<gfx::Size>()); 669 std::vector<gfx::Size>());
571 return; 670 return;
572 } 671 }
573 ++num_download_requests_; 672 ++num_download_requests_;
574 download_request_.Reset(base::Bind(&FaviconHandler::OnDidDownloadFavicon, 673 image_download_request_.Reset(
575 base::Unretained(this), icon_type)); 674 base::Bind(&FaviconHandler::OnDidDownloadFavicon, base::Unretained(this),
675 icon_type));
576 // A max bitmap size is specified to avoid receiving huge bitmaps in 676 // A max bitmap size is specified to avoid receiving huge bitmaps in
577 // OnDidDownloadFavicon(). See FaviconDriver::StartDownload() 677 // OnDidDownloadFavicon(). See FaviconDriver::StartDownload()
578 // for more details about the max bitmap size. 678 // for more details about the max bitmap size.
579 const int download_id = 679 const int download_id =
580 delegate_->DownloadImage(image_url, GetMaximalIconSize(handler_type_), 680 delegate_->DownloadImage(image_url, GetMaximalIconSize(handler_type_),
581 download_request_.callback()); 681 image_download_request_.callback());
582 DCHECK_NE(download_id, 0); 682 DCHECK_NE(download_id, 0);
583 } 683 }
584 684
585 } // namespace favicon 685 } // namespace favicon
OLDNEW

Powered by Google App Engine
This is Rietveld 408576698