Supply default icon to extension icon image.

chrome/browser/extensions/extension_icon_image_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 "chrome/browser/extensions/extension_icon_image.h"
 
 #include "base/json/json_file_value_serializer.h"
 #include "base/message_loop.h"
 #include "base/path_service.h"
+#include "chrome/browser/extensions/image_loading_tracker.h"
 #include "chrome/common/chrome_paths.h"
 #include "chrome/common/extensions/extension.h"
+#include "chrome/common/extensions/extension_constants.h"
 #include "content/public/test/test_browser_thread.h"
+#include "grit/theme_resources.h"
+#include "ui/base/resource/resource_bundle.h"
+#include "ui/gfx/skia_util.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 using content::BrowserThread;
 using extensions::Extension;
 using extensions::IconImage;
 
 namespace {
 
+bool ImageRepsAreEqual(const gfx::ImageSkiaRep& image_rep1,
+                       const gfx::ImageSkiaRep& image_rep2) {
+  return gfx::BitmapsAreEqual(image_rep1.sk_bitmap(), image_rep2.sk_bitmap());

[Jeffrey Yasskin, 2012/09/01 01:10:27]

Sorry for missing this. This isn't actually a correct implementation of equality
for ImageReps, since that would have to include the scale factors. You should
just compare the bitmaps directly in the test.

[tbarzic, 2012/09/04 21:07:09]

Done.

+}
+
+gfx::ImageSkiaRep CreateBlankRep(int size_dip, ui::ScaleFactor scale_factor) {
+    SkBitmap bitmap;
+    const float scale = ui::GetScaleFactorScale(scale_factor);
+    bitmap.setConfig(SkBitmap::kARGB_8888_Config,
+                     static_cast<int>(size_dip * scale),
+                     static_cast<int>(size_dip * scale));
+    bitmap.allocPixels();
+    bitmap.eraseColor(SkColorSetARGB(0, 0, 0, 0));
+    return gfx::ImageSkiaRep(bitmap, scale_factor);
+}
+
+// Helper class for synchronously loading extension image resource.
+class TestImageLoader : public ImageLoadingTracker::Observer {
+ public:
+  explicit TestImageLoader(const Extension* extension)
+      : extension_(extension),
+        waiting_(false),
+        image_loaded_(false),
+        ALLOW_THIS_IN_INITIALIZER_LIST(tracker_(this)) {
+  }
+  virtual ~TestImageLoader() {}
+
+  // ImageLoadingTracker::Observer override.
+  virtual void OnImageLoaded(const gfx::Image& image,
+                             const std::string& extension_id,
+                             int index) OVERRIDE {
+    image_ = image;
+    image_loaded_ = true;
+    if (waiting_)
+      MessageLoop::current()->Quit();
+  }
+
+  gfx::ImageSkia LoadImage(const std::string& path, int size, bool cache) {

[Jeffrey Yasskin, 2012/09/01 01:10:27]

Just use ImageLoadingTracker::CacheParam instead of replacing it with
harder-to-read booleans.

[tbarzic, 2012/09/04 21:07:09]

Done.

+    image_loaded_ = false;
+
+    ImageLoadingTracker::CacheParam cache_param =
+        cache ? ImageLoadingTracker::CACHE : ImageLoadingTracker::DONT_CACHE;
+
+    tracker_.LoadImage(extension_,
+                       extension_->GetResource(path),
+                       gfx::Size(size, size),
+                       cache_param);
+
+    // If |image_| still hasn't been loaded (i.e. it is being loaded
+    // asynchronously), wait for it.
+    if (!image_loaded_) {
+      waiting_ = true;
+      MessageLoop::current()->Run();
+      waiting_ = false;
+    }
+
+    EXPECT_TRUE(image_loaded_);
+
+    return image_.IsEmpty() ? gfx::ImageSkia() : *image_.ToImageSkia();
+  }
+
+ private:
+  const Extension* extension_;
+  bool waiting_;
+  bool image_loaded_;
+  gfx::Image image_;
+  ImageLoadingTracker tracker_;
+
+  DISALLOW_COPY_AND_ASSIGN(TestImageLoader);
+};
+
 class ExtensionIconImageTest : public testing::Test,
                                public IconImage::Observer {
  public:
   ExtensionIconImageTest()
       : image_loaded_count_(0),
-        image_load_failure_count_(0),
         quit_in_image_loaded_(false),
         ui_thread_(BrowserThread::UI, &ui_loop_),
         file_thread_(BrowserThread::FILE),
         io_thread_(BrowserThread::IO) {
   }
 
   virtual ~ExtensionIconImageTest() {}
 
   void WaitForImageLoad() {
-    // ExtensionIconImage may return synchronously, in which case there's
-    // nothing to wait for.
-    if (image_loaded_count_ > 0 || image_load_failure_count_ > 0)
-      return;
     quit_in_image_loaded_ = true;
     MessageLoop::current()->Run();
     quit_in_image_loaded_ = false;
   }
 
   int ImageLoadedCount() {
     int result = image_loaded_count_;
     image_loaded_count_ = 0;
     return result;
   }
 
-  int ImageLoadFailureCount() {
-    int result = image_load_failure_count_;
-    image_load_failure_count_ = 0;
-    return result;
-  }
-
   scoped_refptr<Extension> CreateExtension(const char* name,
                                            Extension::Location location) {
     // Create and load an extension.
     FilePath test_file;
     if (!PathService::Get(chrome::DIR_TEST_DATA, &test_file)) {
       EXPECT_FALSE(true);
       return NULL;
     }
     test_file = test_file.AppendASCII("extensions").AppendASCII(name);
     int error_code = 0;
@@ skipping 20 matching lines @@
     io_thread_.Start();
   }
 
   // IconImage::Delegate overrides:
   virtual void OnExtensionIconImageChanged(IconImage* image) OVERRIDE {
     image_loaded_count_++;
     if (quit_in_image_loaded_)
       MessageLoop::current()->Quit();
   }
 
-  virtual void OnIconImageLoadFailed(IconImage* image,
-                                     ui::ScaleFactor scale_factor) OVERRIDE {
-    image_load_failure_count_++;
-    if (quit_in_image_loaded_)
-      MessageLoop::current()->Quit();
+  gfx::ImageSkia GetDefaultIcon() {
+    return *ui::ResourceBundle::GetSharedInstance().GetImageSkiaNamed(
+        IDR_EXTENSIONS_FAVICON);
   }
 
  private:
   int image_loaded_count_;
-  int image_load_failure_count_;
   bool quit_in_image_loaded_;
   MessageLoop ui_loop_;
   content::TestBrowserThread ui_thread_;
   content::TestBrowserThread file_thread_;
   content::TestBrowserThread io_thread_;
 
   DISALLOW_COPY_AND_ASSIGN(ExtensionIconImageTest);
 };
 
 }  // namespace
 
 TEST_F(ExtensionIconImageTest, Basic) {
   scoped_refptr<Extension> extension(CreateExtension(
       "extension_icon_image", Extension::INVALID));
   ASSERT_TRUE(extension.get() != NULL);
 
-  IconImage image(
-      extension,
-      extension->icons(),
-      extension_misc::EXTENSION_ICON_BITTY,
-      this);
+  gfx::ImageSkia default_icon = GetDefaultIcon();
+
+  // Load images we expect to find as representations in icon_image, so we
+  // can later use them to validate icon_image.
+  TestImageLoader test_image_loader(extension.get());
+  gfx::ImageSkia bitty_image =
+      test_image_loader.LoadImage("16.png",
+                                  extension_misc::EXTENSION_ICON_BITTY,
+                                  false);
+  ASSERT_TRUE(bitty_image.HasRepresentation(ui::SCALE_FACTOR_100P));
+
+  // There is no image of size 32 defined in the extension manifest, so we
+  // should expect manifest image of size 48 resized to size 32.
+  gfx::ImageSkia small_image =
+      test_image_loader.LoadImage("48.png",
+                                  2 * extension_misc::EXTENSION_ICON_BITTY,
+                                  false);
+  ASSERT_TRUE(small_image.HasRepresentation(ui::SCALE_FACTOR_100P));
+
+  IconImage image(extension,
+                  extension->icons(),
+                  extension_misc::EXTENSION_ICON_BITTY,
+                  default_icon,
+                  this);
 
   // No representations in |image_| yet.
   gfx::ImageSkia::ImageSkiaReps image_reps = image.image_skia().image_reps();
   ASSERT_EQ(0u, image_reps.size());
 
   // Gets representation for a scale factor.
-  image.image_skia().GetRepresentation(ui::SCALE_FACTOR_100P);
+  gfx::ImageSkiaRep representation =
+      image.image_skia().GetRepresentation(ui::SCALE_FACTOR_100P);
+  // Before the image representation is loaded, image should contain blank
+  // image representation.
+  EXPECT_TRUE(ImageRepsAreEqual(
+      representation,
+      CreateBlankRep(extension_misc::EXTENSION_ICON_BITTY,
+                     ui::SCALE_FACTOR_100P)));
+
   WaitForImageLoad();
   EXPECT_EQ(1, ImageLoadedCount());
-  EXPECT_EQ(0, ImageLoadFailureCount());
+  ASSERT_EQ(1u, image.image_skia().image_reps().size());
+
+  representation = image.image_skia().GetRepresentation(ui::SCALE_FACTOR_100P);
+  // We should get the right representation now.
+  EXPECT_TRUE(ImageRepsAreEqual(
+      representation,
+      bitty_image.GetRepresentation(ui::SCALE_FACTOR_100P)));
+  EXPECT_EQ(extension_misc::EXTENSION_ICON_BITTY,
+            representation.pixel_width());
 
   // Gets representation for an additional scale factor.
-  image.image_skia().GetRepresentation(ui::SCALE_FACTOR_200P);
+  representation = image.image_skia().GetRepresentation(ui::SCALE_FACTOR_200P);
+
+  EXPECT_TRUE(ImageRepsAreEqual(
+      representation,
+      CreateBlankRep(extension_misc::EXTENSION_ICON_BITTY,
+                     ui::SCALE_FACTOR_200P)));
+
   WaitForImageLoad();
   EXPECT_EQ(1, ImageLoadedCount());
-  EXPECT_EQ(0, ImageLoadFailureCount());
+  ASSERT_EQ(2u, image.image_skia().image_reps().size());
 
-  gfx::ImageSkiaRep image_rep =
-      image.image_skia().GetRepresentation(ui::SCALE_FACTOR_100P);
-  EXPECT_EQ(extension_misc::EXTENSION_ICON_BITTY,
-            image_rep.pixel_width());
-
-  image_rep = image.image_skia().GetRepresentation(ui::SCALE_FACTOR_200P);
+  representation = image.image_skia().GetRepresentation(ui::SCALE_FACTOR_200P);
+  EXPECT_TRUE(ImageRepsAreEqual(
+      representation,
+      small_image.GetRepresentation(ui::SCALE_FACTOR_100P)));
+  // Image should have been resized.
   EXPECT_EQ(extension_misc::EXTENSION_ICON_SMALL,
-            image_rep.pixel_width());
+            representation.pixel_width());
 }
 
 // If we can't load icon with the exact size, but a bigger resource is

