From 14b331731bd1448294497bcf31ffa2116f571d97 Mon Sep 17 00:00:00 2001 From: Evan Stade Date: Thu, 28 Jan 2016 18:34:35 -0800 Subject: [PATCH] Fix the dynamic browser action setIcon path to work with any size icon. BUG=564926,575256 TBR=benwells@chromium.org Review URL: https://codereview.chromium.org/1580983002 Cr-Commit-Position: refs/heads/master@{#371133} (cherry picked from commit 168694381abc2a710a04fe765548849ebbde0e0e) Review URL: https://codereview.chromium.org/1647903003 . Cr-Commit-Position: refs/branch-heads/2623@{#186} Cr-Branched-From: 92d77538a86529ca35f9220bd3cd512cbea1f086-refs/heads/master@{#369907} --- .../browser_action_apitest.cc | 167 +++++++++--------- chrome/browser/extensions/extension_action.cc | 6 +- .../icon_with_badge_image_source.cc | 22 ++- .../ui/views/toolbar/toolbar_action_view.cc | 8 +- .../common/extensions/api/browser_action.json | 14 +- .../extensions/api/declarative_content.json | 16 +- chrome/common/extensions/api/page_action.json | 14 +- .../browser_action/no_icon/background.js | 18 +- .../api_test/browser_action/no_icon/icon.png | Bin 2799 -> 3159 bytes .../api_test/browser_action/no_icon/icon2.png | Bin 0 -> 4055 bytes extensions/renderer/resources/set_icon.js | 94 ++++------ extensions/renderer/set_icon_natives.cc | 18 +- 12 files changed, 168 insertions(+), 209 deletions(-) create mode 100644 chrome/test/data/extensions/api_test/browser_action/no_icon/icon2.png 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 scale, then image with size scale * 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 scale, then image with size scale * 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 scale, then image with size scale * 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 scale, then image with size scale * 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-dip 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 host permissions, but the extension must have page or browser action.

Exactly one of imageData or path must be specified. Both are dictionaries mapping a number of pixels to an image representation. The image representation in imageData is anImageData object, for example from a <canvas> element, while the image representation in path is the path to an image file relative to he extension's manifest. If scale screen pixels fit into a device-independent pixel, the scale * 19 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-dip 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 host permissions, but the extension must have page or browser action.

