manifest: rework icon selector to include small icon cut-off

chrome/browser/manifest/manifest_icon_selector_unittest.cc

 // Copyright 2015 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 "chrome/browser/manifest/manifest_icon_selector.h"
 
 #include <string>
 #include <vector>
 
 #include "base/strings/utf_string_conversions.h"
@@ skipping 65 matching lines @@
       CreateIcon("http://foo.com/icon.png", "", 1.0, std::vector<gfx::Size>()));
 
   GURL url = FindBestMatchingIcon(icons);
   EXPECT_TRUE(url.is_empty());
 }
 
 TEST_F(ManifestIconSelectorTest, MIMETypeFiltering) {
   // Icons with type specified to a MIME type that isn't a valid image MIME type
   // are ignored.
   std::vector<gfx::Size> sizes;
-  sizes.push_back(gfx::Size(10, 10));
+  sizes.push_back(gfx::Size(1024, 1024));
 
   std::vector<content::Manifest::Icon> icons;
   icons.push_back(
       CreateIcon("http://foo.com/icon.png", "image/foo_bar", 1.0, sizes));
   icons.push_back(CreateIcon("http://foo.com/icon.png", "image/", 1.0, sizes));
   icons.push_back(CreateIcon("http://foo.com/icon.png", "image/", 1.0, sizes));
   icons.push_back(
       CreateIcon("http://foo.com/icon.png", "video/mp4", 1.0, sizes));
 
   GURL url = FindBestMatchingIcon(icons);
@@ skipping 47 matching lines @@
   EXPECT_EQ("http://foo.com/icon_x2.png", url.spec());
 
   SetDisplayDeviceScaleFactor(3.0f);
   url = FindBestMatchingIcon(icons);
   EXPECT_EQ("http://foo.com/icon_x3.png", url.spec());
 }
 
 TEST_F(ManifestIconSelectorTest, PreferredSizeOfDefaultDensityIsUsedSecond) {
   // This test has three icons. The first one is of density zero and is marked
   // with three sizes which are the preferred icon size for density 1, 2 and 3.
-  // The icon for density 2 and 3 have a size set to 2x2 and 3x3.
+  // The icon for density 2 and 3 have a size set to 1024x1024.
   // Regardless of the device scale factor, the icon of density 1 is going to be
   // used because it matches the preferred size.
   std::vector<gfx::Size> sizes_1;
   sizes_1.push_back(gfx::Size(GetPreferredIconSizeInDp(),
                               GetPreferredIconSizeInDp()));
   sizes_1.push_back(gfx::Size(GetPreferredIconSizeInDp() * 2,
                               GetPreferredIconSizeInDp() * 2));
   sizes_1.push_back(gfx::Size(GetPreferredIconSizeInDp() * 3,
                               GetPreferredIconSizeInDp() * 3));
 
   std::vector<gfx::Size> sizes_2;
-  sizes_2.push_back(gfx::Size(2, 2));
+  sizes_2.push_back(gfx::Size(1024, 1024));
 
   std::vector<gfx::Size> sizes_3;
-  sizes_3.push_back(gfx::Size(3, 3));
+  sizes_3.push_back(gfx::Size(1024, 1024));
 
   std::vector<content::Manifest::Icon> icons;
   icons.push_back(CreateIcon("http://foo.com/icon_x1.png", "", 1.0, sizes_1));
   icons.push_back(CreateIcon("http://foo.com/icon_x2.png", "", 2.0, sizes_2));
   icons.push_back(CreateIcon("http://foo.com/icon_x3.png", "", 3.0, sizes_3));
 
   SetDisplayDeviceScaleFactor(1.0f);
   GURL url = FindBestMatchingIcon(icons);
   EXPECT_EQ("http://foo.com/icon_x1.png", url.spec());
 
   SetDisplayDeviceScaleFactor(2.0f);
   url = FindBestMatchingIcon(icons);
   EXPECT_EQ("http://foo.com/icon_x1.png", url.spec());
 
   SetDisplayDeviceScaleFactor(3.0f);
   url = FindBestMatchingIcon(icons);
   EXPECT_EQ("http://foo.com/icon_x1.png", url.spec());
 }
 
 TEST_F(ManifestIconSelectorTest, DeviceDensityFirst) {
   // If there is no perfect icon but an icon of the current device density is
   // present, it will be picked.
   // This test has three icons each are marked with sizes set to the preferred
   // icon size for the associated density.
   std::vector<gfx::Size> sizes;
-  sizes.push_back(gfx::Size(2, 2));
+  sizes.push_back(gfx::Size(1024, 1024));
 
   std::vector<content::Manifest::Icon> icons;
   icons.push_back(CreateIcon("http://foo.com/icon_x1.png", "", 1.0, sizes));
   icons.push_back(CreateIcon("http://foo.com/icon_x2.png", "", 2.0, sizes));
   icons.push_back(CreateIcon("http://foo.com/icon_x3.png", "", 3.0, sizes));
 
   SetDisplayDeviceScaleFactor(1.0f);
   GURL url = FindBestMatchingIcon(icons);
   EXPECT_EQ("http://foo.com/icon_x1.png", url.spec());
 
   SetDisplayDeviceScaleFactor(2.0f);
   url = FindBestMatchingIcon(icons);
   EXPECT_EQ("http://foo.com/icon_x2.png", url.spec());
 
   SetDisplayDeviceScaleFactor(3.0f);
   url = FindBestMatchingIcon(icons);
   EXPECT_EQ("http://foo.com/icon_x3.png", url.spec());
 }
 
 TEST_F(ManifestIconSelectorTest, DeviceDensityFallback) {
   // If there is no perfect icon but and no icon of the current display density,
   // an icon of density 1.0 will be used.
   std::vector<gfx::Size> sizes;
-  sizes.push_back(gfx::Size(2, 2));
+  sizes.push_back(gfx::Size(1024, 1024));
 
   std::vector<content::Manifest::Icon> icons;
   icons.push_back(CreateIcon("http://foo.com/icon_x1.png", "", 1.0, sizes));
   icons.push_back(CreateIcon("http://foo.com/icon_x2.png", "", 2.0, sizes));
 
   SetDisplayDeviceScaleFactor(3.0f);
   GURL url = FindBestMatchingIcon(icons);
   EXPECT_EQ("http://foo.com/icon_x1.png", url.spec());
 }
 
