diff options
| -rw-r--r-- | chrome/browser/extensions/extension_menu_manager.cc | 4 | ||||
| -rw-r--r-- | chrome/browser/extensions/extension_menu_manager.h | 1 | ||||
| -rw-r--r-- | chrome/browser/extensions/extension_menu_manager_unittest.cc | 19 |
3 files changed, 22 insertions, 2 deletions
diff --git a/chrome/browser/extensions/extension_menu_manager.cc b/chrome/browser/extensions/extension_menu_manager.cc index 307c339..58bd1af 100644 --- a/chrome/browser/extensions/extension_menu_manager.cc +++ b/chrome/browser/extensions/extension_menu_manager.cc @@ -276,8 +276,10 @@ bool ExtensionMenuManager::RemoveContextMenuItem( items_by_id_.erase(*removed_iter); } - if (list.empty()) + if (list.empty()) { + context_items_.erase(extension_id); icon_manager_.RemoveIcon(extension_id); + } return result; } diff --git a/chrome/browser/extensions/extension_menu_manager.h b/chrome/browser/extensions/extension_menu_manager.h index a58c1df..41edb61 100644 --- a/chrome/browser/extensions/extension_menu_manager.h +++ b/chrome/browser/extensions/extension_menu_manager.h @@ -244,6 +244,7 @@ class ExtensionMenuManager : public NotificationObserver { private: FRIEND_TEST_ALL_PREFIXES(ExtensionMenuManagerTest, DeleteParent); + FRIEND_TEST_ALL_PREFIXES(ExtensionMenuManagerTest, RemoveOneByOne); // 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). diff --git a/chrome/browser/extensions/extension_menu_manager_unittest.cc b/chrome/browser/extensions/extension_menu_manager_unittest.cc index 489b5c6..56c95c4 100644 --- a/chrome/browser/extensions/extension_menu_manager_unittest.cc +++ b/chrome/browser/extensions/extension_menu_manager_unittest.cc @@ -211,7 +211,7 @@ TEST_F(ExtensionMenuManagerTest, DeleteParent) { // Now remove item1 and make sure item2 and item3 are gone as well. ASSERT_TRUE(manager_.RemoveContextMenuItem(item1_id)); - ASSERT_EQ(0u, manager_.MenuItems(extension->id())->size()); + ASSERT_EQ(NULL, manager_.MenuItems(extension->id())); ASSERT_EQ(0u, manager_.items_by_id_.size()); ASSERT_EQ(NULL, manager_.GetItemById(item1_id)); ASSERT_EQ(NULL, manager_.GetItemById(item2_id)); @@ -389,6 +389,23 @@ TEST_F(ExtensionMenuManagerTest, RemoveAll) { EXPECT_EQ(NULL, manager_.MenuItems(extension1->id())); } +// Tests that removing all items one-by-one doesn't leave an entry around. +TEST_F(ExtensionMenuManagerTest, RemoveOneByOne) { + // Add 2 test items. + Extension* extension1 = AddExtension("1111"); + ExtensionMenuItem* item1 = CreateTestItem(extension1); + ExtensionMenuItem* item2 = CreateTestItem(extension1); + ASSERT_TRUE(manager_.AddContextItem(extension1, item1)); + ASSERT_TRUE(manager_.AddContextItem(extension1, item2)); + + ASSERT_FALSE(manager_.context_items_.empty()); + + manager_.RemoveContextMenuItem(item1->id()); + manager_.RemoveContextMenuItem(item2->id()); + + ASSERT_TRUE(manager_.context_items_.empty()); +} + TEST_F(ExtensionMenuManagerTest, ExecuteCommand) { MessageLoopForUI message_loop; BrowserThread ui_thread(BrowserThread::UI, &message_loop); |