Always select best favicon bitmap

components/favicon/core/favicon_handler_unittest.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 <stddef.h>
 
 #include <memory>
 #include <set>
 #include <vector>
 
 #include "base/macros.h"
 #include "base/memory/ptr_util.h"
 #include "base/message_loop/message_loop.h"
 #include "base/run_loop.h"
 #include "base/strings/stringprintf.h"
 #include "components/favicon/core/favicon_client.h"
 #include "components/favicon/core/favicon_driver.h"
 #include "components/favicon/core/test/mock_favicon_service.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/skia/include/core/SkBitmap.h"
 #include "ui/base/layout.h"
 #include "ui/gfx/codec/png_codec.h"
 #include "ui/gfx/favicon_size.h"
 #include "ui/gfx/image/image.h"
+#include "ui/gfx/image/image_skia.h"
 
 namespace favicon {
 namespace {
 
 using favicon_base::FAVICON;
 using favicon_base::FaviconRawBitmapResult;
 using favicon_base::TOUCH_ICON;
 using favicon_base::TOUCH_PRECOMPOSED_ICON;
+using testing::AllOf;
 using testing::AnyNumber;
 using testing::Assign;
 using testing::AtLeast;
 using testing::AtMost;
 using testing::ElementsAre;
 using testing::InSequence;
 using testing::Invoke;
 using testing::IsEmpty;
 using testing::Return;
 using testing::_;
 
 using IntVector = std::vector<int>;
 using URLVector = std::vector<GURL>;
 using BitmapVector = std::vector<SkBitmap>;
 using SizeVector = std::vector<gfx::Size>;
 
 MATCHER_P2(ImageSizeIs, width, height, "") {
   *result_listener << "where size is " << arg.Width() << "x" << arg.Height();
   return arg.Size() == gfx::Size(width, height);
 }
 
+MATCHER_P(HasScaleFactor, scale, "") {
+  return arg.AsImageSkia().HasRepresentation(scale);
+}
+
+MATCHER_P2(ImageColorForScaleIs, scale, expected_color, "") {
+  gfx::ImageSkia image_skia = arg.AsImageSkia();
+  image_skia.EnsureRepsForSupportedScales();
+  const SkBitmap& bitmap = image_skia.GetRepresentation(scale).sk_bitmap();
+  if (bitmap.empty()) {
+    *result_listener << "expected color but no bitmap data available";
+  }
+
+  SkAutoLockPixels lock(bitmap);
+  SkColor actual_color = bitmap.getColor(1, 1);
+  if (actual_color == expected_color) {
+    return true;
+  }
+
+  *result_listener << "expected color "
+                   << base::StringPrintf("%08X", expected_color)
+                   << " but actual color is "
+                   << base::StringPrintf("%08X", actual_color);
+  return false;
+}
+
+MATCHER_P(HasIconURL, expected_url, "") {
+  *result_listener << "where icon URL is " << arg.icon_url;
+  return arg.icon_url == GURL(expected_url);
+}
+
 // Fill the given bmp with some test data.
-SkBitmap CreateBitmap(int w, int h) {
+SkBitmap CreateBitmap(int w, int h, SkColor color) {
   SkBitmap bmp;
   bmp.allocN32Pixels(w, h);
-
-  unsigned char* src_data =
-      reinterpret_cast<unsigned char*>(bmp.getAddr32(0, 0));
-  for (int i = 0; i < w * h; i++) {
-    src_data[i * 4 + 0] = static_cast<unsigned char>(i % 255);
-    src_data[i * 4 + 1] = static_cast<unsigned char>(i % 255);
-    src_data[i * 4 + 2] = static_cast<unsigned char>(i % 255);
-    src_data[i * 4 + 3] = static_cast<unsigned char>(i % 255);
-  }
+  bmp.eraseColor(color);
   return bmp;
 }
 
-// Fill the given data buffer with valid png data.
-void FillBitmap(int w, int h, std::vector<unsigned char>* output) {
-  SkBitmap bitmap = CreateBitmap(w, h);
-  gfx::PNGCodec::EncodeBGRASkBitmap(bitmap, false, output);
-}
-
 std::vector<gfx::Size> CreateSquareSizes(const IntVector& sizes) {
   std::vector<gfx::Size> result;
   for (int size : sizes) {
     result.emplace_back(size, size);
   }
   return result;
 }
 
-std::vector<SkBitmap> CreateBitmaps(const std::vector<gfx::Size>& sizes) {
+std::vector<SkBitmap> CreateBitmaps(const std::vector<gfx::Size>& sizes,
+                                    const std::vector<SkColor>& colors) {
   std::vector<SkBitmap> bitmaps;
-  for (const gfx::Size& size : sizes) {
-    bitmaps.push_back(CreateBitmap(size.width(), size.height()));
+  for (size_t i = 0; i < sizes.size(); ++i) {
+    bitmaps.push_back(CreateBitmap(sizes[i].width(), sizes[i].height(),
+                                   colors[i]));
   }
   return bitmaps;
 }
 
-std::vector<FaviconRawBitmapResult> CreateRawBitmapResult(
+FaviconRawBitmapResult CreateRawBitmapResult(const GURL& icon_url,
+                                             favicon_base::IconType icon_type,
+                                             bool expired,
+                                             const gfx::Size& size,
+                                             SkColor color) {
+  scoped_refptr<base::RefCountedBytes> data(new base::RefCountedBytes());
+  SkBitmap bitmap = CreateBitmap(size.width(), size.height(), color);
+  gfx::PNGCodec::EncodeBGRASkBitmap(bitmap, false, &data->data());
+  FaviconRawBitmapResult bitmap_result;
+  bitmap_result.expired = expired;
+  bitmap_result.bitmap_data = data;
+  bitmap_result.pixel_size = size;
+  bitmap_result.icon_type = icon_type;
+  bitmap_result.icon_url = icon_url;
+  return bitmap_result;
+}
+
+std::vector<FaviconRawBitmapResult> CreateRawBitmapResults(
     const GURL& icon_url,
     favicon_base::IconType icon_type = FAVICON,
     bool expired = false) {
-  scoped_refptr<base::RefCountedBytes> data(new base::RefCountedBytes());
-  FillBitmap(gfx::kFaviconSize, gfx::kFaviconSize, &data->data());
-  FaviconRawBitmapResult bitmap_result;
-  bitmap_result.expired = expired;
-  bitmap_result.bitmap_data = data;
   // Use a pixel size other than (0,0) as (0,0) has a special meaning.
-  bitmap_result.pixel_size = gfx::Size(gfx::kFaviconSize, gfx::kFaviconSize);
-  bitmap_result.icon_type = icon_type;
-  bitmap_result.icon_url = icon_url;
-  return {bitmap_result};
+  return {
+      CreateRawBitmapResult(icon_url, icon_type, expired,
+                            gfx::Size(gfx::kFaviconSize, gfx::kFaviconSize),
+                            SK_ColorRED)};
 }
 
 // Fake that implements the calls to FaviconHalder::Delegate's DownloadImage(),
 // delegated to this class through MockDelegate.
 class FakeImageDownloader {
  public:
   struct Response {
     int http_status_code = 404;
     BitmapVector bitmaps;
     SizeVector original_bitmap_sizes;
@@ skipping 15 matching lines @@
       manual_callback_ = bound_callback;
     else
       base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE, bound_callback);
     return download_id;
   }
 
   void Add(const GURL& icon_url, const IntVector& sizes) {
     AddWithOriginalSizes(icon_url, sizes, sizes);
   }
 
+  void AddWithColors(const GURL& icon_url, const IntVector& sizes,
+                     const std::vector<SkColor>& colors) {
+    AddWithColorsAndOriginalSizes(icon_url, sizes, colors, sizes);
+  }
+
   void AddWithOriginalSizes(const GURL& icon_url,
                             const IntVector& sizes,
                             const IntVector& original_sizes) {
+    std::vector<SkColor> colors;
+    colors.resize(sizes.size(), SK_ColorRED);
+    AddWithColorsAndOriginalSizes(icon_url, sizes, colors, original_sizes);
+  }
+
+  void AddWithColorsAndOriginalSizes(const GURL& icon_url,
+                                     const IntVector& sizes,
+                                     const std::vector<SkColor>& colors,
+                                     const IntVector& original_sizes) {
+    DCHECK_EQ(sizes.size(), colors.size());
     DCHECK_EQ(sizes.size(), original_sizes.size());
     Response response;
     response.http_status_code = 200;
     response.original_bitmap_sizes = CreateSquareSizes(original_sizes);
-    response.bitmaps = CreateBitmaps(CreateSquareSizes(sizes));
+    response.bitmaps = CreateBitmaps(CreateSquareSizes(sizes), colors);
     responses_[icon_url] = response;
   }
 
   void AddError(const GURL& icon_url, int http_status_code) {
     Response response;
     response.http_status_code = http_status_code;
     responses_[icon_url] = response;
   }
 
   // Disables automatic callback for |url|. This is useful for emulating a
@@ skipping 81 matching lines @@
   }
 
   // Returns pending and completed database request URLs.
   const URLVector& db_requests() const { return db_requests_; }
 
   void ClearDbRequests() { db_requests_.clear(); }
 
   base::CancelableTaskTracker::TaskId GetFavicon(
       const GURL& icon_url,
       favicon_base::IconType icon_type,
-      int desired_size_in_dip,
+      const std::vector<int>& desired_sizes_in_pixel,
       const favicon_base::FaviconResultsCallback& callback,
       base::CancelableTaskTracker* tracker) {
     return GetFaviconForPageOrIconURL(icon_url, callback, tracker);
   }
 
   base::CancelableTaskTracker::TaskId GetFaviconForPageURL(
       const GURL& page_url,
       int icon_types,
-      int desired_size_in_dip,
+      const std::vector<int>& desired_sizes_in_pixel,
       const favicon_base::FaviconResultsCallback& callback,
       base::CancelableTaskTracker* tracker) {
     return GetFaviconForPageOrIconURL(page_url, callback, tracker);
   }
 
   base::CancelableTaskTracker::TaskId UpdateFaviconMappingsAndFetch(
       const GURL& page_url,
       const std::vector<GURL>& icon_urls,
       int icon_types,
-      int desired_size_in_dip,
+      const std::vector<int>& desired_sizes_in_pixel,
       const favicon_base::FaviconResultsCallback& callback,
       base::CancelableTaskTracker* tracker) {
     CHECK_EQ(1U, icon_urls.size()) << "Multi-icon lookup not implemented";
     return GetFaviconForPageOrIconURL(icon_urls.front(), callback, tracker);
   }
 
  private:
   base::CancelableTaskTracker::TaskId GetFaviconForPageOrIconURL(
       const GURL& page_or_icon_url,
       const favicon_base::FaviconResultsCallback& callback,
@@ skipping 34 matching lines @@
 
 class FaviconHandlerTest : public testing::Test {
  protected:
   const std::vector<gfx::Size> kEmptySizes;
 
   // Some known icons for which download will succeed.
   const GURL kPageURL = GURL("http://www.google.com");
   const GURL kIconURL10x10 = GURL("http://www.google.com/favicon10x10");
   const GURL kIconURL12x12 = GURL("http://www.google.com/favicon12x12");
   const GURL kIconURL16x16 = GURL("http://www.google.com/favicon16x16");
+  const GURL kIconURL32x32 = GURL("http://www.google.com/favicon32x32");
   const GURL kIconURL64x64 = GURL("http://www.google.com/favicon64x64");
 
   FaviconHandlerTest() {
     // Register various known icon URLs.
     delegate_.fake_downloader().Add(kIconURL10x10, IntVector{10});
     delegate_.fake_downloader().Add(kIconURL12x12, IntVector{12});
     delegate_.fake_downloader().Add(kIconURL16x16, IntVector{16});
+    delegate_.fake_downloader().Add(kIconURL32x32, IntVector{32});
     delegate_.fake_downloader().Add(kIconURL64x64, IntVector{64});
 
     // The score computed by SelectFaviconFrames() is dependent on the supported
     // scale factors of the platform. It is used for determining the goodness of
     // a downloaded bitmap in FaviconHandler::OnDidDownloadFavicon().
     // Force the values of the scale factors so that the tests produce the same
     // results on all platforms.
     scoped_set_supported_scale_factors_.reset(
         new ui::test::ScopedSetSupportedScaleFactors({ui::SCALE_FACTOR_100P}));
   }
@@ skipping 38 matching lines @@
   std::unique_ptr<ui::test::ScopedSetSupportedScaleFactors>
       scoped_set_supported_scale_factors_;
   testing::NiceMock<MockFaviconServiceWithFake> favicon_service_;
   testing::NiceMock<MockDelegate> delegate_;
 };
 
 TEST_F(FaviconHandlerTest, GetFaviconFromHistory) {
   const GURL kIconURL("http://www.google.com/favicon");
 
   favicon_service_.fake()->Store(kPageURL, kIconURL,
-                                 CreateRawBitmapResult(kIconURL));
+                                 CreateRawBitmapResults(kIconURL));
 
   // Shouldn't request to download icon.
   EXPECT_CALL(delegate_, OnFaviconUpdated(
                              kPageURL, FaviconDriverObserver::NON_TOUCH_16_DIP,
                              kIconURL, /*icon_url_changed=*/true, _));
 
   RunHandlerWithSimpleFaviconCandidates({kIconURL});
   EXPECT_THAT(delegate_.downloads(), IsEmpty());
 }
 
@@ skipping 66 matching lines @@
   base::RunLoop().RunUntilIdle();
 
   EXPECT_THAT(delegate_.downloads(), ElementsAre(kIconURL16x16));
 }
 
 // Test that the icon is redownloaded if the icon cached for the page URL
 // expired.
 TEST_F(FaviconHandlerTest, RedownloadExpiredPageUrlFavicon) {
   favicon_service_.fake()->Store(
       kPageURL, kIconURL16x16,
-      CreateRawBitmapResult(kIconURL16x16, FAVICON, /*expired=*/true));
+      CreateRawBitmapResults(kIconURL16x16, FAVICON, /*expired=*/true));
 
   // TODO(crbug.com/700811): It would be nice if we could check whether the two
   // OnFaviconUpdated() calls are called with different gfx::Images (as opposed
   // to calling OnFaviconUpdated() with the expired gfx::Image both times).
   EXPECT_CALL(delegate_, OnFaviconUpdated(_, _, kIconURL16x16, _, _))
       .Times(AtLeast(1));
   EXPECT_CALL(favicon_service_, SetFavicons(_, kIconURL16x16, _, _));
 
   RunHandlerWithSimpleFaviconCandidates({kIconURL16x16});
   // We know from the |kPageUrl| database request that |kIconURL16x16| has
@@ skipping 28 matching lines @@
 // Test that FaviconHandler process finishes when:
 // - The icon URL used by the page has changed.
 // AND
 // - FaviconService::GetFaviconForPageURL() callback returns before
 //   FaviconHandler::OnUpdateFaviconURL() is called.
 TEST_F(FaviconHandlerTest, UpdateAndDownloadFavicon) {
   const GURL kOldIconURL("http://www.google.com/old_favicon");
   const GURL kNewIconURL = kIconURL16x16;
 
   favicon_service_.fake()->Store(kPageURL, kOldIconURL,
-                                 CreateRawBitmapResult(kOldIconURL));
+                                 CreateRawBitmapResults(kOldIconURL));
 
   InSequence seq;
   EXPECT_CALL(delegate_, OnFaviconUpdated(_, _, kOldIconURL, _, _));
   EXPECT_CALL(favicon_service_, SetFavicons(_, kNewIconURL, _, _));
   EXPECT_CALL(delegate_, OnFaviconUpdated(_, _, kNewIconURL, _, _));
 
   RunHandlerWithSimpleFaviconCandidates({kNewIconURL});
   EXPECT_THAT(delegate_.downloads(), ElementsAre(kNewIconURL));
 }
 
@@ skipping 23 matching lines @@
 }
 
 // Test that no downloads are done if a user visits a page which changed its
 // favicon URL to a favicon URL which is already cached in the database.
 TEST_F(FaviconHandlerTest, UpdateFavicon) {
   const GURL kSomePreviousPageURL("https://www.google.com/previous");
   const GURL kIconURL("http://www.google.com/favicon");
   const GURL kNewIconURL("http://www.google.com/new_favicon");
 
   favicon_service_.fake()->Store(kPageURL, kIconURL,
-                                 CreateRawBitmapResult(kIconURL));
+                                 CreateRawBitmapResults(kIconURL));
   favicon_service_.fake()->Store(kSomePreviousPageURL, kNewIconURL,
-                                 CreateRawBitmapResult(kNewIconURL));
+                                 CreateRawBitmapResults(kNewIconURL));
 
   EXPECT_CALL(favicon_service_, SetFavicons(_, _, _, _)).Times(0);
 
   InSequence seq;
   EXPECT_CALL(delegate_, OnFaviconUpdated(_, _, kIconURL, _, _));
   EXPECT_CALL(delegate_, OnFaviconUpdated(_, _, kNewIconURL, _, _));
 
   RunHandlerWithSimpleFaviconCandidates({kNewIconURL});
   ASSERT_THAT(favicon_service_.fake()->db_requests(),
               ElementsAre(kPageURL, kNewIconURL));
   EXPECT_THAT(delegate_.downloads(), IsEmpty());
 }
 
 TEST_F(FaviconHandlerTest, Download2ndFaviconURLCandidate) {
   const GURL kIconURLReturning500("http://www.google.com/500.png");
   delegate_.fake_downloader().AddError(kIconURLReturning500, 500);
 
   favicon_service_.fake()->Store(
       kPageURL, kIconURL64x64,
-      CreateRawBitmapResult(kIconURL64x64, TOUCH_ICON,
-                            /*expired=*/true));
+      CreateRawBitmapResults(kIconURL64x64, TOUCH_ICON,
+                             /*expired=*/true));
 
   EXPECT_CALL(delegate_,
               OnFaviconUpdated(kPageURL, FaviconDriverObserver::TOUCH_LARGEST,
                                kIconURL64x64, /*icon_url_changed=*/true, _));
   EXPECT_CALL(delegate_,
               OnFaviconUpdated(kPageURL, FaviconDriverObserver::TOUCH_LARGEST,
                                kIconURL64x64, /*icon_url_changed=*/false, _));
 
   RunHandlerWithCandidates(
       FaviconDriverObserver::TOUCH_LARGEST,
@@ skipping 211 matching lines @@
 // a little are preferred over huge favicons.
 TEST_F(FaviconHandlerMultipleFaviconsTest,
        ChooseMinorDownsamplingOverHugeIcon) {
   EXPECT_EQ(48, DownloadTillDoneIgnoringHistory(IntVector{256, 48}));
 }
 
 TEST_F(FaviconHandlerMultipleFaviconsTest, ChooseMinorUpsamplingOverHugeIcon) {
   EXPECT_EQ(17, DownloadTillDoneIgnoringHistory(IntVector{17, 256}));
 }
 
+// Test that the FaviconHandler process finishes when the database provides
+// a favicon with multiple bitmaps.
+TEST_F(FaviconHandlerMultipleFaviconsTest,
+       GetFaviconFromHistoryWithMultipleBitmaps) {
+  const GURL kIconURL32_16("http://www.google.com/a");
+  ui::test::ScopedSetSupportedScaleFactors({ui::SCALE_FACTOR_100P, ui::SCALE_FACTOR_200P});
+
+  favicon_service_.fake()->Store(
+      kPageURL, kIconURL32_16,
+      {CreateRawBitmapResult(kIconURL32_16, FAVICON, /*expired=*/false,
+                             gfx::Size(32, 32), SK_ColorGREEN),
+       CreateRawBitmapResult(kIconURL32_16, FAVICON, /*expired=*/false,
+                             gfx::Size(16, 16), SK_ColorBLUE)});
+
+  // Set 16x16 bitmap is needed for sync purposes.
+  EXPECT_CALL(delegate_, OnFaviconUpdated(
+                             _, _, _, _,
+                             AllOf(ImageSizeIs(16, 16),
+                                   ImageColorForScaleIs(1.0f, SK_ColorBLUE),
+                                   ImageColorForScaleIs(2.0f, SK_ColorGREEN))));
+
+  RunHandlerWithSimpleFaviconCandidates({kIconURL32_16});
+}
+
+// Test that the FaviconHandler process finishes when:
+// - There is no data in the database for neither the page URL nor the icon URL.
+// - FaviconService::GetFaviconForPageURL() callback returns before
+//   FaviconHandler::OnUpdateFaviconURL() is called.
+// AND
+// - The page provides a favicon with multiple bitmaps.
+TEST_F(FaviconHandlerMultipleFaviconsTest,
+       DownloadUnknownFaviconWithMultipleBitmaps) {
+  const GURL kIconURL32_16("http://www.google.com/a");
+
+  delegate_.fake_downloader().AddWithColors(
+      kIconURL32_16, IntVector{32, 16}, {SK_ColorGREEN, SK_ColorBLUE});
+
+  // Set 16x16 bitmap is needed for sync purposes.
+  EXPECT_CALL(delegate_, OnFaviconUpdated(_, _, _, _,
+                             AllOf(ImageSizeIs(16, 16),
+                                   ImageColorForScaleIs(1.0f, SK_ColorBLUE),
+                                   ImageColorForScaleIs(2.0f, SK_ColorGREEN))));
+  EXPECT_CALL(favicon_service_,
+              SetFavicons(kPageURL, kIconURL32_16, FAVICON,
+                             AllOf(ImageSizeIs(16, 16),
+                                   ImageColorForScaleIs(1.0f, SK_ColorBLUE),
+                                   ImageColorForScaleIs(2.0f, SK_ColorGREEN))));
+
+  RunHandlerWithSimpleFaviconCandidates({kIconURL32_16});
+}
+
 // Test that the best favicon is selected when:
 // - The page provides several favicons.
 // - Downloading one of the page's icon URLs previously returned a 404.
 // - None of the favicons are cached in the Favicons database.
 TEST_F(FaviconHandlerTest, MultipleFavicons404) {
   ON_CALL(favicon_service_, WasUnableToDownloadFavicon(kIconURL16x16))
       .WillByDefault(Return(true));
 
   EXPECT_CALL(delegate_, OnFaviconUpdated(_, _, kIconURL64x64, _, _));
   RunHandlerWithSimpleFaviconCandidates({kIconURL16x16, kIconURL64x64});
@@ skipping 34 matching lines @@
 TEST_F(FaviconHandlerTest, FaviconInvalidURL) {
   const GURL kInvalidFormatURL("invalid");
   ASSERT_TRUE(kInvalidFormatURL.is_empty());
 
   EXPECT_CALL(delegate_, OnFaviconUpdated(_, _, _, _, _)).Times(0);
 
   RunHandlerWithSimpleFaviconCandidates({kInvalidFormatURL});
   EXPECT_THAT(delegate_.downloads(), IsEmpty());
 }
 
-TEST_F(FaviconHandlerTest, TestSortFavicon) {
+TEST_F(FaviconHandlerTest, TestSortFavicon16DIP) {
   const std::vector<favicon::FaviconURL> kSourceIconURLs{
       FaviconURL(GURL("http://www.google.com/a"), FAVICON,
                  {gfx::Size(1, 1), gfx::Size(17, 17)}),
       FaviconURL(GURL("http://www.google.com/b"), FAVICON,
                  {gfx::Size(1024, 1024), gfx::Size(512, 512)}),
       FaviconURL(GURL("http://www.google.com/c"), FAVICON,
                  {gfx::Size(16, 16), gfx::Size(14, 14)}),
       FaviconURL(GURL("http://www.google.com/d"), FAVICON, kEmptySizes),
-      FaviconURL(GURL("http://www.google.com/e"), FAVICON, kEmptySizes)};
+      FaviconURL(GURL("http://www.google.com/e"), FAVICON, kEmptySizes),
+      FaviconURL(GURL("http://www.google.com/f"), FAVICON,
+                 {gfx::Size(10, 10), gfx::Size(400, 400)})};
+
+  std::unique_ptr<FaviconHandler> handler = RunHandlerWithCandidates(
+      FaviconDriverObserver::NON_TOUCH_16_DIP, kSourceIconURLs);
+
+  EXPECT_THAT(handler->image_urls(),
+              ElementsAre(HasIconURL("http://www.google.com/c"),
+                          HasIconURL("http://www.google.com/a"),
+                          HasIconURL("http://www.google.com/f"),
+                          HasIconURL("http://www.google.com/b"),
+                          // The rest of bitmaps come in order, there is no
+                          // sizes attribute.
+                          HasIconURL("http://www.google.com/d"),
+                          HasIconURL("http://www.google.com/e")));
+}
+
+TEST_F(FaviconHandlerTest, TestSortFaviconLargest) {
+  const std::vector<favicon::FaviconURL> kSourceIconURLs{
+      FaviconURL(GURL("http://www.google.com/a"), FAVICON,
+                 {gfx::Size(1, 1), gfx::Size(17, 17)}),
+      FaviconURL(GURL("http://www.google.com/b"), FAVICON,
+                 {gfx::Size(1024, 1024), gfx::Size(512, 512)}),
+      FaviconURL(GURL("http://www.google.com/c"), FAVICON,
+                 {gfx::Size(16, 16), gfx::Size(14, 14)}),
+      FaviconURL(GURL("http://www.google.com/d"), FAVICON, kEmptySizes),
+      FaviconURL(GURL("http://www.google.com/e"), FAVICON, kEmptySizes),
+      FaviconURL(GURL("http://www.google.com/f"), FAVICON,
+                 {gfx::Size(10, 10), gfx::Size(400, 400)})};
 
   std::unique_ptr<FaviconHandler> handler = RunHandlerWithCandidates(
       FaviconDriverObserver::NON_TOUCH_LARGEST, kSourceIconURLs);
 
-  struct ExpectedResult {
-    // The favicon's index in kSourceIconURLs.
-    size_t favicon_index;
-    // Width of largest bitmap.
-    int width;
-  } results[] = {
-      // First is icon2, though its size larger than maximal.
-      {1, 1024},
-      // Second is icon1
-      // The 17x17 is largest.
-      {0, 17},
-      // Third is icon3 though it has same size as icon1.
-      // The 16x16 is largest.
-      {2, 16},
-      // The rest of bitmaps come in order, there is no sizes attribute.
-      {3, -1},
-      {4, -1},
-  };
-  const std::vector<FaviconURL>& icons = handler->image_urls();
-  ASSERT_EQ(5u, icons.size());
-  for (size_t i = 0; i < icons.size(); ++i) {
-    EXPECT_EQ(kSourceIconURLs[results[i].favicon_index].icon_url,
-              icons[i].icon_url);
-    if (results[i].width != -1)
-      EXPECT_EQ(results[i].width, icons[i].icon_sizes[0].width());
-  }
+  EXPECT_THAT(handler->image_urls(),
+              ElementsAre(HasIconURL("http://www.google.com/f"),
+                          HasIconURL("http://www.google.com/b"),
+                          HasIconURL("http://www.google.com/a"),
+                          HasIconURL("http://www.google.com/c"),
+                          // The rest of bitmaps come in order, there is no
+                          // sizes attribute.
+                          HasIconURL("http://www.google.com/d"),
+                          HasIconURL("http://www.google.com/e")));
+}
+
+TEST_F(FaviconHandlerTest, DownloadNoMoreAfterExactMatch) {
+  RunHandlerWithSimpleFaviconCandidates({kIconURL16x16, kIconURL64x64});
+  EXPECT_THAT(delegate_.downloads(), ElementsAre(kIconURL16x16));
+}
+
+TEST_F(FaviconHandlerTest, DownloadExactMatchFirstIfSizesProvided) {
+  RunHandlerWithCandidates(
+      FaviconDriverObserver::NON_TOUCH_16_DIP,
+      {
+          FaviconURL(kIconURL64x64, FAVICON, SizeVector{gfx::Size(64, 64)}),
+          FaviconURL(kIconURL16x16, FAVICON, SizeVector{gfx::Size(16, 16)}),
+      });
+  EXPECT_THAT(delegate_.downloads(), ElementsAre(kIconURL16x16));
+}
+
+TEST_F(FaviconHandlerTest, DownloadLargeButNotAboveLimit) {
+  const GURL kIconURL192x192 = GURL("http://www.google.com/favicon192x192");
+  const GURL kIconURL512x512 = GURL("http://www.google.com/favicon512x512");
+
+  delegate_.fake_downloader().Add(kIconURL192x192, IntVector{192});
+  delegate_.fake_downloader().Add(kIconURL512x512, IntVector{512});
+
+  RunHandlerWithCandidates(
+      FaviconDriverObserver::NON_TOUCH_LARGEST,
+      {
+          FaviconURL(kIconURL512x512, FAVICON, SizeVector{gfx::Size(512, 512)}),
+          FaviconURL(kIconURL192x192, FAVICON, SizeVector{gfx::Size(192, 192)}),
+      });
+
+  EXPECT_THAT(delegate_.downloads(), ElementsAre(kIconURL192x192));
+}
+
+// Test that the high-resolution favicon is selected when:
+// - The page provides several favicons.
+// - The low-resolution one is cached in the Favicons database (e.g. was written
+//   via sync).
+// AND
+// - The high-resolution candidate is the best match.
+TEST_F(FaviconHandlerTest, DownloadHighResolutionWhenOnlyLowResolutionCached) {
+  favicon_service_.fake()->Store(kPageURL, kIconURL16x16,
+                                 CreateRawBitmapResults(kIconURL16x16, FAVICON,
+                                                        /*expired=*/false));
+
+  EXPECT_CALL(delegate_, OnFaviconUpdated(_, _, kIconURL32x32, _, _));
+
+  RunHandlerWithCandidates(
+      FaviconDriverObserver::NON_TOUCH_LARGEST,
+      {
+          FaviconURL(kIconURL16x16, FAVICON, SizeVector{gfx::Size(16, 16)}),
+          FaviconURL(kIconURL32x32, FAVICON, SizeVector{gfx::Size(32, 32)}),
+      });
+  EXPECT_THAT(delegate_.downloads(), ElementsAre(kIconURL32x32));
 }
 
 TEST_F(FaviconHandlerTest, TestDownloadLargestFavicon) {
   // Names represent the bitmap sizes per icon.
   const GURL kIconURL1024_512("http://www.google.com/a");
   const GURL kIconURL15_14("http://www.google.com/b");
   const GURL kIconURL16_512("http://www.google.com/c");
   const GURL kIconURLWithoutSize1("http://www.google.com/d");
   const GURL kIconURLWithoutSize2("http://www.google.com/e");
 
@@ skipping 33 matching lines @@
 
   RunHandlerWithCandidates(
       FaviconDriverObserver::NON_TOUCH_LARGEST,
       {FaviconURL(kIconURL1, FAVICON, {gfx::Size(15, 15)}),
        FaviconURL(kIconURL2, FAVICON, {gfx::Size(14, 14), gfx::Size(16, 16)})});
 
   EXPECT_THAT(delegate_.downloads(), ElementsAre(kIconURL2));
 }
 
 TEST_F(FaviconHandlerTest, TestFaviconWasScaledAfterDownload) {
-  const int kMaximalSize = FaviconHandler::GetMaximalIconSize(FAVICON);
+#if defined(OS_IOS)
+  const int kMaximalSize = 144;
+#else
+  const int kMaximalSize = 192;
+#endif
 
   const GURL kIconURL1("http://www.google.com/b");
   const GURL kIconURL2("http://www.google.com/c");
 
   const int kOriginalSize1 = kMaximalSize + 1;
   const int kOriginalSize2 = kMaximalSize + 2;
 
   delegate_.fake_downloader().AddWithOriginalSizes(
       kIconURL1, IntVector{kMaximalSize}, IntVector{kOriginalSize1});
   delegate_.fake_downloader().AddWithOriginalSizes(
@@ skipping 24 matching lines @@
 
   RunHandlerWithCandidates(
       FaviconDriverObserver::NON_TOUCH_LARGEST,
       {FaviconURL(kIconURL10x10, FAVICON, SizeVector{gfx::Size(16, 16)}),
        FaviconURL(kIconURL12x12, FAVICON, SizeVector{gfx::Size(15, 15)}),
        FaviconURL(kIconURL16x16, FAVICON, kEmptySizes)});
 }
 
 }  // namespace
 }  // namespace favicon