+TEST_F(ManifestIconSelectorTest, DeviceDensityMatchRejectsTooSmall) {
+  // If we have to resort to density matching to find icons, then an icon of
+  // the correct size has not been found. Make sure that an icon which is just
+  // slightly smaller than one density bucket below the device is not chosen
+  // even if the density matches.
+  std::vector<gfx::Size> sizes_1_2;
+  std::vector<gfx::Size> sizes_3;
+
+  sizes_1_2.push_back(gfx::Size(47, 47));
+  sizes_3.push_back(gfx::Size(95, 95));
+
+  std::vector<content::Manifest::Icon> icons;
+  icons.push_back(CreateIcon("http://foo.com/icon_x1.png", "", 1.0, sizes_1_2));
+  icons.push_back(CreateIcon("http://foo.com/icon_x2.png", "", 2.0, sizes_1_2));
+  icons.push_back(CreateIcon("http://foo.com/icon_x3.png", "", 3.0, sizes_3));
+
+  SetDisplayDeviceScaleFactor(1.0f);

[mlamouri (slow - plz ping), 2015/08/20 22:18:18]

nit: add comments explaining why nothing matches. It's not trivial.

[Lalit Maganti, 2015/08/21 10:46:37]

Done.

+  GURL url = FindBestMatchingIcon(icons);
+  EXPECT_TRUE(url.is_empty());
+
+  SetDisplayDeviceScaleFactor(2.0f);

[mlamouri (slow - plz ping), 2015/08/20 22:18:18]

ditto

[Lalit Maganti, 2015/08/21 10:46:37]

Done.

+  url = FindBestMatchingIcon(icons);
+  EXPECT_TRUE(url.is_empty());
+
+  SetDisplayDeviceScaleFactor(3.0f);

[mlamouri (slow - plz ping), 2015/08/20 22:18:18]

ditto

[Lalit Maganti, 2015/08/21 10:46:37]

Done.

+  url = FindBestMatchingIcon(icons);
+  EXPECT_TRUE(url.is_empty());
+}
+
 TEST_F(ManifestIconSelectorTest, DoNotUseOtherDensities) {
   // If there are only icons of densities that are not the current display
   // density or the default density, they are ignored.
   std::vector<gfx::Size> sizes;
-  sizes.push_back(gfx::Size(2, 2));
+  sizes.push_back(gfx::Size(1024, 1024));
 
   std::vector<content::Manifest::Icon> icons;
   icons.push_back(CreateIcon("http://foo.com/icon_x2.png", "", 2.0, sizes));
 
   SetDisplayDeviceScaleFactor(3.0f);
   GURL url = FindBestMatchingIcon(icons);
   EXPECT_TRUE(url.is_empty());
 }
 
 TEST_F(ManifestIconSelectorTest, NotSquareIconsAreIgnored) {
   std::vector<gfx::Size> sizes;
-  sizes.push_back(gfx::Size(20, 2));
+  sizes.push_back(gfx::Size(1024, 1023));
 
   std::vector<content::Manifest::Icon> icons;
   icons.push_back(CreateIcon("http://foo.com/icon.png", "", 1.0, sizes));
 
   GURL url = FindBestMatchingIcon(icons);
   EXPECT_TRUE(url.is_empty());
 }
 
 TEST_F(ManifestIconSelectorTest, ClosestIconToPreferred) {
   // This test verifies ManifestIconSelector::FindBestMatchingIcon by passing
   // different icon sizes and checking which one is picked.
   // The Device Scale Factor is 1.0 and the preferred icon size is returned by
   // GetPreferredIconSizeInDp().
   int very_small = GetPreferredIconSizeInDp() / 4;
   int small_size = GetPreferredIconSizeInDp() / 2;
   int bit_small = GetPreferredIconSizeInDp() - 1;
   int bit_big = GetPreferredIconSizeInDp() + 1;
   int big = GetPreferredIconSizeInDp() * 2;
   int very_big = GetPreferredIconSizeInDp() * 4;
 
-  // (very_small, bit_small) => bit_small
+  // (very_small, bit_small) => empty (since both are too small)
   {
     std::vector<gfx::Size> sizes_1;
     sizes_1.push_back(gfx::Size(very_small, very_small));
 
     std::vector<gfx::Size> sizes_2;
     sizes_2.push_back(gfx::Size(bit_small, bit_small));
 
     std::vector<content::Manifest::Icon> icons;
     icons.push_back(CreateIcon("http://foo.com/icon_no.png", "", 1.0, sizes_1));
     icons.push_back(CreateIcon("http://foo.com/icon.png", "", 1.0, sizes_2));
 
     GURL url = FindBestMatchingIcon(icons);
-    EXPECT_EQ("http://foo.com/icon.png", url.spec());
+    EXPECT_EQ("", url.spec());
   }
 
-  // (very_small, bit_small, smaller) => bit_small
+  // (very_small, bit_small, smaller) => empty (since both are too small)
   {
     std::vector<gfx::Size> sizes_1;
     sizes_1.push_back(gfx::Size(very_small, very_small));
 
     std::vector<gfx::Size> sizes_2;
     sizes_2.push_back(gfx::Size(bit_small, bit_small));
 
     std::vector<gfx::Size> sizes_3;
     sizes_3.push_back(gfx::Size(small_size, small_size));
 
     std::vector<content::Manifest::Icon> icons;
     icons.push_back(CreateIcon("http://foo.com/icon_no.png", "", 1.0, sizes_1));
     icons.push_back(CreateIcon("http://foo.com/icon.png", "", 1.0, sizes_2));
     icons.push_back(CreateIcon("http://foo.com/icon_no.png", "", 1.0, sizes_3));
 
     GURL url = FindBestMatchingIcon(icons);
-    EXPECT_EQ("http://foo.com/icon.png", url.spec());
+    EXPECT_EQ("", url.spec());
   }
 
   // (very_big, big) => big
   {
     std::vector<gfx::Size> sizes_1;
     sizes_1.push_back(gfx::Size(very_big, very_big));
 
     std::vector<gfx::Size> sizes_2;
     sizes_2.push_back(gfx::Size(big, big));
 
@@ skipping 102 matching lines @@
 
     std::vector<content::Manifest::Icon> icons;
     icons.push_back(CreateIcon("http://foo.com/icon_no.png", "", 1.0, sizes));
     icons.push_back(CreateIcon("http://foo.com/icon.png", "", 3.0, sizes));
 
     SetDisplayDeviceScaleFactor(3.0f);
     GURL url = FindBestMatchingIcon(icons);
     EXPECT_EQ("http://foo.com/icon.png", url.spec());
   }
 }