Always prefer 192x192 on mobile, also for touch icons

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 1192 matching lines @@
 
   EXPECT_THAT(
       handler->GetIconURLs(),
       ElementsAre(
           // The 512x512 bitmap is the best match for the desired size.
           kIconURL1024_512, kIconURL1_17, kIconURL16_14,
           // The rest of bitmaps come in order, there is no "sizes" attribute.
           kIconURLWithoutSize1, kIconURLWithoutSize2));
 }
 
+TEST_F(FaviconHandlerTest, TestSortTouchIconLargest) {
+  const GURL kIconURLWithoutSize("http://www.google.com/touchicon-nosize");
+  const GURL kIconURL144x144("http://www.google.com/touchicon144x144");
+  const GURL kIconURL192x192("http://www.google.com/touchicon192x192");

[noyau (Ping after 24h), 2017/07/04 15:21:00]

I'm trying to decipher these unit tests and at the end of it I just spend 5
minutes figuring out that those URL are never fetched. It would be way clearer
if those URL where guaranteed to fail, ether by targeting example.com, or some
custom scheme. Or both. Actually, in this file, none of the URLs should be
routable.

[mastiz, 2017/07/04 15:36:38]

I can totally imagine the pain of parsing this big file, but wrt to actual
fetches and URLs being routable, it seems in a reasonable state to me
considering there's no actual URL fetcher involved (the icon fetcher is itself a
test double).

Having said that, I'm not against adopting example.com (and in less degree a
custom scheme) if pkotwicz@ agrees, but in any case it'd be outside the scope of
this patch. Peter?

[noyau (Ping after 24h), 2017/07/05 07:28:32]

Agreed about outside of this patch.

+
+  const std::vector<favicon::FaviconURL> kSourceIconURLs{
+      FaviconURL(kIconURLWithoutSize, TOUCH_ICON, kEmptySizes),
+      FaviconURL(kIconURL144x144, TOUCH_ICON,
+                 SizeVector(1U, gfx::Size(144, 144))),
+      FaviconURL(kIconURL192x192, TOUCH_ICON,
+                 SizeVector(1U, gfx::Size(192, 192))),
+  };
+
+  std::unique_ptr<FaviconHandler> handler = RunHandlerWithCandidates(
+      FaviconDriverObserver::TOUCH_LARGEST, kSourceIconURLs);
+
+  EXPECT_THAT(
+      handler->GetIconURLs(),
+      ElementsAre(kIconURL192x192, kIconURL144x144, kIconURLWithoutSize));
+}
+
 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");
 
   RunHandlerWithCandidates(
       FaviconDriverObserver::NON_TOUCH_LARGEST,
@@ skipping 766 matching lines @@
 
   // Complete the download.
   EXPECT_CALL(favicon_service_, SetFavicons(_, kManifestURL2, _, _));
   EXPECT_CALL(delegate_, OnFaviconUpdated(_, _, kManifestURL2, _, _));
   EXPECT_TRUE(delegate_.fake_image_downloader().RunCallbackManually());
   base::RunLoop().RunUntilIdle();
 }
 
 }  // namespace
 }  // namespace favicon