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

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

Issue 2972643002: Always prefer 192x192 on mobile, also for touch icons (Closed)
Patch Set: Created 3 years, 5 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_handler.cc
diff --git a/components/favicon/core/favicon_handler.cc b/components/favicon/core/favicon_handler.cc
index 46d62352af41e15e8745eee07689984707dcc338..fef7214e99400949d7e738880bb1f689908e0914 100644
--- a/components/favicon/core/favicon_handler.cc
+++ b/components/favicon/core/favicon_handler.cc
@@ -27,12 +27,7 @@
namespace favicon {
namespace {
-const int kNonTouchLargestIconSize = 192;
-
-// Size (along each axis) of a touch icon. This currently corresponds to
-// the apple touch icon for iPad.
-// TODO(crbug.com/736290): Consider changing this to 192x192 for Android.
-const int kTouchIconSize = 144;
+const int kLargestIconSize = 192;
// Return true if |bitmap_result| is expired.
bool IsExpired(const favicon_base::FaviconRawBitmapResult& bitmap_result) {
@@ -136,9 +131,8 @@ std::vector<int> GetDesiredPixelSizes(
return pixel_sizes;
}
case FaviconDriverObserver::NON_TOUCH_LARGEST:
- return std::vector<int>(1U, kNonTouchLargestIconSize);
case FaviconDriverObserver::TOUCH_LARGEST:
- return std::vector<int>(1U, kTouchIconSize);
+ return std::vector<int>(1U, kLargestIconSize);
}
NOTREACHED();
return std::vector<int>();
@@ -348,8 +342,7 @@ void FaviconHandler::OnUpdateCandidates(
// If no manifest available, proceed with the regular candidates only.
if (manifest_url_.is_empty()) {
- OnGotFinalIconURLCandidates(candidates,
- GetDesiredPixelSizes(handler_type_));
+ OnGotFinalIconURLCandidates(candidates);
return;
}
@@ -399,8 +392,7 @@ void FaviconHandler::OnDidDownloadManifest(
if (!candidates.empty()) {
// When reading icons from web manifests, prefer kNonTouchLargestIconSize.
pkotwicz 2017/07/12 15:27:30 Should the comment be deleted?
mastiz 2017/07/24 07:52:15 Done.
- OnGotFinalIconURLCandidates(candidates,
- std::vector<int>(1U, kNonTouchLargestIconSize));
+ OnGotFinalIconURLCandidates(candidates);
return;
}
@@ -413,13 +405,14 @@ void FaviconHandler::OnDidDownloadManifest(
service_->UnableToDownloadFavicon(manifest_url_);
manifest_url_ = GURL();
- OnGotFinalIconURLCandidates(non_manifest_original_candidates_,
- GetDesiredPixelSizes(handler_type_));
+ OnGotFinalIconURLCandidates(non_manifest_original_candidates_);
}
void FaviconHandler::OnGotFinalIconURLCandidates(
- const std::vector<FaviconURL>& candidates,
- const std::vector<int>& desired_pixel_sizes) {
+ const std::vector<FaviconURL>& candidates) {
+ const std::vector<int> desired_pixel_sizes =
+ GetDesiredPixelSizes(handler_type_);
+
std::vector<FaviconCandidate> sorted_candidates;
for (const FaviconURL& candidate : candidates) {
if (!candidate.icon_url.is_empty() && (candidate.icon_type & icon_types_)) {

Powered by Google App Engine
This is Rietveld 408576698