Always select best favicon bitmap

components/favicon/core/favicon_handler.cc

Index: components/favicon/core/favicon_handler.cc
diff --git a/components/favicon/core/favicon_handler.cc b/components/favicon/core/favicon_handler.cc
index 8ca5df8a1b250aa1722a999d5166ab0075751d75..35c9fa63229dda18bf409a8b080c3724fe21021b 100644
--- a/components/favicon/core/favicon_handler.cc
+++ b/components/favicon/core/favicon_handler.cc
@@ -23,9 +23,13 @@
 namespace favicon {
 namespace {
 
-// Size (along each axis) of a touch icon. This currently corresponds to
-// the apple touch icon for iPad.
-const int kTouchIconSize = 144;
+// Size (along each axis) of a touch icon.
+#if defined(OS_IOS)
+// This currently corresponds to the apple touch icon for iPad.
+const int kLargestIconSizeInPixels = 144;
+#else
+const int kLargestIconSizeInPixels = 192;
+#endif
 
 bool DoUrlAndIconMatch(const FaviconURL& favicon_url,
                        const GURL& url,
@@ -63,47 +67,11 @@ bool IsValid(const favicon_base::FaviconRawBitmapResult& bitmap_result) {
   return bitmap_result.is_valid();
 }
 
-// Returns true if |bitmap_results| is non-empty and:
-// - At least one of the bitmaps in |bitmap_results| is expired
-// OR
-// - |bitmap_results| is missing favicons for |desired_size_in_dip| and one of
-//   the scale factors in favicon_base::GetFaviconScales().
-bool HasExpiredOrIncompleteResult(
-    int desired_size_in_dip,
+// Returns true at least one of the bitmaps in |bitmap_results| is expired.
+bool HasExpiredResult(
     const std::vector<favicon_base::FaviconRawBitmapResult>& bitmap_results) {
-  if (bitmap_results.empty())
-    return false;
-
-  // Check if at least one of the bitmaps is expired.
-  std::vector<favicon_base::FaviconRawBitmapResult>::const_iterator it =
-      std::find_if(bitmap_results.begin(), bitmap_results.end(), IsExpired);
-  if (it != bitmap_results.end())
-    return true;
-
-  // Any favicon size is good if the desired size is 0.
-  if (desired_size_in_dip == 0)
-    return false;
-
-  // Check if the favicon for at least one of the scale factors is missing.
-  // |bitmap_results| should always be complete for data inserted by
-  // FaviconHandler as the FaviconHandler stores favicons resized to all
-  // of favicon_base::GetFaviconScales() into the history backend.
-  // Examples of when |bitmap_results| can be incomplete:
-  // - Favicons inserted into the history backend by sync.
-  // - Favicons for imported bookmarks.
-  std::vector<gfx::Size> favicon_sizes;
-  for (const auto& bitmap_result : bitmap_results)
-    favicon_sizes.push_back(bitmap_result.pixel_size);
-
-  std::vector<float> favicon_scales = favicon_base::GetFaviconScales();
-  for (float favicon_scale : favicon_scales) {
-    int edge_size_in_pixel = std::ceil(desired_size_in_dip * favicon_scale);
-    auto it = std::find(favicon_sizes.begin(), favicon_sizes.end(),
-                        gfx::Size(edge_size_in_pixel, edge_size_in_pixel));
-    if (it == favicon_sizes.end())
-      return true;
-  }
-  return false;
+  return std::find_if(bitmap_results.begin(), bitmap_results.end(),
+                      IsExpired) != bitmap_results.end();
 }
 
 // Returns true if at least one of |bitmap_results| is valid.
@@ -113,57 +81,43 @@ bool HasValidResult(
       bitmap_results.end();
 }
 
-// Returns the index of the entry with the largest area.
-int GetLargestSizeIndex(const std::vector<gfx::Size>& sizes) {
+// Returns the index of the entry with the best size match.
+int GetBestMatchSizeIndex(const FaviconHandler::TargetSpec& target_size,
+                          const std::vector<gfx::Size>& sizes) {
   DCHECK(!sizes.empty());
   size_t ret = 0;
   for (size_t i = 1; i < sizes.size(); ++i) {
-    if (sizes[ret].GetArea() < sizes[i].GetArea())
+    if (target_size.IsBetterMatch(sizes[i], sizes[ret]))
       ret = i;
   }
   return static_cast<int>(ret);
 }
 
-// Return the index of a size which is same as the given |size|, -1 returned if
-// there is no such bitmap.
-int GetIndexBySize(const std::vector<gfx::Size>& sizes,
-                   const gfx::Size& size) {
-  DCHECK(!sizes.empty());
-  std::vector<gfx::Size>::const_iterator i =
-      std::find(sizes.begin(), sizes.end(), size);
-  if (i == sizes.end())
-    return -1;
-
-  return static_cast<int>(i - sizes.begin());
-}
-
 // Compare function used for std::stable_sort to sort as descend.
-bool CompareIconSize(const FaviconURL& b1, const FaviconURL& b2) {
-  int area1 = 0;
-  if (!b1.icon_sizes.empty())
-    area1 = b1.icon_sizes.front().GetArea();
-
-  int area2 = 0;
-  if (!b2.icon_sizes.empty())
-    area2 = b2.icon_sizes.front().GetArea();
-
-  return area1 > area2;
-}
-
-// Sorts the entries in |image_urls| by icon size in descending order.
-// Discards all but the largest size for each FaviconURL.
-void SortAndPruneImageUrls(std::vector<FaviconURL>* image_urls) {
-  // Not using const-reference since the loop mutates FaviconURL::icon_sizes.
-  for (FaviconURL& image_url : *image_urls) {
-    if (image_url.icon_sizes.empty())
-      continue;
-
-    gfx::Size largest =
-        image_url.icon_sizes[GetLargestSizeIndex(image_url.icon_sizes)];
-    image_url.icon_sizes.clear();
-    image_url.icon_sizes.push_back(largest);
+bool CompareScore(const std::pair<float, FaviconURL>& b1,
+                  const std::pair<float, FaviconURL>& b2) {
+  return b1.first > b2.first;
+}
+
+// Sorts the entries in |image_urls|, best icon size matches first for
+// |target_size|.
+void SortImageUrls(const FaviconHandler::TargetSpec& target_size,
+                   std::vector<FaviconURL>* image_urls) {
+  std::vector<std::pair<float, FaviconURL>> scored_candidates;
+  for (const FaviconURL& image_url : *image_urls) {
+    float max_score = 0;
+    for (const gfx::Size& size : image_url.icon_sizes) {
+      max_score = std::max(
+          max_score,
+          GetFaviconCandidateScore(size, target_size.GetMaxPixelSize()));

[pkotwicz, 2017/03/15 03:49:39]

You should use SelectFaviconFrameIndices() to compute the score.

If you want, you can compute the score of a file by summing up the scores for
each of the desired sizes
SelectFaviconFrameIndices(image_url.icon_sizes, desired_size, nullptr,
&score_out);

A candidate with matches for multiple sizes should have a higher score than a
candidate with a match for just one size

One day, we will evaluate each frame in .ico file separately (so you might pick
the 32x32 and 16x16 icons from different files) but till we support multiple
icon URLs per page URL in the database we need to select a single file as the
best candidate.

[mastiz, 2017/03/15 16:34:33]

I believe this is a moot point now, because of the proposed buckets. I however
still see no use for SelectFaviconFrameIndices().

+    }
+    scored_candidates.emplace_back(max_score, std::move(image_url));
   }
-  std::stable_sort(image_urls->begin(), image_urls->end(), CompareIconSize);
+  std::stable_sort(scored_candidates.begin(), scored_candidates.end(),
+                   CompareScore);
+  image_urls->clear();
+  for (auto& scored_candidate : scored_candidates)
+    image_urls->push_back(std::move(scored_candidate.second));
 }
 
 // Checks whether two FaviconURLs are equal ignoring the icon sizes.
@@ -171,10 +125,118 @@ bool FaviconURLsEqualIgnoringSizes(const FaviconURL& u1, const FaviconURL& u2) {
   return u1.icon_type == u2.icon_type && u1.icon_url == u2.icon_url;
 }
 
+FaviconHandler::TargetSpec GetTargetSpec(
+    FaviconDriverObserver::NotificationIconType handler_type) {
+  switch (handler_type) {
+    case FaviconDriverObserver::NON_TOUCH_16_DIP:
+      return FaviconHandler::TargetSpec::For16x16Dips();
+    case FaviconDriverObserver::NON_TOUCH_LARGEST:
+    case FaviconDriverObserver::TOUCH_LARGEST:
+      return FaviconHandler::TargetSpec::ForLargest();
+  }
+  NOTREACHED();
+}
+
 }  // namespace
 
 ////////////////////////////////////////////////////////////////////////////////
 
+// static
+FaviconHandler::TargetSpec FaviconHandler::TargetSpec::ForLargest() {
+  FaviconHandler::TargetSpec target_size;
+  target_size.pixel_sizes_.insert({kLargestIconSizeInPixels, 1.0f});
+  target_size.ensure_exact_size_ = false;
+  return target_size;
+}
+
+// static
+FaviconHandler::TargetSpec FaviconHandler::TargetSpec::For16x16Dips() {
+  FaviconHandler::TargetSpec target_size;
+  for (float scale : favicon_base::GetFaviconScales()) {
+    target_size.pixel_sizes_.insert(
+        {std::ceil(scale * gfx::kFaviconSize), scale});
+  }
+  target_size.ensure_exact_size_ = true;
+  return target_size;
+}
+
+FaviconHandler::TargetSpec::TargetSpec(const TargetSpec&) = default;
+
+FaviconHandler::TargetSpec::~TargetSpec() = default;
+
+std::vector<int> FaviconHandler::TargetSpec::GetPixelSizes() const {
+  std::vector<int> result;
+  for (const auto& pixel_size : pixel_sizes_)
+    result.push_back(pixel_size.first);
+  return result;
+}
+
+int FaviconHandler::TargetSpec::GetMaxPixelSize() const {
+  return pixel_sizes_.rbegin()->first;
+}
+
+bool FaviconHandler::TargetSpec::WantsBestBitmapOnly() const {
+  return !ensure_exact_size_;
+}
+
+bool FaviconHandler::TargetSpec::HasCompleteResult(
+    const std::vector<favicon_base::FaviconRawBitmapResult>& bitmap_results)
+    const {
+  if (WantsBestBitmapOnly())
+    return HasValidResult(bitmap_results);
+
+  // Check if the favicon for at least one of the scale factors is missing.
+  // |bitmap_results| should always be complete for data inserted by
+  // FaviconHandler as the FaviconHandler stores favicons resized to all
+  // of |favicon_scales| into the history backend.
+  // Examples of when |bitmap_results| can be incomplete:
+  // - Favicons inserted into the history backend by sync.
+  // - Favicons for imported bookmarks.
+  std::vector<gfx::Size> favicon_sizes;
+  for (const auto& bitmap_result : bitmap_results) {
+    if (IsValid(bitmap_result))
+      favicon_sizes.push_back(bitmap_result.pixel_size);
+  }
+
+  for (const auto& pixel_size : pixel_sizes_) {
+    const int edge_size_in_pixel = pixel_size.first;
+    auto it = std::find(favicon_sizes.begin(), favicon_sizes.end(),
+                        gfx::Size(edge_size_in_pixel, edge_size_in_pixel));
+    if (it == favicon_sizes.end())
+      return false;
+  }
+  return true;
+}
+
+bool FaviconHandler::TargetSpec::IsBetterMatch(const gfx::Size& size1,
+                                               const gfx::Size& size2) const {
+  return GetFaviconCandidateScore(size1, GetMaxPixelSize()) >
+         GetFaviconCandidateScore(size2, GetMaxPixelSize());
+}
+
+gfx::ImageSkia FaviconHandler::TargetSpec::CreateImageSkia(
+    const std::vector<SkBitmap>& bitmaps,
+    const std::vector<gfx::Size>& original_bitmap_sizes) const {
+  if (WantsBestBitmapOnly()) {
+    return gfx::ImageSkia(gfx::ImageSkiaRep(
+        bitmaps[GetBestMatchSizeIndex(*this, original_bitmap_sizes)], 1));
+  } else {
+    return CreateFaviconImageSkia(bitmaps, original_bitmap_sizes,
+                                  gfx::kFaviconSize, /*score=*/nullptr);
+  }
+}
+
+gfx::Image FaviconHandler::TargetSpec::CreateImageFromCache(
+    const std::vector<favicon_base::FaviconRawBitmapResult>& bitmap_results)
+    const {
+  return favicon_base::SelectFaviconFramesForPixelSizesFromPNGs(bitmap_results,
+                                                                pixel_sizes_);
+}
+
+FaviconHandler::TargetSpec::TargetSpec() = default;
+
+////////////////////////////////////////////////////////////////////////////////
+
 FaviconHandler::DownloadRequest::DownloadRequest()
     : icon_type(favicon_base::INVALID_ICON) {
 }
@@ -191,8 +253,7 @@ FaviconHandler::DownloadRequest::DownloadRequest(
 ////////////////////////////////////////////////////////////////////////////////
 
 FaviconHandler::FaviconCandidate::FaviconCandidate()
-    : score(0), icon_type(favicon_base::INVALID_ICON) {
-}
+    : icon_type(favicon_base::INVALID_ICON) {}
 
 FaviconHandler::FaviconCandidate::~FaviconCandidate() {
 }
@@ -200,11 +261,9 @@ FaviconHandler::FaviconCandidate::~FaviconCandidate() {
 FaviconHandler::FaviconCandidate::FaviconCandidate(
     const GURL& image_url,
     const gfx::Image& image,
-    float score,
     favicon_base::IconType icon_type)
     : image_url(image_url),
       image(image),
-      score(score),
       icon_type(icon_type) {}
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -218,9 +277,7 @@ FaviconHandler::FaviconHandler(
       initial_history_result_expired_or_incomplete_(false),
       redownload_icons_(false),
       icon_types_(FaviconHandler::GetIconTypesFromHandlerType(handler_type)),
-      download_largest_icon_(
-          handler_type == FaviconDriverObserver::NON_TOUCH_LARGEST ||
-          handler_type == FaviconDriverObserver::TOUCH_LARGEST),
+      target_spec_(GetTargetSpec(handler_type)),
       notification_icon_type_(favicon_base::INVALID_ICON),
       service_(service),
       delegate_(delegate),
@@ -265,7 +322,7 @@ void FaviconHandler::FetchFavicon(const GURL& url) {
   // available.
   if (service_) {
     service_->GetFaviconForPageURL(
-        url_, icon_types_, preferred_icon_size(),
+        url_, icon_types_, target_spec_.GetPixelSizes(),
         base::Bind(
             &FaviconHandler::OnFaviconDataForInitialURLFromFaviconService,
             base::Unretained(this)),
@@ -275,49 +332,42 @@ void FaviconHandler::FetchFavicon(const GURL& url) {
 
 bool FaviconHandler::UpdateFaviconCandidate(const GURL& image_url,
                                             const gfx::Image& image,
-                                            float score,
                                             favicon_base::IconType icon_type) {
-  bool replace_best_favicon_candidate = false;
-  bool exact_match = false;
-  if (download_largest_icon_) {
-    replace_best_favicon_candidate =
-        image.Size().GetArea() >
-        best_favicon_candidate_.image.Size().GetArea();
-
-    gfx::Size largest = best_favicon_candidate_.image.Size();
-    if (replace_best_favicon_candidate)
-      largest = image.Size();
-
-    // The size of the downloaded icon may not match the declared size. Stop
-    // downloading if:
-    // - current candidate is only candidate.
-    // - next candidate doesn't have sizes attributes, in this case, the rest
-    //   candidates don't have sizes attribute either, stop downloading now,
-    //   otherwise, all favicon without sizes attribute are downloaded.
-    // - next candidate has sizes attribute and it is not larger than largest,
-    // - current candidate is maximal one we want.
-    const int maximal_size = GetMaximalIconSize(icon_type);
-    if (current_candidate_index_ + 1 >= image_urls_.size()) {
-      exact_match = true;
-    } else {
-      FaviconURL next_image_url = image_urls_[current_candidate_index_ + 1];
-      exact_match = next_image_url.icon_sizes.empty() ||
-          next_image_url.icon_sizes[0].GetArea() <= largest.GetArea() ||
-          (image.Size().width() == maximal_size &&
-           image.Size().height() == maximal_size);
-    }
-  } else {
-    exact_match = score == 1 || preferred_icon_size() == 0;
-    replace_best_favicon_candidate =
-        exact_match ||
-        best_favicon_candidate_.icon_type == favicon_base::INVALID_ICON ||
-        score > best_favicon_candidate_.score;
+  // The size of the downloaded icon may not match the declared size so compare
+  // before overriding |best_favicon_candidate_|.
+  if (target_spec_.IsBetterMatch(image.Size(),
+                                 best_favicon_candidate_.image.Size())) {
+    best_favicon_candidate_ = FaviconCandidate(image_url, image, icon_type);
+  }
+
+  // Stop downloading if current candidate is only candidate.
+  if (current_candidate_index_ + 1 >= image_urls_.size()) {
+    return true;
   }
-  if (replace_best_favicon_candidate) {
-    best_favicon_candidate_ =
-        FaviconCandidate(image_url, image, score, icon_type);
+  const FaviconURL& next_image_url = image_urls_[current_candidate_index_ + 1];
+
+  // For exact sizes (i.e. not best only), we download all candidates until an
+  // exact match is found.
+  if (!target_spec_.WantsBestBitmapOnly()) {
+    const int desired_max_size = target_spec_.GetMaxPixelSize();
+    return best_favicon_candidate_.image.Size() ==
+           gfx::Size(desired_max_size, desired_max_size);
   }
-  return exact_match;
+
+  // The size of the downloaded icon may not match the declared size. Stop
+  // downloading if:
+  // - next candidate doesn't have sizes attributes, in this case, the rest
+  //   candidates don't have sizes attribute either, stop downloading now,
+  //   otherwise, all favicon without sizes attribute are downloaded.
+  // - next candidate has sizes attribute and it is not better than best,
+  if (next_image_url.icon_sizes.empty())
+    return true;
+
+  const gfx::Size& best_next_size =
+      next_image_url.icon_sizes[
+          GetBestMatchSizeIndex(target_spec_, next_image_url.icon_sizes)];
+  return !target_spec_.IsBetterMatch(best_next_size,
+                                     best_favicon_candidate_.image.Size());
 }
 
 void FaviconHandler::SetFavicon(const GURL& icon_url,
@@ -335,10 +385,9 @@ void FaviconHandler::NotifyFaviconUpdated(
   if (favicon_bitmap_results.empty())
     return;
 
-  gfx::Image resized_image = favicon_base::SelectFaviconFramesFromPNGs(
-      favicon_bitmap_results,
-      favicon_base::GetFaviconScales(),
-      preferred_icon_size());
+  gfx::Image resized_image =
+      target_spec_.CreateImageFromCache(favicon_bitmap_results);
+
   // The history service sends back results for a single icon URL and icon
   // type, so it does not matter which result we get |icon_url| and |icon_type|
   // from.
@@ -376,8 +425,7 @@ void FaviconHandler::OnUpdateFaviconURL(
       pruned_candidates.push_back(candidate);
   }
 
-  if (download_largest_icon_)
-    SortAndPruneImageUrls(&pruned_candidates);
+  SortImageUrls(target_spec_, &pruned_candidates);
 
   // Ignore FaviconURL::icon_sizes because FaviconURL::icon_sizes is not stored
   // in the history database.
@@ -398,24 +446,6 @@ void FaviconHandler::OnUpdateFaviconURL(
     OnGotInitialHistoryDataAndIconURLCandidates();
 }
 
-int FaviconHandler::GetMaximalIconSize(favicon_base::IconType icon_type) {
-  switch (icon_type) {
-    case favicon_base::FAVICON:
-#if defined(OS_ANDROID)
-      return 192;
-#else
-      return gfx::ImageSkia::GetMaxSupportedScale() * gfx::kFaviconSize;
-#endif
-    case favicon_base::TOUCH_ICON:
-    case favicon_base::TOUCH_PRECOMPOSED_ICON:
-      return kTouchIconSize;
-    case favicon_base::INVALID_ICON:
-      return 0;
-  }
-  NOTREACHED();
-  return 0;
-}
-
 void FaviconHandler::OnGotInitialHistoryDataAndIconURLCandidates() {
   if (!initial_history_result_expired_or_incomplete_ &&
       DoUrlAndIconMatch(*current_candidate(), notification_icon_url_,
@@ -465,34 +495,15 @@ void FaviconHandler::OnDidDownloadFavicon(
 
   bool request_next_icon = true;
   if (!bitmaps.empty()) {
-    float score = 0.0f;
-    gfx::ImageSkia image_skia;
-    if (download_largest_icon_) {
-      int index = -1;
-      // Use the largest bitmap if FaviconURL doesn't have sizes attribute.
-      if (current_candidate()->icon_sizes.empty()) {
-        index = GetLargestSizeIndex(original_bitmap_sizes);
-      } else {
-        index = GetIndexBySize(original_bitmap_sizes,
-                               current_candidate()->icon_sizes[0]);
-        // Find largest bitmap if there is no one exactly matched.
-        if (index == -1)
-          index = GetLargestSizeIndex(original_bitmap_sizes);
-      }
-      image_skia = gfx::ImageSkia(gfx::ImageSkiaRep(bitmaps[index], 1));
-    } else {
-      image_skia = CreateFaviconImageSkia(bitmaps,
-                                          original_bitmap_sizes,
-                                          preferred_icon_size(),
-                                          &score);
-    }
+    gfx::ImageSkia image_skia =
+        target_spec_.CreateImageSkia(bitmaps, original_bitmap_sizes);
 
     if (!image_skia.isNull()) {
       gfx::Image image(image_skia);
       // The downloaded icon is still valid when there is no FaviconURL update
       // during the downloading.
-      request_next_icon = !UpdateFaviconCandidate(image_url, image, score,
-                                                  download_request.icon_type);
+      request_next_icon =
+          !UpdateFaviconCandidate(image_url, image, download_request.icon_type);

[pkotwicz, 2017/03/15 03:49:39]

UpdateFaviconCandidate() should be fed |bitmaps| and |original_bitmap_sizes|

UpdateFaviconCandidate() should iterate through all of the buckets and find the
best candidate for each bucket and compute the score summing the scores for each
match up

     }
   }
 
@@ -534,9 +545,8 @@ void FaviconHandler::OnFaviconDataForInitialURLFromFaviconService(
   got_favicon_from_history_ = true;
   bool has_valid_result = HasValidResult(favicon_bitmap_results);
   initial_history_result_expired_or_incomplete_ =
-      !has_valid_result ||
-      HasExpiredOrIncompleteResult(preferred_icon_size(),
-                                   favicon_bitmap_results);
+      !has_valid_result || HasExpiredResult(favicon_bitmap_results) ||
+      !target_spec_.HasCompleteResult(favicon_bitmap_results);
   redownload_icons_ = initial_history_result_expired_or_incomplete_ &&
                       !favicon_bitmap_results.empty();
 
@@ -567,7 +577,8 @@ void FaviconHandler::DownloadCurrentCandidateOrAskFaviconService() {
     // favicon given the favicon URL.
     if (delegate_->IsOffTheRecord()) {
       service_->GetFavicon(
-          icon_url, icon_type, preferred_icon_size(),
+          icon_url, icon_type,
+          target_spec_.GetPixelSizes(),
           base::Bind(&FaviconHandler::OnFaviconData, base::Unretained(this)),
           &cancelable_task_tracker_);
     } else {
@@ -579,7 +590,7 @@ void FaviconHandler::DownloadCurrentCandidateOrAskFaviconService() {
       // TODO(pkotwicz): pass in all of |image_urls_| to
       // UpdateFaviconMappingsAndFetch().
       service_->UpdateFaviconMappingsAndFetch(
-          url_, {icon_url}, icon_type, preferred_icon_size(),
+          url_, {icon_url}, icon_type, target_spec_.GetPixelSizes(),
           base::Bind(&FaviconHandler::OnFaviconData, base::Unretained(this)),
           &cancelable_task_tracker_);
     }
@@ -591,8 +602,8 @@ void FaviconHandler::OnFaviconData(const std::vector<
   bool has_results = !favicon_bitmap_results.empty();
   bool has_valid_result = HasValidResult(favicon_bitmap_results);
   bool has_expired_or_incomplete_result =
-      !has_valid_result || HasExpiredOrIncompleteResult(preferred_icon_size(),
-                                                        favicon_bitmap_results);
+      !has_valid_result || HasExpiredResult(favicon_bitmap_results) ||
+      !target_spec_.HasCompleteResult(favicon_bitmap_results);
 
   if (has_valid_result) {
     // There is a valid favicon. Notify any observers. It is useful to notify
@@ -631,7 +642,7 @@ void FaviconHandler::ScheduleDownload(const GURL& image_url,
   // OnDidDownloadFavicon(). See FaviconDriver::StartDownload()
   // for more details about the max bitmap size.
   const int download_id =
-      delegate_->DownloadImage(image_url, GetMaximalIconSize(icon_type),
+      delegate_->DownloadImage(image_url, target_spec_.GetMaxPixelSize(),
                                base::Bind(&FaviconHandler::OnDidDownloadFavicon,
                                           weak_ptr_factory_.GetWeakPtr()));
   DCHECK_NE(download_id, 0);