[Extensions UI] Clean up extension icon generation

chrome/browser/extensions/api/extension_action/browser_action_apitest.cc

Index: chrome/browser/extensions/api/extension_action/browser_action_apitest.cc
diff --git a/chrome/browser/extensions/api/extension_action/browser_action_apitest.cc b/chrome/browser/extensions/api/extension_action/browser_action_apitest.cc
index 6322c6a53a7a54ec3fe1db58a869836db73329cd..f4567fce73f8a6f1f3335be28536ed8ee192c7fb 100644
--- a/chrome/browser/extensions/api/extension_action/browser_action_apitest.cc
+++ b/chrome/browser/extensions/api/extension_action/browser_action_apitest.cc
@@ -33,6 +33,7 @@
 #include "ui/base/resource/resource_bundle.h"
 #include "ui/gfx/geometry/rect.h"
 #include "ui/gfx/geometry/size.h"
+#include "ui/gfx/image/canvas_image_source.h"
 #include "ui/gfx/image/image_skia.h"
 #include "ui/gfx/image/image_skia_operations.h"
 #include "ui/gfx/skia_util.h"
@@ -42,21 +43,38 @@ using content::WebContents;
 namespace extensions {
 namespace {
 
+// An ImageSkia source that will do nothing (i.e., have a blank skia). We need
+// this because we need a blank skia at a certain size, and that can't be done
+// by an empty skia.

[Avi (use Gerrit), 2015/07/02 00:33:23]

"have a blank skia"? "empty skia"?

s/skia/canvas/ perhaps?

[Devlin, 2015/07/06 19:16:07]

For "blank", yeah, blank canvas is better.  For "empty", I think skia is right,
but I'll change to "null", since that matches the isNull terminology in the skia
api.  (This comment is just trying to explain why it's not enough to just
super-impose it with an ImageSkia()).

[Avi (use Gerrit), 2015/07/06 19:33:30]

What is a "skia"? That's my question. ImageSkia is a Skia (the graphics
library)-backed Image.

But your use of the word "skia" as a noun is weird, because it's clearly not a
reference to the graphics library, but I can't figure out what it _is_ a
reference to.

[Devlin, 2015/07/06 19:35:52]

That's fair.  s/skia/ImageSkia.

+class BlankImageSource : public gfx::CanvasImageSource {
+ public:
+  explicit BlankImageSource(const gfx::Size& size)
+     : gfx::CanvasImageSource(size, false) {}
+  ~BlankImageSource() override {}
+
+  void Draw(gfx::Canvas* canvas) override {}
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(BlankImageSource);
+};
+
 const char kEmptyImageDataError[] =
     "The imageData property must contain an ImageData object or dictionary "
     "of ImageData objects.";
 const char kEmptyPathError[] = "The path property must not be empty.";
 
-// Views implementation of browser action button will return icon whose
-// background will be set.
-gfx::ImageSkia AddBackgroundForViews(const gfx::ImageSkia& icon) {
+// Views platforms have the icon superimposed over a button's background.
+// Macs don't, but still need a 29x29-sized image (and the easiest way to do
+// that is to superimpose the icon over a blank background).
+gfx::ImageSkia AddBackground(const gfx::ImageSkia& icon) {
 #if !defined(OS_MACOSX)
   ui::ResourceBundle& rb = ui::ResourceBundle::GetSharedInstance();
   gfx::ImageSkia bg = *rb.GetImageSkiaNamed(IDR_BROWSER_ACTION);
-  return gfx::ImageSkiaOperations::CreateSuperimposedImage(bg, icon);
 #else
-  return icon;
+  gfx::Size size(29, 29);  // Size of browser actions buttons.

[Finnur, 2015/07/06 10:27:09]

nit: const-ify.

[Devlin, 2015/07/06 19:16:07]

Done.

+  gfx::ImageSkia bg(new BlankImageSource(size), size);
 #endif
+  return gfx::ImageSkiaOperations::CreateSuperimposedImage(bg, icon);
 }
 
 bool ImagesAreEqualAtScale(const gfx::ImageSkia& i1,
@@ -182,7 +200,7 @@ IN_PROC_BROWSER_TEST_F(BrowserActionApiTest, DynamicBrowserAction) {
   EXPECT_FALSE(action_icon.ToImageSkia()->HasRepresentation(2.0f));
 
   EXPECT_TRUE(
-      ImagesAreEqualAtScale(AddBackgroundForViews(*action_icon.ToImageSkia()),
+      ImagesAreEqualAtScale(AddBackground(*action_icon.ToImageSkia()),
                             *GetBrowserActionsBar()->GetIcon(0).ToImageSkia(),
                             1.0f));
 
@@ -200,7 +218,7 @@ IN_PROC_BROWSER_TEST_F(BrowserActionApiTest, DynamicBrowserAction) {
       action_icon.ToImageSkia()->HasRepresentation(2.0f));
 
   EXPECT_TRUE(
-      ImagesAreEqualAtScale(AddBackgroundForViews(*action_icon.ToImageSkia()),
+      ImagesAreEqualAtScale(AddBackground(*action_icon.ToImageSkia()),
                             *GetBrowserActionsBar()->GetIcon(0).ToImageSkia(),
                             1.0f));
 
@@ -218,7 +236,7 @@ IN_PROC_BROWSER_TEST_F(BrowserActionApiTest, DynamicBrowserAction) {
   EXPECT_TRUE(action_icon.ToImageSkia()->HasRepresentation(2.0f));
 
   EXPECT_TRUE(
-      ImagesAreEqualAtScale(AddBackgroundForViews(*action_icon.ToImageSkia()),
+      ImagesAreEqualAtScale(AddBackground(*action_icon.ToImageSkia()),
                             *GetBrowserActionsBar()->GetIcon(0).ToImageSkia(),
                             1.0f));
 
@@ -235,7 +253,7 @@ IN_PROC_BROWSER_TEST_F(BrowserActionApiTest, DynamicBrowserAction) {
   EXPECT_TRUE(action_icon.ToImageSkia()->HasRepresentation(2.0f));
 
   EXPECT_TRUE(
-      ImagesAreEqualAtScale(AddBackgroundForViews(*action_icon.ToImageSkia()),
+      ImagesAreEqualAtScale(AddBackground(*action_icon.ToImageSkia()),
                             *GetBrowserActionsBar()->GetIcon(0).ToImageSkia(),
                             1.0f));
 
@@ -253,7 +271,7 @@ IN_PROC_BROWSER_TEST_F(BrowserActionApiTest, DynamicBrowserAction) {
   EXPECT_FALSE(action_icon.ToImageSkia()->HasRepresentation(2.0f));
 
   EXPECT_TRUE(
-      ImagesAreEqualAtScale(AddBackgroundForViews(*action_icon.ToImageSkia()),
+      ImagesAreEqualAtScale(AddBackground(*action_icon.ToImageSkia()),
                             *GetBrowserActionsBar()->GetIcon(0).ToImageSkia(),
                             1.0f));
 
@@ -271,7 +289,7 @@ IN_PROC_BROWSER_TEST_F(BrowserActionApiTest, DynamicBrowserAction) {
   EXPECT_FALSE(action_icon.ToImageSkia()->HasRepresentation(2.0f));
 
   EXPECT_TRUE(
-      ImagesAreEqualAtScale(AddBackgroundForViews(*action_icon.ToImageSkia()),
+      ImagesAreEqualAtScale(AddBackground(*action_icon.ToImageSkia()),
                             *GetBrowserActionsBar()->GetIcon(0).ToImageSkia(),
                             1.0f));
 
@@ -296,7 +314,7 @@ IN_PROC_BROWSER_TEST_F(BrowserActionApiTest, DynamicBrowserAction) {
       action_icon_skia->GetRepresentation(2.0f).sk_bitmap()));
 
   EXPECT_TRUE(
-      ImagesAreEqualAtScale(AddBackgroundForViews(*action_icon_skia),
+      ImagesAreEqualAtScale(AddBackground(*action_icon_skia),
                             *GetBrowserActionsBar()->GetIcon(0).ToImageSkia(),
                             2.0f));