Refactored loading of applications / extensions icons.

chrome/browser/extensions/image_loader_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/image_loader.h"
 
 #include "base/json/json_file_value_serializer.h"
 #include "base/message_loop/message_loop.h"
 #include "base/path_service.h"
 #include "chrome/browser/chrome_notification_types.h"
@@ skipping 29 matching lines @@
         io_thread_(BrowserThread::IO) {
   }
 
   void OnImageLoaded(const gfx::Image& image) {
     image_loaded_count_++;
     if (quit_in_image_loaded_)
       base::MessageLoop::current()->Quit();
     image_ = image;
   }
 
+  void OnImageURLLoaded(const GURL& url) {
+    image_loaded_count_++;
+    if (quit_in_image_loaded_)
+      base::MessageLoop::current()->Quit();
+    url_ = url;
+  }
+
   void WaitForImageLoad() {
     quit_in_image_loaded_ = true;
     base::MessageLoop::current()->Run();
     quit_in_image_loaded_ = false;

[Finnur, 2013/10/02 09:52:15]

Do you know why it is necessary to set this to false here?
Actually, why is this variable necessary?

[dvh-g, 2013/10/02 15:08:13]

I don't think that the case where the it's really needed actually happen.
I removed it.

   }
 
   int image_loaded_count() {
     int result = image_loaded_count_;
     image_loaded_count_ = 0;
     return result;
   }
 
   scoped_refptr<Extension> CreateExtension(const char* name,
                                            Manifest::Location location) {
@@ skipping 24 matching lines @@
         EXPECT_FALSE(true);
         return NULL;
       }
       test_file = test_file.AppendASCII(name);
     }
     return Extension::Create(test_file, location, *valid_value,
                              Extension::NO_FLAGS, &error);
   }
 
   gfx::Image image_;
+  GURL url_;

[Finnur, 2013/10/02 09:52:15]

Document:
// Holds the image loaded by the test.
and
// Holds the data url retrieved by the test.

[dvh-g, 2013/10/02 15:08:13]

Done.

[dvh-g, 2013/10/02 15:08:13]

Done.

 
  private:
   virtual void SetUp() OVERRIDE {
     testing::Test::SetUp();
     file_thread_.Start();
     io_thread_.Start();
   }
 
   int image_loaded_count_;
   bool quit_in_image_loaded_;

[Finnur, 2013/10/02 09:52:15]

Please rename: quit_when_image_loaded_.

[dvh-g, 2013/10/02 15:08:13]

Done.

   base::MessageLoop ui_loop_;
   content::TestBrowserThread ui_thread_;
   content::TestBrowserThread file_thread_;
   content::TestBrowserThread io_thread_;
 };
 
 // Tests loading an image works correctly.
 TEST_F(ImageLoaderTest, LoadImage) {
   scoped_refptr<Extension> extension(CreateExtension(
       "image_loading_tracker", Manifest::INVALID_LOCATION));
   ASSERT_TRUE(extension.get() != NULL);
 
   ExtensionResource image_resource = extensions::IconsInfo::GetIconResource(
       extension.get(),
       extension_misc::EXTENSION_ICON_SMALLISH,
       ExtensionIconSet::MATCH_EXACTLY);
   gfx::Size max_size(extension_misc::EXTENSION_ICON_SMALLISH,
                      extension_misc::EXTENSION_ICON_SMALLISH);
-  ImageLoader loader;
+  ImageLoader loader(NULL);
   loader.LoadImageAsync(extension.get(),
                         image_resource,
                         max_size,
                         base::Bind(&ImageLoaderTest::OnImageLoaded,
                                    base::Unretained(this)));
 
   // The image isn't cached, so we should not have received notification.
   EXPECT_EQ(0, image_loaded_count());
 
   WaitForImageLoad();
@@ skipping 12 matching lines @@
   scoped_refptr<Extension> extension(CreateExtension(
       "image_loading_tracker", Manifest::INVALID_LOCATION));
   ASSERT_TRUE(extension.get() != NULL);
 
   ExtensionResource image_resource = extensions::IconsInfo::GetIconResource(
       extension.get(),
       extension_misc::EXTENSION_ICON_SMALLISH,
       ExtensionIconSet::MATCH_EXACTLY);
   gfx::Size max_size(extension_misc::EXTENSION_ICON_SMALLISH,
                      extension_misc::EXTENSION_ICON_SMALLISH);
-  ImageLoader loader;
+  ImageLoader loader(NULL);
   std::set<int> sizes;
   sizes.insert(extension_misc::EXTENSION_ICON_SMALLISH);
   loader.LoadImageAsync(extension.get(),
                         image_resource,
                         max_size,
                         base::Bind(&ImageLoaderTest::OnImageLoaded,
                                    base::Unretained(this)));
 
   // The image isn't cached, so we should not have received notification.
   EXPECT_EQ(0, image_loaded_count());
@@ skipping 33 matching lines @@
   for (size_t i = 0; i < arraysize(sizes); ++i) {
     ExtensionResource resource = extensions::IconsInfo::GetIconResource(
         extension.get(), sizes[i], ExtensionIconSet::MATCH_EXACTLY);
     info_list.push_back(ImageLoader::ImageRepresentation(
         resource,
         ImageLoader::ImageRepresentation::RESIZE_WHEN_LARGER,
         gfx::Size(sizes[i], sizes[i]),
         ui::SCALE_FACTOR_NONE));
   }
 
-  ImageLoader loader;
+  ImageLoader loader(NULL);
   loader.LoadImagesAsync(extension.get(), info_list,
                          base::Bind(&ImageLoaderTest::OnImageLoaded,
                                     base::Unretained(this)));
 
   // The image isn't cached, so we should not have received notification.
   EXPECT_EQ(0, image_loaded_count());
 
   WaitForImageLoad();
 
   // We should have gotten the image.
@@ skipping 27 matching lines @@
 
 #if defined(FILE_MANAGER_EXTENSION)
   int resource_id;
   ASSERT_EQ(true,
             ImageLoader::IsComponentExtensionResource(extension->path(),
                                                       resource.relative_path(),
                                                       &resource_id));
   ASSERT_EQ(IDR_FILE_MANAGER_ICON_16, resource_id);
 #endif
 }