Exactly one of imageData or path must be specified. Both are dictionaries mapping a number of pixels to an image representation. The image representation in imageData is anImageData object, for example from a <canvas> element, while the image representation in path is the path to an image file relative to he extension's manifest. If scale screen pixels fit into a device-independent pixel, the scale * n 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 scale, then image with size scale * 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 scale, then image with size scale * 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 scale, then image with size scale * 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 scale, then image with size scale * 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 scale, then image with size scale * 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 scale, then image with size scale * 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 scale, then image with size scale * 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 scale, then image with size scale * 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 84c4be344f2e49f07828f00ab94f42f06cf8e615..06dff11ca02f7c7e5242a4d1789cec43f5b1645e 100644 GIT binary patch delta 531 zcmV+u0_^?o71tOciBL{Q4GJ0x0000DNk~Le0000L0000L2nGNE0I3(HmXRSJ0(jc7 zLN*G2V>2z=00006VoOIv0RI600RN!9r;`8x010qNS#tmY3ljhU3ljkVnw%H_000Mc zNliru-~kL56)C|;?56+#0f|XOK~y-)t<=wpk5Lo`@XwbiHJSxdij*c&Q%p8ivXN4= zR4gs5)GTGAlpR?Ro@+~*a`9u`99h&kdFF+-eBam4i&(e;=sMDKOr zw-Os8R*6j-)|;X}+<~9X{2gDhA<->3(1HIh>g^j9oxn&3zL@zSMq5PtSg0I5a5w9J zOT}h&NFR$K-TVvd6L%83kVA^K+t5Cw{rH71c!%3q!F`-Tuk}qY4>74_>y9MnRPI%${|!r7PY&RW;$qfsGgn0yih8ob z8_&GbB0AqF`YH3y7SZ+VG=_Z*4?o&sa-}A60LLnvWnxG2W`<4wC$Txn(*roqi9^Xb zUHqi~anv&2;}KR1|3A*WgwJ@2C;9!Raiiv_D?=Q#mDo+3B(4%4I!@4wHS&L5e*oIq VgxiUx))oK&002ovPDHLkV1ldx=l=iz delta 149 zcmcaE@m^H1Gr-TCmrII^fq{Y7)59eQNDF{42OE%-|NK93qM|&L#p#VsW?TkQaa|ce zQH5lWAYTTCDpdxChGqtapZ|gMO9qBg0|tgy2@DKYGZ+}e^C!h0b(`GBC800n>Eak- vaXR_W`2!#3H!>Zt$U3lsaWNZ@I5Wf2EAk(c8dUBA&1CR&^>bP0l+XkKN=7Tr 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 0000000000000000000000000000000000000000..b6170c44037186ae03479ce88eb65cdeada0cd72 GIT binary patch literal 4055 zcmV;|4=C`7P)Oz@Z0f2-7z;ux~O9+4z06=<WDR*FRcSTFz- zW=q650N5=6FiBTtNC2?60Km==3$g$R3;-}uh=nNt1bYBr$Ri_o0EC$U6h`t_Jn<{8 z5a%iY0C<_QJh>z}MS)ugEpZ1|S1ukX&Pf+56gFW3VVXcL!g-k)GJ!M?;PcD?0HBc- z5#WRK{dmp}uFlRjj{U%*%WZ25jX z{P*?XzTzZ-GF^d31o+^>%=Ap99M6&ogks$0k4OBs3;+Bb(;~!4V!2o<6ys46agIcq zjPo+3B8fthDa9qy|77CdEc*jK-!%ZRYCZvbku9iQV*~a}ClFY4z~c7+0P?$U!PF=S z1Au6Q;m>#f??3%Vpd|o+W=WE9003S@Bra6Svp>fO002awfhw>;8}z{#EWidF!3EsG z3;bXU&9EIRU@z1_9W=mEXoiz;4lcq~xDGvV5BgyU zp1~-*fe8db$Osc*A=-!mVv1NJjtCc-h4>-CNCXm#Bp}I%6j35eku^v$Qi@a{RY)E3 zJ#qp$hg?Rwkvqr$GJ^buyhkyVfwECO)C{#lxu`c9ghrwZ&}4KmnvWKso6vH!8a<3Q zq36)6Xb;+tK10Vaz~~qUGsJ8#F2=(`u{bOVlVi)VBCHIn#u~6ztOL7=^<&SmcLWlF zMZgI*1b0FpVIDz9SWH+>*hr`#93(Um+6gxa1B6k+CnA%mOSC4s5&6UzVlpv@SV$}* z))J2sFA#f(L&P^E5{W}HC%KRUNwK6<(h|}}(r!{C=`5+6G)NjFlgZj-YqAG9lq?`C z$c5yc>d>VnA`E_*3F2Qp##d8RZb=H01_mm@+|Cqnc9PsG(F5HIG_C zt)aG3uTh7n6Et<2In9F>NlT@zqLtGcXcuVrX|L#Xx)I%#9!{6gSJKPrN9dR61N3(c z4Tcqi$B1Vr8Jidf7-t!G7_XR2rWwr)$3XQ?}=hpK0&Z&W{| zep&sA23f;Q!%st`QJ}G3cbou<7-yIK2z4nfCCCtN2-XOGSWo##{8Q{ATurxr~;I`ytDs%xbip}RzP zziy}Qn4Z2~fSycmr`~zJ=lUFdFa1>gZThG6M+{g7vkW8#+YHVaJjFF}Z#*3@$J_By zLtVo_L#1JrVVB{Ak-5=4qt!-@Mh}c>#$4kh<88)m#-k<%CLtzEP3leVno>={htGUuD;o7bD)w_sX$S}eAxwzy?UvgBH(S?;#HZiQMoS*2K2 zT3xe7t(~nU*1N5{rxB;QPLocnp4Ml>u<^FZwyC!nu;thW+pe~4wtZn|Vi#w(#jeBd zlf9FDx_yoPJqHbk*$%56S{;6Kv~mM9!g3B(KJ}#RZ#@)!hR|78Dq|Iq-afF%KE1Brn_fm;Im z_u$xr8UFki1L{Ox>G0o)(&RAZ;=|I=wN2l97;cLaHH6leTB-XXa*h%dBOEvi`+x zi?=Txl?TadvyiL>SuF~-LZ;|cS}4~l2eM~nS7yJ>iOM;atDY;(?aZ^v+mJV$@1Ote z62cPUlD4IWOIIx&SmwQ~YB{nzae3Pc;}r!fhE@iwJh+OsDs9zItL;~pu715HdQEGA zUct(O!LkCy1<%NCg+}G`0PgpNm-?d@-hMgNe6^V+j6x$b<6@S<$+<4_1hi}Ti zncS4LsjI}fWY1>OX6feMEuLErma3QLmkw?X+1j)X-&VBk_4Y;EFPF_I+q;9dL%E~B zJh;4Nr^(LEJ3myURP{Rblsw%57T)g973R8o)DE9*xN#~;4_o$q%o z4K@u`jhx2fBXC4{U8Qn{*%*B$Ge=nny$HAYq{=vy|sI0 z_vss+H_qMky?OB#|JK!>IX&II^LlUh#rO5!7TtbwC;iULyV-Xq?ybB}ykGP{?LpZ? z-G|jbTmIbG@7#ZCz;~eY(cDM(28Dyq{*m>M4?_iynUBkc4TkHUI6gT!;y-fz>HMcd z&t%Ugo)`Y2{>!cx7B7DI)$7;J(U{Spm-3gBzioV_{p!H$8L!*M!p0uH$#^p{Ui4P` z?ZJ24cOCDe-w#jZd?0@)|7iKK^;6KN`;!@ylm7$*nDhK&GcDTy000JJOGiWi{{a60 z|De66lK=n!32;bRa{vGf6951U69E94oEQKA00(qQO+^Rb0TdP#5ED5-sQ>^42}wjj zR9M5sncHtvRTPK6=}c!jy<#cl4&@RgT0td7j0R2AClmfF{tVxIGx1Gc)p$cN8Uz#~ zpcOB*g+kk5rtQpheECjTIhoSA9O6!PX6BrI_WA8~`>nMm*Mb&Zj&^9vB8jfKk`9fK4aBfU^SJ z18x9=KnXYu40%7-i$php8ZZys1Qxw^1Nhh%BDm*)7d_B4(C{EtpZguS1Kbkq7%&Tb zpmb+}@qQ#a1RMaSfHhy7(;Sjf295x)0q0y_cl~3fe^=FS06zmuz_;#S0uJ>HbU;;> zoQWOf!nzW05I73#0VaW_Qf+$gl=Beq1@Jkr3VbEN_dH-vKuH10E2UbnJv_)>V81U- z*5*@tjk{kBxGvDDX0hy=o`I&Lz`S!kTpv`eQ*FS-Q2ovM@3NX6cq&kkn=w7vzuQZ2*syN*d^{ zZGdh9KgmX40)GO>fOiCXQjhOD;5W8#f+Db6+8jx(k4ecKa79U$ftP@H1$sP15{0bd zXFAaDkvd0$^p=(W9qsX!Y*qr!3-ncaf3yvIWgh5jdhvIre1!lq0eabc4Ku7^V4n%d z*cLRKN&Jiwod-SwK2ch^p!S9#Y*i_jUH^^uhRi`Ho)yrIR5Fs6aUQqHwu&L;s{;># zU;N(E?$0RUXQuj`o_pSmXfx$ZRi(LP8h6#2tD;k02d){8%BFz%tRx!iq)}i7c-zdr z8Nk*Bx+2iKmM7Qr{D<22B}u1nQhNH0=ncupK)UHG%s|wrc1);`hF7wDJ_{%y+TRsJy-m{A(0=pb+Z) z0BvcLkHU%5)Y;9(C7I|>pv;<@YWBnRF)TGwVW|_2Zx_tRjh*qiYQI1?bf9+B*-d&E zH56yNG0;!^Txb*MdjXmxH%qC54+nsKtG?H&bGuSx#K~vIRsoTag$(Ci3X&p3)KGAO}l6dJ_2-r#K(P>wtB5hu@i}$P74*;jl z17^BO^J0fMX&KI$>BLRO1i2RMxSnb=HoX48hS(M0Q$Y^MPG{|<&UBMFw|rSmwJZXE z*l1lz^@#GWo%eV0+^Vw@o=-Z=bMkv$Ri87DJ#CkyW1#CMwf9VN=X}vk?K9T(e_8sj zNQawBx8nV(T1eWH3rcyvl6)O_(;RlZT|f3|J^aPc1^!HRHB+%Ojv;3uP+=}KZ%cad zgrsm-&pxhgPJ3^l+u|~jbN<8^%r<1&3OQ5Y(<@kS({{YdYuRG$j&ieoW002ov JPDHLkV1mngn^gb+ literal 0 HcmV?d00001 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 property_names(image_data_set->GetOwnPropertyNames()); + for (size_t i = 0; i < property_names->Length(); ++i) { + v8::Local key(property_names->Get(i)); + v8::String::Utf8Value utf8_key(key); + int size; + if (!base::StringToInt(std::string(*utf8_key), &size)) continue; v8::Local image_data = - image_data_set->Get(v8::String::NewFromUtf8(isolate, kImageSizeKeys[i])) - ->ToObject(isolate); + image_data_set->Get(key)->ToObject(isolate); v8::Local 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; }