Make some updates to extension iconography.

chrome/browser/extensions/extension_icon_manager_unittest.cc

Index: chrome/browser/extensions/extension_icon_manager_unittest.cc
diff --git a/chrome/browser/extensions/extension_icon_manager_unittest.cc b/chrome/browser/extensions/extension_icon_manager_unittest.cc
index d474e5c7e6d83e8d3305975fb9af8f0f567a5a62..d522187fbe6d7278661d9cf3e870b2494e70ad57 100644
--- a/chrome/browser/extensions/extension_icon_manager_unittest.cc
+++ b/chrome/browser/extensions/extension_icon_manager_unittest.cc
@@ -16,11 +16,17 @@
 #include "content/public/test/test_browser_thread.h"
 #include "extensions/common/extension.h"
 #include "testing/gtest/include/gtest/gtest.h"
+#include "ui/base/layout.h"
+#include "ui/gfx/favicon_size.h"
+#include "ui/gfx/image/image.h"
+#include "ui/gfx/image/image_skia.h"
+#include "ui/gfx/image/image_unittest_util.h"
 #include "ui/gfx/skia_util.h"
 
+namespace extensions {
+namespace {
+
 using content::BrowserThread;
-using extensions::Extension;
-using extensions::Manifest;
 
 // Our test class that takes care of managing the necessary threads for loading
 // extension icons, and waiting for those loads to happen.
@@ -95,7 +101,7 @@ class TestIconManager : public ExtensionIconManager {
 
 // Returns the default icon that ExtensionIconManager gives when an extension
 // doesn't have an icon.
-SkBitmap GetDefaultIcon() {
+gfx::Image GetDefaultIcon() {
   std::string dummy_id = crx_file::id_util::GenerateId("whatever");
   ExtensionIconManager manager;
   return manager.GetIcon(dummy_id);
@@ -104,7 +110,7 @@ SkBitmap GetDefaultIcon() {
 // Tests loading an icon for an extension, removing it, then re-loading it.
 TEST_F(ExtensionIconManagerTest, LoadRemoveLoad) {
   std::unique_ptr<Profile> profile(new TestingProfile());
-  SkBitmap default_icon = GetDefaultIcon();
+  gfx::Image default_icon = GetDefaultIcon();
 
   base::FilePath test_dir;
   ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &test_dir));
@@ -123,11 +129,11 @@ TEST_F(ExtensionIconManagerTest, LoadRemoveLoad) {
   ASSERT_TRUE(extension.get());
   TestIconManager icon_manager(this);
 
-  // Load the icon and grab the bitmap.
+  // Load the icon.
   icon_manager.LoadIcon(profile.get(), extension.get());
   WaitForImageLoad();
-  SkBitmap first_icon = icon_manager.GetIcon(extension->id());
-  EXPECT_FALSE(gfx::BitmapsAreEqual(first_icon, default_icon));
+  gfx::Image first_icon = icon_manager.GetIcon(extension->id());
+  EXPECT_FALSE(gfx::test::AreImagesEqual(first_icon, default_icon));
 
   // Remove the icon from the manager.
   icon_manager.RemoveIcon(extension->id());
@@ -136,17 +142,17 @@ TEST_F(ExtensionIconManagerTest, LoadRemoveLoad) {
   // default icon).
   icon_manager.LoadIcon(profile.get(), extension.get());
   WaitForImageLoad();
-  SkBitmap second_icon = icon_manager.GetIcon(extension->id());
-  EXPECT_FALSE(gfx::BitmapsAreEqual(second_icon, default_icon));
+  gfx::Image second_icon = icon_manager.GetIcon(extension->id());
+  EXPECT_FALSE(gfx::test::AreImagesEqual(second_icon, default_icon));
 
-  EXPECT_TRUE(gfx::BitmapsAreEqual(first_icon, second_icon));
+  EXPECT_TRUE(gfx::test::AreImagesEqual(first_icon, second_icon));
 }
 
 #if defined(OS_CHROMEOS)
 // Tests loading an icon for a component extension.
 TEST_F(ExtensionIconManagerTest, LoadComponentExtensionResource) {
   std::unique_ptr<Profile> profile(new TestingProfile());
-  SkBitmap default_icon = GetDefaultIcon();
+  gfx::Image default_icon = GetDefaultIcon();
 
   base::FilePath test_dir;
   ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &test_dir));
@@ -165,11 +171,11 @@ TEST_F(ExtensionIconManagerTest, LoadComponentExtensionResource) {
   ASSERT_TRUE(extension.get());
 
   TestIconManager icon_manager(this);
-  // Load the icon and grab the bitmap.
+  // Load the icon.
   icon_manager.LoadIcon(profile.get(), extension.get());
   WaitForImageLoad();
-  SkBitmap first_icon = icon_manager.GetIcon(extension->id());
-  EXPECT_FALSE(gfx::BitmapsAreEqual(first_icon, default_icon));
+  gfx::Image first_icon = icon_manager.GetIcon(extension->id());
+  EXPECT_FALSE(gfx::test::AreImagesEqual(first_icon, default_icon));
 
   // Remove the icon from the manager.
   icon_manager.RemoveIcon(extension->id());
@@ -178,9 +184,94 @@ TEST_F(ExtensionIconManagerTest, LoadComponentExtensionResource) {
   // default icon).
   icon_manager.LoadIcon(profile.get(), extension.get());
   WaitForImageLoad();
-  SkBitmap second_icon = icon_manager.GetIcon(extension->id());
-  EXPECT_FALSE(gfx::BitmapsAreEqual(second_icon, default_icon));
+  gfx::Image second_icon = icon_manager.GetIcon(extension->id());
+  EXPECT_FALSE(gfx::test::AreImagesEqual(second_icon, default_icon));
 
-  EXPECT_TRUE(gfx::BitmapsAreEqual(first_icon, second_icon));
+  EXPECT_TRUE(gfx::test::AreImagesEqual(first_icon, second_icon));
 }
 #endif
+
+// Test what bitmaps are loaded when various combinations of scale factors are
+// supported.
+TEST_F(ExtensionIconManagerTest, ScaleFactors) {
+  std::unique_ptr<Profile> profile(new TestingProfile());

[Devlin, 2016/12/15 17:14:41]

nit: I think we prefer MakeUnique now, so
auto profile = base::MakeUnique<TestingProfile>();

[Evan Stade, 2016/12/15 23:49:02]

Done.

+  const gfx::Image default_icon = GetDefaultIcon();
+
+  base::FilePath test_dir;
+  ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &test_dir));
+  base::FilePath manifest_path =
+      test_dir.AppendASCII("extensions/context_menus/icons/manifest.json");
+
+  JSONFileValueDeserializer deserializer(manifest_path);
+  std::unique_ptr<base::DictionaryValue> manifest =
+      base::DictionaryValue::From(deserializer.Deserialize(nullptr, nullptr));
+  ASSERT_TRUE(manifest);
+
+  std::string error;
+  scoped_refptr<Extension> extension(
+      Extension::Create(manifest_path.DirName(), Manifest::INVALID_LOCATION,
+                        *manifest, Extension::NO_FLAGS, &error));
+  ASSERT_TRUE(extension);
+
+  int kMaxIconSizeInManifest = 32;
+  std::vector<std::vector<ui::ScaleFactor>> supported_scales = {
+      // Base case.
+      {ui::SCALE_FACTOR_100P},
+      // Two scale factors.
+      {ui::SCALE_FACTOR_100P, ui::SCALE_FACTOR_200P},
+      // A scale factor that is in between two of the provided icon sizes
+      // (should use the larger one and scale down).
+      {ui::SCALE_FACTOR_125P},
+      // One scale factor for which we have an icon, one scale factor for which
+      // we don't.
+      {ui::SCALE_FACTOR_100P, ui::SCALE_FACTOR_300P},
+      // Just a scale factor where we don't have any icon. This falls back to
+      // the default icon.
+      {ui::SCALE_FACTOR_300P}};
+
+  for (size_t i = 0; i < supported_scales.size(); ++i) {
+    SCOPED_TRACE(testing::Message() << "Test case: " << i);

[Devlin, 2016/12/15 17:14:41]

mind blown.  Had no idea this existed.  Awesome.

[Evan Stade, 2016/12/15 23:49:02]

Acknowledged.

+    ui::test::ScopedSetSupportedScaleFactors scoped(supported_scales[i]);
+    TestIconManager icon_manager(this);
+
+    icon_manager.LoadIcon(profile.get(), extension.get());
+    WaitForImageLoad();
+    gfx::Image icon = icon_manager.GetIcon(extension->id());
+
+    // Determine if the default icon fallback will be used. We'll use the
+    // default when none of the supported scale factors can find an appropriate
+    // icon.
+    bool should_fall_back_to_default = true;
+    for (auto supported_scale : supported_scales[i]) {
+      if (gfx::kFaviconSize * ui::GetScaleForScaleFactor(supported_scale) <=
+          kMaxIconSizeInManifest) {
+        should_fall_back_to_default = false;
+        break;
+      }
+    }
+    if (should_fall_back_to_default) {
+      EXPECT_TRUE(gfx::test::AreImagesEqual(icon, default_icon));
+      continue;
+    }
+
+    gfx::ImageSkia image_skia = icon.AsImageSkia();
+
+    for (int dsf = ui::SCALE_FACTOR_NONE + 1; dsf < ui::NUM_SCALE_FACTORS;

[Devlin, 2016/12/15 17:14:41]

what is dsf?

[Evan Stade, 2016/12/15 23:49:02]

stands for device scale factor. Changed to scale_factor_iter

+         ++dsf) {
+      auto scale_factor = static_cast<ui::ScaleFactor>(dsf);
+      float scale = ui::GetScaleForScaleFactor(scale_factor);
+      SCOPED_TRACE(testing::Message() << "Scale: " << scale);
+
+      const bool has_representation = image_skia.HasRepresentation(scale);
+      // We shouldn't have a representation if the extension didn't provide a
+      // big enough icon.
+      if (gfx::kFaviconSize * scale > kMaxIconSizeInManifest)
+        EXPECT_FALSE(has_representation);
+      else
+        EXPECT_EQ(ui::IsSupportedScale(scale), has_representation);
+    }
+  }
+}
+
+}  // namespace
+}  // namespace extensions