[Jeffrey Yasskin, 2012/09/01 01:10:27]

Hasn't this already been tested in the SCALE_FACTOR_200P part of the previous
test?

[tbarzic, 2012/09/04 21:07:09]

Done.

 // available.
 TEST_F(ExtensionIconImageTest, FallbackToBigger) {
   scoped_refptr<Extension> extension(CreateExtension(
       "extension_icon_image", Extension::INVALID));
   ASSERT_TRUE(extension.get() != NULL);
 
-  IconImage image(
-      extension,
-      extension->icons(),
-      extension_misc::EXTENSION_ICON_BITTY,
-      this);
+  gfx::ImageSkia default_icon = GetDefaultIcon();
+
+  // Load images we expect to find as representations in icon_image, so we
+  // can later use them to validate icon_image.
+  TestImageLoader test_image_loader(extension.get());
+  gfx::ImageSkia small_image =
+      test_image_loader.LoadImage("48.png",
+                                  2 * extension_misc::EXTENSION_ICON_BITTY,
+                                  false);
+  ASSERT_TRUE(small_image.HasRepresentation(ui::SCALE_FACTOR_100P));
+
+  IconImage image(extension,
+                  extension->icons(),
+                  extension_misc::EXTENSION_ICON_BITTY,
+                  default_icon,
+                  this);
 
   // Get representation for 2x.
-  image.image_skia().GetRepresentation(ui::SCALE_FACTOR_200P);
+  gfx::ImageSkiaRep representation =
+      image.image_skia().GetRepresentation(ui::SCALE_FACTOR_200P);
 
   WaitForImageLoad();
   EXPECT_EQ(1, ImageLoadedCount());
-  EXPECT_EQ(0, ImageLoadFailureCount());
+  ASSERT_EQ(1u, image.image_skia().image_reps().size());
 
-  gfx::ImageSkiaRep image_rep =
-      image.image_skia().GetRepresentation(ui::SCALE_FACTOR_200P);
+  representation = image.image_skia().GetRepresentation(ui::SCALE_FACTOR_200P);
+  EXPECT_TRUE(ImageRepsAreEqual(
+      representation,
+      small_image.GetRepresentation(ui::SCALE_FACTOR_100P)));
 
   // We should have found a bigger resource and it should have been resized.
-  EXPECT_EQ(ui::SCALE_FACTOR_200P, image_rep.scale_factor());
+  EXPECT_EQ(ui::SCALE_FACTOR_200P, representation.scale_factor());
   EXPECT_EQ(2 * extension_misc::EXTENSION_ICON_BITTY,
-            image_rep.pixel_width());
+            representation.pixel_width());
 }
 
 // There is no resource with either exact or bigger size, but there is a smaller
 // resource.
 TEST_F(ExtensionIconImageTest, FallbackToSmallerWhenNoBigger) {
   scoped_refptr<Extension> extension(CreateExtension(
       "extension_icon_image", Extension::INVALID));
   ASSERT_TRUE(extension.get() != NULL);
 
-  IconImage image(
-      extension,
-      extension->icons(),
-      extension_misc::EXTENSION_ICON_SMALL,
-      this);
+  gfx::ImageSkia default_icon = GetDefaultIcon();
+
+  // Load images we expect to find as representations in icon_image, so we
+  // can later use them to validate icon_image.
+  // The image should not be resized.

[Jeffrey Yasskin, 2012/09/01 01:10:27]

"should not be" makes me nervous in a test. Either you should trust the
underlying implementations and say "is not" or you should EXPECT to validate the
assumptions.

[tbarzic, 2012/09/04 21:07:09]

yeah, that comment is a bit misplaced.. removed it

+  TestImageLoader test_image_loader(extension.get());
+  gfx::ImageSkia expected_image =
+      test_image_loader.LoadImage("48.png",
+                                  extension_misc::EXTENSION_ICON_MEDIUM,
+                                  false);
+  ASSERT_TRUE(expected_image.HasRepresentation(ui::SCALE_FACTOR_100P));
+
+  IconImage image(extension,
+                  extension->icons(),
+                  extension_misc::EXTENSION_ICON_SMALL,
+                  default_icon,
+                  this);
 
   // Attempt to get representation for 2x.
-  image.image_skia().GetRepresentation(ui::SCALE_FACTOR_200P);
+  gfx::ImageSkiaRep representation =
+      image.image_skia().GetRepresentation(ui::SCALE_FACTOR_200P);
 
   WaitForImageLoad();
   EXPECT_EQ(1, ImageLoadedCount());
-  EXPECT_EQ(0, ImageLoadFailureCount());
+  ASSERT_EQ(1u, image.image_skia().image_reps().size());
 
-  gfx::ImageSkiaRep image_rep =
-      image.image_skia().GetRepresentation(ui::SCALE_FACTOR_200P);
+  representation = image.image_skia().GetRepresentation(ui::SCALE_FACTOR_200P);
+  EXPECT_TRUE(ImageRepsAreEqual(
+      representation,
+      expected_image.GetRepresentation(ui::SCALE_FACTOR_100P)));
 
   // We should have loaded the biggest smaller resource. In this case the
   // loaded resource should not be resized.
-  EXPECT_EQ(ui::SCALE_FACTOR_200P, image_rep.scale_factor());
+  EXPECT_EQ(ui::SCALE_FACTOR_200P, representation.scale_factor());
   EXPECT_EQ(extension_misc::EXTENSION_ICON_MEDIUM,
-            image_rep.pixel_width());
+            representation.pixel_width());
 }
 
 // There is no resource with exact size, but there is a smaller and a bigger
 // one. Requested size is smaller than 32 though, so the smaller resource should
 // be loaded.
 TEST_F(ExtensionIconImageTest, FallbackToSmaller) {
   scoped_refptr<Extension> extension(CreateExtension(
       "extension_icon_image", Extension::INVALID));
   ASSERT_TRUE(extension.get() != NULL);
 
-  IconImage image(
-      extension,
-      extension->icons(),
-      17,
-      this);
+  gfx::ImageSkia default_icon = GetDefaultIcon();
+
+  // Load images we expect to find as representations in icon_image, so we
+  // can later use them to validate icon_image.
+  TestImageLoader test_image_loader(extension.get());
+  gfx::ImageSkia expected_image =
+      test_image_loader.LoadImage("16.png",

[Jeffrey Yasskin, 2012/09/01 01:10:27]

Please extract all of these image loads into a helper class that you instantiate
in each test case, so you don't have to reproduce the loading code over and
over. Be sure to give them short descriptive field names like "icon_16" or
"icon_48_resized_to_32"

[tbarzic, 2012/09/04 21:07:09]

Done.

+                                  extension_misc::EXTENSION_ICON_BITTY,
+                                  false);
+  ASSERT_TRUE(expected_image.HasRepresentation(ui::SCALE_FACTOR_100P));
+
+  IconImage image(extension, extension->icons(), 17, default_icon, this);
 
   // Attempt to get representation for 1x.
-  image.image_skia().GetRepresentation(ui::SCALE_FACTOR_100P);
+  gfx::ImageSkiaRep representation =
+      image.image_skia().GetRepresentation(ui::SCALE_FACTOR_100P);
 
   WaitForImageLoad();
   EXPECT_EQ(1, ImageLoadedCount());
-  EXPECT_EQ(0, ImageLoadFailureCount());
+  ASSERT_EQ(1u, image.image_skia().image_reps().size());
 
-  gfx::ImageSkiaRep image_rep =
-      image.image_skia().GetRepresentation(ui::SCALE_FACTOR_100P);
+  representation = image.image_skia().GetRepresentation(ui::SCALE_FACTOR_100P);
+  EXPECT_TRUE(ImageRepsAreEqual(
+      representation,
+      expected_image.GetRepresentation(ui::SCALE_FACTOR_100P)));
 
   // We should have loaded smaller (not resized) resource.
-  EXPECT_EQ(ui::SCALE_FACTOR_100P, image_rep.scale_factor());
+  EXPECT_EQ(ui::SCALE_FACTOR_100P, representation.scale_factor());
   EXPECT_EQ(extension_misc::EXTENSION_ICON_BITTY,
-            image_rep.pixel_width());
+            representation.pixel_width());
 }
 
 // If resource set is empty, failure should be reported.

