[Extensions UI] Clean up extension icon generation

chrome/browser/extensions/api/extension_action/browser_action_apitest.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 "build/build_config.h"
 #include "chrome/browser/extensions/api/extension_action/extension_action_api.h"
 #include "chrome/browser/extensions/browser_action_test_util.h"
 #include "chrome/browser/extensions/extension_action.h"
 #include "chrome/browser/extensions/extension_action_icon_factory.h"
 #include "chrome/browser/extensions/extension_action_manager.h"
@@ skipping 15 matching lines @@
 #include "extensions/browser/extension_system.h"
 #include "extensions/browser/notification_types.h"
 #include "extensions/browser/process_manager.h"
 #include "extensions/browser/test_extension_registry_observer.h"
 #include "extensions/common/feature_switch.h"
 #include "extensions/test/result_catcher.h"
 #include "grit/theme_resources.h"
 #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"
 
 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);
+#else
+  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);
-#else
-  return icon;
-#endif
 }
 
 bool ImagesAreEqualAtScale(const gfx::ImageSkia& i1,
                            const gfx::ImageSkia& i2,
                            float scale) {
   SkBitmap bitmap1 = i1.GetRepresentation(scale).sk_bitmap();
   SkBitmap bitmap2 = i2.GetRepresentation(scale).sk_bitmap();
   return gfx::BitmapsAreEqual(bitmap1, bitmap2);
 }
 
@@ skipping 105 matching lines @@
 
   action_icon = icon_factory.GetIcon(0);
 
   action_icon_current_id = action_icon.ToSkBitmap()->getGenerationID();
   EXPECT_GT(action_icon_current_id, action_icon_last_id);
   action_icon_last_id = action_icon_current_id;
 
   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));
 
   // Tell the extension to update the icon using path.
   GetBrowserActionsBar()->Press(0);
   ASSERT_TRUE(catcher.GetNextResult());
 
   action_icon = icon_factory.GetIcon(0);
 
   action_icon_current_id = action_icon.ToSkBitmap()->getGenerationID();
   EXPECT_GT(action_icon_current_id, action_icon_last_id);
   action_icon_last_id = action_icon_current_id;
 
   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));
 
   // Tell the extension to update the icon using dictionary of ImageData
   // objects.
   GetBrowserActionsBar()->Press(0);
   ASSERT_TRUE(catcher.GetNextResult());
 
   action_icon = icon_factory.GetIcon(0);
 
   action_icon_current_id = action_icon.ToSkBitmap()->getGenerationID();
   EXPECT_GT(action_icon_current_id, action_icon_last_id);
   action_icon_last_id = action_icon_current_id;
 
   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));
 
   // Tell the extension to update the icon using dictionary of paths.
   GetBrowserActionsBar()->Press(0);
   ASSERT_TRUE(catcher.GetNextResult());
 
   action_icon = icon_factory.GetIcon(0);
 
   action_icon_current_id = action_icon.ToSkBitmap()->getGenerationID();
   EXPECT_GT(action_icon_current_id, action_icon_last_id);
   action_icon_last_id = action_icon_current_id;
 
   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));
 
   // Tell the extension to update the icon using dictionary of ImageData
   // objects, but setting only size 19.
   GetBrowserActionsBar()->Press(0);
   ASSERT_TRUE(catcher.GetNextResult());
 
   action_icon = icon_factory.GetIcon(0);
 
   action_icon_current_id = action_icon.ToSkBitmap()->getGenerationID();
   EXPECT_GT(action_icon_current_id, action_icon_last_id);
   action_icon_last_id = action_icon_current_id;
 
   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));
 
   // Tell the extension to update the icon using dictionary of paths, but
   // setting only size 19.
   GetBrowserActionsBar()->Press(0);
   ASSERT_TRUE(catcher.GetNextResult());
 
   action_icon = icon_factory.GetIcon(0);
 
   action_icon_current_id = action_icon.ToSkBitmap()->getGenerationID();
   EXPECT_GT(action_icon_current_id, action_icon_last_id);
   action_icon_last_id = action_icon_current_id;
 
   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));
 
   // Tell the extension to update the icon using dictionary of ImageData
   // objects, but setting only size 38.
   GetBrowserActionsBar()->Press(0);
   ASSERT_TRUE(catcher.GetNextResult());
 
   action_icon = icon_factory.GetIcon(0);
 
   const gfx::ImageSkia* action_icon_skia = action_icon.ToImageSkia();
 
   EXPECT_FALSE(action_icon_skia->HasRepresentation(1.0f));
   EXPECT_TRUE(action_icon_skia->HasRepresentation(2.0f));
 
   action_icon_current_id = action_icon.ToSkBitmap()->getGenerationID();
   EXPECT_GT(action_icon_current_id, action_icon_last_id);
   action_icon_last_id = action_icon_current_id;
 
   EXPECT_TRUE(gfx::BitmapsAreEqual(
       *action_icon.ToSkBitmap(),
       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));
 
   // Try setting icon with empty dictionary of ImageData objects.
   GetBrowserActionsBar()->Press(0);
   ASSERT_FALSE(catcher.GetNextResult());
   EXPECT_EQ(kEmptyImageDataError, catcher.message());
 
   // Try setting icon with empty dictionary of path objects.
   GetBrowserActionsBar()->Press(0);
@@ skipping 344 matching lines @@
   const std::string script =
       "window.domAutomationController.send(document.body.style."
       "backgroundColor);";
   std::string result;
   EXPECT_TRUE(content::ExecuteScriptAndExtractString(tab, script, &result));
   EXPECT_EQ(result, "red");
 }
 
 }  // namespace
 }  // namespace extensions