Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(178)

Issues Search
    My Issues | Starred     Open | Closed | All

Unified Diff: chrome/browser/extensions/api/extension_action/browser_action_apitest.cc

Issue 1580983002: Fix the dynamic browser action setIcon path to work with any size icon. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: mac compatible image equality check Created 4 years, 11 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | chrome/browser/ui/extensions/icon_with_badge_image_source.cc » ('j') | chrome/browser/ui/extensions/icon_with_badge_image_source.cc » ('J')
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
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 4426fa7b827fd06aea84f80f478f5677e12452e5..27abc86082d8dbc4cf4fa2104e974adeaa3daf45 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,28 +69,6 @@ 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);
-}
-
class BrowserActionApiTest : public ExtensionApiTest {
public:
BrowserActionApiTest() {}
@@ -188,140 +167,135 @@ 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;
- EXPECT_FALSE(action_icon.ToImageSkia()->HasRepresentation(2.0f));
-
- EXPECT_TRUE(
- ImagesAreEqualAtScale(AddBackground(*action_icon.ToImageSkia()),
- *GetBrowserActionsBar()->GetIcon(0).ToImageSkia(),
- 1.0f));
+ 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);
+
Devlin 2016/01/19 22:44:40 extra newline
Evan Stade 2016/01/20 23:04:36 Done.
+ 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(AddBackground(*action_icon.ToImageSkia()),
- *GetBrowserActionsBar()->GetIcon(0).ToImageSkia(),
- 1.0f));
+ EXPECT_TRUE(action_icon.ToImageSkia()->HasRepresentation(kSmallIconScale));
Devlin 2016/01/19 22:44:40 nit: please comment above these why we expect/don'
Evan Stade 2016/01/20 23:04:36 done, also see comment above the kSmallIconScale d
+ 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;
- EXPECT_TRUE(action_icon.ToImageSkia()->HasRepresentation(2.0f));
-
- EXPECT_TRUE(
- ImagesAreEqualAtScale(AddBackground(*action_icon.ToImageSkia()),
- *GetBrowserActionsBar()->GetIcon(0).ToImageSkia(),
- 1.0f));
+ EXPECT_TRUE(action_icon.ToImageSkia()->HasRepresentation(kSmallIconScale));
+ EXPECT_TRUE(action_icon.ToImageSkia()->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;
- EXPECT_TRUE(action_icon.ToImageSkia()->HasRepresentation(2.0f));
-
- EXPECT_TRUE(
- ImagesAreEqualAtScale(AddBackground(*action_icon.ToImageSkia()),
- *GetBrowserActionsBar()->GetIcon(0).ToImageSkia(),
- 1.0f));
+ EXPECT_TRUE(action_icon.ToImageSkia()->HasRepresentation(kSmallIconScale));
+ EXPECT_TRUE(action_icon.ToImageSkia()->HasRepresentation(kLargeIconScale));
// 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);
+ 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_FALSE(action_icon.ToImageSkia()->HasRepresentation(2.0f));
-
- EXPECT_TRUE(
- ImagesAreEqualAtScale(AddBackground(*action_icon.ToImageSkia()),
- *GetBrowserActionsBar()->GetIcon(0).ToImageSkia(),
- 1.0f));
+ 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.
Devlin 2016/01/19 22:44:40 These comments (also on line 250) need to be updat
Evan Stade 2016/01/20 23:04:36 Done.
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;
- EXPECT_FALSE(action_icon.ToImageSkia()->HasRepresentation(2.0f));
-
- EXPECT_TRUE(
- ImagesAreEqualAtScale(AddBackground(*action_icon.ToImageSkia()),
- *GetBrowserActionsBar()->GetIcon(0).ToImageSkia(),
- 1.0f));
+ 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));
+ EXPECT_FALSE(action_icon.ToImageSkia()->HasRepresentation(kSmallIconScale));
+ EXPECT_TRUE(action_icon.ToImageSkia()->HasRepresentation(kLargeIconScale));
// Try setting icon with empty dictionary of ImageData objects.
GetBrowserActionsBar()->Press(0);
« no previous file with comments | « no previous file | chrome/browser/ui/extensions/icon_with_badge_image_source.cc » ('j') | chrome/browser/ui/extensions/icon_with_badge_image_source.cc » ('J')

Powered by Google App Engine
This is Rietveld 408576698