[Jeffrey Yasskin, 2012/09/01 01:10:27]

"failure should be reported" is no longer true.

[tbarzic, 2012/09/04 21:07:09]

Done.

 TEST_F(ExtensionIconImageTest, NoResources) {
   scoped_refptr<Extension> extension(CreateExtension(
       "extension_icon_image", Extension::INVALID));
   ASSERT_TRUE(extension.get() != NULL);
 
   ExtensionIconSet empty_icon_set;
+  gfx::ImageSkia default_icon = GetDefaultIcon();
 
-  IconImage image(
-      extension,
-      empty_icon_set,
-      extension_misc::EXTENSION_ICON_SMALLISH,
-      this);
+  IconImage image(extension,
+                  empty_icon_set,
+                  extension_misc::EXTENSION_ICON_SMALLISH,
+                  default_icon,
+                  this);
 
   // Attempt to get representation for 2x.
-  image.image_skia().GetRepresentation(ui::SCALE_FACTOR_200P);
+  gfx::ImageSkiaRep representation =
+      image.image_skia().GetRepresentation(ui::SCALE_FACTOR_100P);
+  EXPECT_TRUE(ImageRepsAreEqual(
+      representation,
+      default_icon.GetRepresentation(ui::SCALE_FACTOR_100P)));
+
+  EXPECT_EQ(0, ImageLoadedCount());
+  // We should still have default icon representation.
+  ASSERT_EQ(1u, image.image_skia().image_reps().size());
+
+  representation = image.image_skia().GetRepresentation(ui::SCALE_FACTOR_100P);
+  EXPECT_TRUE(ImageRepsAreEqual(
+      representation,
+      default_icon.GetRepresentation(ui::SCALE_FACTOR_100P)));
+}
+
+// If resource set is empty, failure should be reported.