+
+// Tests loading an image works correctly.
+TEST_F(ImageLoaderTest, LoadIcon) {
+  scoped_refptr<Extension> extension(CreateExtension(
+      "image_loading_tracker", Manifest::INVALID_LOCATION));
+  ASSERT_TRUE(extension.get() != NULL);
+
+  ImageLoader loader(NULL);
+  loader.LoadExtensionIconAsync(extension.get(),
+                                extension_misc::EXTENSION_ICON_SMALLISH,
+                                false,
+                                base::Bind(&ImageLoaderTest::OnImageLoaded,
+                                    base::Unretained(this)));
+
+  // The image isn't cached, so we should not have received notification.
+  EXPECT_EQ(0, image_loaded_count());
+
+  WaitForImageLoad();
+
+  // We should have gotten the image.
+  EXPECT_EQ(1, image_loaded_count());
+
+  // Check that the image was loaded.
+  EXPECT_EQ(extension_misc::EXTENSION_ICON_SMALLISH,
+            image_.ToSkBitmap()->width());
+}
+
+// Tests loading an icon in grayscale works correctly.
+TEST_F(ImageLoaderTest, LoadIconGrayscale) {
+  scoped_refptr<Extension> extension(CreateExtension(
+      "image_loading_tracker", Manifest::INVALID_LOCATION));
+  ASSERT_TRUE(extension.get() != NULL);
+
+  ImageLoader loader(NULL);
+  loader.LoadExtensionIconAsync(extension.get(),
+                                extension_misc::EXTENSION_ICON_SMALLISH,
+                                true,
+                                base::Bind(&ImageLoaderTest::OnImageLoaded,
+                                    base::Unretained(this)));
+
+  // The image isn't cached, so we should not have received notification.
+  EXPECT_EQ(0, image_loaded_count());
+
+  WaitForImageLoad();
+
+  // We should have gotten the image.
+  EXPECT_EQ(1, image_loaded_count());
+
+  // Check that the image was loaded.
+  EXPECT_EQ(extension_misc::EXTENSION_ICON_SMALLISH,
+            image_.ToSkBitmap()->width());
+}
+
+// Tests loading an icon URL works correctly.
+TEST_F(ImageLoaderTest, LoadIconURL) {
+  scoped_refptr<Extension> extension(CreateExtension(
+      "image_loading_tracker", Manifest::INVALID_LOCATION));
+  ASSERT_TRUE(extension.get() != NULL);
+
+  ImageLoader loader(NULL);
+  loader.LoadExtensionIconDataURLAsync(
+      extension.get(),
+      extension_misc::EXTENSION_ICON_SMALLISH,
+      true,
+      base::Bind(&ImageLoaderTest::OnImageURLLoaded, base::Unretained(this)));
+
+  // The image isn't cached, so we should not have received notification.
+  EXPECT_EQ(0, image_loaded_count());
+
+  WaitForImageLoad();
+
+  // We should have gotten the image.
+  EXPECT_EQ(1, image_loaded_count());

[Finnur, 2013/10/02 09:52:15]

We could test against an actual data URL. Or is that not advisable?

[dvh-g, 2013/10/02 15:08:13]

Even though lossless, I think that PNG compression is not consistent, depending
on the algorithm chosen. It would make the test flacky.
A second issue is that the resize algorithm can give different pixel results
depending on the algorithm too.

[Finnur, 2013/10/02 15:19:27]

Fair enough.

+}
+
+// Tests loading the default icon works correctly.
+TEST_F(ImageLoaderTest, LoadDefaultIcon) {
+  ImageLoader loader(NULL);
+  loader.LoadExtensionIconAsync(
+      NULL,
+      -1,
+      false,
+      base::Bind(&ImageLoaderTest::OnImageLoaded, base::Unretained(this)));
+
+  // The image isn't cached, so we should not have received notification.
+  EXPECT_EQ(0, image_loaded_count());
+
+  WaitForImageLoad();
+
+  // We should have gotten the image.
+  EXPECT_EQ(1, image_loaded_count());
+}
+
+// Tests loading the icon of an extension without icon works correctly.
+TEST_F(ImageLoaderTest, LoadExtensionWithoutIcon) {
+  scoped_refptr<Extension> extension(CreateExtension(
+      "default_image_loading_tracker", Manifest::INVALID_LOCATION));
+  ASSERT_TRUE(extension.get() != NULL);
+
+  ImageLoader loader(NULL);
+  loader.LoadExtensionIconAsync(extension.get(),
+                                extension_misc::EXTENSION_ICON_SMALLISH,
+                                false,
+                                base::Bind(&ImageLoaderTest::OnImageLoaded,
+                                    base::Unretained(this)));
+
+  // The image isn't cached, so we should not have received notification.
+  EXPECT_EQ(0, image_loaded_count());
+
+  WaitForImageLoad();
+
+  // We should have gotten the image.
+  EXPECT_EQ(1, image_loaded_count());
+
+  // Check that the image was loaded.
+  EXPECT_EQ(extension_misc::EXTENSION_ICON_SMALLISH,
+            image_.ToSkBitmap()->width());
+}