summaryrefslogtreecommitdiffstats
path: root/chrome/browser
diff options
context:
space:
mode:
authorhclam@chromium.org <hclam@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2010-06-25 06:46:10 +0000
committerhclam@chromium.org <hclam@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2010-06-25 06:46:10 +0000
commit4930760d08e0d3afef5e99684fc7450b15d0cf78 (patch)
treed4d02e538c145096e8ba74bc8cb34a72a6d83647 /chrome/browser
parent022a8ca1d96755b98c26c6211968415fe7c2f458 (diff)
downloadchromium_src-4930760d08e0d3afef5e99684fc7450b15d0cf78.zip
chromium_src-4930760d08e0d3afef5e99684fc7450b15d0cf78.tar.gz
chromium_src-4930760d08e0d3afef5e99684fc7450b15d0cf78.tar.bz2
Revert r50779 as it breaks memory tests
TBR=asargent@chromium.org TEST=Tree goes green Review URL: http://codereview.chromium.org/2812025 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@50828 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'chrome/browser')
-rw-r--r--chrome/browser/extensions/extension_context_menu_api.cc2
-rw-r--r--chrome/browser/extensions/extension_menu_manager.cc101
-rw-r--r--chrome/browser/extensions/extension_menu_manager.h40
-rw-r--r--chrome/browser/extensions/extension_menu_manager_unittest.cc157
-rw-r--r--chrome/browser/extensions/extension_protocols.cc9
-rw-r--r--chrome/browser/extensions/extensions_service_unittest.cc11
-rw-r--r--chrome/browser/extensions/file_reader_unittest.cc10
-rw-r--r--chrome/browser/extensions/image_loading_tracker.cc30
-rw-r--r--chrome/browser/extensions/image_loading_tracker.h7
-rw-r--r--chrome/browser/extensions/image_loading_tracker_unittest.cc12
-rw-r--r--chrome/browser/tab_contents/render_view_context_menu.cc16
-rw-r--r--chrome/browser/tab_contents/render_view_context_menu.h2
12 files changed, 125 insertions, 272 deletions
diff --git a/chrome/browser/extensions/extension_context_menu_api.cc b/chrome/browser/extensions/extension_context_menu_api.cc
index 1c63dd1..554cc37 100644
--- a/chrome/browser/extensions/extension_context_menu_api.cc
+++ b/chrome/browser/extensions/extension_context_menu_api.cc
@@ -188,7 +188,7 @@ bool CreateContextMenuFunction::RunImpl() {
}
id = menu_manager->AddChildItem(parent_id, item.release());
} else {
- id = menu_manager->AddContextItem(GetExtension(), item.release());
+ id = menu_manager->AddContextItem(item.release());
}
if (id <= 0)
diff --git a/chrome/browser/extensions/extension_menu_manager.cc b/chrome/browser/extensions/extension_menu_manager.cc
index 42e3842..9563a0a 100644
--- a/chrome/browser/extensions/extension_menu_manager.cc
+++ b/chrome/browser/extensions/extension_menu_manager.cc
@@ -6,7 +6,6 @@
#include <algorithm>
-#include "app/resource_bundle.h"
#include "base/logging.h"
#include "base/string_util.h"
#include "base/utf_string_conversions.h"
@@ -17,11 +16,6 @@
#include "chrome/browser/extensions/extension_tabs_module.h"
#include "chrome/browser/profile.h"
#include "chrome/common/extensions/extension.h"
-#include "chrome/common/extensions/extension_resource.h"
-#include "gfx/favicon_size.h"
-#include "gfx/size.h"
-#include "grit/theme_resources.h"
-#include "skia/ext/image_operations.h"
#include "webkit/glue/context_menu.h"
ExtensionMenuItem::ExtensionMenuItem(const std::string& extension_id,
@@ -102,9 +96,7 @@ void ExtensionMenuItem::AddChild(ExtensionMenuItem* item) {
children_.push_back(item);
}
-ExtensionMenuManager::ExtensionMenuManager()
- : next_item_id_(1),
- ALLOW_THIS_IN_INITIALIZER_LIST(image_tracker_(this)) {
+ExtensionMenuManager::ExtensionMenuManager() : next_item_id_(1) {
registrar_.Add(this, NotificationType::EXTENSION_UNLOADED,
NotificationService::AllSources());
}
@@ -134,38 +126,21 @@ const ExtensionMenuItem::List* ExtensionMenuManager::MenuItems(
return NULL;
}
-int ExtensionMenuManager::AddContextItem(Extension* extension,
- ExtensionMenuItem* item) {
+int ExtensionMenuManager::AddContextItem(ExtensionMenuItem* item) {
const std::string& extension_id = item->extension_id();
// The item must have a non-empty extension id.
if (extension_id.empty())
return 0;
- DCHECK_EQ(extension->id(), extension_id);
-
DCHECK_EQ(0, item->id());
item->set_id(next_item_id_++);
- bool first_item = !ContainsKey(context_items_, extension_id);
context_items_[extension_id].push_back(item);
items_by_id_[item->id()] = item;
if (item->type() == ExtensionMenuItem::RADIO && item->checked())
RadioItemSelected(item);
- // If this is the first item for this extension, start loading its icon.
- if (first_item) {
- ExtensionResource icon_resource;
- extension->GetIconPathAllowLargerSize(&icon_resource,
- Extension::EXTENSION_ICON_BITTY);
- if (!icon_resource.extension_root().empty()) {
- image_tracker_.LoadImage(extension,
- icon_resource,
- gfx::Size(kFavIconSize, kFavIconSize),
- ImageLoadingTracker::CACHE);
- }
- }
-
return item->id();
}
@@ -258,7 +233,6 @@ bool ExtensionMenuManager::RemoveContextMenuItem(int id) {
return false;
}
- bool result = false;
ExtensionMenuItem::List& list = i->second;
ExtensionMenuItem::List::iterator j;
for (j = list.begin(); j < list.end(); ++j) {
@@ -267,20 +241,14 @@ bool ExtensionMenuManager::RemoveContextMenuItem(int id) {
delete *j;
list.erase(j);
items_by_id_.erase(id);
- result = true;
- break;
+ return true;
} else if ((*j)->RemoveChild(id)) {
items_by_id_.erase(id);
- result = true;
- break;
+ return true;
}
}
- DCHECK(result); // The check at the very top should have prevented this.
-
- if (list.empty() && ContainsKey(extension_icons_, extension_id))
- extension_icons_.erase(extension_id);
-
- return result;
+ NOTREACHED(); // The check at the very top should prevent getting here.
+ return false;
}
void ExtensionMenuManager::RemoveAllContextItems(std::string extension_id) {
@@ -299,9 +267,6 @@ void ExtensionMenuManager::RemoveAllContextItems(std::string extension_id) {
}
STLDeleteElements(&context_items_[extension_id]);
context_items_.erase(extension_id);
-
- if (ContainsKey(extension_icons_, extension_id))
- extension_icons_.erase(extension_id);
}
ExtensionMenuItem* ExtensionMenuManager::GetItemById(int id) const {
@@ -457,57 +422,3 @@ void ExtensionMenuManager::Observe(NotificationType type,
RemoveAllContextItems(extension->id());
}
}
-
-const SkBitmap& ExtensionMenuManager::GetIconForExtension(
- const std::string& extension_id) {
- const SkBitmap* result = NULL;
- if (ContainsKey(extension_icons_, extension_id)) {
- result = &(extension_icons_[extension_id]);
- } else {
- EnsureDefaultIcon();
- result = &default_icon_;
- }
- DCHECK(result);
- DCHECK(result->width() == kFavIconSize);
- DCHECK(result->height() == kFavIconSize);
- return *result;
-}
-
-void ExtensionMenuManager::OnImageLoaded(SkBitmap* image,
- ExtensionResource resource,
- int index) {
- if (!image)
- return;
-
- const std::string extension_id = resource.extension_id();
-
- // Make sure we still have menu items for this extension (since image loading
- // is asynchronous, there's a slight chance they may have all been removed
- // while the icon was loading).
- if (!ContainsKey(context_items_, extension_id))
- return;
-
- if (image->width() == kFavIconSize && image->height() == kFavIconSize)
- extension_icons_[extension_id] = *image;
- else
- extension_icons_[extension_id] = ScaleToFavIconSize(*image);
-}
-
-void ExtensionMenuManager::EnsureDefaultIcon() {
- if (default_icon_.empty()) {
- ResourceBundle& rb = ResourceBundle::GetSharedInstance();
- SkBitmap* src = rb.GetBitmapNamed(IDR_EXTENSIONS_SECTION);
- if (src->width() == kFavIconSize && src->height() == kFavIconSize) {
- default_icon_ = *src;
- } else {
- default_icon_ = SkBitmap(ScaleToFavIconSize(*src));
- }
- }
-}
-
-SkBitmap ExtensionMenuManager::ScaleToFavIconSize(const SkBitmap& source) {
- return skia::ImageOperations::Resize(source,
- skia::ImageOperations::RESIZE_LANCZOS3,
- kFavIconSize,
- kFavIconSize);
-}
diff --git a/chrome/browser/extensions/extension_menu_manager.h b/chrome/browser/extensions/extension_menu_manager.h
index c3f82c5..e85f5d5 100644
--- a/chrome/browser/extensions/extension_menu_manager.h
+++ b/chrome/browser/extensions/extension_menu_manager.h
@@ -14,14 +14,11 @@
#include "base/linked_ptr.h"
#include "base/stl_util-inl.h"
#include "base/string16.h"
-#include "chrome/browser/extensions/image_loading_tracker.h"
#include "chrome/common/notification_observer.h"
#include "chrome/common/notification_registrar.h"
-#include "third_party/skia/include/core/SkBitmap.h"
struct ContextMenuParams;
-class Extension;
class ExtensionMessageService;
class Profile;
class TabContents;
@@ -170,9 +167,7 @@ class ExtensionMenuItem {
};
// This class keeps track of menu items added by extensions.
-class ExtensionMenuManager
- : public ImageLoadingTracker::Observer,
- public NotificationObserver {
+class ExtensionMenuManager : public NotificationObserver {
public:
ExtensionMenuManager();
virtual ~ExtensionMenuManager();
@@ -187,11 +182,9 @@ class ExtensionMenuManager
// put them into a submenu if there are more than 1.
const ExtensionMenuItem::List* MenuItems(const std::string& extension_id);
- // Adds a top-level menu item for an extension, requiring the |extension|
- // pointer so it can load the icon for the extension. Takes ownership of
- // |item|. Returns the id assigned to the item. Has the side-effect of
- // incrementing the next_item_id_ member.
- int AddContextItem(Extension* extension, ExtensionMenuItem* item);
+ // Takes ownership of |item|. Returns the id assigned to the item. Has the
+ // side-effect of incrementing the next_item_id_ member.
+ int AddContextItem(ExtensionMenuItem* item);
// Add an item as a child of another item which has been previously added, and
// takes ownership of |item|. Returns the id assigned to the item, or 0 on
@@ -224,15 +217,6 @@ class ExtensionMenuManager
virtual void Observe(NotificationType type, const NotificationSource& source,
const NotificationDetails& details);
- // This returns a bitmap of width/height kFavIconSize, loaded either from an
- // entry specified in the extension's 'icon' section of the manifest, or a
- // default extension icon.
- const SkBitmap& GetIconForExtension(const std::string& extension_id);
-
- // Implements the ImageLoadingTracker::Observer interface.
- virtual void OnImageLoaded(SkBitmap* image, ExtensionResource resource,
- int index);
-
private:
// This is a helper function which takes care of de-selecting any other radio
// items in the same group (i.e. that are adjacent in the list).
@@ -241,13 +225,6 @@ class ExtensionMenuManager
// Returns true if item is a descendant of an item with id |ancestor_id|.
bool DescendantOf(ExtensionMenuItem* item, int ancestor_id);
- // Makes sure we've done one-time initialization of the default extension icon
- // default_icon_.
- void EnsureDefaultIcon();
-
- // Helper function to return a copy of |src| scaled to kFavIconSize.
- SkBitmap ScaleToFavIconSize(const SkBitmap& src);
-
// We keep items organized by mapping an extension id to a list of items.
typedef std::map<std::string, ExtensionMenuItem::List> MenuItemMap;
MenuItemMap context_items_;
@@ -262,15 +239,6 @@ class ExtensionMenuManager
NotificationRegistrar registrar_;
- // Used for loading extension icons.
- ImageLoadingTracker image_tracker_;
-
- // Maps extension id to an SkBitmap with the icon for that extension.
- std::map<std::string, SkBitmap> extension_icons_;
-
- // The default icon we'll use if an extension doesn't have one.
- SkBitmap default_icon_;
-
DISALLOW_COPY_AND_ASSIGN(ExtensionMenuManager);
};
diff --git a/chrome/browser/extensions/extension_menu_manager_unittest.cc b/chrome/browser/extensions/extension_menu_manager_unittest.cc
index 04baf5ff..2abeef1 100644
--- a/chrome/browser/extensions/extension_menu_manager_unittest.cc
+++ b/chrome/browser/extensions/extension_menu_manager_unittest.cc
@@ -10,7 +10,6 @@
#include "base/values.h"
#include "chrome/browser/extensions/extension_menu_manager.h"
#include "chrome/browser/extensions/extension_message_service.h"
-#include "chrome/browser/extensions/test_extension_prefs.h"
#include "chrome/common/chrome_paths.h"
#include "chrome/common/extensions/extension.h"
#include "chrome/common/extensions/extension_constants.h"
@@ -31,24 +30,25 @@ class ExtensionMenuManagerTest : public testing::Test {
ExtensionMenuManagerTest() {}
~ExtensionMenuManagerTest() {}
- // Returns a test item.
- static ExtensionMenuItem* CreateTestItem(Extension* extension) {
+ // Returns a test item with some default values you can override if you want
+ // to by passing in |properties| (currently just extension_id). Caller owns
+ // the return value and is responsible for freeing it.
+ static ExtensionMenuItem* CreateTestItem(DictionaryValue* properties) {
+ std::string extension_id = "0123456789"; // A default dummy value.
+ if (properties && properties->HasKey(L"extension_id"))
+ EXPECT_TRUE(properties->GetString(L"extension_id", &extension_id));
+
ExtensionMenuItem::Type type = ExtensionMenuItem::NORMAL;
ExtensionMenuItem::ContextList contexts(ExtensionMenuItem::ALL);
ExtensionMenuItem::ContextList enabled_contexts = contexts;
- return new ExtensionMenuItem(extension->id(), "test", false, type, contexts,
- enabled_contexts);
- }
+ std::string title = "test";
- // Creates and returns a test Extension. The caller does *not* own the return
- // value.
- Extension* AddExtension(std::string name) {
- return prefs_.AddExtension(name);
+ return new ExtensionMenuItem(extension_id, title, false, type, contexts,
+ enabled_contexts);
}
protected:
ExtensionMenuManager manager_;
- TestExtensionPrefs prefs_;
private:
DISALLOW_COPY_AND_ASSIGN(ExtensionMenuManagerTest);
@@ -56,12 +56,10 @@ class ExtensionMenuManagerTest : public testing::Test {
// Tests adding, getting, and removing items.
TEST_F(ExtensionMenuManagerTest, AddGetRemoveItems) {
- Extension* extension = AddExtension("test");
-
// Add a new item, make sure you can get it back.
- ExtensionMenuItem* item1 = CreateTestItem(extension);
+ ExtensionMenuItem* item1 = CreateTestItem(NULL);
ASSERT_TRUE(item1 != NULL);
- int id1 = manager_.AddContextItem(extension, item1);
+ int id1 = manager_.AddContextItem(item1); // Ownership transferred.
ASSERT_GT(id1, 0);
ASSERT_EQ(item1, manager_.GetItemById(id1));
const ExtensionMenuItem::List* items =
@@ -70,8 +68,8 @@ TEST_F(ExtensionMenuManagerTest, AddGetRemoveItems) {
ASSERT_EQ(item1, items->at(0));
// Add a second item, make sure it comes back too.
- ExtensionMenuItem* item2 = CreateTestItem(extension);
- int id2 = manager_.AddContextItem(extension, item2);
+ ExtensionMenuItem* item2 = CreateTestItem(NULL);
+ int id2 = manager_.AddContextItem(item2); // Ownership transferred.
ASSERT_GT(id2, 0);
ASSERT_NE(id1, id2);
ASSERT_EQ(item2, manager_.GetItemById(id2));
@@ -81,9 +79,9 @@ TEST_F(ExtensionMenuManagerTest, AddGetRemoveItems) {
ASSERT_EQ(item2, items->at(1));
// Try adding item 3, then removing it.
- ExtensionMenuItem* item3 = CreateTestItem(extension);
+ ExtensionMenuItem* item3 = CreateTestItem(NULL);
std::string extension_id = item3->extension_id();
- int id3 = manager_.AddContextItem(extension, item3);
+ int id3 = manager_.AddContextItem(item3); // Ownership transferred.
ASSERT_GT(id3, 0);
ASSERT_EQ(item3, manager_.GetItemById(id3));
ASSERT_EQ(3u, manager_.MenuItems(extension_id)->size());
@@ -97,22 +95,23 @@ TEST_F(ExtensionMenuManagerTest, AddGetRemoveItems) {
// Test adding/removing child items.
TEST_F(ExtensionMenuManagerTest, ChildFunctions) {
- Extension* extension1 = AddExtension("1111");
- Extension* extension2 = AddExtension("2222");
- Extension* extension3 = AddExtension("3333");
+ DictionaryValue properties;
+ properties.SetString(L"extension_id", "1111");
+ ExtensionMenuItem* item1 = CreateTestItem(&properties);
- ExtensionMenuItem* item1 = CreateTestItem(extension1);
- ExtensionMenuItem* item2 = CreateTestItem(extension2);
- ExtensionMenuItem* item2_child = CreateTestItem(extension2);
- ExtensionMenuItem* item2_grandchild = CreateTestItem(extension2);
+ properties.SetString(L"extension_id", "2222");
+ ExtensionMenuItem* item2 = CreateTestItem(&properties);
+ ExtensionMenuItem* item2_child = CreateTestItem(&properties);
+ ExtensionMenuItem* item2_grandchild = CreateTestItem(&properties);
// This third item we expect to fail inserting, so we use a scoped_ptr to make
// sure it gets deleted.
- scoped_ptr<ExtensionMenuItem> item3(CreateTestItem(extension3));
+ properties.SetString(L"extension_id", "3333");
+ scoped_ptr<ExtensionMenuItem> item3(CreateTestItem(&properties));
// Add in the first two items.
- int id1 = manager_.AddContextItem(extension1, item1);
- int id2 = manager_.AddContextItem(extension2, item2);
+ int id1 = manager_.AddContextItem(item1); // Ownership transferred.
+ int id2 = manager_.AddContextItem(item2); // Ownership transferred.
ASSERT_NE(id1, id2);
@@ -151,15 +150,13 @@ TEST_F(ExtensionMenuManagerTest, ChildFunctions) {
// Tests changing parents.
TEST_F(ExtensionMenuManagerTest, ChangeParent) {
- Extension* extension1 = AddExtension("1111");
-
// First create two items and add them both to the manager.
- ExtensionMenuItem* item1 = CreateTestItem(extension1);
- ExtensionMenuItem* item2 = CreateTestItem(extension1);
+ ExtensionMenuItem* item1 = CreateTestItem(NULL);
+ ExtensionMenuItem* item2 = CreateTestItem(NULL);
- int id1 = manager_.AddContextItem(extension1, item1);
+ int id1 = manager_.AddContextItem(item1);
ASSERT_GT(id1, 0);
- int id2 = manager_.AddContextItem(extension1, item2);
+ int id2 = manager_.AddContextItem(item2);
ASSERT_GT(id2, 0);
const ExtensionMenuItem::List* items =
@@ -170,7 +167,7 @@ TEST_F(ExtensionMenuManagerTest, ChangeParent) {
// Now create a third item, initially add it as a child of item1, then move
// it to be a child of item2.
- ExtensionMenuItem* item3 = CreateTestItem(extension1);
+ ExtensionMenuItem* item3 = CreateTestItem(NULL);
int id3 = manager_.AddChildItem(id1, item3);
ASSERT_GT(id3, 0);
@@ -221,9 +218,10 @@ TEST_F(ExtensionMenuManagerTest, ChangeParent) {
ASSERT_EQ(item3, item1->children()[0]);
// Make sure you can't move a node to be a child of another extension's item.
- Extension* extension2 = AddExtension("2222");
- ExtensionMenuItem* item4 = CreateTestItem(extension2);
- int id4 = manager_.AddContextItem(extension2, item4);
+ DictionaryValue properties;
+ properties.SetString(L"extension_id", "4444");
+ ExtensionMenuItem* item4 = CreateTestItem(&properties);
+ int id4 = manager_.AddContextItem(item4);
ASSERT_GT(id4, 0);
ASSERT_FALSE(manager_.ChangeParent(id4, id1));
ASSERT_FALSE(manager_.ChangeParent(id1, id4));
@@ -235,33 +233,46 @@ TEST_F(ExtensionMenuManagerTest, ChangeParent) {
// Tests that we properly remove an extension's menu item when that extension is
// unloaded.
TEST_F(ExtensionMenuManagerTest, ExtensionUnloadRemovesMenuItems) {
+ ScopedTempDir temp_dir;
+ ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
+
NotificationService* notifier = NotificationService::current();
ASSERT_TRUE(notifier != NULL);
// Create a test extension.
- Extension* extension1 = AddExtension("1111");
+ DictionaryValue extension_properties;
+ extension_properties.SetString(extension_manifest_keys::kVersion, "1");
+ extension_properties.SetString(extension_manifest_keys::kName, "Test");
+ Extension extension(temp_dir.path().AppendASCII("extension"));
+ std::string errors;
+ ASSERT_TRUE(extension.InitFromValue(extension_properties,
+ false, // No public key required.
+ &errors)) << errors;
// Create an ExtensionMenuItem and put it into the manager.
- ExtensionMenuItem* item1 = CreateTestItem(extension1);
- ASSERT_EQ(extension1->id(), item1->extension_id());
- int id1 = manager_.AddContextItem(extension1, item1);
+ DictionaryValue item_properties;
+ item_properties.SetString(L"extension_id", extension.id());
+ ExtensionMenuItem* item1 = CreateTestItem(&item_properties);
+ ASSERT_EQ(extension.id(), item1->extension_id());
+ int id1 = manager_.AddContextItem(item1); // Ownership transferred.
ASSERT_GT(id1, 0);
- ASSERT_EQ(1u, manager_.MenuItems(extension1->id())->size());
+ ASSERT_EQ(1u, manager_.MenuItems(extension.id())->size());
// Create a menu item with a different extension id and add it to the manager.
- Extension* extension2 = AddExtension("2222");
- ExtensionMenuItem* item2 = CreateTestItem(extension2);
+ std::string alternate_extension_id = "0000";
+ item_properties.SetString(L"extension_id", alternate_extension_id);
+ ExtensionMenuItem* item2 = CreateTestItem(&item_properties);
ASSERT_NE(item1->extension_id(), item2->extension_id());
- int id2 = manager_.AddContextItem(extension2, item2);
+ int id2 = manager_.AddContextItem(item2); // Ownership transferred.
ASSERT_GT(id2, 0);
// Notify that the extension was unloaded, and make sure the right item is
// gone.
notifier->Notify(NotificationType::EXTENSION_UNLOADED,
Source<Profile>(NULL),
- Details<Extension>(extension1));
- ASSERT_EQ(NULL, manager_.MenuItems(extension1->id()));
- ASSERT_EQ(1u, manager_.MenuItems(extension2->id())->size());
+ Details<Extension>(&extension));
+ ASSERT_EQ(NULL, manager_.MenuItems(extension.id()));
+ ASSERT_EQ(1u, manager_.MenuItems(alternate_extension_id)->size());
ASSERT_TRUE(manager_.GetItemById(id1) == NULL);
ASSERT_TRUE(manager_.GetItemById(id2) != NULL);
}
@@ -297,34 +308,35 @@ TEST_F(ExtensionMenuManagerTest, RemoveAll) {
// Try removing all items for an extension id that doesn't have any items.
manager_.RemoveAllContextItems("CCCC");
- // Add 2 top-level and one child item for extension 1.
- Extension* extension1 = AddExtension("1111");
- ExtensionMenuItem* item1 = CreateTestItem(extension1);
- ExtensionMenuItem* item2 = CreateTestItem(extension1);
- ExtensionMenuItem* item3 = CreateTestItem(extension1);
- int id1 = manager_.AddContextItem(extension1, item1);
- int id2 = manager_.AddContextItem(extension1, item2);
+ // Add 2 top-level and one child item for extension id AAAA.
+ DictionaryValue properties;
+ properties.SetString(L"extension_id", "AAAA");
+ ExtensionMenuItem* item1 = CreateTestItem(&properties);
+ ExtensionMenuItem* item2 = CreateTestItem(&properties);
+ ExtensionMenuItem* item3 = CreateTestItem(&properties);
+ int id1 = manager_.AddContextItem(item1);
+ int id2 = manager_.AddContextItem(item2);
EXPECT_GT(id1, 0);
EXPECT_GT(id2, 0);
int id3 = manager_.AddChildItem(id1, item3);
EXPECT_GT(id3, 0);
- // Add one top-level item for extension 2.
- Extension* extension2 = AddExtension("2222");
- ExtensionMenuItem* item4 = CreateTestItem(extension2);
- manager_.AddContextItem(extension2, item4);
+ // Add one top-level item for extension id BBBB.
+ properties.SetString(L"extension_id", "BBBB");
+ ExtensionMenuItem* item4 = CreateTestItem(&properties);
+ manager_.AddContextItem(item4);
- EXPECT_EQ(2u, manager_.MenuItems(extension1->id())->size());
- EXPECT_EQ(1u, manager_.MenuItems(extension2->id())->size());
+ EXPECT_EQ(2u, manager_.MenuItems("AAAA")->size());
+ EXPECT_EQ(1u, manager_.MenuItems("BBBB")->size());
- // Remove extension2's item.
- manager_.RemoveAllContextItems(extension2->id());
- EXPECT_EQ(2u, manager_.MenuItems(extension1->id())->size());
- EXPECT_EQ(NULL, manager_.MenuItems(extension2->id()));
+ // Remove the BBBB item.
+ manager_.RemoveAllContextItems("BBBB");
+ EXPECT_EQ(2u, manager_.MenuItems("AAAA")->size());
+ EXPECT_EQ(NULL, manager_.MenuItems("BBBB"));
- // Remove extension1's items.
- manager_.RemoveAllContextItems(extension1->id());
- EXPECT_EQ(NULL, manager_.MenuItems(extension1->id()));
+ // Remove the AAAA items.
+ manager_.RemoveAllContextItems("AAAA");
+ EXPECT_EQ(NULL, manager_.MenuItems("AAAA"));
}
TEST_F(ExtensionMenuManagerTest, ExecuteCommand) {
@@ -343,9 +355,8 @@ TEST_F(ExtensionMenuManagerTest, ExecuteCommand) {
params.selection_text = L"Hello World";
params.is_editable = false;
- Extension* extension = AddExtension("test");
- ExtensionMenuItem* item = CreateTestItem(extension);
- int id = manager_.AddContextItem(extension, item);
+ ExtensionMenuItem* item = CreateTestItem(NULL);
+ int id = manager_.AddContextItem(item); // Ownership transferred.
ASSERT_GT(id, 0);
EXPECT_CALL(profile, GetExtensionMessageService())
diff --git a/chrome/browser/extensions/extension_protocols.cc b/chrome/browser/extensions/extension_protocols.cc
index 83ef404..0a868f4 100644
--- a/chrome/browser/extensions/extension_protocols.cc
+++ b/chrome/browser/extensions/extension_protocols.cc
@@ -85,10 +85,9 @@ static URLRequestJob* CreateExtensionURLRequestJob(URLRequest* request,
return new URLRequestErrorJob(request, net::ERR_ADDRESS_UNREACHABLE);
// chrome-extension://extension-id/resource/path.js
- const std::string& extension_id = request->url().host();
- FilePath directory_path = context->GetPathForExtension(extension_id);
+ FilePath directory_path = context->GetPathForExtension(request->url().host());
if (directory_path.value().empty()) {
- LOG(WARNING) << "Failed to GetPathForExtension: " << extension_id;
+ LOG(WARNING) << "Failed to GetPathForExtension: " << request->url().host();
return NULL;
}
@@ -118,7 +117,7 @@ static URLRequestJob* CreateExtensionURLRequestJob(URLRequest* request,
}
// TODO(tc): Move all of these files into resources.pak so we don't break
// when updating on Linux.
- ExtensionResource resource(extension_id, directory_path,
+ ExtensionResource resource(directory_path,
extension_file_util::ExtensionURLToRelativeFilePath(request->url()));
return new URLRequestFileJob(request,
@@ -135,7 +134,7 @@ static URLRequestJob* CreateUserScriptURLRequestJob(URLRequest* request,
// chrome-user-script:/user-script-name.user.js
FilePath directory_path = context->user_script_dir_path();
- ExtensionResource resource(request->url().host(), directory_path,
+ ExtensionResource resource(directory_path,
extension_file_util::ExtensionURLToRelativeFilePath(request->url()));
return new URLRequestFileJob(request, resource.GetFilePath());
diff --git a/chrome/browser/extensions/extensions_service_unittest.cc b/chrome/browser/extensions/extensions_service_unittest.cc
index 6ce579c..a6bbb3c 100644
--- a/chrome/browser/extensions/extensions_service_unittest.cc
+++ b/chrome/browser/extensions/extensions_service_unittest.cc
@@ -257,7 +257,7 @@ void ExtensionsServiceTestBase::InitializeExtensionsService(
new JsonPrefStore( /* user defined preference values */
pref_file,
ChromeThread::GetMessageLoopProxyForThread(ChromeThread::FILE)),
- NULL /* No suggested preference values */)));
+ NULL /* No suggested preference values */ )));
Profile::RegisterUserPrefs(prefs_.get());
browser::RegisterUserPrefs(prefs_.get());
@@ -663,14 +663,12 @@ TEST_F(ExtensionsServiceTest, LoadAllExtensionsFromDirectorySuccess) {
EXPECT_EQ("https://*.google.com/*",
scripts[0].url_patterns()[2].GetAsString());
EXPECT_EQ(2u, scripts[0].js_scripts().size());
- ExtensionResource resource00(extension->id(),
- scripts[0].js_scripts()[0].extension_root(),
+ ExtensionResource resource00(scripts[0].js_scripts()[0].extension_root(),
scripts[0].js_scripts()[0].relative_path());
FilePath expected_path(extension->path().AppendASCII("script1.js"));
ASSERT_TRUE(file_util::AbsolutePath(&expected_path));
EXPECT_TRUE(resource00.ComparePathWithDefault(expected_path));
- ExtensionResource resource01(extension->id(),
- scripts[0].js_scripts()[1].extension_root(),
+ ExtensionResource resource01(scripts[0].js_scripts()[1].extension_root(),
scripts[0].js_scripts()[1].relative_path());
expected_path = extension->path().AppendASCII("script2.js");
ASSERT_TRUE(file_util::AbsolutePath(&expected_path));
@@ -678,8 +676,7 @@ TEST_F(ExtensionsServiceTest, LoadAllExtensionsFromDirectorySuccess) {
EXPECT_TRUE(extension->plugins().empty());
EXPECT_EQ(1u, scripts[1].url_patterns().size());
EXPECT_EQ("http://*.news.com/*", scripts[1].url_patterns()[0].GetAsString());
- ExtensionResource resource10(extension->id(),
- scripts[1].js_scripts()[0].extension_root(),
+ ExtensionResource resource10(scripts[1].js_scripts()[0].extension_root(),
scripts[1].js_scripts()[0].relative_path());
expected_path =
extension->path().AppendASCII("js_files").AppendASCII("script3.js");
diff --git a/chrome/browser/extensions/file_reader_unittest.cc b/chrome/browser/extensions/file_reader_unittest.cc
index b45afeb..dbe9ceb 100644
--- a/chrome/browser/extensions/file_reader_unittest.cc
+++ b/chrome/browser/extensions/file_reader_unittest.cc
@@ -10,7 +10,6 @@
#include "chrome/browser/chrome_thread.h"
#include "chrome/browser/extensions/file_reader.h"
#include "chrome/common/chrome_paths.h"
-#include "chrome/common/extensions/extension.h"
#include "chrome/common/extensions/extension_resource.h"
#include "testing/gtest/include/gtest/gtest.h"
@@ -52,10 +51,7 @@ class Receiver {
void RunBasicTest(const char* filename) {
FilePath path;
PathService::Get(chrome::DIR_TEST_DATA, &path);
- std::string extension_id;
- Extension::GenerateId("test", &extension_id);
- ExtensionResource resource(extension_id, path,
- FilePath().AppendASCII(filename));
+ ExtensionResource resource(path, FilePath().AppendASCII(filename));
path = path.AppendASCII(filename);
std::string file_contents;
@@ -84,9 +80,7 @@ TEST_F(FileReaderTest, BiggerFile) {
TEST_F(FileReaderTest, NonExistantFile) {
FilePath path;
PathService::Get(chrome::DIR_TEST_DATA, &path);
- std::string extension_id;
- Extension::GenerateId("test", &extension_id);
- ExtensionResource resource(extension_id, path, FilePath(
+ ExtensionResource resource(path, FilePath(
FILE_PATH_LITERAL("file_that_does_not_exist")));
path = path.AppendASCII("file_that_does_not_exist");
diff --git a/chrome/browser/extensions/image_loading_tracker.cc b/chrome/browser/extensions/image_loading_tracker.cc
index 5be22d8..ea174ff 100644
--- a/chrome/browser/extensions/image_loading_tracker.cc
+++ b/chrome/browser/extensions/image_loading_tracker.cc
@@ -14,8 +14,6 @@
#include "third_party/skia/include/core/SkBitmap.h"
#include "webkit/glue/image_decoder.h"
-ImageLoadingTracker::Observer::~Observer() {}
-
////////////////////////////////////////////////////////////////////////////////
// ImageLoadingTracker::ImageLoader
@@ -56,7 +54,7 @@ class ImageLoadingTracker::ImageLoader
std::string file_contents;
FilePath path = resource.GetFilePath();
if (path.empty() || !file_util::ReadFileToString(path, &file_contents)) {
- ReportBack(NULL, resource, gfx::Size(), id);
+ ReportBack(NULL, resource, id);
return;
}
@@ -67,12 +65,10 @@ class ImageLoadingTracker::ImageLoader
scoped_ptr<SkBitmap> decoded(new SkBitmap());
*decoded = decoder.Decode(data, file_contents.length());
if (decoded->empty()) {
- ReportBack(NULL, resource, gfx::Size(), id);
+ ReportBack(NULL, resource, id);
return; // Unable to decode.
}
- gfx::Size original_size(decoded->width(), decoded->height());
-
if (decoded->width() > max_size.width() ||
decoded->height() > max_size.height()) {
// The bitmap is too big, re-sample.
@@ -81,25 +77,25 @@ class ImageLoadingTracker::ImageLoader
max_size.width(), max_size.height());
}
- ReportBack(decoded.release(), resource, original_size, id);
+ ReportBack(decoded.release(), resource, id);
}
void ReportBack(SkBitmap* image, const ExtensionResource& resource,
- const gfx::Size& original_size, int id) {
+ int id) {
DCHECK(ChromeThread::CurrentlyOn(ChromeThread::FILE));
ChromeThread::PostTask(
callback_thread_id_, FROM_HERE,
NewRunnableMethod(this, &ImageLoader::ReportOnUIThread,
- image, resource, original_size, id));
+ image, resource, id));
}
void ReportOnUIThread(SkBitmap* image, ExtensionResource resource,
- const gfx::Size& original_size, int id) {
+ int id) {
DCHECK(!ChromeThread::CurrentlyOn(ChromeThread::FILE));
if (tracker_)
- tracker_->OnImageLoaded(image, resource, original_size, id);
+ tracker_->OnImageLoaded(image, resource, id);
delete image;
}
@@ -142,16 +138,16 @@ void ImageLoadingTracker::LoadImage(Extension* extension,
// back.
int id = next_id_++;
if (resource.relative_path().empty()) {
- OnImageLoaded(NULL, resource, max_size, id);
+ OnImageLoaded(NULL, resource, id);
return;
}
DCHECK(extension->path() == resource.extension_root());
// See if the extension has the image already.
- if (extension->HasCachedImage(resource, max_size)) {
- SkBitmap image = extension->GetCachedImage(resource, max_size);
- OnImageLoaded(&image, resource, max_size, id);
+ if (extension->HasCachedImage(resource)) {
+ SkBitmap image = extension->GetCachedImage(resource);
+ OnImageLoaded(&image, resource, id);
return;
}
@@ -169,12 +165,10 @@ void ImageLoadingTracker::LoadImage(Extension* extension,
void ImageLoadingTracker::OnImageLoaded(
SkBitmap* image,
const ExtensionResource& resource,
- const gfx::Size& original_size,
int id) {
LoadMap::iterator i = load_map_.find(id);
if (i != load_map_.end()) {
- i->second->SetCachedImage(resource, image ? *image : SkBitmap(),
- original_size);
+ i->second->SetCachedImage(resource, image ? *image : SkBitmap());
load_map_.erase(i);
}
diff --git a/chrome/browser/extensions/image_loading_tracker.h b/chrome/browser/extensions/image_loading_tracker.h
index e32166a..653a9a9 100644
--- a/chrome/browser/extensions/image_loading_tracker.h
+++ b/chrome/browser/extensions/image_loading_tracker.h
@@ -43,8 +43,6 @@ class ImageLoadingTracker : public NotificationObserver {
class Observer {
public:
- virtual ~Observer();
-
// Will be called when the image with the given index has loaded.
// The |image| is owned by the tracker, so the observer should make a copy
// if they need to access it after this call. |image| can be null if valid
@@ -74,11 +72,10 @@ class ImageLoadingTracker : public NotificationObserver {
// When an image has finished loaded and been resized on the file thread, it
// is posted back to this method on the original thread. This method then
// calls the observer's OnImageLoaded and deletes the ImageLoadingTracker if
- // it was the last image in the list. The |original_size| should be the size
- // of the image before any resizing was done.
+ // it was the last image in the list.
// |image| may be null if the file failed to decode.
void OnImageLoaded(SkBitmap* image, const ExtensionResource& resource,
- const gfx::Size& original_size, int id);
+ int id);
// NotificationObserver method. If an extension is uninstalled while we're
// waiting for the image we remove the entry from load_map_.
diff --git a/chrome/browser/extensions/image_loading_tracker_unittest.cc b/chrome/browser/extensions/image_loading_tracker_unittest.cc
index 974ce54..4659496 100644
--- a/chrome/browser/extensions/image_loading_tracker_unittest.cc
+++ b/chrome/browser/extensions/image_loading_tracker_unittest.cc
@@ -102,12 +102,11 @@ TEST_F(ImageLoadingTrackerTest, Cache) {
ExtensionResource image_resource =
extension->GetIconPath(Extension::EXTENSION_ICON_SMALLISH);
- gfx::Size max_size(Extension::EXTENSION_ICON_SMALLISH,
- Extension::EXTENSION_ICON_SMALLISH);
ImageLoadingTracker loader(static_cast<ImageLoadingTracker::Observer*>(this));
loader.LoadImage(extension.get(),
image_resource,
- max_size,
+ gfx::Size(Extension::EXTENSION_ICON_SMALLISH,
+ Extension::EXTENSION_ICON_SMALLISH),
ImageLoadingTracker::CACHE);
// The image isn't cached, so we should not have received notification.
@@ -122,16 +121,17 @@ TEST_F(ImageLoadingTrackerTest, Cache) {
EXPECT_EQ(Extension::EXTENSION_ICON_SMALLISH, image_.width());
// The image should be cached in the Extension.
- EXPECT_TRUE(extension->HasCachedImage(image_resource, max_size));
+ EXPECT_TRUE(extension->HasCachedImage(image_resource));
// Make sure the image is in the extension.
EXPECT_EQ(Extension::EXTENSION_ICON_SMALLISH,
- extension->GetCachedImage(image_resource, max_size).width());
+ extension->GetCachedImage(image_resource).width());
// Ask the tracker for the image again, this should call us back immediately.
loader.LoadImage(extension.get(),
image_resource,
- max_size,
+ gfx::Size(Extension::EXTENSION_ICON_SMALLISH,
+ Extension::EXTENSION_ICON_SMALLISH),
ImageLoadingTracker::CACHE);
// We should have gotten the image.
EXPECT_EQ(1, image_loaded_count());
diff --git a/chrome/browser/tab_contents/render_view_context_menu.cc b/chrome/browser/tab_contents/render_view_context_menu.cc
index aaed42e..ef4e6d7 100644
--- a/chrome/browser/tab_contents/render_view_context_menu.cc
+++ b/chrome/browser/tab_contents/render_view_context_menu.cc
@@ -41,7 +41,6 @@
#include "chrome/common/chrome_switches.h"
#include "chrome/common/pref_names.h"
#include "chrome/common/url_constants.h"
-#include "gfx/favicon_size.h"
#include "grit/generated_resources.h"
#include "net/base/escape.h"
#include "net/url_request/url_request.h"
@@ -196,7 +195,6 @@ void RenderViewContextMenu::AppendExtensionItems(
menu_model_.AddSubMenu(menu_id, title, submenu);
RecursivelyAppendExtensionItems(submenu_items, submenu, index);
}
- SetExtensionIcon(extension_id);
}
void RenderViewContextMenu::RecursivelyAppendExtensionItems(
@@ -255,20 +253,6 @@ void RenderViewContextMenu::RecursivelyAppendExtensionItems(
}
}
-void RenderViewContextMenu::SetExtensionIcon(const std::string& extension_id) {
- ExtensionsService* service = profile_->GetExtensionsService();
- ExtensionMenuManager* menu_manager = service->menu_manager();
-
- int index = menu_model_.GetItemCount() - 1;
- DCHECK(index >= 0);
-
- const SkBitmap& icon = menu_manager->GetIconForExtension(extension_id);
- DCHECK(icon.width() == kFavIconSize);
- DCHECK(icon.height() == kFavIconSize);
-
- menu_model_.SetIcon(index, icon);
-}
-
void RenderViewContextMenu::AppendAllExtensionItems() {
extension_item_map_.clear();
ExtensionsService* service = profile_->GetExtensionsService();
diff --git a/chrome/browser/tab_contents/render_view_context_menu.h b/chrome/browser/tab_contents/render_view_context_menu.h
index 2874aee..97d0d5c 100644
--- a/chrome/browser/tab_contents/render_view_context_menu.h
+++ b/chrome/browser/tab_contents/render_view_context_menu.h
@@ -91,8 +91,6 @@ class RenderViewContextMenu : public menus::SimpleMenuModel::Delegate {
const std::vector<ExtensionMenuItem*>& items,
menus::SimpleMenuModel* menu_model,
int *index);
- // This will set the icon on the most recently-added item in the menu_model_.
- void SetExtensionIcon(const std::string& extension_id);
// Opens the specified URL string in a new tab. If |in_current_window| is
// false, a new window is created to hold the new tab.