[Jeffrey Yasskin, 2012/09/01 01:10:27]

Please update comments when you copy them.

[tbarzic, 2012/09/04 21:07:09]

Done.

+TEST_F(ExtensionIconImageTest, InvalidResource) {
+  scoped_refptr<Extension> extension(CreateExtension(
+      "extension_icon_image", Extension::INVALID));
+  ASSERT_TRUE(extension.get() != NULL);
+
+  ExtensionIconSet invalid_icon_set;
+  invalid_icon_set.Add(extension_misc::EXTENSION_ICON_SMALLISH, "invalid.png");
+  gfx::ImageSkia default_icon = GetDefaultIcon();
+
+  IconImage image(extension,
+                  invalid_icon_set,
+                  extension_misc::EXTENSION_ICON_SMALLISH,
+                  default_icon,
+                  this);
+
+  // Attempt to get representation for 2x.
+  gfx::ImageSkiaRep representation =
+      image.image_skia().GetRepresentation(ui::SCALE_FACTOR_100P);
+  EXPECT_TRUE(ImageRepsAreEqual(
+      representation,
+      CreateBlankRep(extension_misc::EXTENSION_ICON_SMALLISH,
+                     ui::SCALE_FACTOR_100P)));
 
   WaitForImageLoad();
+  EXPECT_EQ(1, ImageLoadedCount());
+  // We should still have default icon representation.

[Jeffrey Yasskin, 2012/09/01 01:10:27]

What do you mean "still"? This has changed from the original representation,
which was blank.

[tbarzic, 2012/09/04 21:07:09]

Done.

+  ASSERT_EQ(1u, image.image_skia().image_reps().size());
+
+  representation = image.image_skia().GetRepresentation(ui::SCALE_FACTOR_100P);
+  EXPECT_TRUE(ImageRepsAreEqual(
+      representation,
+      default_icon.GetRepresentation(ui::SCALE_FACTOR_100P)));
+}
+
+TEST_F(ExtensionIconImageTest, LoadPrecachedImage) {
+  scoped_refptr<Extension> extension(CreateExtension(
+      "extension_icon_image", Extension::INVALID));
+  ASSERT_TRUE(extension.get() != NULL);
+
+  gfx::ImageSkia default_icon = GetDefaultIcon();
+
+  TestImageLoader test_image_loader(extension.get());
+  // Load image and cache it.
+  gfx::ImageSkia bitty_image =
+      test_image_loader.LoadImage("16.png",
+                                  extension_misc::EXTENSION_ICON_BITTY,
+                                  true);
+  ASSERT_TRUE(bitty_image.HasRepresentation(ui::SCALE_FACTOR_100P));
+
+  IconImage image(extension,
+                  extension->icons(),
+                  extension_misc::EXTENSION_ICON_BITTY,
+                  default_icon,
+                  this);
+
+  // No representations in |image_| yet.
+  gfx::ImageSkia::ImageSkiaReps image_reps = image.image_skia().image_reps();
+  ASSERT_EQ(0u, image_reps.size());
+
+  // Gets representation for a scale factor.
+  // Since the icon representation is precached, it should be returned right
+  // away. Also, we should not received any notifications.
+  gfx::ImageSkiaRep representation =
+      image.image_skia().GetRepresentation(ui::SCALE_FACTOR_100P);
+  EXPECT_TRUE(ImageRepsAreEqual(
+      representation,
+      bitty_image.GetRepresentation(ui::SCALE_FACTOR_100P)));
+
   EXPECT_EQ(0, ImageLoadedCount());
-  EXPECT_EQ(1, ImageLoadFailureCount());
+  ASSERT_EQ(1u, image.image_skia().image_reps().size());
 }