Always prefer 192x192 on mobile, also for touch icons

components/favicon/core/favicon_handler.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "components/favicon/core/favicon_handler.h"
 
 #include <algorithm>
 #include <cmath>
 #include <utility>
 #include <vector>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/feature_list.h"
 #include "base/memory/ref_counted_memory.h"
 #include "base/metrics/histogram_macros.h"
 #include "build/build_config.h"
 #include "components/favicon/core/favicon_service.h"
 #include "components/favicon/core/features.h"
 #include "components/favicon_base/favicon_util.h"
 #include "components/favicon_base/select_favicon_frames.h"
 #include "skia/ext/image_operations.h"
 #include "ui/gfx/codec/png_codec.h"
 #include "ui/gfx/image/image_skia.h"
 #include "ui/gfx/image/image_util.h"
 
 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) {
   return bitmap_result.expired;
 }
 
 // Return true if |bitmap_result| is valid.
 bool IsValid(const favicon_base::FaviconRawBitmapResult& bitmap_result) {
   return bitmap_result.is_valid();
 }
@@ skipping 83 matching lines @@
   switch (handler_type) {
     case FaviconDriverObserver::NON_TOUCH_16_DIP: {
       std::vector<int> pixel_sizes;
       for (float scale_factor : favicon_base::GetFaviconScales()) {
         pixel_sizes.push_back(
             static_cast<int>(ceil(scale_factor * gfx::kFaviconSize)));
       }
       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>();
 }
 
 bool FaviconURLEquals(const FaviconURL& lhs, const FaviconURL& rhs) {
   return lhs.icon_url == rhs.icon_url && lhs.icon_type == rhs.icon_type &&
          lhs.icon_sizes == rhs.icon_sizes;
 }
 
@@ skipping 189 matching lines @@
   // Check if the manifest was previously blacklisted (e.g. returned a 404) and
   // ignore the manifest URL if that's the case.
   if (!manifest_url_.is_empty() &&
       service_->WasUnableToDownloadFavicon(manifest_url_)) {
     DVLOG(1) << "Skip failed Manifest: " << manifest_url;
     manifest_url_ = GURL();
   }
 
   // If no manifest available, proceed with the regular candidates only.
   if (manifest_url_.is_empty()) {
-    OnGotFinalIconURLCandidates(candidates,
-                                GetDesiredPixelSizes(handler_type_));
+    OnGotFinalIconURLCandidates(candidates);
     return;
   }
 
   // See if there is a cached favicon for the manifest. This will update the DB
   // mappings only if the manifest URL is cached.
   GetFaviconAndUpdateMappingsUnlessIncognito(
       /*icon_url=*/manifest_url_, favicon_base::WEB_MANIFEST_ICON,
       base::Bind(&FaviconHandler::OnFaviconDataForManifestFromFaviconService,
                  base::Unretained(this)));
 }
@@ skipping 28 matching lines @@
                                 manifest_download_request_.callback());
   }
 }
 
 void FaviconHandler::OnDidDownloadManifest(
     const std::vector<FaviconURL>& candidates) {
   // Mark manifest download as finished.
   manifest_download_request_.Cancel();
 
   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;
   }
 
   // If either the downloading of the manifest failed, OR the manifest contains
   // no icons, proceed with the list of icons listed in the HTML.
   DVLOG(1) << "Could not fetch Manifest icons from " << manifest_url_
            << ", falling back to inlined ones, which are "
            << non_manifest_original_candidates_.size();
 
   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_)) {
       sorted_candidates.push_back(
           FaviconCandidate::FromFaviconURL(candidate, desired_pixel_sizes));
     }
   }
 
   std::stable_sort(sorted_candidates.begin(), sorted_candidates.end(),
                    &FaviconCandidate::CompareScore);
@@ skipping 235 matching lines @@
   // A max bitmap size is specified to avoid receiving huge bitmaps in
   // OnDidDownloadFavicon(). See FaviconDriver::StartDownload()
   // for more details about the max bitmap size.
   const int download_id =
       delegate_->DownloadImage(image_url, GetMaximalIconSize(handler_type_),
                                image_download_request_.callback());
   DCHECK_NE(download_id, 0);
 }
 
 }  // namespace favicon