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

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

Issue 2986033002: Revert of Always prefer 192x192 on mobile, also for touch icons (Closed)
Patch Set: Created 3 years, 4 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/feature_list.h" 14 #include "base/feature_list.h"
15 #include "base/memory/ref_counted_memory.h" 15 #include "base/memory/ref_counted_memory.h"
16 #include "base/metrics/histogram_macros.h" 16 #include "base/metrics/histogram_macros.h"
17 #include "build/build_config.h" 17 #include "build/build_config.h"
18 #include "components/favicon/core/favicon_service.h" 18 #include "components/favicon/core/favicon_service.h"
19 #include "components/favicon/core/features.h" 19 #include "components/favicon/core/features.h"
20 #include "components/favicon_base/favicon_util.h" 20 #include "components/favicon_base/favicon_util.h"
21 #include "components/favicon_base/select_favicon_frames.h" 21 #include "components/favicon_base/select_favicon_frames.h"
22 #include "skia/ext/image_operations.h" 22 #include "skia/ext/image_operations.h"
23 #include "ui/gfx/codec/png_codec.h" 23 #include "ui/gfx/codec/png_codec.h"
24 #include "ui/gfx/image/image_skia.h" 24 #include "ui/gfx/image/image_skia.h"
25 #include "ui/gfx/image/image_util.h" 25 #include "ui/gfx/image/image_util.h"
26 26
27 namespace favicon { 27 namespace favicon {
28 namespace { 28 namespace {
29 29
30 const int kLargestIconSize = 192; 30 const int kNonTouchLargestIconSize = 192;
31
32 // Size (along each axis) of a touch icon. This currently corresponds to
33 // the apple touch icon for iPad.
34 // TODO(crbug.com/736290): Consider changing this to 192x192 for Android.
35 const int kTouchIconSize = 144;
31 36
32 // Return true if |bitmap_result| is expired. 37 // Return true if |bitmap_result| is expired.
33 bool IsExpired(const favicon_base::FaviconRawBitmapResult& bitmap_result) { 38 bool IsExpired(const favicon_base::FaviconRawBitmapResult& bitmap_result) {
34 return bitmap_result.expired; 39 return bitmap_result.expired;
35 } 40 }
36 41
37 // Return true if |bitmap_result| is valid. 42 // Return true if |bitmap_result| is valid.
38 bool IsValid(const favicon_base::FaviconRawBitmapResult& bitmap_result) { 43 bool IsValid(const favicon_base::FaviconRawBitmapResult& bitmap_result) {
39 return bitmap_result.is_valid(); 44 return bitmap_result.is_valid();
40 } 45 }
(...skipping 83 matching lines...) Expand 10 before | Expand all | Expand 10 after
124 switch (handler_type) { 129 switch (handler_type) {
125 case FaviconDriverObserver::NON_TOUCH_16_DIP: { 130 case FaviconDriverObserver::NON_TOUCH_16_DIP: {
126 std::vector<int> pixel_sizes; 131 std::vector<int> pixel_sizes;
127 for (float scale_factor : favicon_base::GetFaviconScales()) { 132 for (float scale_factor : favicon_base::GetFaviconScales()) {
128 pixel_sizes.push_back( 133 pixel_sizes.push_back(
129 static_cast<int>(ceil(scale_factor * gfx::kFaviconSize))); 134 static_cast<int>(ceil(scale_factor * gfx::kFaviconSize)));
130 } 135 }
131 return pixel_sizes; 136 return pixel_sizes;
132 } 137 }
133 case FaviconDriverObserver::NON_TOUCH_LARGEST: 138 case FaviconDriverObserver::NON_TOUCH_LARGEST:
139 return std::vector<int>(1U, kNonTouchLargestIconSize);
134 case FaviconDriverObserver::TOUCH_LARGEST: 140 case FaviconDriverObserver::TOUCH_LARGEST:
135 return std::vector<int>(1U, kLargestIconSize); 141 return std::vector<int>(1U, kTouchIconSize);
136 } 142 }
137 NOTREACHED(); 143 NOTREACHED();
138 return std::vector<int>(); 144 return std::vector<int>();
139 } 145 }
140 146
141 bool FaviconURLEquals(const FaviconURL& lhs, const FaviconURL& rhs) { 147 bool FaviconURLEquals(const FaviconURL& lhs, const FaviconURL& rhs) {
142 return lhs.icon_url == rhs.icon_url && lhs.icon_type == rhs.icon_type && 148 return lhs.icon_url == rhs.icon_url && lhs.icon_type == rhs.icon_type &&
143 lhs.icon_sizes == rhs.icon_sizes; 149 lhs.icon_sizes == rhs.icon_sizes;
144 } 150 }
145 151
(...skipping 189 matching lines...) Expand 10 before | Expand all | Expand 10 after
335 // Check if the manifest was previously blacklisted (e.g. returned a 404) and 341 // Check if the manifest was previously blacklisted (e.g. returned a 404) and
336 // ignore the manifest URL if that's the case. 342 // ignore the manifest URL if that's the case.
337 if (!manifest_url_.is_empty() && 343 if (!manifest_url_.is_empty() &&
338 service_->WasUnableToDownloadFavicon(manifest_url_)) { 344 service_->WasUnableToDownloadFavicon(manifest_url_)) {
339 DVLOG(1) << "Skip failed Manifest: " << manifest_url; 345 DVLOG(1) << "Skip failed Manifest: " << manifest_url;
340 manifest_url_ = GURL(); 346 manifest_url_ = GURL();
341 } 347 }
342 348
343 // If no manifest available, proceed with the regular candidates only. 349 // If no manifest available, proceed with the regular candidates only.
344 if (manifest_url_.is_empty()) { 350 if (manifest_url_.is_empty()) {
345 OnGotFinalIconURLCandidates(candidates); 351 OnGotFinalIconURLCandidates(candidates,
352 GetDesiredPixelSizes(handler_type_));
346 return; 353 return;
347 } 354 }
348 355
349 // See if there is a cached favicon for the manifest. This will update the DB 356 // See if there is a cached favicon for the manifest. This will update the DB
350 // mappings only if the manifest URL is cached. 357 // mappings only if the manifest URL is cached.
351 GetFaviconAndUpdateMappingsUnlessIncognito( 358 GetFaviconAndUpdateMappingsUnlessIncognito(
352 /*icon_url=*/manifest_url_, favicon_base::WEB_MANIFEST_ICON, 359 /*icon_url=*/manifest_url_, favicon_base::WEB_MANIFEST_ICON,
353 base::Bind(&FaviconHandler::OnFaviconDataForManifestFromFaviconService, 360 base::Bind(&FaviconHandler::OnFaviconDataForManifestFromFaviconService,
354 base::Unretained(this))); 361 base::Unretained(this)));
355 } 362 }
(...skipping 28 matching lines...) Expand all
384 manifest_download_request_.callback()); 391 manifest_download_request_.callback());
385 } 392 }
386 } 393 }
387 394
388 void FaviconHandler::OnDidDownloadManifest( 395 void FaviconHandler::OnDidDownloadManifest(
389 const std::vector<FaviconURL>& candidates) { 396 const std::vector<FaviconURL>& candidates) {
390 // Mark manifest download as finished. 397 // Mark manifest download as finished.
391 manifest_download_request_.Cancel(); 398 manifest_download_request_.Cancel();
392 399
393 if (!candidates.empty()) { 400 if (!candidates.empty()) {
394 OnGotFinalIconURLCandidates(candidates); 401 // When reading icons from web manifests, prefer kNonTouchLargestIconSize.
402 OnGotFinalIconURLCandidates(candidates,
403 std::vector<int>(1U, kNonTouchLargestIconSize));
395 return; 404 return;
396 } 405 }
397 406
398 // If either the downloading of the manifest failed, OR the manifest contains 407 // If either the downloading of the manifest failed, OR the manifest contains
399 // no icons, proceed with the list of icons listed in the HTML. 408 // no icons, proceed with the list of icons listed in the HTML.
400 DVLOG(1) << "Could not fetch Manifest icons from " << manifest_url_ 409 DVLOG(1) << "Could not fetch Manifest icons from " << manifest_url_
401 << ", falling back to inlined ones, which are " 410 << ", falling back to inlined ones, which are "
402 << non_manifest_original_candidates_.size(); 411 << non_manifest_original_candidates_.size();
403 412
404 service_->UnableToDownloadFavicon(manifest_url_); 413 service_->UnableToDownloadFavicon(manifest_url_);
405 manifest_url_ = GURL(); 414 manifest_url_ = GURL();
406 415
407 OnGotFinalIconURLCandidates(non_manifest_original_candidates_); 416 OnGotFinalIconURLCandidates(non_manifest_original_candidates_,
417 GetDesiredPixelSizes(handler_type_));
408 } 418 }
409 419
410 void FaviconHandler::OnGotFinalIconURLCandidates( 420 void FaviconHandler::OnGotFinalIconURLCandidates(
411 const std::vector<FaviconURL>& candidates) { 421 const std::vector<FaviconURL>& candidates,
412 const std::vector<int> desired_pixel_sizes = 422 const std::vector<int>& desired_pixel_sizes) {
413 GetDesiredPixelSizes(handler_type_);
414
415 std::vector<FaviconCandidate> sorted_candidates; 423 std::vector<FaviconCandidate> sorted_candidates;
416 for (const FaviconURL& candidate : candidates) { 424 for (const FaviconURL& candidate : candidates) {
417 if (!candidate.icon_url.is_empty() && (candidate.icon_type & icon_types_)) { 425 if (!candidate.icon_url.is_empty() && (candidate.icon_type & icon_types_)) {
418 sorted_candidates.push_back( 426 sorted_candidates.push_back(
419 FaviconCandidate::FromFaviconURL(candidate, desired_pixel_sizes)); 427 FaviconCandidate::FromFaviconURL(candidate, desired_pixel_sizes));
420 } 428 }
421 } 429 }
422 430
423 std::stable_sort(sorted_candidates.begin(), sorted_candidates.end(), 431 std::stable_sort(sorted_candidates.begin(), sorted_candidates.end(),
424 &FaviconCandidate::CompareScore); 432 &FaviconCandidate::CompareScore);
(...skipping 235 matching lines...) Expand 10 before | Expand all | Expand 10 after
660 // A max bitmap size is specified to avoid receiving huge bitmaps in 668 // A max bitmap size is specified to avoid receiving huge bitmaps in
661 // OnDidDownloadFavicon(). See FaviconDriver::StartDownload() 669 // OnDidDownloadFavicon(). See FaviconDriver::StartDownload()
662 // for more details about the max bitmap size. 670 // for more details about the max bitmap size.
663 const int download_id = 671 const int download_id =
664 delegate_->DownloadImage(image_url, GetMaximalIconSize(handler_type_), 672 delegate_->DownloadImage(image_url, GetMaximalIconSize(handler_type_),
665 image_download_request_.callback()); 673 image_download_request_.callback());
666 DCHECK_NE(download_id, 0); 674 DCHECK_NE(download_id, 0);
667 } 675 }
668 676
669 } // namespace favicon 677 } // 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