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 4426fa7b827fd..edb1f355dff91 100644 --- a/chrome/browser/extensions/api/extension_action/browser_action_apitest.cc +++ b/chrome/browser/extensions/api/extension_action/browser_action_apitest.cc @@ -41,6 +41,7 @@ #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/image/image_unittest_util.h" #include "ui/gfx/skia_util.h" using content::WebContents; @@ -68,26 +69,19 @@ const char kEmptyImageDataError[] = "of ImageData objects."; const char kEmptyPathError[] = "The path property must not be empty."; -// 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 - const gfx::Size size(29, 29); // Size of browser actions buttons. - gfx::ImageSkia bg(new BlankImageSource(size), size); -#endif - return gfx::ImageSkiaOperations::CreateSuperimposedImage(bg, icon); -} - -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); +// Makes sure |bar_rendering| has |model_icon| in the middle (there's additional +// padding that correlates to the rest of the button, and this is ignored). +void VerifyIconsMatch(const gfx::Image& bar_rendering, + const gfx::Image& model_icon) { + gfx::Rect icon_portion(gfx::Point(), bar_rendering.Size()); + icon_portion.ClampToCenteredSize(model_icon.Size()); + + EXPECT_TRUE(gfx::test::AreBitmapsEqual( + model_icon.AsImageSkia().GetRepresentation(1.0f).sk_bitmap(), + gfx::ImageSkiaOperations::ExtractSubset(bar_rendering.AsImageSkia(), + icon_portion) + .GetRepresentation(1.0f) + .sk_bitmap())); } class BrowserActionApiTest : public ExtensionApiTest { @@ -188,140 +182,147 @@ IN_PROC_BROWSER_TEST_F(BrowserActionApiTest, DynamicBrowserAction) { ASSERT_EQ(action_icon_last_id, icon_factory.GetIcon(0).ToSkBitmap()->getGenerationID()); - uint32_t action_icon_current_id = 0; + gfx::Image last_bar_icon = GetBrowserActionsBar()->GetIcon(0); + EXPECT_TRUE(gfx::test::AreImagesEqual(last_bar_icon, + GetBrowserActionsBar()->GetIcon(0))); - ResultCatcher catcher; + // The reason we don't test more standard scales (like 1x, 2x, etc.) is that + // these may be generated from the provided scales. + float kSmallIconScale = 21.f / ExtensionAction::ActionIconSize(); + float kLargeIconScale = 42.f / ExtensionAction::ActionIconSize(); + ASSERT_FALSE(ui::IsSupportedScale(kSmallIconScale)); + ASSERT_FALSE(ui::IsSupportedScale(kLargeIconScale)); // Tell the extension to update the icon using ImageData object. + ResultCatcher catcher; GetBrowserActionsBar()->Press(0); ASSERT_TRUE(catcher.GetNextResult()); - action_icon = icon_factory.GetIcon(0); + EXPECT_FALSE(gfx::test::AreImagesEqual(last_bar_icon, + GetBrowserActionsBar()->GetIcon(0))); + last_bar_icon = GetBrowserActionsBar()->GetIcon(0); - action_icon_current_id = action_icon.ToSkBitmap()->getGenerationID(); + action_icon = icon_factory.GetIcon(0); + uint32_t 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; + VerifyIconsMatch(last_bar_icon, action_icon); - EXPECT_FALSE(action_icon.ToImageSkia()->HasRepresentation(2.0f)); - - EXPECT_TRUE( - ImagesAreEqualAtScale(AddBackground(*action_icon.ToImageSkia()), - *GetBrowserActionsBar()->GetIcon(0).ToImageSkia(), - 1.0f)); + // Check that only the smaller size was set (only a 21px icon was provided). + EXPECT_TRUE(action_icon.ToImageSkia()->HasRepresentation(kSmallIconScale)); + EXPECT_FALSE(action_icon.ToImageSkia()->HasRepresentation(kLargeIconScale)); // Tell the extension to update the icon using path. GetBrowserActionsBar()->Press(0); ASSERT_TRUE(catcher.GetNextResult()); - action_icon = icon_factory.GetIcon(0); + // Make sure the browser action bar updated. + EXPECT_FALSE(gfx::test::AreImagesEqual(last_bar_icon, + GetBrowserActionsBar()->GetIcon(0))); + last_bar_icon = GetBrowserActionsBar()->GetIcon(0); + 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; + VerifyIconsMatch(last_bar_icon, action_icon); - EXPECT_FALSE( - action_icon.ToImageSkia()->HasRepresentation(2.0f)); - - EXPECT_TRUE( - ImagesAreEqualAtScale(AddBackground(*action_icon.ToImageSkia()), - *GetBrowserActionsBar()->GetIcon(0).ToImageSkia(), - 1.0f)); + // Check that only the smaller size was set (only a 21px icon was provided). + EXPECT_TRUE(action_icon.ToImageSkia()->HasRepresentation(kSmallIconScale)); + EXPECT_FALSE(action_icon.ToImageSkia()->HasRepresentation(kLargeIconScale)); // 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); + EXPECT_FALSE(gfx::test::AreImagesEqual(last_bar_icon, + GetBrowserActionsBar()->GetIcon(0))); + last_bar_icon = GetBrowserActionsBar()->GetIcon(0); + 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; + VerifyIconsMatch(last_bar_icon, action_icon); - EXPECT_TRUE(action_icon.ToImageSkia()->HasRepresentation(2.0f)); - - EXPECT_TRUE( - ImagesAreEqualAtScale(AddBackground(*action_icon.ToImageSkia()), - *GetBrowserActionsBar()->GetIcon(0).ToImageSkia(), - 1.0f)); + // Check both sizes were set (as two icon sizes were provided). + EXPECT_TRUE(action_icon.ToImageSkia()->HasRepresentation(kSmallIconScale)); + EXPECT_TRUE(action_icon.AsImageSkia().HasRepresentation(kLargeIconScale)); // Tell the extension to update the icon using dictionary of paths. GetBrowserActionsBar()->Press(0); ASSERT_TRUE(catcher.GetNextResult()); - action_icon = icon_factory.GetIcon(0); + EXPECT_FALSE(gfx::test::AreImagesEqual(last_bar_icon, + GetBrowserActionsBar()->GetIcon(0))); + last_bar_icon = GetBrowserActionsBar()->GetIcon(0); + 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; + VerifyIconsMatch(last_bar_icon, action_icon); - EXPECT_TRUE(action_icon.ToImageSkia()->HasRepresentation(2.0f)); - - EXPECT_TRUE( - ImagesAreEqualAtScale(AddBackground(*action_icon.ToImageSkia()), - *GetBrowserActionsBar()->GetIcon(0).ToImageSkia(), - 1.0f)); + // Check both sizes were set (as two icon sizes were provided). + EXPECT_TRUE(action_icon.ToImageSkia()->HasRepresentation(kSmallIconScale)); + EXPECT_TRUE(action_icon.AsImageSkia().HasRepresentation(kLargeIconScale)); // Tell the extension to update the icon using dictionary of ImageData - // objects, but setting only size 19. + // objects, but setting only one size. GetBrowserActionsBar()->Press(0); ASSERT_TRUE(catcher.GetNextResult()); - action_icon = icon_factory.GetIcon(0); + EXPECT_FALSE(gfx::test::AreImagesEqual(last_bar_icon, + GetBrowserActionsBar()->GetIcon(0))); + last_bar_icon = GetBrowserActionsBar()->GetIcon(0); + 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; + VerifyIconsMatch(last_bar_icon, action_icon); - EXPECT_FALSE(action_icon.ToImageSkia()->HasRepresentation(2.0f)); - - EXPECT_TRUE( - ImagesAreEqualAtScale(AddBackground(*action_icon.ToImageSkia()), - *GetBrowserActionsBar()->GetIcon(0).ToImageSkia(), - 1.0f)); + // Check that only the smaller size was set (only a 21px icon was provided). + EXPECT_TRUE(action_icon.ToImageSkia()->HasRepresentation(kSmallIconScale)); + EXPECT_FALSE(action_icon.ToImageSkia()->HasRepresentation(kLargeIconScale)); // Tell the extension to update the icon using dictionary of paths, but - // setting only size 19. + // setting only one size. GetBrowserActionsBar()->Press(0); ASSERT_TRUE(catcher.GetNextResult()); - action_icon = icon_factory.GetIcon(0); + EXPECT_FALSE(gfx::test::AreImagesEqual(last_bar_icon, + GetBrowserActionsBar()->GetIcon(0))); + last_bar_icon = GetBrowserActionsBar()->GetIcon(0); + 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; + VerifyIconsMatch(last_bar_icon, action_icon); - EXPECT_FALSE(action_icon.ToImageSkia()->HasRepresentation(2.0f)); - - EXPECT_TRUE( - ImagesAreEqualAtScale(AddBackground(*action_icon.ToImageSkia()), - *GetBrowserActionsBar()->GetIcon(0).ToImageSkia(), - 1.0f)); + // Check that only the smaller size was set (only a 21px icon was provided). + EXPECT_TRUE(action_icon.ToImageSkia()->HasRepresentation(kSmallIconScale)); + EXPECT_FALSE(action_icon.ToImageSkia()->HasRepresentation(kLargeIconScale)); // Tell the extension to update the icon using dictionary of ImageData - // objects, but setting only size 38. + // objects, but setting only size 42. 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)); + EXPECT_FALSE(gfx::test::AreImagesEqual(last_bar_icon, + GetBrowserActionsBar()->GetIcon(0))); + last_bar_icon = GetBrowserActionsBar()->GetIcon(0); + 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(gfx::BitmapsAreEqual( - *action_icon.ToSkBitmap(), - action_icon_skia->GetRepresentation(2.0f).sk_bitmap())); - - EXPECT_TRUE( - ImagesAreEqualAtScale(AddBackground(*action_icon_skia), - *GetBrowserActionsBar()->GetIcon(0).ToImageSkia(), - 2.0f)); + // Check that only the larger size was set (only a 42px icon was provided). + EXPECT_FALSE(action_icon.ToImageSkia()->HasRepresentation(kSmallIconScale)); + EXPECT_TRUE(action_icon.AsImageSkia().HasRepresentation(kLargeIconScale)); // Try setting icon with empty dictionary of ImageData objects. GetBrowserActionsBar()->Press(0); diff --git a/chrome/browser/extensions/extension_action.cc b/chrome/browser/extensions/extension_action.cc index 5476478e1e5bf..4742e75b488fa 100644 --- a/chrome/browser/extensions/extension_action.cc +++ b/chrome/browser/extensions/extension_action.cc @@ -130,8 +130,12 @@ bool ExtensionAction::ParseIconFromCanvasDictionary( for (base::DictionaryValue::Iterator iter(dict); !iter.IsAtEnd(); iter.Advance()) { int icon_size = 0; - if (!base::StringToInt(iter.key(), &icon_size)) + // 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) { continue; + } const base::BinaryValue* image_data; std::string binary_string64; diff --git a/chrome/browser/ui/extensions/icon_with_badge_image_source.cc b/chrome/browser/ui/extensions/icon_with_badge_image_source.cc index 322dcce754393..71609344f7cd0 100644 --- a/chrome/browser/ui/extensions/icon_with_badge_image_source.cc +++ b/chrome/browser/ui/extensions/icon_with_badge_image_source.cc @@ -90,14 +90,13 @@ SkPaint* GetBadgeTextPaintSingleton() { } gfx::ImageSkiaRep ScaleImageSkiaRep(const gfx::ImageSkiaRep& rep, + int target_width, float target_scale) { - gfx::Size scaled_size = - gfx::ScaleToCeiledSize(rep.pixel_size(), target_scale / rep.scale()); - return gfx::ImageSkiaRep(skia::ImageOperations::Resize( - rep.sk_bitmap(), - skia::ImageOperations::RESIZE_BEST, - scaled_size.width(), - scaled_size.height()), target_scale); + return gfx::ImageSkiaRep( + skia::ImageOperations::Resize(rep.sk_bitmap(), + skia::ImageOperations::RESIZE_BEST, + target_width, target_width), + target_scale); } } // namespace @@ -129,11 +128,10 @@ void IconWithBadgeImageSource::Draw(gfx::Canvas* canvas) { return; gfx::ImageSkia skia = icon_.AsImageSkia(); - // TODO(estade): Fix setIcon and enable this on !MD. - if (ui::MaterialDesignController::IsModeMaterial()) { - gfx::ImageSkiaRep rep = skia.GetRepresentation(canvas->image_scale()); - if (rep.scale() != canvas->image_scale()) - skia.AddRepresentation(ScaleImageSkiaRep(rep, canvas->image_scale())); + gfx::ImageSkiaRep rep = skia.GetRepresentation(canvas->image_scale()); + if (rep.scale() != canvas->image_scale()) { + skia.AddRepresentation( + ScaleImageSkiaRep(rep, skia.width(), canvas->image_scale())); } if (grayscale_) skia = gfx::ImageSkiaOperations::CreateHSLShiftedImage(skia, {-1, 0, 0.6}); diff --git a/chrome/browser/ui/views/toolbar/toolbar_action_view.cc b/chrome/browser/ui/views/toolbar/toolbar_action_view.cc index 13198fb2a94c5..237c76e208bb1 100644 --- a/chrome/browser/ui/views/toolbar/toolbar_action_view.cc +++ b/chrome/browser/ui/views/toolbar/toolbar_action_view.cc @@ -158,12 +158,8 @@ void ToolbarActionView::UpdateState() { view_controller_->GetIcon(web_contents, GetPreferredSize()).AsImageSkia()); - if (!icon.isNull()) { - gfx::ImageSkia bg = *ResourceBundle::GetSharedInstance().GetImageSkiaNamed( - IDR_BROWSER_ACTION); - SetImage(views::Button::STATE_NORMAL, - gfx::ImageSkiaOperations::CreateSuperimposedImage(bg, icon)); - } + if (!icon.isNull()) + SetImage(views::Button::STATE_NORMAL, icon); SetTooltipText(view_controller_->GetTooltip(web_contents)); SetAccessibleName(view_controller_->GetAccessibleName(web_contents)); diff --git a/chrome/common/extensions/api/browser_action.json b/chrome/common/extensions/api/browser_action.json index d7ff7438db65c..8b7898c68f27a 100644 --- a/chrome/common/extensions/api/browser_action.json +++ b/chrome/common/extensions/api/browser_action.json @@ -91,28 +91,22 @@ { "$ref": "ImageDataType" }, { "type": "object", - "properties": { - "19": {"$ref": "ImageDataType", "optional": true}, - "38": {"$ref": "ImageDataType", "optional": true} - } + "additionalProperties": { "type": "any" } } ], "optional": true, - "description": "Either an ImageData object or a dictionary {size -> ImageData} representing icon to be set. If the icon is specified as a dictionary, the actual image to be used is chosen depending on screen's pixel density. If the number of image pixels that fit into one screen space unit equals <code>scale</code>, then image with size <code>scale</code> * 19 will be selected. Initially only scales 1 and 2 will be supported. At least one image must be specified. Note that 'details.imageData = foo' is equivalent to 'details.imageData = {'19': foo}'" + "description": "Either an ImageData object or a dictionary {size -> ImageData} representing icon to be set. If the icon is specified as a dictionary, the actual image to be used is chosen depending on screen's pixel density. If the number of image pixels that fit into one screen space unit equals <code>scale</code>, then image with size <code>scale</code> * n will be selected, where n is the size of the icon in the UI. At least one image must be specified. Note that 'details.imageData = foo' is equivalent to 'details.imageData = {'16': foo}'" }, "path": { "choices": [ { "type": "string" }, { "type": "object", - "properties": { - "19": {"type": "string", "optional": true}, - "38": {"type": "string", "optional": true} - } + "additionalProperties": { "type": "any" } } ], "optional": true, - "description": "Either a relative image path or a dictionary {size -> relative image path} pointing to icon to be set. If the icon is specified as a dictionary, the actual image to be used is chosen depending on screen's pixel density. If the number of image pixels that fit into one screen space unit equals <code>scale</code>, then image with size <code>scale</code> * 19 will be selected. Initially only scales 1 and 2 will be supported. At least one image must be specified. Note that 'details.path = foo' is equivalent to 'details.imageData = {'19': foo}'" + "description": "Either a relative image path or a dictionary {size -> relative image path} pointing to icon to be set. If the icon is specified as a dictionary, the actual image to be used is chosen depending on screen's pixel density. If the number of image pixels that fit into one screen space unit equals <code>scale</code>, then image with size <code>scale</code> * n will be selected, where n is the size of the icon in the UI. At least one image must be specified. Note that 'details.path = foo' is equivalent to 'details.path = {'16': foo}'" }, "tabId": { "type": "integer", diff --git a/chrome/common/extensions/api/declarative_content.json b/chrome/common/extensions/api/declarative_content.json index adb05fe00437d..bb9488d3dd281 100644 --- a/chrome/common/extensions/api/declarative_content.json +++ b/chrome/common/extensions/api/declarative_content.json @@ -64,7 +64,7 @@ }, { "id": "SetIcon", - "description": "Declarative event action that sets the 19-<abbr title=\"device-independent pixel\">dip</abbr> square icon for the extension's $(ref:pageAction page action) or $(ref:browserAction browser action) while the corresponding conditions are met. This action can be used without <a href=\"declare_permissions.html#host-permissions\">host permissions</a>, but the extension must have page or browser action.<p>Exactly one of <code>imageData</code> or <code>path</code> must be specified. Both are dictionaries mapping a number of pixels to an image representation. The image representation in <code>imageData</code> is an<a href=\"https://developer.mozilla.org/en-US/docs/Web/API/ImageData\">ImageData</a> object, for example from a <code><canvas></code> element, while the image representation in <code>path</code> is the path to an image file relative to he extension's manifest. If <code>scale</code> screen pixels fit into a device-independent pixel, the <code>scale * 19</code> icon will be used. If that scale is missing, the other image will be resized to the needed size.", + "description": "Declarative event action that sets the n-<abbr title=\"device-independent pixel\">dip</abbr> square icon for the extension's $(ref:pageAction page action) or $(ref:browserAction browser action) while the corresponding conditions are met. This action can be used without <a href=\"declare_permissions.html#host-permissions\">host permissions</a>, but the extension must have page or browser action.<p>Exactly one of <code>imageData</code> or <code>path</code> must be specified. Both are dictionaries mapping a number of pixels to an image representation. The image representation in <code>imageData</code> is an<a href=\"https://developer.mozilla.org/en-US/docs/Web/API/ImageData\">ImageData</a> object, for example from a <code><canvas></code> element, while the image representation in <code>path</code> is the path to an image file relative to he extension's manifest. If <code>scale</code> screen pixels fit into a device-independent pixel, the <code>scale * n</code> icon will be used. If that scale is missing, another image will be resized to the needed size.", "type": "object", "properties": { "instanceType": { @@ -76,28 +76,22 @@ { "$ref": "ImageDataType" }, { "type": "object", - "properties": { - "19": {"$ref": "ImageDataType", "optional": true}, - "38": {"$ref": "ImageDataType", "optional": true} - } + "additionalProperties": { "type": "any" } } ], "optional": true, - "description": "Either an ImageData object or a dictionary {size -> ImageData} representing icon to be set. If the icon is specified as a dictionary, the actual image to be used is chosen depending on screen's pixel density. If the number of image pixels that fit into one screen space unit equals <code>scale</code>, then image with size <code>scale</code> * 19 will be selected. Initially only scales 1 and 2 will be supported. At least one image must be specified. Note that 'details.imageData = foo' is equivalent to 'details.imageData = {'19': foo}'" + "description": "Either an ImageData object or a dictionary {size -> ImageData} representing icon to be set. If the icon is specified as a dictionary, the actual image to be used is chosen depending on screen's pixel density. If the number of image pixels that fit into one screen space unit equals <code>scale</code>, then image with size <code>scale</code> * n will be selected, where n is the size of the icon in the UI. At least one image must be specified. Note that 'details.imageData = foo' is equivalent to 'details.imageData = {'16': foo}'" } // TODO: "path": { // "choices": [ // { "type": "string" }, // { // "type": "object", -// "properties": { -// "19": {"type": "string", "optional": true}, -// "38": {"type": "string", "optional": true} -// } +// "additionalProperties": { "type": "any" } // } // ], // "optional": true, -// "description": "Either a relative image path or a dictionary {size -> relative image path} pointing to icon to be set. If the icon is specified as a dictionary, the actual image to be used is chosen depending on screen's pixel density. If the number of image pixels that fit into one screen space unit equals <code>scale</code>, then image with size <code>scale</code> * 19 will be selected. Initially only scales 1 and 2 will be supported. At least one image must be specified. Note that 'details.path = foo' is equivalent to 'details.imageData = {'19': foo}'" +// "description": "Either a relative image path or a dictionary {size -> relative image path} pointing to icon to be set. If the icon is specified as a dictionary, the actual image to be used is chosen depending on screen's pixel density. If the number of image pixels that fit into one screen space unit equals <code>scale</code>, then image with size <code>scale</code> * n will be selected, where n is the size of the icon in the UI. At least one image must be specified. Note that 'details.path = foo' is equivalent to 'details.path = {'16': foo}'" // } } }, diff --git a/chrome/common/extensions/api/page_action.json b/chrome/common/extensions/api/page_action.json index 09c496431c0d5..bd0bd0ff0f39b 100644 --- a/chrome/common/extensions/api/page_action.json +++ b/chrome/common/extensions/api/page_action.json @@ -89,28 +89,22 @@ { "$ref": "ImageDataType" }, { "type": "object", - "properties": { - "19": {"$ref": "ImageDataType", "optional": true}, - "38": {"$ref": "ImageDataType", "optional": true} - } + "additionalProperties": { "type": "any" } } ], "optional": true, - "description": "Either an ImageData object or a dictionary {size -> ImageData} representing icon to be set. If the icon is specified as a dictionary, the actual image to be used is chosen depending on screen's pixel density. If the number of image pixels that fit into one screen space unit equals <code>scale</code>, then image with size <code>scale</code> * 19 will be selected. Initially only scales 1 and 2 will be supported. At least one image must be specified. Note that 'details.imageData = foo' is equivalent to 'details.imageData = {'19': foo}'" + "description": "Either an ImageData object or a dictionary {size -> ImageData} representing icon to be set. If the icon is specified as a dictionary, the actual image to be used is chosen depending on screen's pixel density. If the number of image pixels that fit into one screen space unit equals <code>scale</code>, then image with size <code>scale</code> * n will be selected, where n is the size of the icon in the UI. At least one image must be specified. Note that 'details.imageData = foo' is equivalent to 'details.imageData = {'16': foo}'" }, "path": { "choices": [ { "type": "string" }, { "type": "object", - "properties": { - "19": {"type": "string", "optional": true}, - "38": {"type": "string", "optional": true} - } + "additionalProperties": { "type": "any" } } ], "optional": true, - "description": "Either a relative image path or a dictionary {size -> relative image path} pointing to icon to be set. If the icon is specified as a dictionary, the actual image to be used is chosen depending on screen's pixel density. If the number of image pixels that fit into one screen space unit equals <code>scale</code>, then image with size <code>scale</code> * 19 will be selected. Initially only scales 1 and 2 will be supported. At least one image must be specified. Note that 'details.path = foo' is equivalent to 'details.imageData = {'19': foo}'" + "description": "Either a relative image path or a dictionary {size -> relative image path} pointing to icon to be set. If the icon is specified as a dictionary, the actual image to be used is chosen depending on screen's pixel density. If the number of image pixels that fit into one screen space unit equals <code>scale</code>, then image with size <code>scale</code> * n will be selected, where n is the size of the icon in the UI. At least one image must be specified. Note that 'details.path = foo' is equivalent to 'details.path = {'16': foo}'" }, "iconIndex": { "type": "integer", diff --git a/chrome/test/data/extensions/api_test/browser_action/no_icon/background.js b/chrome/test/data/extensions/api_test/browser_action/no_icon/background.js index 7d8c6c9e814d4..95a747f108108 100644 --- a/chrome/test/data/extensions/api_test/browser_action/no_icon/background.js +++ b/chrome/test/data/extensions/api_test/browser_action/no_icon/background.js @@ -2,19 +2,19 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -var canvas = document.getElementById("canvas").getContext('2d'). - getImageData(0, 0, 19, 19); -var canvasHD = document.getElementById("canvas").getContext('2d'). - getImageData(0, 0, 38, 38); +var canvas = document.getElementById("canvas").getContext('2d').getImageData( + 0, 0, 21, 21); +var canvasHD = document.getElementById("canvas").getContext('2d').getImageData( + 0, 0, 42, 42); var setIconParamQueue = [ {imageData: canvas}, {path: 'icon.png'}, - {imageData: {'19': canvas, '38': canvasHD}}, - {path: {'19': 'icon.png', '38': 'icon.png'}}, - {imageData: {'19': canvas}}, - {path: {'19': 'icon.png'}}, - {imageData: {'38': canvasHD}}, + {imageData: {'21': canvas, '42': canvasHD}}, + {path: {'21': 'icon.png', '42': 'icon2.png'}}, + {imageData: {'21': canvas}}, + {path: {'21': 'icon.png'}}, + {imageData: {'42': canvasHD}}, {imageData: {}}, {path: {}}, ]; diff --git a/chrome/test/data/extensions/api_test/browser_action/no_icon/icon.png b/chrome/test/data/extensions/api_test/browser_action/no_icon/icon.png index 84c4be344f2e4..06dff11ca02f7 100644 Binary files a/chrome/test/data/extensions/api_test/browser_action/no_icon/icon.png and b/chrome/test/data/extensions/api_test/browser_action/no_icon/icon.png differ diff --git a/chrome/test/data/extensions/api_test/browser_action/no_icon/icon2.png b/chrome/test/data/extensions/api_test/browser_action/no_icon/icon2.png new file mode 100644 index 0000000000000..b6170c4403718 Binary files /dev/null and b/chrome/test/data/extensions/api_test/browser_action/no_icon/icon2.png differ diff --git a/extensions/renderer/resources/set_icon.js b/extensions/renderer/resources/set_icon.js index 5b66d6b359701..b0ed2b5b5fab3 100644 --- a/extensions/renderer/resources/set_icon.js +++ b/extensions/renderer/resources/set_icon.js @@ -5,15 +5,15 @@ var SetIconCommon = requireNative('setIcon').SetIconCommon; var sendRequest = require('sendRequest').sendRequest; -function loadImagePath(path, iconSize, callback) { +function loadImagePath(path, callback) { var img = new Image(); img.onerror = function() { console.error('Could not load action icon \'' + path + '\'.'); }; img.onload = function() { var canvas = document.createElement('canvas'); - canvas.width = img.width > iconSize ? iconSize : img.width; - canvas.height = img.height > iconSize ? iconSize : img.height; + canvas.width = img.width; + canvas.height = img.height; var canvas_context = canvas.getContext('2d'); canvas_context.clearRect(0, 0, canvas.width, canvas.height); @@ -25,27 +25,23 @@ function loadImagePath(path, iconSize, callback) { img.src = path; } -function verifyImageData(imageData, iconSize) { - // Verify that this at least looks like an ImageData element. +function smellsLikeImageData(imageData) { + // See if this object at least looks like an ImageData element. // Unfortunately, we cannot use instanceof because the ImageData // constructor is not public. // // We do this manually instead of using JSONSchema to avoid having these // properties show up in the doc. - if (!('width' in imageData) || - !('height' in imageData) || - !('data' in imageData)) { + return (typeof imageData == 'object') && ('width' in imageData) && + ('height' in imageData) && ('data' in imageData); +} + +function verifyImageData(imageData) { + if (!smellsLikeImageData(imageData)) { throw new Error( 'The imageData property must contain an ImageData object or' + ' dictionary of ImageData objects.'); } - - if (imageData.width > iconSize || - imageData.height > iconSize) { - throw new Error( - 'The imageData property must contain an ImageData object that ' + - 'is no larger than ' + iconSize + ' pixels square.'); - } } /** @@ -59,7 +55,6 @@ function verifyImageData(imageData, iconSize) { * callback may be called reentrantly. */ function setIcon(details, callback) { - var iconSizes = [19, 38]; // Note that iconIndex is actually deprecated, and only available to the // pageAction API. // TODO(kalman): Investigate whether this is for the pageActions API, and if @@ -70,24 +65,19 @@ function setIcon(details, callback) { } if ('imageData' in details) { - var isEmpty = true; - for (var i = 0; i < iconSizes.length; i++) { - var sizeKey = iconSizes[i].toString(); - if (sizeKey in details.imageData) { - verifyImageData(details.imageData[sizeKey], iconSizes[i]); - isEmpty =false; - } - } - - if (isEmpty) { - // If details.imageData is not dictionary with keys in set {'19', '38'}, - // it must be an ImageData object. - var sizeKey = iconSizes[0].toString(); + if (smellsLikeImageData(details.imageData)) { var imageData = details.imageData; details.imageData = {}; - details.imageData[sizeKey] = imageData; - verifyImageData(details.imageData[sizeKey], iconSizes[0]); + details.imageData[imageData.width.toString()] = imageData; + } else if (typeof details.imageData == 'object' && + Object.getOwnPropertyNames(details.imageData).length !== 0) { + for (var sizeKey in details.imageData) { + verifyImageData(details.imageData[sizeKey]); + } + } else { + verifyImageData(false); } + callback(SetIconCommon(details)); return; } @@ -95,37 +85,23 @@ function setIcon(details, callback) { if ('path' in details) { if (typeof details.path == 'object') { details.imageData = {}; - var isEmpty = true; - var processIconSize = function(index) { - if (index == iconSizes.length) { - delete details.path; - if (isEmpty) - throw new Error('The path property must not be empty.'); - callback(SetIconCommon(details)); - return; - } - var sizeKey = iconSizes[index].toString(); - if (!(sizeKey in details.path)) { - processIconSize(index + 1); - return; - } - isEmpty = false; - loadImagePath(details.path[sizeKey], iconSizes[index], - function(imageData) { - details.imageData[sizeKey] = imageData; - processIconSize(index + 1); - }); + var detailKeyCount = 0; + for (var iconSize in details.path) { + ++detailKeyCount; + loadImagePath(details.path[iconSize], function(size, imageData) { + details.imageData[size] = imageData; + if (--detailKeyCount == 0) + callback(SetIconCommon(details)); + }.bind(null, iconSize)); } - processIconSize(0); + if (detailKeyCount == 0) + throw new Error('The path property must not be empty.'); } else if (typeof details.path == 'string') { - var sizeKey = iconSizes[0].toString(); details.imageData = {}; - loadImagePath(details.path, iconSizes[0], - function(imageData) { - details.imageData[sizeKey] = imageData; - delete details.path; - callback(SetIconCommon(details)); - return; + loadImagePath(details.path, function(imageData) { + details.imageData[imageData.width.toString()] = imageData; + delete details.path; + callback(SetIconCommon(details)); }); } return; diff --git a/extensions/renderer/set_icon_natives.cc b/extensions/renderer/set_icon_natives.cc index 426acfe90ff49..2b6b097c46071 100644 --- a/extensions/renderer/set_icon_natives.cc +++ b/extensions/renderer/set_icon_natives.cc @@ -11,6 +11,7 @@ #include "base/macros.h" #include "base/memory/scoped_ptr.h" +#include "base/strings/string_number_conversions.h" #include "content/public/common/common_param_traits.h" #include "extensions/renderer/request_sender.h" #include "extensions/renderer/script_context.h" @@ -21,7 +22,6 @@ namespace { -const char* kImageSizeKeys[] = {"19", "38"}; const char kInvalidDimensions[] = "ImageData has invalid dimensions."; const char kInvalidData[] = "ImageData data length does not match dimensions."; const char kNoMemory[] = "Chrome was unable to initialize icon."; @@ -115,18 +115,20 @@ bool SetIconNatives::ConvertImageDataSetToBitmapValueSet( ->ToObject(isolate); DCHECK(bitmap_set_value); - for (size_t i = 0; i < arraysize(kImageSizeKeys); i++) { - if (!image_data_set->Has( - v8::String::NewFromUtf8(isolate, kImageSizeKeys[i]))) + + v8::Local<v8::Array> property_names(image_data_set->GetOwnPropertyNames()); + for (size_t i = 0; i < property_names->Length(); ++i) { + v8::Local<v8::Value> key(property_names->Get(i)); + v8::String::Utf8Value utf8_key(key); + int size; + if (!base::StringToInt(std::string(*utf8_key), &size)) continue; v8::Local<v8::Object> image_data = - image_data_set->Get(v8::String::NewFromUtf8(isolate, kImageSizeKeys[i])) - ->ToObject(isolate); + image_data_set->Get(key)->ToObject(isolate); v8::Local<v8::Value> image_data_bitmap; if (!ConvertImageDataToBitmapValue(image_data, &image_data_bitmap)) return false; - (*bitmap_set_value)->Set( - v8::String::NewFromUtf8(isolate, kImageSizeKeys[i]), image_data_bitmap); + (*bitmap_set_value)->Set(key, image_data_bitmap); } return true; }