Fix broken icons in new context menu items after removing previous items

BUG=59603
TEST=Have an extension add several context menu items. Verify that the context
menu properly shows the extension's icon. Now call chrome.contextMenus.remove
repeatedly to remove all the items. Then add one or more new ones. The icon
should still show up properly, and not revert to the default puzzle piece
icon.


Review URL: http://codereview.chromium.org/4170008

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@64337 0039d316-1c4b-4281-b951-d872f2087c98

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);