summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authortbarzic@chromium.org <tbarzic@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2012-09-18 05:53:45 +0000
committertbarzic@chromium.org <tbarzic@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2012-09-18 05:53:45 +0000
commit684cc6a02fb5e536a52cbccece01ef5c319252f1 (patch)
treea79b30a8b8d997a71e8e323fa10672c2497a3436
parent910b34bb21c120b78aa84648d09330a415d538dc (diff)
downloadchromium_src-684cc6a02fb5e536a52cbccece01ef5c319252f1.zip
chromium_src-684cc6a02fb5e536a52cbccece01ef5c319252f1.tar.gz
chromium_src-684cc6a02fb5e536a52cbccece01ef5c319252f1.tar.bz2
Change browser/page action default icon defined in manifest to support hidpi.
To support hidpi for browser action default icons, ability to define dictionary of icon in manifest as default icons is added. Defining images of sizes 19 and 38 is allowed (other dictionary values will be ignored). The image to be painted will be determined based on screen density. Similary, for script badges, default icon is determined using 16 and 32 px icon defined in extension manifest. I have extracted actual icon loading code to extension_action_icon_factory.h/.cc, so it doesn't have to be implemented for all platform specific solutions. BUG=138025,135271 Review URL: https://chromiumcodereview.appspot.com/10905005 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@157309 0039d316-1c4b-4281-b951-d872f2087c98
-rw-r--r--build/android/gtest_filter/unit_tests_disabled1
-rw-r--r--chrome/browser/extensions/api/extension_action/browser_action_apitest.cc25
-rw-r--r--chrome/browser/extensions/api/extension_action/page_action_apitest.cc7
-rw-r--r--chrome/browser/extensions/api/extension_action/page_as_browser_action_apitest.cc7
-rw-r--r--chrome/browser/extensions/extension_action_icon_factory.cc66
-rw-r--r--chrome/browser/extensions/extension_action_icon_factory.h66
-rw-r--r--chrome/browser/extensions/extension_action_icon_factory_unittest.cc266
-rw-r--r--chrome/browser/extensions/extension_icon_image.cc2
-rw-r--r--chrome/browser/extensions/extension_icon_image_unittest.cc26
-rw-r--r--chrome/browser/ui/cocoa/extensions/browser_action_button.h4
-rw-r--r--chrome/browser/ui/cocoa/extensions/browser_action_button.mm48
-rw-r--r--chrome/browser/ui/cocoa/location_bar/page_action_decoration.h21
-rw-r--r--chrome/browser/ui/cocoa/location_bar/page_action_decoration.mm18
-rw-r--r--chrome/browser/ui/gtk/browser_actions_toolbar_gtk.cc49
-rw-r--r--chrome/browser/ui/gtk/location_bar_view_gtk.cc19
-rw-r--r--chrome/browser/ui/gtk/location_bar_view_gtk.h18
-rw-r--r--chrome/browser/ui/views/browser_action_view.cc36
-rw-r--r--chrome/browser/ui/views/browser_action_view.h30
-rw-r--r--chrome/browser/ui/views/browser_actions_container_browsertest.cc13
-rw-r--r--chrome/browser/ui/views/location_bar/page_action_image_view.cc32
-rw-r--r--chrome/browser/ui/views/location_bar/page_action_image_view.h20
-rw-r--r--chrome/chrome_browser_extensions.gypi2
-rw-r--r--chrome/chrome_tests.gypi2
-rw-r--r--chrome/common/extensions/docs/templates/intros/browserAction.html45
-rw-r--r--chrome/common/extensions/docs/templates/intros/pageAction.html29
-rw-r--r--chrome/common/extensions/extension.cc135
-rw-r--r--chrome/common/extensions/extension.h3
-rw-r--r--chrome/common/extensions/extension_action.cc60
-rw-r--r--chrome/common/extensions/extension_action.h46
-rw-r--r--chrome/common/extensions/extension_action_unittest.cc54
-rw-r--r--chrome/common/extensions/extension_constants.cc16
-rw-r--r--chrome/common/extensions/extension_constants.h9
-rw-r--r--chrome/common/extensions/extension_file_util.cc59
-rw-r--r--chrome/common/extensions/extension_unittest.cc6
-rw-r--r--chrome/common/extensions/manifest_tests/extension_manifests_browseraction_unittest.cc102
-rw-r--r--chrome/common/extensions/manifest_tests/extension_manifests_pageaction_unittest.cc2
-rw-r--r--chrome/common/extensions/manifest_tests/extension_manifests_scriptbadge_unittest.cc54
-rw-r--r--chrome/test/data/extensions/manifest_tests/script_badge_basic.json1
-rw-r--r--ui/gfx/image/image_skia_operations.cc11
39 files changed, 988 insertions, 422 deletions
diff --git a/build/android/gtest_filter/unit_tests_disabled b/build/android/gtest_filter/unit_tests_disabled
index 60d74cf..ca23cd6 100644
--- a/build/android/gtest_filter/unit_tests_disabled
+++ b/build/android/gtest_filter/unit_tests_disabled
@@ -95,6 +95,7 @@ ExtensionSettingsSyncTest.*
ExtensionUpdaterTest.*
UserScriptListenerTest.*
WebApplicationTest.GetShortcutInfoForTab
+ExtensionActionIconFactoryTest.*
# crbug.com/139411
AutocompleteProviderTest.*
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 11f8ccb..f1daae3 100644
--- a/chrome/browser/extensions/api/extension_action/browser_action_apitest.cc
+++ b/chrome/browser/extensions/api/extension_action/browser_action_apitest.cc
@@ -9,6 +9,7 @@
#endif
#include "chrome/browser/extensions/browser_action_test_util.h"
+#include "chrome/browser/extensions/extension_action_icon_factory.h"
#include "chrome/browser/extensions/extension_apitest.h"
#include "chrome/browser/extensions/extension_service.h"
#include "chrome/browser/extensions/extension_tab_util.h"
@@ -131,17 +132,21 @@ IN_PROC_BROWSER_TEST_F(BrowserActionApiTest, DynamicBrowserAction) {
gfx::test::SetSupportedScaleFactorsTo1xAnd2x();
#endif
+ // We should not be creating icons asynchronously, so we don't need an
+ // observer.
+ ExtensionActionIconFactory icon_factory(extension,
+ extension->browser_action(),
+ NULL);
// Test that there is a browser action in the toolbar.
ASSERT_EQ(1, GetBrowserActionsBar().NumberOfBrowserActions());
EXPECT_TRUE(GetBrowserActionsBar().HasIcon(0));
- gfx::Image action_icon = extension->browser_action()->GetIcon(0);
+ gfx::Image action_icon = icon_factory.GetIcon(0);
uint32_t action_icon_last_id = action_icon.ToSkBitmap()->getGenerationID();
// Let's check that |GetIcon| doesn't always return bitmap with new id.
ASSERT_EQ(action_icon_last_id,
- extension->browser_action()->GetIcon(0).ToSkBitmap()->
- getGenerationID());
+ icon_factory.GetIcon(0).ToSkBitmap()->getGenerationID());
uint32_t action_icon_current_id = 0;
@@ -151,7 +156,7 @@ IN_PROC_BROWSER_TEST_F(BrowserActionApiTest, DynamicBrowserAction) {
GetBrowserActionsBar().Press(0);
ASSERT_TRUE(catcher.GetNextResult());
- action_icon = extension->browser_action()->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);
@@ -169,7 +174,7 @@ IN_PROC_BROWSER_TEST_F(BrowserActionApiTest, DynamicBrowserAction) {
GetBrowserActionsBar().Press(0);
ASSERT_TRUE(catcher.GetNextResult());
- action_icon = extension->browser_action()->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);
@@ -188,7 +193,7 @@ IN_PROC_BROWSER_TEST_F(BrowserActionApiTest, DynamicBrowserAction) {
GetBrowserActionsBar().Press(0);
ASSERT_TRUE(catcher.GetNextResult());
- action_icon = extension->browser_action()->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);
@@ -206,7 +211,7 @@ IN_PROC_BROWSER_TEST_F(BrowserActionApiTest, DynamicBrowserAction) {
GetBrowserActionsBar().Press(0);
ASSERT_TRUE(catcher.GetNextResult());
- action_icon = extension->browser_action()->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);
@@ -225,7 +230,7 @@ IN_PROC_BROWSER_TEST_F(BrowserActionApiTest, DynamicBrowserAction) {
GetBrowserActionsBar().Press(0);
ASSERT_TRUE(catcher.GetNextResult());
- action_icon = extension->browser_action()->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);
@@ -244,7 +249,7 @@ IN_PROC_BROWSER_TEST_F(BrowserActionApiTest, DynamicBrowserAction) {
GetBrowserActionsBar().Press(0);
ASSERT_TRUE(catcher.GetNextResult());
- action_icon = extension->browser_action()->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);
@@ -263,7 +268,7 @@ IN_PROC_BROWSER_TEST_F(BrowserActionApiTest, DynamicBrowserAction) {
GetBrowserActionsBar().Press(0);
ASSERT_TRUE(catcher.GetNextResult());
- action_icon = extension->browser_action()->GetIcon(0);
+ action_icon = icon_factory.GetIcon(0);
const gfx::ImageSkia* action_icon_skia = action_icon.ToImageSkia();
diff --git a/chrome/browser/extensions/api/extension_action/page_action_apitest.cc b/chrome/browser/extensions/api/extension_action/page_action_apitest.cc
index aafb5c1..4e18704 100644
--- a/chrome/browser/extensions/api/extension_action/page_action_apitest.cc
+++ b/chrome/browser/extensions/api/extension_action/page_action_apitest.cc
@@ -3,6 +3,7 @@
// found in the LICENSE file.
#include "chrome/browser/extensions/browser_event_router.h"
+#include "chrome/browser/extensions/extension_action_icon_factory.h"
#include "chrome/browser/extensions/extension_apitest.h"
#include "chrome/browser/extensions/extension_service.h"
#include "chrome/browser/extensions/extension_tab_util.h"
@@ -71,10 +72,14 @@ IN_PROC_BROWSER_TEST_F(ExtensionApiTest, PageAction) {
ASSERT_TRUE(catcher.GetNextResult());
}
+ // We should not be creating icons asynchronously, so we don't need an
+ // observer.
+ ExtensionActionIconFactory icon_factory(extension, action, NULL);
+
// Test that we received the changes.
tab_id = SessionTabHelper::FromWebContents(
chrome::GetActiveWebContents(browser()))->session_id().id();
- EXPECT_FALSE(action->GetIcon(tab_id).IsEmpty());
+ EXPECT_FALSE(icon_factory.GetIcon(tab_id).IsEmpty());
}
// Test that calling chrome.pageAction.setPopup() can enable a popup.
diff --git a/chrome/browser/extensions/api/extension_action/page_as_browser_action_apitest.cc b/chrome/browser/extensions/api/extension_action/page_as_browser_action_apitest.cc
index 95136cd..53dfcb7 100644
--- a/chrome/browser/extensions/api/extension_action/page_as_browser_action_apitest.cc
+++ b/chrome/browser/extensions/api/extension_action/page_as_browser_action_apitest.cc
@@ -4,6 +4,7 @@
#include "base/command_line.h"
#include "chrome/browser/extensions/browser_action_test_util.h"
+#include "chrome/browser/extensions/extension_action_icon_factory.h"
#include "chrome/browser/extensions/extension_apitest.h"
#include "chrome/browser/extensions/extension_prefs.h"
#include "chrome/browser/extensions/extension_service.h"
@@ -93,8 +94,12 @@ IN_PROC_BROWSER_TEST_F(PageAsBrowserActionApiTest, Basic) {
ASSERT_TRUE(catcher.GetNextResult());
}
+ // We should not be creating icons asynchronously, so we don't need an
+ // observer.
+ ExtensionActionIconFactory icon_factory(extension, action, NULL);
+
// Test that we received the changes.
- EXPECT_FALSE(action->GetIcon(tab_id).IsEmpty());
+ EXPECT_FALSE(icon_factory.GetIcon(tab_id).IsEmpty());
}
// Test that calling chrome.pageAction.setPopup() can enable a popup.
diff --git a/chrome/browser/extensions/extension_action_icon_factory.cc b/chrome/browser/extensions/extension_action_icon_factory.cc
new file mode 100644
index 0000000..8638159
--- /dev/null
+++ b/chrome/browser/extensions/extension_action_icon_factory.cc
@@ -0,0 +1,66 @@
+// 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 "chrome/browser/extensions/extension_action_icon_factory.h"
+
+#include "chrome/common/extensions/extension.h"
+#include "chrome/common/extensions/extension_action.h"
+#include "chrome/common/extensions/extension_icon_set.h"
+#include "grit/theme_resources.h"
+#include "ui/base/resource/resource_bundle.h"
+#include "ui/gfx/image/image_skia.h"
+
+using extensions::Extension;
+using extensions::IconImage;
+
+namespace {
+
+gfx::ImageSkia GetDefaultIcon() {
+ return *ui::ResourceBundle::GetSharedInstance().GetImageNamed(
+ IDR_EXTENSIONS_FAVICON).ToImageSkia();
+}
+
+} // namespace
+
+ExtensionActionIconFactory::ExtensionActionIconFactory(
+ const Extension* extension,
+ const ExtensionAction* action,
+ Observer* observer)
+ : extension_(extension),
+ action_(action),
+ observer_(observer) {
+ if (action_->default_icon()) {
+ default_icon_.reset(new IconImage(
+ extension_,
+ *action_->default_icon(),
+ ExtensionAction::GetIconSizeForType(action_->action_type()),
+ GetDefaultIcon(),
+ this));
+ }
+}
+
+ExtensionActionIconFactory::~ExtensionActionIconFactory() {}
+
+// extensions::IconImage::Observer overrides.
+void ExtensionActionIconFactory::OnExtensionIconImageChanged(IconImage* image) {
+ if (observer_)
+ observer_->OnIconUpdated();
+}
+
+gfx::Image ExtensionActionIconFactory::GetIcon(int tab_id) {
+ gfx::ImageSkia base_icon = GetBaseIconFromAction(tab_id);
+ return action_->ApplyAttentionAndAnimation(base_icon, tab_id);
+}
+
+gfx::ImageSkia ExtensionActionIconFactory::GetBaseIconFromAction(int tab_id) {
+ gfx::ImageSkia icon = action_->GetExplicitlySetIcon(tab_id);
+ if (!icon.isNull())
+ return icon;
+
+ if (default_icon_.get())
+ return default_icon_->image_skia();
+
+ return GetDefaultIcon();
+}
+
diff --git a/chrome/browser/extensions/extension_action_icon_factory.h b/chrome/browser/extensions/extension_action_icon_factory.h
new file mode 100644
index 0000000..a93c9a1
--- /dev/null
+++ b/chrome/browser/extensions/extension_action_icon_factory.h
@@ -0,0 +1,66 @@
+// 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.
+
+#ifndef CHROME_BROWSER_EXTENSIONS_EXTENSION_ACTION_ICON_FACTORY_H_
+#define CHROME_BROWSER_EXTENSIONS_EXTENSION_ACTION_ICON_FACTORY_H_
+
+#include "base/memory/scoped_ptr.h"
+#include "chrome/browser/extensions/extension_icon_image.h"
+
+class ExtensionAction;
+class ExtensionIconSet;
+
+namespace extensions {
+class Extension;
+}
+
+// Used to get an icon to be used in the UI for an extension action.
+// If the extension action icon is the default icon defined in the extension's
+// manifest, it is loaded using extensions::IconImage. This icon can be loaded
+// asynchronously. The factory observes underlying IconImage and notifies its
+// own observer when the icon image changes.
+class ExtensionActionIconFactory : public extensions::IconImage::Observer {
+ public:
+ class Observer {
+ public:
+ virtual ~Observer() {}
+ // Called when the underlying icon image changes.
+ virtual void OnIconUpdated() = 0;
+ };
+
+ // Observer should outlive this.
+ ExtensionActionIconFactory(const extensions::Extension* extension,
+ const ExtensionAction* action,
+ Observer* observer);
+ virtual ~ExtensionActionIconFactory();
+
+ // extensions::IconImage override.
+ virtual void OnExtensionIconImageChanged(
+ extensions::IconImage* image) OVERRIDE;
+
+ // Gets the extension action icon for the tab.
+ // If there is an icon set using |SetIcon|, that icon is returned.
+ // Else, if there is a default icon set for the extension action, the icon is
+ // created using IconImage. Observer is triggered wheniever the icon gets
+ // updated.
+ // Else, extension favicon is returned.
+ // In all cases, action's attention and animation icon transformations are
+ // applied on the icon.
+ gfx::Image GetIcon(int tab_id);
+
+ private:
+ // Gets the icon that should be returned by |GetIcon| (without the attention
+ // and animation transformations).
+ gfx::ImageSkia GetBaseIconFromAction(int tab_id);
+
+ const extensions::Extension* extension_;
+ const ExtensionAction* action_;
+ Observer* observer_;
+ // Underlying icon image for the default icon.
+ scoped_ptr<extensions::IconImage> default_icon_;
+
+ DISALLOW_COPY_AND_ASSIGN(ExtensionActionIconFactory);
+};
+
+#endif // CHROME_BROWSER_EXTENSIONS_EXTENSION_ACTION_ICON_FACTORY_H_
diff --git a/chrome/browser/extensions/extension_action_icon_factory_unittest.cc b/chrome/browser/extensions/extension_action_icon_factory_unittest.cc
new file mode 100644
index 0000000..cea9a78
--- /dev/null
+++ b/chrome/browser/extensions/extension_action_icon_factory_unittest.cc
@@ -0,0 +1,266 @@
+// 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 "chrome/browser/extensions/extension_action_icon_factory.h"
+
+#include "base/file_util.h"
+#include "base/json/json_file_value_serializer.h"
+#include "base/message_loop.h"
+#include "base/path_service.h"
+#include "chrome/common/chrome_paths.h"
+#include "chrome/common/extensions/extension.h"
+#include "chrome/common/extensions/extension_action.h"
+#include "content/public/test/test_browser_thread.h"
+#include "grit/theme_resources.h"
+#include "skia/ext/image_operations.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "ui/base/resource/resource_bundle.h"
+#include "ui/gfx/image/image_skia.h"
+#include "ui/gfx/skia_util.h"
+#include "webkit/glue/image_decoder.h"
+
+using content::BrowserThread;
+using extensions::Extension;
+
+namespace {
+
+bool ImageRepsAreEqual(const gfx::ImageSkiaRep& image_rep1,
+ const gfx::ImageSkiaRep& image_rep2) {
+ return image_rep1.scale_factor() == image_rep2.scale_factor() &&
+ gfx::BitmapsAreEqual(image_rep1.sk_bitmap(), image_rep2.sk_bitmap());
+}
+
+gfx::Image EnsureImageSize(const gfx::Image& original, int size) {
+ const SkBitmap* original_bitmap = original.ToSkBitmap();
+ if (original_bitmap->width() == size && original_bitmap->height() == size)
+ return original;
+
+ SkBitmap resized = skia::ImageOperations::Resize(
+ *original.ToSkBitmap(), skia::ImageOperations::RESIZE_LANCZOS3,
+ size, size);
+ return gfx::Image(resized);
+}
+
+gfx::ImageSkiaRep CreateBlankRep(int size_dip, ui::ScaleFactor scale_factor) {
+ SkBitmap bitmap;
+ const float scale = ui::GetScaleFactorScale(scale_factor);
+ bitmap.setConfig(SkBitmap::kARGB_8888_Config,
+ static_cast<int>(size_dip * scale),
+ static_cast<int>(size_dip * scale));
+ bitmap.allocPixels();
+ bitmap.eraseColor(SkColorSetARGB(0, 0, 0, 0));
+ return gfx::ImageSkiaRep(bitmap, scale_factor);
+}
+
+gfx::Image LoadIcon(const std::string& filename) {
+ FilePath path;
+ PathService::Get(chrome::DIR_TEST_DATA, &path);
+ path = path.AppendASCII("extensions/api_test").AppendASCII(filename);
+
+ std::string file_contents;
+ file_util::ReadFileToString(path, &file_contents);
+ const unsigned char* data =
+ reinterpret_cast<const unsigned char*>(file_contents.data());
+
+ SkBitmap bitmap;
+ webkit_glue::ImageDecoder decoder;
+ bitmap = decoder.Decode(data, file_contents.length());
+
+ return gfx::Image(bitmap);
+}
+
+class ExtensionActionIconFactoryTest
+ : public testing::Test,
+ public ExtensionActionIconFactory::Observer {
+ public:
+ ExtensionActionIconFactoryTest()
+ : quit_in_icon_updated_(false),
+ ui_thread_(BrowserThread::UI, &ui_loop_),
+ file_thread_(BrowserThread::FILE),
+ io_thread_(BrowserThread::IO) {
+ }
+
+ virtual ~ExtensionActionIconFactoryTest() {}
+
+ void WaitForIconUpdate() {
+ quit_in_icon_updated_ = true;
+ MessageLoop::current()->Run();
+ quit_in_icon_updated_ = false;
+ }
+
+ scoped_refptr<Extension> CreateExtension(const char* name,
+ Extension::Location location) {
+ // Create and load an extension.
+ FilePath test_file;
+ if (!PathService::Get(chrome::DIR_TEST_DATA, &test_file)) {
+ EXPECT_FALSE(true);
+ return NULL;
+ }
+ test_file = test_file.AppendASCII("extensions/api_test").AppendASCII(name);
+ int error_code = 0;
+ std::string error;
+ JSONFileValueSerializer serializer(test_file.AppendASCII("manifest.json"));
+ scoped_ptr<DictionaryValue> valid_value(
+ static_cast<DictionaryValue*>(serializer.Deserialize(&error_code,
+ &error)));
+ EXPECT_EQ(0, error_code) << error;
+ if (error_code != 0)
+ return NULL;
+
+ EXPECT_TRUE(valid_value.get());
+ if (!valid_value.get())
+ return NULL;
+
+ return Extension::Create(test_file, location, *valid_value,
+ Extension::NO_FLAGS, &error);
+ }
+
+ // testing::Test overrides:
+ virtual void SetUp() OVERRIDE {
+ file_thread_.Start();
+ io_thread_.Start();
+ }
+
+ // ExtensionActionIconFactory::Observer overrides:
+ virtual void OnIconUpdated() OVERRIDE {
+ if (quit_in_icon_updated_)
+ MessageLoop::current()->Quit();
+ }
+
+ gfx::ImageSkia GetFavicon() {
+ return *ui::ResourceBundle::GetSharedInstance().GetImageSkiaNamed(
+ IDR_EXTENSIONS_FAVICON);
+ }
+
+ private:
+ bool quit_in_icon_updated_;
+ MessageLoop ui_loop_;
+ content::TestBrowserThread ui_thread_;
+ content::TestBrowserThread file_thread_;
+ content::TestBrowserThread io_thread_;
+
+ DISALLOW_COPY_AND_ASSIGN(ExtensionActionIconFactoryTest);
+};
+
+// If there is no default icon, and the icon has not been set using |SetIcon|,
+// the factory should return favicon.
+TEST_F(ExtensionActionIconFactoryTest, NoIcons) {
+ // Load an extension that has browser action without default icon set in the
+ // manifest and does not call |SetIcon| by default.
+ scoped_refptr<Extension> extension(CreateExtension(
+ "browser_action/no_icon", Extension::INVALID));
+ ASSERT_TRUE(extension.get() != NULL);
+ ASSERT_TRUE(extension->browser_action());
+ ASSERT_FALSE(extension->browser_action()->default_icon());
+ ASSERT_TRUE(
+ extension->browser_action()->GetExplicitlySetIcon(0 /*tab id*/).isNull());
+
+ gfx::ImageSkia favicon = GetFavicon();
+
+ ExtensionActionIconFactory icon_factory(extension,
+ extension->browser_action(),
+ this);
+
+ gfx::Image icon = icon_factory.GetIcon(0);
+
+ EXPECT_TRUE(ImageRepsAreEqual(
+ favicon.GetRepresentation(ui::SCALE_FACTOR_100P),
+ icon.ToImageSkia()->GetRepresentation(ui::SCALE_FACTOR_100P)));
+}
+
+// If the icon has been set using |SetIcon|, the factory should return that
+// icon.
+TEST_F(ExtensionActionIconFactoryTest, AfterSetIcon) {
+ // Load an extension that has browser action without default icon set in the
+ // manifest and does not call |SetIcon| by default (but has an browser action
+ // icon resource).
+ scoped_refptr<Extension> extension(CreateExtension(
+ "browser_action/no_icon", Extension::INVALID));
+ ASSERT_TRUE(extension.get() != NULL);
+ ASSERT_TRUE(extension->browser_action());
+ ASSERT_FALSE(extension->browser_action()->default_icon());
+ ASSERT_TRUE(
+ extension->browser_action()->GetExplicitlySetIcon(0 /*tab id*/).isNull());
+
+ gfx::Image set_icon = LoadIcon("browser_action/no_icon/icon.png");
+ ASSERT_FALSE(set_icon.IsEmpty());
+
+ extension->browser_action()->SetIcon(0, set_icon);
+
+ ASSERT_FALSE(
+ extension->browser_action()->GetExplicitlySetIcon(0 /*tab id*/).isNull());
+
+ ExtensionActionIconFactory icon_factory(extension,
+ extension->browser_action(),
+ this);
+
+ gfx::Image icon = icon_factory.GetIcon(0);
+
+ EXPECT_TRUE(ImageRepsAreEqual(
+ set_icon.ToImageSkia()->GetRepresentation(ui::SCALE_FACTOR_100P),
+ icon.ToImageSkia()->GetRepresentation(ui::SCALE_FACTOR_100P)));
+
+ // It should still return favicon for another tabs.
+ icon = icon_factory.GetIcon(1);
+
+ EXPECT_TRUE(ImageRepsAreEqual(
+ GetFavicon().GetRepresentation(ui::SCALE_FACTOR_100P),
+ icon.ToImageSkia()->GetRepresentation(ui::SCALE_FACTOR_100P)));
+}
+
+// If there is a default icon, and the icon has not been set using |SetIcon|,
+// the factory should return the default icon.
+TEST_F(ExtensionActionIconFactoryTest, DefaultIcon) {
+ // Load an extension that has browser action without default icon set in the
+ // manifest and does not call |SetIcon| by default (but has an browser action
+ // icon resource).
+ scoped_refptr<Extension> extension(CreateExtension(
+ "browser_action/no_icon", Extension::INVALID));
+ ASSERT_TRUE(extension.get() != NULL);
+ ASSERT_TRUE(extension->browser_action());
+ ASSERT_FALSE(extension->browser_action()->default_icon());
+ ASSERT_TRUE(
+ extension->browser_action()->GetExplicitlySetIcon(0 /*tab id*/).isNull());
+
+ gfx::Image default_icon =
+ EnsureImageSize(LoadIcon("browser_action/no_icon/icon.png"), 19);
+ ASSERT_FALSE(default_icon.IsEmpty());
+
+ scoped_ptr<ExtensionIconSet> default_icon_set(new ExtensionIconSet());
+ default_icon_set->Add(19, "icon.png");
+
+ extension->browser_action()->set_default_icon(default_icon_set.Pass());
+ ASSERT_TRUE(extension->browser_action()->default_icon());
+
+ ExtensionActionIconFactory icon_factory(extension,
+ extension->browser_action(),
+ this);
+
+ gfx::Image icon = icon_factory.GetIcon(0);
+
+ // The icon should be loaded asynchronously. Initially a transparent icon
+ // should be returned.
+ EXPECT_TRUE(ImageRepsAreEqual(
+ CreateBlankRep(19, ui::SCALE_FACTOR_100P),
+ icon.ToImageSkia()->GetRepresentation(ui::SCALE_FACTOR_100P)));
+
+ WaitForIconUpdate();
+
+ icon = icon_factory.GetIcon(0);
+
+ // The default icon representation should be loaded at this point.
+ EXPECT_TRUE(ImageRepsAreEqual(
+ default_icon.ToImageSkia()->GetRepresentation(ui::SCALE_FACTOR_100P),
+ icon.ToImageSkia()->GetRepresentation(ui::SCALE_FACTOR_100P)));
+
+ // The same icon should be returned for the other tabs.
+ icon = icon_factory.GetIcon(1);
+
+ EXPECT_TRUE(ImageRepsAreEqual(
+ default_icon.ToImageSkia()->GetRepresentation(ui::SCALE_FACTOR_100P),
+ icon.ToImageSkia()->GetRepresentation(ui::SCALE_FACTOR_100P)));
+
+}
+
+} // namespace
diff --git a/chrome/browser/extensions/extension_icon_image.cc b/chrome/browser/extensions/extension_icon_image.cc
index 24125c4..8259861 100644
--- a/chrome/browser/extensions/extension_icon_image.cc
+++ b/chrome/browser/extensions/extension_icon_image.cc
@@ -193,7 +193,7 @@ gfx::ImageSkiaRep IconImage::LoadImageForScaleFactor(
std::vector<ImageLoadingTracker::ImageRepresentation> info_list;
info_list.push_back(ImageLoadingTracker::ImageRepresentation(
resource,
- ImageLoadingTracker::ImageRepresentation::RESIZE_WHEN_LARGER,
+ ImageLoadingTracker::ImageRepresentation::ALWAYS_RESIZE,
gfx::Size(resource_size_in_dip_, resource_size_in_dip_).Scale(scale),
scale_factor));
tracker_.LoadImages(extension_, info_list, ImageLoadingTracker::DONT_CACHE);
diff --git a/chrome/browser/extensions/extension_icon_image_unittest.cc b/chrome/browser/extensions/extension_icon_image_unittest.cc
index cba8c93a..8ca3747 100644
--- a/chrome/browser/extensions/extension_icon_image_unittest.cc
+++ b/chrome/browser/extensions/extension_icon_image_unittest.cc
@@ -13,6 +13,7 @@
#include "chrome/common/extensions/extension_constants.h"
#include "content/public/test/test_browser_thread.h"
#include "grit/theme_resources.h"
+#include "skia/ext/image_operations.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "ui/base/resource/resource_bundle.h"
#include "ui/gfx/image/image_skia_source.h"
@@ -35,6 +36,15 @@ SkBitmap CreateBlankBitmapForScale(int size_dip, ui::ScaleFactor scale_factor) {
return bitmap;
}
+SkBitmap EnsureBitmapSize(const SkBitmap& original, int size) {
+ if (original.width() == size && original.height() == size)
+ return original;
+
+ SkBitmap resized = skia::ImageOperations::Resize(
+ original, skia::ImageOperations::RESIZE_LANCZOS3, size, size);
+ return resized;
+}
+
// Used to test behaviour including images defined by an image skia source.
// |GetImageForScale| simply returns image representation from the image given
// in the ctor.
@@ -291,11 +301,12 @@ TEST_F(ExtensionIconImageTest, FallbackToSmallerWhenNoBigger) {
representation = image.image_skia().GetRepresentation(ui::SCALE_FACTOR_200P);
- // We should have loaded the biggest smaller resource. In this case the
- // loaded resource should not be resized.
+ // We should have loaded the biggest smaller resource resized to the actual
+ // size.
EXPECT_EQ(ui::SCALE_FACTOR_200P, representation.scale_factor());
- EXPECT_EQ(48, representation.pixel_width());
- EXPECT_TRUE(gfx::BitmapsAreEqual(representation.sk_bitmap(), bitmap_48));
+ EXPECT_EQ(64, representation.pixel_width());
+ EXPECT_TRUE(gfx::BitmapsAreEqual(representation.sk_bitmap(),
+ EnsureBitmapSize(bitmap_48, 64)));
}
// There is no resource with exact size, but there is a smaller and a bigger
@@ -325,10 +336,11 @@ TEST_F(ExtensionIconImageTest, FallbackToSmaller) {
representation = image.image_skia().GetRepresentation(ui::SCALE_FACTOR_100P);
- // We should have loaded smaller (not resized) resource.
+ // We should have loaded smaller (resized) resource.
EXPECT_EQ(ui::SCALE_FACTOR_100P, representation.scale_factor());
- EXPECT_EQ(16, representation.pixel_width());
- EXPECT_TRUE(gfx::BitmapsAreEqual(representation.sk_bitmap(), bitmap_16));
+ EXPECT_EQ(17, representation.pixel_width());
+ EXPECT_TRUE(gfx::BitmapsAreEqual(representation.sk_bitmap(),
+ EnsureBitmapSize(bitmap_16, 17)));
}
// If resource set is empty, |GetRepresentation| should synchronously return
diff --git a/chrome/browser/ui/cocoa/extensions/browser_action_button.h b/chrome/browser/ui/cocoa/extensions/browser_action_button.h
index 359ec7a..3b2d77c 100644
--- a/chrome/browser/ui/cocoa/extensions/browser_action_button.h
+++ b/chrome/browser/ui/cocoa/extensions/browser_action_button.h
@@ -13,7 +13,7 @@
class Browser;
class ExtensionAction;
-class ExtensionImageTrackerBridge;
+class ExtensionActionIconFactoryBridge;
namespace extensions {
class Extension;
@@ -28,7 +28,7 @@ extern NSString* const kBrowserActionButtonDragEndNotification;
@private
// Bridge to proxy Chrome notifications to the Obj-C class as well as load the
// extension's icon.
- scoped_ptr<ExtensionImageTrackerBridge> imageLoadingBridge_;
+ scoped_ptr<ExtensionActionIconFactoryBridge> iconFactoryBridge_;
// Used to move the button and query whether a button is currently animating.
scoped_nsobject<NSViewAnimation> moveAnimation_;
diff --git a/chrome/browser/ui/cocoa/extensions/browser_action_button.mm b/chrome/browser/ui/cocoa/extensions/browser_action_button.mm
index d188840..93fa879 100644
--- a/chrome/browser/ui/cocoa/extensions/browser_action_button.mm
+++ b/chrome/browser/ui/cocoa/extensions/browser_action_button.mm
@@ -9,7 +9,7 @@
#include "base/logging.h"
#include "base/sys_string_conversions.h"
-#include "chrome/browser/extensions/image_loading_tracker.h"
+#include "chrome/browser/extensions/extension_action_icon_factory.h"
#include "chrome/browser/ui/cocoa/extensions/extension_action_context_menu.h"
#include "chrome/common/chrome_notification_types.h"
#include "chrome/common/extensions/extension.h"
@@ -40,35 +40,24 @@ const CGFloat kAnimationDuration = 0.2;
// A helper class to bridge the asynchronous Skia bitmap loading mechanism to
// the extension's button.
-class ExtensionImageTrackerBridge : public content::NotificationObserver,
- public ImageLoadingTracker::Observer {
+class ExtensionActionIconFactoryBridge
+ : public content::NotificationObserver,
+ public ExtensionActionIconFactory::Observer {
public:
- ExtensionImageTrackerBridge(BrowserActionButton* owner,
- const Extension* extension)
+ ExtensionActionIconFactoryBridge(BrowserActionButton* owner,
+ const Extension* extension)
: owner_(owner),
- tracker_(this),
+ icon_factory_(extension, extension->browser_action(), this),
browser_action_(extension->browser_action()) {
- // The Browser Action API does not allow the default icon path to be
- // changed at runtime, so we can load this now and cache it.
- std::string path = extension->browser_action()->default_icon_path();
- if (!path.empty()) {
- tracker_.LoadImage(extension, extension->GetResource(path),
- gfx::Size(Extension::kBrowserActionIconMaxSize,
- Extension::kBrowserActionIconMaxSize),
- ImageLoadingTracker::DONT_CACHE);
- }
registrar_.Add(
this, chrome::NOTIFICATION_EXTENSION_BROWSER_ACTION_UPDATED,
content::Source<ExtensionAction>(browser_action_));
}
- ~ExtensionImageTrackerBridge() {}
+ virtual ~ExtensionActionIconFactoryBridge() {}
// ImageLoadingTracker::Observer implementation.
- void OnImageLoaded(const gfx::Image& image,
- const std::string& extension_id,
- int index) OVERRIDE {
- browser_action_->CacheIcon(image);
+ void OnIconUpdated() OVERRIDE {
[owner_ updateState];
}
@@ -82,12 +71,19 @@ class ExtensionImageTrackerBridge : public content::NotificationObserver,
NOTREACHED();
}
+ gfx::Image GetIcon(int tabId) {
+ return icon_factory_.GetIcon(tabId);
+ }
+
private:
// Weak. Owns us.
BrowserActionButton* owner_;
- // Loads the button's icons for us on the file thread.
- ImageLoadingTracker tracker_;
+ // The object that will be used to get the browser action icon for us.
+ // It may load the icon asynchronously (in which case the initial icon
+ // returned by the factory will be transparent), so we have to observe it for
+ // updates to the icon.
+ ExtensionActionIconFactory icon_factory_;
// The browser action whose images we're loading.
ExtensionAction* const browser_action_;
@@ -95,7 +91,7 @@ class ExtensionImageTrackerBridge : public content::NotificationObserver,
// Used for registering to receive notifications and automatic clean up.
content::NotificationRegistrar registrar_;
- DISALLOW_COPY_AND_ASSIGN(ExtensionImageTrackerBridge);
+ DISALLOW_COPY_AND_ASSIGN(ExtensionActionIconFactoryBridge);
};
@interface BrowserActionCell (Internals)
@@ -151,7 +147,8 @@ class ExtensionImageTrackerBridge : public content::NotificationObserver,
tabId_ = tabId;
extension_ = extension;
- imageLoadingBridge_.reset(new ExtensionImageTrackerBridge(self, extension));
+ iconFactoryBridge_.reset(
+ new ExtensionActionIconFactoryBridge(self, extension));
moveAnimation_.reset([[NSViewAnimation alloc] init]);
[moveAnimation_ gtm_setDuration:kAnimationDuration
@@ -250,7 +247,8 @@ class ExtensionImageTrackerBridge : public content::NotificationObserver,
[self setToolTip:base::SysUTF8ToNSString(tooltip)];
}
- gfx::Image image = extension_->browser_action()->GetIcon(tabId_);
+ gfx::Image image = iconFactoryBridge_->GetIcon(tabId_);
+
if (!image.IsEmpty())
[self setImage:image.ToNSImage()];
diff --git a/chrome/browser/ui/cocoa/location_bar/page_action_decoration.h b/chrome/browser/ui/cocoa/location_bar/page_action_decoration.h
index 1bc290b..bedb045 100644
--- a/chrome/browser/ui/cocoa/location_bar/page_action_decoration.h
+++ b/chrome/browser/ui/cocoa/location_bar/page_action_decoration.h
@@ -5,9 +5,11 @@
#ifndef CHROME_BROWSER_UI_COCOA_LOCATION_BAR_PAGE_ACTION_DECORATION_H_
#define CHROME_BROWSER_UI_COCOA_LOCATION_BAR_PAGE_ACTION_DECORATION_H_
-#include "chrome/browser/extensions/image_loading_tracker.h"
+#include "chrome/browser/extensions/extension_action_icon_factory.h"
#import "chrome/browser/ui/cocoa/location_bar/image_decoration.h"
#include "chrome/common/extensions/extension_action.h"
+#include "content/public/browser/notification_observer.h"
+#include "content/public/browser/notification_registrar.h"
#include "googleurl/src/gurl.h"
@class ExtensionActionContextMenu;
@@ -22,7 +24,7 @@ class WebContents;
// Action and notify the extension when the icon is clicked.
class PageActionDecoration : public ImageDecoration,
- public ImageLoadingTracker::Observer,
+ public ExtensionActionIconFactory::Observer,
public content::NotificationObserver,
public ExtensionAction::IconAnimation::Observer {
public:
@@ -36,10 +38,8 @@ class PageActionDecoration : public ImageDecoration,
void set_preview_enabled(bool enabled) { preview_enabled_ = enabled; }
bool preview_enabled() const { return preview_enabled_; }
- // Overridden from |ImageLoadingTracker::Observer|.
- virtual void OnImageLoaded(const gfx::Image& image,
- const std::string& extension_id,
- int index) OVERRIDE;
+ // Overridden from |ExtensionActionIconFactory::Observer|.
+ virtual void OnIconUpdated() OVERRIDE;
// Called to notify the Page Action that it should determine whether
// to be visible or hidden. |contents| is the WebContents that is
@@ -87,9 +87,12 @@ class PageActionDecoration : public ImageDecoration,
// profile.
ExtensionAction* page_action_;
- // The object that is waiting for the image loading to complete
- // asynchronously.
- ImageLoadingTracker tracker_;
+
+ // The object that will be used to get the page action icon for us.
+ // It may load the icon asynchronously (in which case the initial icon
+ // returned by the factory will be transparent), so we have to observe it for
+ // updates to the icon.
+ scoped_ptr<ExtensionActionIconFactory> icon_factory_;
// The tab id we are currently showing the icon for.
int current_tab_id_;
diff --git a/chrome/browser/ui/cocoa/location_bar/page_action_decoration.mm b/chrome/browser/ui/cocoa/location_bar/page_action_decoration.mm
index 4027e97..2866f95 100644
--- a/chrome/browser/ui/cocoa/location_bar/page_action_decoration.mm
+++ b/chrome/browser/ui/cocoa/location_bar/page_action_decoration.mm
@@ -49,7 +49,6 @@ PageActionDecoration::PageActionDecoration(
: owner_(NULL),
browser_(browser),
page_action_(page_action),
- tracker_(this),
current_tab_id_(-1),
preview_enabled_(false),
ALLOW_THIS_IN_INITIALIZER_LIST(scoped_icon_animation_observer_(
@@ -60,13 +59,8 @@ PageActionDecoration::PageActionDecoration(
GetExtensionById(page_action->extension_id(), false);
DCHECK(extension);
- std::string path = page_action_->default_icon_path();
- if (!path.empty()) {
- tracker_.LoadImage(extension, extension->GetResource(path),
- gfx::Size(Extension::kPageActionIconMaxSize,
- Extension::kPageActionIconMaxSize),
- ImageLoadingTracker::DONT_CACHE);
- }
+ icon_factory_.reset(
+ new ExtensionActionIconFactory(extension, page_action, this));
registrar_.Add(this, chrome::NOTIFICATION_EXTENSION_HOST_VIEW_SHOULD_CLOSE,
content::Source<Profile>(browser_->profile()));
@@ -135,11 +129,7 @@ bool PageActionDecoration::ActivatePageAction(NSRect frame) {
return true;
}
-void PageActionDecoration::OnImageLoaded(const gfx::Image& image,
- const std::string& extension_id,
- int index) {
- page_action_->CacheIcon(image);
-
+void PageActionDecoration::OnIconUpdated() {
// If we have no owner, that means this class is still being constructed.
TabContents* tab_contents = owner_ ? owner_->GetTabContents() : NULL;
if (tab_contents) {
@@ -161,7 +151,7 @@ void PageActionDecoration::UpdateVisibility(WebContents* contents,
SetToolTip(page_action_->GetTitle(current_tab_id_));
// Set the image.
- gfx::Image icon = page_action_->GetIcon(current_tab_id_);
+ gfx::Image icon = icon_factory_->GetIcon(current_tab_id_);
if (!icon.IsEmpty()) {
SetImage(icon.ToNSImage());
} else if (!GetImage()) {
diff --git a/chrome/browser/ui/gtk/browser_actions_toolbar_gtk.cc b/chrome/browser/ui/gtk/browser_actions_toolbar_gtk.cc
index 9a8d17e..b8114dc 100644
--- a/chrome/browser/ui/gtk/browser_actions_toolbar_gtk.cc
+++ b/chrome/browser/ui/gtk/browser_actions_toolbar_gtk.cc
@@ -15,9 +15,9 @@
#include "base/utf_string_conversions.h"
#include "chrome/browser/extensions/api/commands/command_service.h"
#include "chrome/browser/extensions/api/commands/command_service_factory.h"
+#include "chrome/browser/extensions/extension_action_icon_factory.h"
#include "chrome/browser/extensions/extension_context_menu_model.h"
#include "chrome/browser/extensions/extension_service.h"
-#include "chrome/browser/extensions/image_loading_tracker.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/sessions/session_tab_helper.h"
#include "chrome/browser/ui/browser.h"
@@ -91,7 +91,7 @@ gint WidthForIconCount(gint icon_count) {
using ui::SimpleMenuModel;
class BrowserActionButton : public content::NotificationObserver,
- public ImageLoadingTracker::Observer,
+ public ExtensionActionIconFactory::Observer,
public ExtensionContextMenuModel::PopupDelegate,
public MenuGtk::Delegate {
public:
@@ -101,9 +101,7 @@ class BrowserActionButton : public content::NotificationObserver,
: toolbar_(toolbar),
extension_(extension),
image_(NULL),
- tracker_(this),
- tab_specific_icon_(NULL),
- default_icon_(NULL),
+ icon_factory_(extension, extension->browser_action(), this),
accel_group_(NULL) {
button_.reset(new CustomDrawButton(
theme_provider,
@@ -119,16 +117,6 @@ class BrowserActionButton : public content::NotificationObserver,
DCHECK(extension_->browser_action());
- // The Browser Action API does not allow the default icon path to be
- // changed at runtime, so we can load this now and cache it.
- std::string path = extension_->browser_action()->default_icon_path();
- if (!path.empty()) {
- tracker_.LoadImage(extension_, extension_->GetResource(path),
- gfx::Size(Extension::kBrowserActionIconMaxSize,
- Extension::kBrowserActionIconMaxSize),
- ImageLoadingTracker::DONT_CACHE);
- }
-
UpdateState();
signals_.Connect(button(), "button-press-event",
@@ -170,12 +158,6 @@ class BrowserActionButton : public content::NotificationObserver,
~BrowserActionButton() {
DisconnectBrowserActionPopupAccelerator();
- if (tab_specific_icon_)
- g_object_unref(tab_specific_icon_);
-
- if (default_icon_)
- g_object_unref(default_icon_);
-
alignment_.Destroy();
}
@@ -218,11 +200,8 @@ class BrowserActionButton : public content::NotificationObserver,
}
}
- // ImageLoadingTracker::Observer implementation.
- void OnImageLoaded(const gfx::Image& image,
- const std::string& extension_id,
- int index) OVERRIDE {
- extension_->browser_action()->CacheIcon(image);
+ // ExtensionActionIconFactory::Observer implementation.
+ void OnIconUpdated() OVERRIDE {
UpdateState();
}
@@ -239,7 +218,7 @@ class BrowserActionButton : public content::NotificationObserver,
else
gtk_widget_set_tooltip_text(button(), tooltip.c_str());
- gfx::Image image = extension_->browser_action()->GetIcon(tab_id);
+ gfx::Image image = icon_factory_.GetIcon(tab_id);
if (!image.IsEmpty())
SetImage(image.ToGdkPixbuf());
bool enabled = extension_->browser_action()->GetIsVisible(tab_id);
@@ -249,8 +228,7 @@ class BrowserActionButton : public content::NotificationObserver,
}
gfx::Image GetIcon() {
- return extension_->browser_action()->GetIcon(
- toolbar_->GetCurrentTabId());
+ return icon_factory_.GetIcon(toolbar_->GetCurrentTabId());
}
MenuGtk* GetContextMenu() {
@@ -461,14 +439,11 @@ class BrowserActionButton : public content::NotificationObserver,
// extensions change browser action icon in a loop.
GtkWidget* image_;
- // Loads the button's icons for us on the file thread.
- ImageLoadingTracker tracker_;
-
- // If we are displaying a tab-specific icon, it will be here.
- GdkPixbuf* tab_specific_icon_;
-
- // If the browser action has a default icon, it will be here.
- GdkPixbuf* default_icon_;
+ // The object that will be used to get the browser action icon for us.
+ // It may load the icon asynchronously (in which case the initial icon
+ // returned by the factory will be transparent), so we have to observe it for
+ // updates to the icon.
+ ExtensionActionIconFactory icon_factory_;
// Same as |default_icon_|, but stored as SkBitmap.
SkBitmap default_skbitmap_;
diff --git a/chrome/browser/ui/gtk/location_bar_view_gtk.cc b/chrome/browser/ui/gtk/location_bar_view_gtk.cc
index 1badb15..8f86c17 100644
--- a/chrome/browser/ui/gtk/location_bar_view_gtk.cc
+++ b/chrome/browser/ui/gtk/location_bar_view_gtk.cc
@@ -1790,7 +1790,6 @@ LocationBarViewGtk::PageActionViewGtk::PageActionViewGtk(
ExtensionAction* page_action)
: owner_(NULL),
page_action_(page_action),
- tracker_(this),
current_tab_id_(-1),
window_(NULL),
accel_group_(NULL),
@@ -1821,13 +1820,8 @@ LocationBarViewGtk::PageActionViewGtk::PageActionViewGtk(
false);
DCHECK(extension);
- std::string path = page_action_->default_icon_path();
- if (!path.empty()) {
- tracker_.LoadImage(extension, extension->GetResource(path),
- gfx::Size(Extension::kPageActionIconMaxSize,
- Extension::kPageActionIconMaxSize),
- ImageLoadingTracker::DONT_CACHE);
- }
+ icon_factory_.reset(
+ new ExtensionActionIconFactory(extension, page_action, this));
// We set the owner last of all so that we can determine whether we are in
// the process of initializing this class or not.
@@ -1860,7 +1854,7 @@ void LocationBarViewGtk::PageActionViewGtk::UpdateVisibility(
page_action_->GetTitle(current_tab_id_).c_str());
// Set the image.
- gfx::Image icon = page_action_->GetIcon(current_tab_id_);
+ gfx::Image icon = icon_factory_->GetIcon(current_tab_id_);
if (!icon.IsEmpty()) {
GdkPixbuf* pixbuf = icon.ToGdkPixbuf();
DCHECK(pixbuf);
@@ -1882,12 +1876,7 @@ void LocationBarViewGtk::PageActionViewGtk::UpdateVisibility(
}
}
-void LocationBarViewGtk::PageActionViewGtk::OnImageLoaded(
- const gfx::Image& image,
- const std::string& extension_id,
- int index) {
- page_action_->CacheIcon(image);
-
+void LocationBarViewGtk::PageActionViewGtk::OnIconUpdated() {
// If we have no owner, that means this class is still being constructed.
TabContents* tab_contents = owner_ ? owner_->GetTabContents() : NULL;
if (tab_contents)
diff --git a/chrome/browser/ui/gtk/location_bar_view_gtk.h b/chrome/browser/ui/gtk/location_bar_view_gtk.h
index 448645a..a14a656 100644
--- a/chrome/browser/ui/gtk/location_bar_view_gtk.h
+++ b/chrome/browser/ui/gtk/location_bar_view_gtk.h
@@ -17,8 +17,8 @@
#include "base/memory/scoped_vector.h"
#include "base/memory/weak_ptr.h"
#include "chrome/browser/api/prefs/pref_member.h"
+#include "chrome/browser/extensions/extension_action_icon_factory.h"
#include "chrome/browser/extensions/extension_context_menu_model.h"
-#include "chrome/browser/extensions/image_loading_tracker.h"
#include "chrome/browser/ui/gtk/bubble/bubble_gtk.h"
#include "chrome/browser/ui/gtk/menu_gtk.h"
#include "chrome/browser/ui/omnibox/location_bar.h"
@@ -227,7 +227,7 @@ class LocationBarViewGtk : public OmniboxEditController,
private:
class PageActionViewGtk :
- public ImageLoadingTracker::Observer,
+ public ExtensionActionIconFactory::Observer,
public content::NotificationObserver,
public ExtensionContextMenuModel::PopupDelegate,
public ExtensionAction::IconAnimation::Observer {
@@ -250,10 +250,8 @@ class LocationBarViewGtk : public OmniboxEditController,
// is the current page URL.
void UpdateVisibility(content::WebContents* contents, const GURL& url);
- // A callback from ImageLoadingTracker for when the image has loaded.
- virtual void OnImageLoaded(const gfx::Image& image,
- const std::string& extension_id,
- int index) OVERRIDE;
+ // Overriden from ExtensionActionIconFactory::Observer.
+ virtual void OnIconUpdated() OVERRIDE;
// Simulate left mouse click on the page action button.
void TestActivatePageAction();
@@ -297,9 +295,11 @@ class LocationBarViewGtk : public OmniboxEditController,
// us, it resides in the extension of this particular profile.
ExtensionAction* page_action_;
- // The object that is waiting for the image loading to complete
- // asynchronously.
- ImageLoadingTracker tracker_;
+ // The object that will be used to get the extension action icon for us.
+ // It may load the icon asynchronously (in which case the initial icon
+ // returned by the factory will be transparent), so we have to observe it
+ // for updates to the icon.
+ scoped_ptr<ExtensionActionIconFactory> icon_factory_;
// The widgets for this page action.
ui::OwnedWidgetGtk event_box_;
diff --git a/chrome/browser/ui/views/browser_action_view.cc b/chrome/browser/ui/views/browser_action_view.cc
index 2d8ac71..f7774cc 100644
--- a/chrome/browser/ui/views/browser_action_view.cc
+++ b/chrome/browser/ui/views/browser_action_view.cc
@@ -62,7 +62,7 @@ gfx::ImageSkia BrowserActionView::GetIconWithBadge() {
const ExtensionAction* action = button_->extension()->browser_action();
gfx::Size spacing(0, ToolbarView::kVertSpacing);
- gfx::ImageSkia icon = *action->GetIcon(tab_id).ToImageSkia();
+ gfx::ImageSkia icon = *button_->icon_factory().GetIcon(tab_id).ToImageSkia();
if (!button_->IsEnabled(tab_id))
icon = gfx::ImageSkiaOperations::CreateTransparentImage(icon, .25);
return action->GetIconWithBadge(icon, tab_id, spacing);
@@ -111,7 +111,8 @@ BrowserActionButton::BrowserActionButton(const Extension* extension,
browser_(browser),
browser_action_(extension->browser_action()),
extension_(extension),
- ALLOW_THIS_IN_INITIALIZER_LIST(tracker_(this)),
+ ALLOW_THIS_IN_INITIALIZER_LIST(
+ icon_factory_(extension, extension->browser_action(), this)),
delegate_(delegate),
context_menu_(NULL),
called_registered_extension_command_(false) {
@@ -145,21 +146,6 @@ void BrowserActionButton::Destroy() {
void BrowserActionButton::ViewHierarchyChanged(
bool is_add, View* parent, View* child) {
- if (is_add && child == this) {
- // The Browser Action API does not allow the default icon path to be
- // changed at runtime, so we can load this now and cache it.
- std::string relative_path = browser_action_->default_icon_path();
- if (!relative_path.empty()) {
- // LoadImage is not guaranteed to be synchronous, so we might see the
- // callback OnImageLoaded execute immediately. It (through UpdateState)
- // expects parent() to return the owner for this button, so this
- // function is as early as we can start this request.
- tracker_.LoadImage(extension_, extension_->GetResource(relative_path),
- gfx::Size(Extension::kBrowserActionIconMaxSize,
- Extension::kBrowserActionIconMaxSize),
- ImageLoadingTracker::DONT_CACHE);
- }
- }
if (is_add && !called_registered_extension_command_ && GetFocusManager()) {
MaybeRegisterExtensionCommand();
@@ -214,16 +200,6 @@ void BrowserActionButton::ShowContextMenuForView(View* source,
context_menu_ = NULL;
}
-void BrowserActionButton::OnImageLoaded(const gfx::Image& image,
- const std::string& extension_id,
- int index) {
- browser_action_->CacheIcon(image);
-
- // Call back to UpdateState() because a more specific icon might have been set
- // while the load was outstanding.
- UpdateState();
-}
-
void BrowserActionButton::UpdateState() {
int tab_id = delegate_->GetCurrentTabId();
if (tab_id < 0)
@@ -239,7 +215,7 @@ void BrowserActionButton::UpdateState() {
views::CustomButton::BS_NORMAL);
}
- gfx::ImageSkia icon = *browser_action()->GetIcon(tab_id).ToImageSkia();
+ gfx::ImageSkia icon = *icon_factory_.GetIcon(tab_id).ToImageSkia();
if (!icon.isNull()) {
if (!browser_action()->GetIsVisible(tab_id))
@@ -308,6 +284,10 @@ void BrowserActionButton::Observe(int type,
}
}
+void BrowserActionButton::OnIconUpdated() {
+ UpdateState();
+}
+
bool BrowserActionButton::Activate() {
if (!IsPopup())
return true;
diff --git a/chrome/browser/ui/views/browser_action_view.h b/chrome/browser/ui/views/browser_action_view.h
index 61c1c9c..3d6bcd5 100644
--- a/chrome/browser/ui/views/browser_action_view.h
+++ b/chrome/browser/ui/views/browser_action_view.h
@@ -7,9 +7,10 @@
#include <string>
+#include "chrome/browser/extensions/extension_action_icon_factory.h"
#include "chrome/browser/extensions/extension_context_menu_model.h"
-#include "chrome/browser/extensions/image_loading_tracker.h"
#include "content/public/browser/notification_observer.h"
+#include "content/public/browser/notification_registrar.h"
#include "ui/views/context_menu_controller.h"
#include "ui/views/controls/button/menu_button.h"
#include "ui/views/controls/button/menu_button_listener.h"
@@ -107,8 +108,8 @@ class BrowserActionView : public views::View {
class BrowserActionButton : public views::MenuButton,
public views::ButtonListener,
public views::ContextMenuController,
- public ImageLoadingTracker::Observer,
- public content::NotificationObserver {
+ public content::NotificationObserver,
+ public ExtensionActionIconFactory::Observer {
public:
BrowserActionButton(const extensions::Extension* extension,
Browser* browser_,
@@ -139,16 +140,14 @@ class BrowserActionButton : public views::MenuButton,
virtual void ShowContextMenuForView(View* source,
const gfx::Point& point) OVERRIDE;
- // Overridden from ImageLoadingTracker.
- virtual void OnImageLoaded(const gfx::Image& image,
- const std::string& extension_id,
- int index) OVERRIDE;
-
// Overridden from content::NotificationObserver:
virtual void Observe(int type,
const content::NotificationSource& source,
const content::NotificationDetails& details) OVERRIDE;
+ // Overriden from ExtensionActionIconFactory::Observer.
+ virtual void OnIconUpdated() OVERRIDE;
+
// MenuButton behavior overrides. These methods all default to TextButton
// behavior unless this button is a popup. In that case, it uses MenuButton
// behavior. MenuButton has the notion of a child popup being shown where the
@@ -173,6 +172,9 @@ class BrowserActionButton : public views::MenuButton,
// receive drag events.
bool IsEnabled(int tab_id) const;
+ // Returns icon factory for the button.
+ ExtensionActionIconFactory& icon_factory() { return icon_factory_; }
+
// Returns button icon so it can be accessed during tests.
gfx::ImageSkia GetIconForTest();
@@ -202,13 +204,11 @@ class BrowserActionButton : public views::MenuButton,
// The extension associated with the browser action we're displaying.
const extensions::Extension* extension_;
- // The object that is waiting for the image loading to complete
- // asynchronously.
- ImageLoadingTracker tracker_;
-
- // The default icon for our browser action. This might be non-empty if the
- // browser action had a value for default_icon in the manifest.
- SkBitmap default_icon_;
+ // The object that will be used to get the browser action icon for us.
+ // It may load the icon asynchronously (in which case the initial icon
+ // returned by the factory will be transparent), so we have to observe it for
+ // updates to the icon.
+ ExtensionActionIconFactory icon_factory_;
// Delegate that usually represents a container for BrowserActionView.
BrowserActionView::Delegate* delegate_;
diff --git a/chrome/browser/ui/views/browser_actions_container_browsertest.cc b/chrome/browser/ui/views/browser_actions_container_browsertest.cc
index 8888178..ebdf5f1 100644
--- a/chrome/browser/ui/views/browser_actions_container_browsertest.cc
+++ b/chrome/browser/ui/views/browser_actions_container_browsertest.cc
@@ -10,6 +10,8 @@
#include "chrome/browser/ui/views/browser_actions_container.h"
#include "chrome/common/chrome_notification_types.h"
#include "chrome/common/extensions/extension_action.h"
+#include "chrome/common/extensions/extension_constants.h"
+#include "chrome/common/extensions/extension_icon_set.h"
#include "chrome/common/extensions/extension_resource.h"
#include "content/public/test/test_utils.h"
@@ -272,10 +274,13 @@ IN_PROC_BROWSER_TEST_F(BrowserActionsContainerTest, TestCrash57536) {
gfx::Size size(Extension::kBrowserActionIconMaxSize,
Extension::kBrowserActionIconMaxSize);
- extension->SetCachedImage(
- extension->GetResource(extension->browser_action()->default_icon_path()),
- bitmap,
- size);
+ const ExtensionIconSet* default_icon =
+ extension->browser_action()->default_icon();
+ const std::string path =
+ default_icon->Get(extension_misc::EXTENSION_ICON_ACTION,
+ ExtensionIconSet::MATCH_EXACTLY);
+
+ extension->SetCachedImage(extension->GetResource(path), bitmap, size);
LOG(INFO) << "Disabling extension\n" << std::flush;
DisableExtension(extension->id());
diff --git a/chrome/browser/ui/views/location_bar/page_action_image_view.cc b/chrome/browser/ui/views/location_bar/page_action_image_view.cc
index f44eb23..f0ef3b3 100644
--- a/chrome/browser/ui/views/location_bar/page_action_image_view.cc
+++ b/chrome/browser/ui/views/location_bar/page_action_image_view.cc
@@ -7,6 +7,7 @@
#include "base/utf_string_conversions.h"
#include "chrome/browser/extensions/api/commands/command_service.h"
#include "chrome/browser/extensions/api/commands/command_service_factory.h"
+#include "chrome/browser/extensions/extension_action_icon_factory.h"
#include "chrome/browser/extensions/extension_context_menu_model.h"
#include "chrome/browser/extensions/extension_service.h"
#include "chrome/browser/extensions/extension_tab_util.h"
@@ -43,7 +44,6 @@ PageActionImageView::PageActionImageView(LocationBarView* owner,
: owner_(owner),
page_action_(page_action),
browser_(browser),
- ALLOW_THIS_IN_INITIALIZER_LIST(tracker_(this)),
current_tab_id_(-1),
preview_enabled_(false),
popup_(NULL),
@@ -55,13 +55,8 @@ PageActionImageView::PageActionImageView(LocationBarView* owner,
GetExtensionById(page_action->extension_id(), false);
DCHECK(extension);
- std::string path = page_action_->default_icon_path();
- if (!path.empty()) {
- tracker_.LoadImage(extension, extension->GetResource(path),
- gfx::Size(Extension::kPageActionIconMaxSize,
- Extension::kPageActionIconMaxSize),
- ImageLoadingTracker::DONT_CACHE);
- }
+ icon_factory_.reset(
+ new ExtensionActionIconFactory(extension, page_action, this));
registrar_.Add(this, chrome::NOTIFICATION_EXTENSION_UNLOADED,
content::Source<Profile>(
@@ -188,17 +183,6 @@ bool PageActionImageView::OnKeyPressed(const ui::KeyEvent& event) {
return false;
}
-void PageActionImageView::OnImageLoaded(const gfx::Image& image,
- const std::string& extension_id,
- int index) {
- page_action_->CacheIcon(image);
-
- // During object construction owner_ will be NULL.
- TabContents* tab_contents = owner_ ? owner_->GetTabContents() : NULL;
- if (tab_contents)
- UpdateVisibility(tab_contents->web_contents(), current_url_);
-}
-
void PageActionImageView::ShowContextMenuForView(View* source,
const gfx::Point& point) {
const Extension* extension = owner_->profile()->GetExtensionService()->
@@ -251,7 +235,7 @@ void PageActionImageView::UpdateVisibility(WebContents* contents,
SetTooltipText(UTF8ToUTF16(tooltip_));
// Set the image.
- gfx::Image icon = page_action_->GetIcon(current_tab_id_);
+ gfx::Image icon = icon_factory_->GetIcon(current_tab_id_);
if (!icon.IsEmpty())
SetImage(*icon.ToImageSkia());
@@ -274,16 +258,20 @@ void PageActionImageView::Observe(int type,
DCHECK_EQ(chrome::NOTIFICATION_EXTENSION_UNLOADED, type);
const Extension* unloaded_extension =
content::Details<extensions::UnloadedExtensionInfo>(details)->extension;
- if (page_action_ == unloaded_extension ->page_action())
+ if (page_action_ == unloaded_extension->page_action())
owner_->UpdatePageActions();
}
-void PageActionImageView::OnIconChanged() {
+void PageActionImageView::OnIconUpdated() {
TabContents* tab_contents = owner_->GetTabContents();
if (tab_contents)
UpdateVisibility(tab_contents->web_contents(), current_url_);
}
+void PageActionImageView::OnIconChanged() {
+ OnIconUpdated();
+}
+
void PageActionImageView::ShowPopupWithURL(
const GURL& popup_url,
ExtensionPopup::ShowAction show_action) {
diff --git a/chrome/browser/ui/views/location_bar/page_action_image_view.h b/chrome/browser/ui/views/location_bar/page_action_image_view.h
index 1416cc6..76dac12 100644
--- a/chrome/browser/ui/views/location_bar/page_action_image_view.h
+++ b/chrome/browser/ui/views/location_bar/page_action_image_view.h
@@ -9,8 +9,8 @@
#include <string>
#include "base/memory/scoped_ptr.h"
+#include "chrome/browser/extensions/extension_action_icon_factory.h"
#include "chrome/browser/extensions/extension_context_menu_model.h"
-#include "chrome/browser/extensions/image_loading_tracker.h"
#include "chrome/browser/ui/views/extensions/extension_popup.h"
#include "chrome/common/extensions/extension_action.h"
#include "ui/views/context_menu_controller.h"
@@ -30,11 +30,11 @@ class MenuRunner;
// PageActionImageView is used by the LocationBarView to display the icon for a
// given PageAction and notify the extension when the icon is clicked.
class PageActionImageView : public views::ImageView,
- public ImageLoadingTracker::Observer,
public ExtensionContextMenuModel::PopupDelegate,
public views::WidgetObserver,
public views::ContextMenuController,
public content::NotificationObserver,
+ public ExtensionActionIconFactory::Observer,
public ExtensionAction::IconAnimation::Observer {
public:
PageActionImageView(LocationBarView* owner,
@@ -56,11 +56,6 @@ class PageActionImageView : public views::ImageView,
virtual void OnMouseReleased(const ui::MouseEvent& event) OVERRIDE;
virtual bool OnKeyPressed(const ui::KeyEvent& event) OVERRIDE;
- // Overridden from ImageLoadingTracker:
- virtual void OnImageLoaded(const gfx::Image& image,
- const std::string& extension_id,
- int index) OVERRIDE;
-
// Overridden from ExtensionContextMenuModel::Delegate
virtual void InspectPopup(ExtensionAction* action) OVERRIDE;
@@ -76,6 +71,9 @@ class PageActionImageView : public views::ImageView,
const content::NotificationSource& source,
const content::NotificationDetails& details) OVERRIDE;
+ // Overriden from ExtensionActionIconFactory::Observer.
+ virtual void OnIconUpdated() OVERRIDE;
+
// Overridden from ui::AcceleratorTarget:
virtual bool AcceleratorPressed(const ui::Accelerator& accelerator) OVERRIDE;
virtual bool CanHandleAccelerators() const OVERRIDE;
@@ -109,9 +107,11 @@ class PageActionImageView : public views::ImageView,
// The corresponding browser.
Browser* browser_;
- // The object that is waiting for the image loading to complete
- // asynchronously.
- ImageLoadingTracker tracker_;
+ // The object that will be used to get the page action icon for us.
+ // It may load the icon asynchronously (in which case the initial icon
+ // returned by the factory will be transparent), so we have to observe it for
+ // updates to the icon.
+ scoped_ptr<ExtensionActionIconFactory> icon_factory_;
// The tab id we are currently showing the icon for.
int current_tab_id_;
diff --git a/chrome/chrome_browser_extensions.gypi b/chrome/chrome_browser_extensions.gypi
index 19fe96e..d9cbcfb 100644
--- a/chrome/chrome_browser_extensions.gypi
+++ b/chrome/chrome_browser_extensions.gypi
@@ -373,6 +373,8 @@
'browser/extensions/event_router.h',
'browser/extensions/event_router_forwarder.cc',
'browser/extensions/event_router_forwarder.h',
+ 'browser/extensions/extension_action_icon_factory.cc',
+ 'browser/extensions/extension_action_icon_factory.h',
'browser/extensions/extension_context_menu_model.cc',
'browser/extensions/extension_context_menu_model.h',
'browser/extensions/extension_creator.cc',
diff --git a/chrome/chrome_tests.gypi b/chrome/chrome_tests.gypi
index 6bcc252..edb3824 100644
--- a/chrome/chrome_tests.gypi
+++ b/chrome/chrome_tests.gypi
@@ -1270,6 +1270,7 @@
'browser/extensions/default_apps_unittest.cc',
'browser/extensions/event_listener_map_unittest.cc',
'browser/extensions/event_router_forwarder_unittest.cc',
+ 'browser/extensions/extension_action_icon_factory_unittest.cc',
'browser/extensions/extension_creator_filter_unittest.cc',
'browser/extensions/extension_function_test_utils.cc',
'browser/extensions/extension_function_test_utils.h',
@@ -1974,6 +1975,7 @@
'common/extensions/manifest_tests/extension_manifest_test.cc',
'common/extensions/manifest_tests/extension_manifests_auth_unittest.cc',
'common/extensions/manifest_tests/extension_manifests_background_unittest.cc',
+ 'common/extensions/manifest_tests/extension_manifests_browseraction_unittest.cc',
'common/extensions/manifest_tests/extension_manifests_chromepermission_unittest.cc',
'common/extensions/manifest_tests/extension_manifests_command_unittest.cc',
'common/extensions/manifest_tests/extension_manifests_contentscript_unittest.cc',
diff --git a/chrome/common/extensions/docs/templates/intros/browserAction.html b/chrome/common/extensions/docs/templates/intros/browserAction.html
index d4c7476..c414c96 100644
--- a/chrome/common/extensions/docs/templates/intros/browserAction.html
+++ b/chrome/common/extensions/docs/templates/intros/browserAction.html
@@ -44,13 +44,34 @@ like this:
"name": "My extension",
...
<b>"browser_action": {
- "default_icon": "images/icon19.png", <em>// optional</em>
+ "default_icon": { <em>// optional</em>
+ "19": "images/icon19.png", <em>// optional</em>
+ "38": "images/icon38.png" <em>// optional</em>
+ },
"default_title": "Google Mail", <em>// optional; shown in tooltip</em>
"default_popup": "popup.html" <em>// optional</em>
}</b>,
...
}</pre>
+<p>
+If you only provide one of the 19px or 38px icon size, the extension system will
+scale the icon you provide to the pixel density of the user's display, which
+can lose detail or make it look fuzzy. The old syntax for registering the
+default icon is still supported:
+</p>
+
+<pre>{
+ "name": "My extension",
+ ...
+ <b>"browser_action": {
+ ...
+ "default_icon": "images/icon19.png" <em>// optional</em>
+ <em>// equivalent to "default_icon": { "19": "images/icon19.png" }</em>
+ }</b>,
+ ...
+}</pre>
+
<h2 id="ui">Parts of the UI</h2>
<p>
@@ -62,9 +83,9 @@ and a <a href="#popups">popup</a>.
<h3 id="icon">Icon</h3>
-<p>Browser action icons can be up to 19 pixels wide and high.
- Larger icons are resized to fit, but for best results,
- use a 19-pixel square icon.</p>
+<p>Browser action icons can be up to 19 dips (device-independent pixels)
+ wide and high. Larger icons are resized to fit, but for best results,
+ use a 19-dip square icon.</p>
<p>You can set the icon in two ways:
using a static image or using the
@@ -80,10 +101,18 @@ and a <a href="#popups">popup</a>.
</p>
<p>To set the icon,
-use the <b>default_icon</b> field of <b>browser_action</b>
-in the <a href="#manifest">manifest</a>,
-or call the <a href="#method-setIcon">setIcon()</a> method.
+ use the <b>default_icon</b> field of <b>browser_action</b>
+ in the <a href="#manifest">manifest</a>,
+ or call the <a href="#method-setIcon">setIcon()</a> method.
+ </p>
+<p>To properly display icon when screen pixel density (ratio
+ <code>size_in_pixel / size_in_dip</code>) is different than 1,
+ the icon can be defined as set of images with different sizes.
+ The actual image to display will be selected from the set to
+ best fit the pixel size of 19 dip icon.
+ At the moment, the icon set can contain images with pixel sizes 19 and 38.
+ </p>
<h3 id="tooltip">Tooltip</h3>
@@ -169,4 +198,4 @@ You can find simple examples of using browser actions in the
directory.
For other examples and for help in viewing the source code, see
<a href="samples.html">Samples</a>.
-</p> \ No newline at end of file
+</p>
diff --git a/chrome/common/extensions/docs/templates/intros/pageAction.html b/chrome/common/extensions/docs/templates/intros/pageAction.html
index 86930f1..c490abf 100644
--- a/chrome/common/extensions/docs/templates/intros/pageAction.html
+++ b/chrome/common/extensions/docs/templates/intros/pageAction.html
@@ -43,9 +43,30 @@ like this:
"name": "My extension",
...
<b>"page_action": {
- "default_icon": "icons/foo.png", <em>// optional</em>
- "default_title": "Do action", <em>// optional; shown in tooltip</em>
- "default_popup": "popup.html" <em>// optional</em>
+ "default_icon": { <em>// optional</em>
+ "19": "images/icon19.png", <em>// optional</em>
+ "38": "images/icon38.png" <em>// optional</em>
+ },
+ "default_title": "Google Mail", <em>// optional; shown in tooltip</em>
+ "default_popup": "popup.html" <em>// optional</em>
+ }</b>,
+ ...
+}</pre>
+
+<p>
+If you only provide one of the 19px or 38px icon size, the extension system will
+scale the icon you provide to the pixel density of the user's display, which
+can lose detail or make it look fuzzy. The old syntax for registering the
+default icon is still supported:
+</p>
+
+<pre>{
+ "name": "My extension",
+ ...
+ <b>"page_action": {
+ ...
+ "default_icon": "images/icon19.png" <em>// optional</em>
+ <em>// equivalent to "default_icon": { "19": "images/icon19.png" }</em>
}</b>,
...
}</pre>
@@ -105,4 +126,4 @@ You can find simple examples of using page actions in the
directory.
For other examples and for help in viewing the source code, see
<a href="samples.html">Samples</a>.
-</p> \ No newline at end of file
+</p>
diff --git a/chrome/common/extensions/extension.cc b/chrome/common/extensions/extension.cc
index faa4933..f042c988e 100644
--- a/chrome/common/extensions/extension.cc
+++ b/chrome/common/extensions/extension.cc
@@ -130,6 +130,40 @@ static void ConvertHexadecimalToIDAlphabet(std::string* id) {
}
}
+// Loads icon paths defined in dictionary |icons_value| into ExtensionIconSet
+// |icons|. |icons_value| is a dictionary value {icon size -> icon path}. Icons
+// in |icons_value| whose size is not in |icon_sizes| will be ignored.
+// Returns success. If load fails, |error| will be set.
+bool LoadIconsFromDictionary(const DictionaryValue* icons_value,
+ const int* icon_sizes,
+ size_t num_icon_sizes,
+ ExtensionIconSet* icons,
+ string16* error) {
+ DCHECK(icons);
+ for (size_t i = 0; i < num_icon_sizes; ++i) {
+ std::string key = base::IntToString(icon_sizes[i]);
+ if (icons_value->HasKey(key)) {
+ std::string icon_path;
+ if (!icons_value->GetString(key, &icon_path)) {
+ *error = ExtensionErrorUtils::FormatErrorMessageUTF16(
+ errors::kInvalidIconPath, key);
+ return false;
+ }
+
+ if (!icon_path.empty() && icon_path[0] == '/')
+ icon_path = icon_path.substr(1);
+
+ if (icon_path.empty()) {
+ *error = ExtensionErrorUtils::FormatErrorMessageUTF16(
+ errors::kInvalidIconPath, key);
+ return false;
+ }
+ icons->Add(icon_sizes[i], icon_path);
+ }
+ }
+ return true;
+}
+
// A singleton object containing global data needed by the extension objects.
class ExtensionConfig {
public:
@@ -823,7 +857,9 @@ scoped_ptr<ExtensionAction> Extension::LoadExtensionActionHelper(
return scoped_ptr<ExtensionAction>();
}
- result->set_default_icon_path(path);
+ scoped_ptr<ExtensionIconSet> icon_set(new ExtensionIconSet);
+ icon_set->Add(extension_misc::EXTENSION_ICON_ACTION, path);
+ result->set_default_icon(icon_set.Pass());
break;
}
}
@@ -838,16 +874,34 @@ scoped_ptr<ExtensionAction> Extension::LoadExtensionActionHelper(
}
}
- std::string default_icon;
// Read the page action |default_icon| (optional).
+ // The |default_icon| value can be either dictionary {icon size -> icon path}
+ // or non empty string value.
if (extension_action->HasKey(keys::kPageActionDefaultIcon)) {
- if (!extension_action->GetString(keys::kPageActionDefaultIcon,
- &default_icon) ||
- default_icon.empty()) {
+ const DictionaryValue* icons_value = NULL;
+ std::string default_icon;
+ if (extension_action->GetDictionary(keys::kPageActionDefaultIcon,
+ &icons_value)) {
+ scoped_ptr<ExtensionIconSet> default_icons(new ExtensionIconSet());
+ if (!LoadIconsFromDictionary(icons_value,
+ extension_misc::kExtensionActionIconSizes,
+ extension_misc::kNumExtensionActionIconSizes,
+ default_icons.get(),
+ error)) {
+ return scoped_ptr<ExtensionAction>();
+ }
+
+ result->set_default_icon(default_icons.Pass());
+ } else if (extension_action->GetString(keys::kPageActionDefaultIcon,
+ &default_icon) &&
+ !default_icon.empty()) {
+ scoped_ptr<ExtensionIconSet> icon_set(new ExtensionIconSet);
+ icon_set->Add(extension_misc::EXTENSION_ICON_ACTION, default_icon);
+ result->set_default_icon(icon_set.Pass());
+ } else {
*error = ASCIIToUTF16(errors::kInvalidPageActionIconPath);
return scoped_ptr<ExtensionAction>();
}
- result->set_default_icon_path(default_icon);
}
// Read the page action title from |default_title| if present, |name| if not
@@ -1375,28 +1429,11 @@ bool Extension::LoadIcons(string16* error) {
return false;
}
- for (size_t i = 0; i < extension_misc::kNumExtensionIconSizes; ++i) {
- std::string key = base::IntToString(extension_misc::kExtensionIconSizes[i]);
- if (icons_value->HasKey(key)) {
- std::string icon_path;
- if (!icons_value->GetString(key, &icon_path)) {
- *error = ExtensionErrorUtils::FormatErrorMessageUTF16(
- errors::kInvalidIconPath, key);
- return false;
- }
-
- if (!icon_path.empty() && icon_path[0] == '/')
- icon_path = icon_path.substr(1);
-
- if (icon_path.empty()) {
- *error = ExtensionErrorUtils::FormatErrorMessageUTF16(
- errors::kInvalidIconPath, key);
- return false;
- }
- icons_.Add(extension_misc::kExtensionIconSizes[i], icon_path);
- }
- }
- return true;
+ return LoadIconsFromDictionary(icons_value,
+ extension_misc::kExtensionIconSizes,
+ extension_misc::kNumExtensionIconSizes,
+ &icons_,
+ error);
}
bool Extension::LoadCommands(string16* error) {
@@ -2369,20 +2406,26 @@ bool Extension::LoadScriptBadge(string16* error) {
}
script_badge_->SetTitle(ExtensionAction::kDefaultTabId, name());
- if (!script_badge_->default_icon_path().empty()) {
+ if (script_badge_->default_icon()) {
install_warnings_.push_back(
InstallWarning(InstallWarning::FORMAT_TEXT,
errors::kScriptBadgeIconIgnored));
}
- std::string icon16_path = icons().Get(extension_misc::EXTENSION_ICON_BITTY,
- ExtensionIconSet::MATCH_EXACTLY);
- if (!icon16_path.empty()) {
- script_badge_->set_default_icon_path(icon16_path);
+
+ scoped_ptr<ExtensionIconSet> icon_set(new ExtensionIconSet);
+
+ for (size_t i = 0; i < extension_misc::kNumScriptBadgeIconSizes; i++) {
+ std::string path = icons().Get(extension_misc::kScriptBadgeIconSizes[i],
+ ExtensionIconSet::MATCH_EXACTLY);
+ if (!path.empty()) {
+ icon_set->Add(extension_misc::kScriptBadgeIconSizes[i], path);
+ }
+ }
+
+ if (!icon_set->map().empty()) {
+ script_badge_->set_default_icon(icon_set.Pass());
} else {
- script_badge_->SetIcon(
- ExtensionAction::kDefaultTabId,
- ui::ResourceBundle::GetSharedInstance().GetImageNamed(
- IDR_EXTENSIONS_FAVICON));
+ script_badge_->set_default_icon(scoped_ptr<ExtensionIconSet>());
}
return true;
@@ -3200,16 +3243,22 @@ std::set<FilePath> Extension::GetBrowserImages() const {
}
}
- // Page action icons.
if (page_action()) {
- image_paths.insert(FilePath::FromWStringHack(UTF8ToWide(
- page_action()->default_icon_path())));
+ for (ExtensionIconSet::IconMap::const_iterator iter =
+ page_action()->default_icon()->map().begin();
+ iter != page_action()->default_icon()->map().end();
+ ++iter) {
+ image_paths.insert(FilePath::FromWStringHack(UTF8ToWide(iter->second)));
+ }
}
- // Browser action icons.
if (browser_action()) {
- image_paths.insert(FilePath::FromWStringHack(UTF8ToWide(
- browser_action()->default_icon_path())));
+ for (ExtensionIconSet::IconMap::const_iterator iter =
+ browser_action()->default_icon()->map().begin();
+ iter != browser_action()->default_icon()->map().end();
+ ++iter) {
+ image_paths.insert(FilePath::FromWStringHack(UTF8ToWide(iter->second)));
+ }
}
return image_paths;
diff --git a/chrome/common/extensions/extension.h b/chrome/common/extensions/extension.h
index 55e8d2a..d0aef2a 100644
--- a/chrome/common/extensions/extension.h
+++ b/chrome/common/extensions/extension.h
@@ -629,8 +629,7 @@ class Extension : public base::RefCountedThreadSafe<Extension> {
return !omnibox_keyword().empty() ||
browser_action() ||
(page_action() &&
- (page_action_command() ||
- !page_action()->default_icon_path().empty()));
+ (page_action_command() || page_action()->default_icon()));
}
const FileBrowserHandlerList* file_browser_handlers() const {
return file_browser_handlers_.get();
diff --git a/chrome/common/extensions/extension_action.cc b/chrome/common/extensions/extension_action.cc
index 448aa11..c857864 100644
--- a/chrome/common/extensions/extension_action.cc
+++ b/chrome/common/extensions/extension_action.cc
@@ -10,6 +10,7 @@
#include "base/logging.h"
#include "base/message_loop.h"
#include "chrome/common/badge_util.h"
+#include "chrome/common/extensions/extension_constants.h"
#include "googleurl/src/gurl.h"
#include "grit/theme_resources.h"
#include "grit/ui_resources.h"
@@ -253,11 +254,28 @@ scoped_ptr<ExtensionAction> ExtensionAction::CopyForTest() const {
copy->badge_text_color_ = badge_text_color_;
copy->appearance_ = appearance_;
copy->icon_animation_ = icon_animation_;
- copy->default_icon_path_ = default_icon_path_;
copy->id_ = id_;
+
+ if (default_icon_.get())
+ copy->default_icon_.reset(new ExtensionIconSet(*default_icon_));
+
return copy.Pass();
}
+// static
+int ExtensionAction::GetIconSizeForType(ExtensionAction::Type type) {
+ switch (type) {
+ case ExtensionAction::TYPE_BROWSER:
+ case ExtensionAction::TYPE_PAGE:
+ return extension_misc::EXTENSION_ICON_ACTION;
+ case ExtensionAction::TYPE_SCRIPT_BADGE:
+ return extension_misc::EXTENSION_ICON_BITTY;
+ default:
+ NOTREACHED();
+ return 0;
+ }
+}
+
void ExtensionAction::SetPopupUrl(int tab_id, const GURL& url) {
// We store |url| even if it is empty, rather than removing a URL from the
// map. If an extension has a default popup, and removes it for a tab via
@@ -275,27 +293,14 @@ GURL ExtensionAction::GetPopupUrl(int tab_id) const {
return GetValue(&popup_url_, tab_id);
}
-void ExtensionAction::CacheIcon(const gfx::Image& icon) {
- if (!icon.IsEmpty())
- cached_icon_.reset(new gfx::ImageSkia(*icon.ToImageSkia()));
-}
-
void ExtensionAction::SetIcon(int tab_id, const gfx::Image& image) {
SetValue(&icon_, tab_id, image.AsImageSkia());
}
-gfx::Image ExtensionAction::GetIcon(int tab_id) const {
- // Check if a specific icon is set for this tab.
- gfx::ImageSkia icon = GetExplicitlySetIcon(tab_id);
- if (icon.isNull()) {
- if (cached_icon_.get()) {
- icon = *cached_icon_;
- } else {
- icon = *ui::ResourceBundle::GetSharedInstance().GetImageSkiaNamed(
- IDR_EXTENSIONS_FAVICON);
- }
- }
-
+gfx::Image ExtensionAction::ApplyAttentionAndAnimation(
+ const gfx::ImageSkia& original_icon,
+ int tab_id) const {
+ gfx::ImageSkia icon = original_icon;
if (GetValue(&appearance_, tab_id) == WANTS_ATTENTION)
icon = gfx::ImageSkia(new GetAttentionImageSource(icon), icon.size());
@@ -343,7 +348,7 @@ void ExtensionAction::PaintBadge(gfx::Canvas* canvas,
GetBadgeText(tab_id),
GetBadgeTextColor(tab_id),
GetBadgeBackgroundColor(tab_id),
- GetValue(&icon_, tab_id).size().width());
+ GetIconWidth(tab_id));
}
gfx::ImageSkia ExtensionAction::GetIconWithBadge(
@@ -362,6 +367,23 @@ gfx::ImageSkia ExtensionAction::GetIconWithBadge(
icon.size());
}
+// Determines which icon would be returned by |GetIcon|, and returns its width.
+int ExtensionAction::GetIconWidth(int tab_id) const {
+ // If icon has been set, return its width.
+ gfx::ImageSkia icon = GetValue(&icon_, tab_id);
+ if (!icon.isNull())
+ return icon.width();
+ // If there is a default icon, the icon width will be set depending on our
+ // action type.
+ if (default_icon_.get())
+ return GetIconSizeForType(action_type());
+
+ // If no icon has been set and there is no default icon, we need favicon
+ // width.
+ return ui::ResourceBundle::GetSharedInstance().GetImageNamed(
+ IDR_EXTENSIONS_FAVICON).ToImageSkia()->width();
+}
+
// static
void ExtensionAction::DoPaintBadge(gfx::Canvas* canvas,
const gfx::Rect& bounds,
diff --git a/chrome/common/extensions/extension_action.h b/chrome/common/extensions/extension_action.h
index b5a1339..1dee6c2 100644
--- a/chrome/common/extensions/extension_action.h
+++ b/chrome/common/extensions/extension_action.h
@@ -12,8 +12,10 @@
#include "base/basictypes.h"
#include "base/memory/linked_ptr.h"
#include "base/memory/scoped_ptr.h"
+#include "base/memory/scoped_vector.h"
#include "base/memory/weak_ptr.h"
#include "base/observer_list.h"
+#include "chrome/common/extensions/extension_icon_set.h"
#include "third_party/skia/include/core/SkColor.h"
#include "ui/base/animation/linear_animation.h"
@@ -123,6 +125,11 @@ class ExtensionAction {
// It doesn't make sense to copy of an ExtensionAction except in tests.
scoped_ptr<ExtensionAction> CopyForTest() const;
+ // Given the extension action type, returns the size the extension action icon
+ // should have. The icon should be square, so only one dimension is
+ // returned.
+ static int GetIconSizeForType(Type type);
+
// extension id
const std::string& extension_id() const { return extension_id_; }
@@ -158,33 +165,28 @@ class ExtensionAction {
// Icons are a bit different because the default value can be set to either a
// bitmap or a path. However, conceptually, there is only one default icon.
// Setting the default icon using a path clears the bitmap and vice-versa.
-
- // Since ExtensionAction, living in common/, can't interact with the browser
- // to load images, the UI code needs to load the icon. Load the image there
- // using an ImageLoadingTracker and call CacheIcon(image) with the result.
- //
- // If an image is cached redundantly, the first load will be used.
- void CacheIcon(const gfx::Image& icon);
+ // To retrieve the icon for the extension action, use
+ // ExtensionActionIconFactory.
// Set this action's icon bitmap on a specific tab.
void SetIcon(int tab_id, const gfx::Image& image);
- // Get the icon for a tab, or the default if no icon was set for this tab,
- // retrieving icons that have been specified by path from the previous
- // arguments to CacheIcon(). If the default icon isn't found in the cache,
- // returns the puzzle piece icon.
- gfx::Image GetIcon(int tab_id) const;
+ // Applies the attention and animation image transformations registered for
+ // the tab on the provided icon.
+ gfx::Image ApplyAttentionAndAnimation(const gfx::ImageSkia& icon,
+ int tab_id) const;
// Gets the icon that has been set using |SetIcon| for the tab.
gfx::ImageSkia GetExplicitlySetIcon(int tab_id) const;
// Non-tab-specific icon path. This is used to support the default_icon key of
// page and browser actions.
- void set_default_icon_path(const std::string& path) {
- default_icon_path_ = path;
+ void set_default_icon(scoped_ptr<ExtensionIconSet> icon_set) {
+ default_icon_ = icon_set.Pass();
}
- const std::string& default_icon_path() const {
- return default_icon_path_;
+
+ const ExtensionIconSet* default_icon() const {
+ return default_icon_.get();
}
// Set this action's badge text on a specific tab.
@@ -252,6 +254,11 @@ class ExtensionAction {
gfx::ImageSkia ApplyIconAnimation(int tab_id,
const gfx::ImageSkia& orig) const;
+ // Returns width of the current icon for tab_id.
+ // TODO(tbarzic): The icon selection is done in ExtensionActionIconFactory.
+ // We should probably move this there too.
+ int GetIconWidth(int tab_id) const;
+
// Paints badge with specified parameters to |canvas|.
static void DoPaintBadge(gfx::Canvas* canvas,
const gfx::Rect& bounds,
@@ -306,15 +313,14 @@ class ExtensionAction {
// NULLs to prevent the map from growing without bound.
mutable std::map<int, base::WeakPtr<IconAnimation> > icon_animation_;
- std::string default_icon_path_;
+ // ExtensionIconSet containing paths to bitmaps from which default icon's
+ // image representations will be selected.
+ scoped_ptr<const ExtensionIconSet> default_icon_;
// The id for the ExtensionAction, for example: "RssPageAction". This is
// needed for compat with an older version of the page actions API.
std::string id_;
- // Saves the arguments from CacheIcon() calls.
- scoped_ptr<gfx::ImageSkia> cached_icon_;
-
// True if the ExtensionAction's settings have changed from what was
// specified in the manifest.
bool has_changed_;
diff --git a/chrome/common/extensions/extension_action_unittest.cc b/chrome/common/extensions/extension_action_unittest.cc
index f34ab88..7a52911 100644
--- a/chrome/common/extensions/extension_action_unittest.cc
+++ b/chrome/common/extensions/extension_action_unittest.cc
@@ -2,44 +2,13 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
-#include "base/file_path.h"
-#include "base/file_util.h"
#include "base/message_loop.h"
-#include "base/path_service.h"
-#include "chrome/common/chrome_paths.h"
#include "chrome/common/extensions/extension_action.h"
#include "googleurl/src/gurl.h"
-#include "grit/theme_resources.h"
#include "testing/gtest/include/gtest/gtest.h"
-#include "third_party/skia/include/core/SkBitmap.h"
-#include "ui/base/resource/resource_bundle.h"
-#include "ui/gfx/image/image_skia.h"
-#include "ui/gfx/skia_util.h"
-#include "webkit/glue/image_decoder.h"
namespace {
-bool ImagesAreEqual(const gfx::Image& i1, const gfx::Image& i2) {
- return gfx::BitmapsAreEqual(*i1.ToSkBitmap(), *i2.ToSkBitmap());
-}
-
-gfx::Image LoadIcon(const std::string& filename) {
- FilePath path;
- PathService::Get(chrome::DIR_TEST_DATA, &path);
- path = path.AppendASCII("extensions").AppendASCII(filename);
-
- std::string file_contents;
- file_util::ReadFileToString(path, &file_contents);
- const unsigned char* data =
- reinterpret_cast<const unsigned char*>(file_contents.data());
-
- SkBitmap bitmap;
- webkit_glue::ImageDecoder decoder;
- bitmap = decoder.Decode(data, file_contents.length());
-
- return gfx::Image(bitmap);
-}
-
class ExtensionActionTest : public testing::Test {
public:
ExtensionActionTest()
@@ -63,29 +32,6 @@ TEST_F(ExtensionActionTest, Title) {
ASSERT_EQ("baz", action.GetTitle(100));
}
-TEST_F(ExtensionActionTest, Icon) {
- gfx::Image puzzle_piece =
- ui::ResourceBundle::GetSharedInstance().GetImageNamed(
- IDR_EXTENSIONS_FAVICON);
- gfx::Image icon1 = LoadIcon("icon1.png");
- gfx::Image icon2 = LoadIcon("icon2.png");
- ASSERT_TRUE(ImagesAreEqual(puzzle_piece, action.GetIcon(1)));
-
- action.set_default_icon_path("the_default.png");
- ASSERT_TRUE(ImagesAreEqual(puzzle_piece, action.GetIcon(1)))
- << "Still returns the puzzle piece because the image isn't loaded yet.";
- action.CacheIcon(icon2);
- ASSERT_TRUE(ImagesAreEqual(icon2, action.GetIcon(1)));
-
- action.SetIcon(ExtensionAction::kDefaultTabId, icon1);
- ASSERT_TRUE(ImagesAreEqual(icon1, action.GetIcon(100)))
- << "SetIcon(kDefaultTabId) overrides the default_icon_path.";
-
- action.SetIcon(100, icon2);
- ASSERT_TRUE(ImagesAreEqual(icon1, action.GetIcon(1)));
- ASSERT_TRUE(ImagesAreEqual(icon2, action.GetIcon(100)));
-}
-
TEST_F(ExtensionActionTest, Visibility) {
// Supports the icon animation.
MessageLoop message_loop;
diff --git a/chrome/common/extensions/extension_constants.cc b/chrome/common/extensions/extension_constants.cc
index b96ba511..bb54bf8 100644
--- a/chrome/common/extensions/extension_constants.cc
+++ b/chrome/common/extensions/extension_constants.cc
@@ -164,4 +164,20 @@ const int kExtensionIconSizes[] = {
const size_t kNumExtensionIconSizes =
arraysize(kExtensionIconSizes);
+const int kExtensionActionIconSizes[] = {
+ EXTENSION_ICON_ACTION, // 19,
+ 2 * EXTENSION_ICON_ACTION // 38
+};
+
+const size_t kNumExtensionActionIconSizes =
+ arraysize(kExtensionActionIconSizes);
+
+const int kScriptBadgeIconSizes[] = {
+ EXTENSION_ICON_BITTY, // 16
+ 2 * EXTENSION_ICON_BITTY // 32
+};
+
+const size_t kNumScriptBadgeIconSizes =
+ arraysize(kScriptBadgeIconSizes);
+
} // namespace extension_misc
diff --git a/chrome/common/extensions/extension_constants.h b/chrome/common/extensions/extension_constants.h
index 33b5434..e00658b 100644
--- a/chrome/common/extensions/extension_constants.h
+++ b/chrome/common/extensions/extension_constants.h
@@ -263,6 +263,7 @@ namespace extension_misc {
EXTENSION_ICON_MEDIUM = 48,
EXTENSION_ICON_SMALL = 32,
EXTENSION_ICON_SMALLISH = 24,
+ EXTENSION_ICON_ACTION = 19,
EXTENSION_ICON_BITTY = 16,
EXTENSION_ICON_INVALID = 0,
};
@@ -271,6 +272,14 @@ namespace extension_misc {
extern const int kExtensionIconSizes[];
extern const size_t kNumExtensionIconSizes;
+ // List of sizes for extension icons that can be defined in the manifest.
+ extern const int kExtensionActionIconSizes[];
+ extern const size_t kNumExtensionActionIconSizes;
+
+ // List of sizes for extension icons that can be defined in the manifest.
+ extern const int kScriptBadgeIconSizes[];
+ extern const size_t kNumScriptBadgeIconSizes;
+
} // extension_misc
#endif // CHROME_COMMON_EXTENSIONS_EXTENSION_CONSTANTS_H_
diff --git a/chrome/common/extensions/extension_file_util.cc b/chrome/common/extensions/extension_file_util.cc
index 2b94f67..fce46dd 100644
--- a/chrome/common/extensions/extension_file_util.cc
+++ b/chrome/common/extensions/extension_file_util.cc
@@ -35,6 +35,27 @@ using extensions::Extension;
namespace errors = extension_manifest_errors;
+namespace {
+
+bool ValidateExtensionIconSet(const ExtensionIconSet* icon_set,
+ const Extension* extension,
+ int error_message_id,
+ std::string* error) {
+ for (ExtensionIconSet::IconMap::const_iterator iter = icon_set->map().begin();
+ iter != icon_set->map().end();
+ ++iter) {
+ const FilePath path = extension->GetResource(iter->second).GetFilePath();
+ if (!extension_file_util::ValidateFilePath(path)) {
+ *error = l10n_util::GetStringFUTF8(error_message_id,
+ UTF8ToUTF16(iter->second));
+ return false;
+ }
+ }
+ return true;
+}
+
+} // namespace
+
namespace extension_file_util {
// Validates locale info. Doesn't check if messages.json files are valid.
@@ -320,36 +341,18 @@ bool ValidateExtension(const Extension* extension,
}
}
- // Validate icon location and icon file size for page actions.
- ExtensionAction* page_action = extension->page_action();
- if (page_action) {
- std::string path = page_action->default_icon_path();
- if (!path.empty()) {
- const FilePath file_path = extension->GetResource(path).GetFilePath();
- if (!ValidateFilePath(file_path)) {
- *error =
- l10n_util::GetStringFUTF8(
- IDS_EXTENSION_LOAD_ICON_FOR_PAGE_ACTION_FAILED,
- UTF8ToUTF16(path));
- return false;
- }
- }
+ const ExtensionAction* action = extension->page_action();
+ if (action && action->default_icon() &&
+ !ValidateExtensionIconSet(action->default_icon(), extension,
+ IDS_EXTENSION_LOAD_ICON_FOR_PAGE_ACTION_FAILED, error)) {
+ return false;
}
- // Validate icon location and icon file size for browser actions.
- ExtensionAction* browser_action = extension->browser_action();
- if (browser_action) {
- std::string path = browser_action->default_icon_path();
- if (!path.empty()) {
- const FilePath file_path = extension->GetResource(path).GetFilePath();
- if (!ValidateFilePath(file_path)) {
- *error =
- l10n_util::GetStringFUTF8(
- IDS_EXTENSION_LOAD_ICON_FOR_BROWSER_ACTION_FAILED,
- UTF8ToUTF16(path));
- return false;
- }
- }
+ action = extension->browser_action();
+ if (action && action->default_icon() &&
+ !ValidateExtensionIconSet(action->default_icon(), extension,
+ IDS_EXTENSION_LOAD_ICON_FOR_BROWSER_ACTION_FAILED, error)) {
+ return false;
}
// Validate that background scripts exist.
diff --git a/chrome/common/extensions/extension_unittest.cc b/chrome/common/extensions/extension_unittest.cc
index 5f5c329..7529734 100644
--- a/chrome/common/extensions/extension_unittest.cc
+++ b/chrome/common/extensions/extension_unittest.cc
@@ -229,7 +229,9 @@ TEST(ExtensionTest, LoadPageActionHelper) {
// No title, so fall back to name.
ASSERT_EQ(name, action->GetTitle(1));
- ASSERT_EQ(img1, action->default_icon_path());
+ ASSERT_EQ(img1,
+ action->default_icon()->Get(extension_misc::EXTENSION_ICON_ACTION,
+ ExtensionIconSet::MATCH_EXACTLY));
// Same test with explicitly set type.
action = LoadAction("page_action_type.json");
@@ -256,7 +258,7 @@ TEST(ExtensionTest, LoadPageActionHelper) {
action = LoadAction("page_action_new_format.json");
ASSERT_TRUE(action.get());
ASSERT_EQ(kTitle, action->GetTitle(1));
- ASSERT_FALSE(action->default_icon_path().empty());
+ ASSERT_TRUE(action->default_icon());
// Invalid title should give an error even with a valid name.
LoadActionAndExpectError("page_action_invalid_title.json",
diff --git a/chrome/common/extensions/manifest_tests/extension_manifests_browseraction_unittest.cc b/chrome/common/extensions/manifest_tests/extension_manifests_browseraction_unittest.cc
new file mode 100644
index 0000000..7c6a53b
--- /dev/null
+++ b/chrome/common/extensions/manifest_tests/extension_manifests_browseraction_unittest.cc
@@ -0,0 +1,102 @@
+// 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 "chrome/common/extensions/extension_action.h"
+#include "chrome/common/extensions/extension_builder.h"
+#include "chrome/common/extensions/extension_error_utils.h"
+#include "chrome/common/extensions/extension_icon_set.h"
+#include "chrome/common/extensions/extension_manifest_constants.h"
+#include "chrome/common/extensions/manifest_tests/extension_manifest_test.h"
+#include "chrome/common/extensions/value_builder.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace errors = extension_manifest_errors;
+
+namespace extensions {
+namespace {
+
+TEST_F(ExtensionManifestTest, BrowserActionManifestIcons_NoDefaultIcons) {
+ scoped_refptr<const Extension> extension =
+ ExtensionBuilder()
+ .SetManifest(DictionaryBuilder()
+ .Set("name", "No default properties")
+ .Set("version", "1.0.0")
+ .Set("manifest_version", 2)
+ .Set("browser_action", DictionaryBuilder()
+ .Set("default_title", "Title")))
+ .Build();
+
+ ASSERT_TRUE(extension.get());
+ ASSERT_TRUE(extension->browser_action());
+ EXPECT_FALSE(extension->browser_action()->default_icon());
+}
+
+
+TEST_F(ExtensionManifestTest, BrowserActionManifestIcons_StringDefaultIcon) {
+ scoped_refptr<const Extension> extension =
+ ExtensionBuilder()
+ .SetManifest(DictionaryBuilder()
+ .Set("name", "String default icon")
+ .Set("version", "1.0.0")
+ .Set("manifest_version", 2)
+ .Set("browser_action", DictionaryBuilder()
+ .Set("default_icon", "icon.png")))
+ .Build();
+
+ ASSERT_TRUE(extension.get());
+ ASSERT_TRUE(extension->browser_action());
+ ASSERT_TRUE(extension->browser_action()->default_icon());
+
+ const ExtensionIconSet* icons = extension->browser_action()->default_icon();
+
+ EXPECT_EQ(1u, icons->map().size());
+ EXPECT_EQ("icon.png", icons->Get(19, ExtensionIconSet::MATCH_EXACTLY));
+}
+
+TEST_F(ExtensionManifestTest, BrowserActionManifestIcons_DictDefaultIcon) {
+ scoped_refptr<const Extension> extension =
+ ExtensionBuilder()
+ .SetManifest(DictionaryBuilder()
+ .Set("name", "Dictionary default icon")
+ .Set("version", "1.0.0")
+ .Set("manifest_version", 2)
+ .Set("browser_action", DictionaryBuilder()
+ .Set("default_icon", DictionaryBuilder()
+ .Set("19", "icon19.png")
+ .Set("24", "icon24.png") // Should be ignored.
+ .Set("38", "icon38.png"))))
+ .Build();
+
+ ASSERT_TRUE(extension.get());
+ ASSERT_TRUE(extension->browser_action());
+ ASSERT_TRUE(extension->browser_action()->default_icon());
+
+ const ExtensionIconSet* icons = extension->browser_action()->default_icon();
+
+ // 24px icon should be ignored.
+ EXPECT_EQ(2u, icons->map().size());
+ EXPECT_EQ("icon19.png", icons->Get(19, ExtensionIconSet::MATCH_EXACTLY));
+ EXPECT_EQ("icon38.png", icons->Get(38, ExtensionIconSet::MATCH_EXACTLY));
+}
+
+TEST_F(ExtensionManifestTest, BrowserActionManifestIcons_InvalidDefaultIcon) {
+ scoped_ptr<DictionaryValue> manifest_value = DictionaryBuilder()
+ .Set("name", "Invalid default icon")
+ .Set("version", "1.0.0")
+ .Set("manifest_version", 2)
+ .Set("browser_action", DictionaryBuilder()
+ .Set("default_icon", DictionaryBuilder()
+ .Set("19", "") // Invalid value.
+ .Set("24", "icon24.png")
+ .Set("38", "icon38.png")))
+ .Build();
+
+ string16 error = ExtensionErrorUtils::FormatErrorMessageUTF16(
+ errors::kInvalidIconPath, "19");
+ LoadAndExpectError(Manifest(manifest_value.get(), "Invalid default icon"),
+ errors::kInvalidIconPath);
+}
+
+} // namespace
+} // namespace extensions
diff --git a/chrome/common/extensions/manifest_tests/extension_manifests_pageaction_unittest.cc b/chrome/common/extensions/manifest_tests/extension_manifests_pageaction_unittest.cc
index 40d4149..354440a 100644
--- a/chrome/common/extensions/manifest_tests/extension_manifests_pageaction_unittest.cc
+++ b/chrome/common/extensions/manifest_tests/extension_manifests_pageaction_unittest.cc
@@ -17,7 +17,7 @@ TEST_F(ExtensionManifestTest, PageActionManifestVersion2) {
ASSERT_TRUE(extension->page_action());
EXPECT_EQ("", extension->page_action()->id());
- EXPECT_TRUE(extension->page_action()->default_icon_path().empty());
+ EXPECT_FALSE(extension->page_action()->default_icon());
EXPECT_EQ("", extension->page_action()->GetTitle(
ExtensionAction::kDefaultTabId));
EXPECT_FALSE(extension->page_action()->HasPopup(
diff --git a/chrome/common/extensions/manifest_tests/extension_manifests_scriptbadge_unittest.cc b/chrome/common/extensions/manifest_tests/extension_manifests_scriptbadge_unittest.cc
index 6b2c39d..1ba17b4 100644
--- a/chrome/common/extensions/manifest_tests/extension_manifests_scriptbadge_unittest.cc
+++ b/chrome/common/extensions/manifest_tests/extension_manifests_scriptbadge_unittest.cc
@@ -2,17 +2,14 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
-#include "chrome/common/extensions/manifest_tests/extension_manifest_test.h"
-
#include "chrome/common/extensions/extension_action.h"
+#include "chrome/common/extensions/extension_constants.h"
+#include "chrome/common/extensions/extension_icon_set.h"
#include "chrome/common/extensions/extension_manifest_constants.h"
#include "chrome/common/extensions/extension_switch_utils.h"
-#include "grit/theme_resources.h"
+#include "chrome/common/extensions/manifest_tests/extension_manifest_test.h"
#include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h"
-#include "ui/base/resource/resource_bundle.h"
-#include "ui/gfx/image/image.h"
-#include "ui/gfx/skia_util.h"
namespace errors = extension_manifest_errors;
namespace switch_utils = extensions::switch_utils;
@@ -20,10 +17,6 @@ using extensions::Extension;
namespace {
-bool ImagesAreEqual(const gfx::Image& i1, const gfx::Image& i2) {
- return gfx::BitmapsAreEqual(*i1.ToSkBitmap(), *i2.ToSkBitmap());
-}
-
std::vector<Extension::InstallWarning> StripMissingFlagWarning(
const std::vector<Extension::InstallWarning>& install_warnings) {
std::vector<Extension::InstallWarning> result;
@@ -42,15 +35,24 @@ TEST_F(ExtensionManifestTest, ScriptBadgeBasic) {
EXPECT_THAT(StripMissingFlagWarning(extension->install_warnings()),
testing::ElementsAre(/*empty*/));
+ const ExtensionIconSet* default_icon =
+ extension->script_badge()->default_icon();
+ // Default icon set should not be NULL.
+ ASSERT_TRUE(default_icon);
+
+ // Verify that correct icon paths are registered in default_icon.
+ EXPECT_EQ(2u, default_icon->map().size());
+ EXPECT_EQ("icon16.png",
+ default_icon->Get(extension_misc::EXTENSION_ICON_BITTY,
+ ExtensionIconSet::MATCH_EXACTLY));
+ EXPECT_EQ("icon32.png",
+ default_icon->Get(2 * extension_misc::EXTENSION_ICON_BITTY,
+ ExtensionIconSet::MATCH_EXACTLY));
+
EXPECT_EQ("my extension", extension->script_badge()->GetTitle(
ExtensionAction::kDefaultTabId));
EXPECT_TRUE(extension->script_badge()->HasPopup(
ExtensionAction::kDefaultTabId));
- EXPECT_TRUE(ImagesAreEqual(
- ui::ResourceBundle::GetSharedInstance().GetImageNamed(
- IDR_EXTENSIONS_FAVICON),
- extension->script_badge()->GetIcon(ExtensionAction::kDefaultTabId)));
- EXPECT_EQ("icon16.png", extension->script_badge()->default_icon_path());
}
TEST_F(ExtensionManifestTest, ScriptBadgeExplicitTitleAndIconsIgnored) {
@@ -67,13 +69,18 @@ TEST_F(ExtensionManifestTest, ScriptBadgeExplicitTitleAndIconsIgnored) {
Extension::InstallWarning(
Extension::InstallWarning::FORMAT_TEXT,
errors::kScriptBadgeIconIgnored)));
+
+ const ExtensionIconSet* default_icon =
+ extension->script_badge()->default_icon();
+ ASSERT_TRUE(default_icon);
+
+ EXPECT_EQ(1u, default_icon->map().size());
+ EXPECT_EQ("icon16.png",
+ default_icon->Get(extension_misc::EXTENSION_ICON_BITTY,
+ ExtensionIconSet::MATCH_EXACTLY));
+
EXPECT_EQ("my extension", extension->script_badge()->GetTitle(
ExtensionAction::kDefaultTabId));
- EXPECT_TRUE(ImagesAreEqual(
- ui::ResourceBundle::GetSharedInstance().GetImageNamed(
- IDR_EXTENSIONS_FAVICON),
- extension->script_badge()->GetIcon(ExtensionAction::kDefaultTabId)));
- EXPECT_EQ("icon16.png", extension->script_badge()->default_icon_path());
}
TEST_F(ExtensionManifestTest, ScriptBadgeIconFallsBackToPuzzlePiece) {
@@ -84,12 +91,9 @@ TEST_F(ExtensionManifestTest, ScriptBadgeIconFallsBackToPuzzlePiece) {
EXPECT_THAT(extension->install_warnings(),
testing::ElementsAre(/*empty*/));
- EXPECT_EQ("", extension->script_badge()->default_icon_path())
+ EXPECT_FALSE(extension->script_badge()->default_icon())
<< "Should not fall back to the 64px icon.";
- EXPECT_FALSE(extension->script_badge()->GetIcon(
- ExtensionAction::kDefaultTabId).IsEmpty())
- << "Should set the puzzle piece as the default, but there's no way "
- << "to assert in a unittest what the image looks like.";
+ EXPECT_EQ(NULL, extension->script_badge()->default_icon());
}
} // namespace
diff --git a/chrome/test/data/extensions/manifest_tests/script_badge_basic.json b/chrome/test/data/extensions/manifest_tests/script_badge_basic.json
index faecd0e..50d7cbc 100644
--- a/chrome/test/data/extensions/manifest_tests/script_badge_basic.json
+++ b/chrome/test/data/extensions/manifest_tests/script_badge_basic.json
@@ -5,6 +5,7 @@
"description": "Check that a simple script_badge section parses",
"icons": {
"16": "icon16.png",
+ "32": "icon32.png",
"19": "icon19.png",
"48": "icon48.png"
},
diff --git a/ui/gfx/image/image_skia_operations.cc b/ui/gfx/image/image_skia_operations.cc
index 9657de9..a9809b3 100644
--- a/ui/gfx/image/image_skia_operations.cc
+++ b/ui/gfx/image/image_skia_operations.cc
@@ -131,15 +131,14 @@ class TransparentImageSource : public gfx::ImageSkiaSource {
virtual ImageSkiaRep GetImageForScale(ui::ScaleFactor scale_factor) OVERRIDE {
ImageSkiaRep image_rep = image_.GetRepresentation(scale_factor);
SkBitmap alpha;
- const float scale = ui::GetScaleFactorScale(image_rep.scale_factor());
alpha.setConfig(SkBitmap::kARGB_8888_Config,
- static_cast<int>(image_.size().width() * scale),
- static_cast<int>(image_.size().height() * scale));
+ image_rep.pixel_width(),
+ image_rep.pixel_height());
alpha.allocPixels();
alpha.eraseColor(SkColorSetARGB(alpha_ * 255, 0, 0, 0));
- return ImageSkiaRep(SkBitmapOperations::CreateMaskedBitmap(
- image_rep.sk_bitmap(), alpha),
- image_rep.scale_factor());
+ return ImageSkiaRep(
+ SkBitmapOperations::CreateMaskedBitmap(image_rep.sk_bitmap(), alpha),
+ image_rep.scale_factor());
}
ImageSkia image_;