Fix bug in browser action icon scaling for non-1x scale factors.

chrome/browser/extensions/extension_action.cc

Index: chrome/browser/extensions/extension_action.cc
diff --git a/chrome/browser/extensions/extension_action.cc b/chrome/browser/extensions/extension_action.cc
index d0b284c5370b9a78cb6f111d254de7ea01a412eb..1c8f76f050923ccbdbb85fd409e4df42c6c10fd6 100644
--- a/chrome/browser/extensions/extension_action.cc
+++ b/chrome/browser/extensions/extension_action.cc
@@ -129,14 +129,6 @@ bool ExtensionAction::ParseIconFromCanvasDictionary(
     gfx::ImageSkia* icon) {
   for (base::DictionaryValue::Iterator iter(dict); !iter.IsAtEnd();
        iter.Advance()) {
-    int icon_size = 0;
-    // Chrome helpfully scales the provided icon(s), but let's not go overboard.
-    const int kActionIconMaxSize = 10 * extension_misc::EXTENSION_ICON_ACTION;
-    if (!base::StringToInt(iter.key(), &icon_size) || icon_size <= 0 ||
-        icon_size > kActionIconMaxSize) {

[Devlin, 2016/01/26 19:08:48]

Do we not want any checks for ridiculous scaling?  (Agreed that the index into
the dictionary might not be useful, but we could check the bitmap size itself.)

[Evan Stade, 2016/01/26 19:50:07]

righto, baby rescued from bathwater

-      continue;
-    }
-
     const base::BinaryValue* image_data;
     std::string binary_string64;
     IPC::Message pickle;
@@ -155,8 +147,10 @@ bool ExtensionAction::ParseIconFromCanvasDictionary(
     if (!IPC::ReadParam(&pickle, &pickle_iter, &bitmap))
       return false;
     CHECK(!bitmap.isNull());
+    if (bitmap.width() != bitmap.height())

[Devlin, 2016/01/26 19:08:48]

I wonder if we really *need* this to be a square.  But we've always kind of
assumed it was, so this is fine for now.

Could you add a
// TODO(devlin): Can we support rectangular icons?

[Evan Stade, 2016/01/26 19:50:07]

Why would we want to let developers do that? Is there any advantage?

If we determine we need to scale the icon, we'll scale it to square, which looks
bad if it's changing dimensions. If we don't think we need to scale (based on
width), we'll use it as is, which will look bad if height > width (the icon
would extend below the 16x16 box it's supposed to be inside).

[Devlin, 2016/01/26 19:58:25]

Well, we can scale it to be proportionate and maintain the aspect ratio, and
then center the icon.  This is already what any developer without a perfectly
square icon would do and then provide a square icon with a transparent
background, but we could save them that step.

But, low priority.

[Evan Stade, 2016/01/26 22:33:50]

IMO, better not to save them that step. It adds code complexity and hurts
performance.

+      continue;
     float scale =
-        static_cast<float>(icon_size) / ExtensionAction::ActionIconSize();
+        static_cast<float>(bitmap.width()) / ExtensionAction::ActionIconSize();
     icon->AddRepresentation(gfx::ImageSkiaRep(bitmap, scale));
   }
   return true;