Make FaviconService::GetRawFaviconForPageURL() select the best candidate among all the icon types

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 <map>
 #include <memory>
@@ skipping 243 matching lines @@
       const GURL& page_url,
       int icon_types,
       int desired_size_in_dip,
       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,
+      const GURL& icon_url,
+      favicon_base::IconType icon_type,

[mastiz, 2017/04/18 08:04:34]

This signature change is orthogonal. Can you please split it to a dedicated CL?
That part should be trivial so we can have a smaller diff for your CL that
actually changed behavior.

[pkotwicz, 2017/04/20 04:06:19]

Done.

       int desired_size_in_dip,
       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);
+    return GetFaviconForPageOrIconURL(icon_url, callback, tracker);
   }
 
  private:
   base::CancelableTaskTracker::TaskId GetFaviconForPageOrIconURL(
       const GURL& page_or_icon_url,
       const favicon_base::FaviconResultsCallback& callback,
       base::CancelableTaskTracker* tracker) {
     db_requests_.push_back(page_or_icon_url);
 
     return tracker->PostTask(base::ThreadTaskRunnerHandle::Get().get(),
@@ skipping 110 matching lines @@
                              kPageURL, FaviconDriverObserver::NON_TOUCH_16_DIP,
                              kIconURL, /*icon_url_changed=*/true, _));
 
   RunHandlerWithSimpleFaviconCandidates({kIconURL});
   EXPECT_THAT(delegate_.downloads(), IsEmpty());
 }
 
 // Test that UpdateFaviconsAndFetch() is called with the appropriate parameters
 // when there is data in the database for neither the page URL nor the icon URL.
 TEST_F(FaviconHandlerTest, UpdateFaviconMappingsAndFetch) {
-  EXPECT_CALL(favicon_service_, UpdateFaviconMappingsAndFetch(
-                                    kPageURL, URLVector{kIconURL16x16}, FAVICON,
-                                    /*desired_size_in_dip=*/16, _, _));
+  EXPECT_CALL(favicon_service_,
+              UpdateFaviconMappingsAndFetch(kPageURL, kIconURL16x16, FAVICON,
+                                            /*desired_size_in_dip=*/16, _, _));
 
   RunHandlerWithSimpleFaviconCandidates({kIconURL16x16});
 }
 
 // Test that the FaviconHandler process finishes when:
 // - There is data in the database for neither the page URL nor the icon URL.
 // AND
 // - FaviconService::GetFaviconForPageURL() callback returns before
 //   FaviconHandler::OnUpdateFaviconURL() is called.
 TEST_F(FaviconHandlerTest, DownloadUnknownFaviconIfCandidatesSlower) {
@@ skipping 612 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