Add MenuItemView API to add and remove items at a particular index.

views/controls/menu/menu_item_view.cc

Index: views/controls/menu/menu_item_view.cc
diff --git a/views/controls/menu/menu_item_view.cc b/views/controls/menu/menu_item_view.cc
index 4af6da783ad34d467d491f37a71070a275c235fd..40aa2356f251b0599c853c500cd19dbb63a60367 100644
--- a/views/controls/menu/menu_item_view.cc
+++ b/views/controls/menu/menu_item_view.cc
@@ -101,6 +101,7 @@ MenuItemView::~MenuItemView() {
   // MenuController should be the only place handling deletion of the menu.
   // (57890).
   delete submenu_;
+  DeleteRemovedItems();
 }
 
 bool MenuItemView::GetTooltipText(const gfx::Point& p, std::wstring* tooltip) {
@@ -250,6 +251,47 @@ void MenuItemView::Cancel() {
   }
 }
 
+MenuItemView* MenuItemView::AddMenuItemAt(int index,
+                                          int item_id,
+                                          const std::wstring& label,
+                                          const SkBitmap& icon,
+                                          Type type) {
+  DCHECK_LE(0, index);
+  if (!submenu_)
+    CreateSubmenu();
+  DCHECK_GE(submenu_->child_count(), index);
+  if (type == SEPARATOR) {
+    submenu_->AddChildViewAt(new MenuSeparator(), index);
+    return NULL;
+  }
+  MenuItemView* item = new MenuItemView(this, item_id, type);
+  if (label.empty() && GetDelegate())
+    item->SetTitle(GetDelegate()->GetLabel(item_id));
+  else
+    item->SetTitle(label);
+  item->SetIcon(icon);
+  if (type == SUBMENU)
+    item->CreateSubmenu();
+  submenu_->AddChildViewAt(item, index);
+  return item;
+}
+
+void MenuItemView::RemoveMenuItemAt(int index)
+{

[sky, 2011/05/06 16:17:00]

{ on previous line.

[rhashimoto, 2011/05/11 00:44:53]

Done.

+  DCHECK(submenu_);
+  DCHECK_LE(0, index);
+  DCHECK_GT(submenu_->child_count(), index);
+
+  View* item = submenu_->GetChildViewAt(index);
+  DCHECK(item);
+  submenu_->RemoveChildView(item);
+
+  // RemoveChildView() does not delete the item, which is a good thing
+  // in case a submenu is being displayed while items are being removed.
+  // Deletion will be done in DeleteRemovedItems().
+  removed_items_.push_back(item);

[sky, 2011/05/06 16:17:00]

Why do we need to do this? BookmarkMenuController removes menu items. Are you
wanting to use this to remove submenus?

[rhashimoto, 2011/05/11 00:44:53]

The use case is for the ChromiumOS network menu which asynchronously refreshes
the list of available networks of various kinds, some on the top-level menu and
some on a submenu.  There is no anticipated case of adding or removing submenus
while a menu is posted (it is needed when the menu is offline).

When BookmarkMenuController removes items, it uses a similar approach of saving
removed items in a container, calling ChildrenChanged(), and then deleting the
items in the container.  It does this with a method that removes multiple items
at once (BookmarkMenuController::WillRemoveBookmarks), calling ChildrenChanged()
and deleting items internally.

As an alternative to this proposed API on MenuItemView, I could implement a bulk
API like BookmarkMenuController (handling multiple adds/removes at once) or
forego an API completely (have the network code manipulate menu items directly)
if that is preferable.

+}
+
 MenuItemView* MenuItemView::AppendMenuItemFromModel(ui::MenuModel* model,
                                                     int index,
                                                     int id) {
@@ -292,22 +334,8 @@ MenuItemView* MenuItemView::AppendMenuItemImpl(int item_id,
                                                const std::wstring& label,
                                                const SkBitmap& icon,
                                                Type type) {
-  if (!submenu_)
-    CreateSubmenu();
-  if (type == SEPARATOR) {
-    submenu_->AddChildView(new MenuSeparator());
-    return NULL;
-  }
-  MenuItemView* item = new MenuItemView(this, item_id, type);
-  if (label.empty() && GetDelegate())
-    item->SetTitle(GetDelegate()->GetLabel(item_id));
-  else
-    item->SetTitle(label);
-  item->SetIcon(icon);
-  if (type == SUBMENU)
-    item->CreateSubmenu();
-  submenu_->AddChildView(item);
-  return item;
+  const int index = submenu_ ? submenu_->child_count() : 0;
+  return AddMenuItemAt(index, item_id, label, icon, type);
 }
 
 SubmenuView* MenuItemView::CreateSubmenu() {
@@ -420,24 +448,25 @@ MenuItemView* MenuItemView::GetMenuItemByID(int id) {
 
 void MenuItemView::ChildrenChanged() {
   MenuController* controller = GetMenuController();
-  if (!controller)
-    return;  // We're not showing, nothing to do.
-
-  // Handles the case where we were empty and are no longer empty.
-  RemoveEmptyMenus();
+  if (controller) {
+    // Handles the case where we were empty and are no longer empty.
+    RemoveEmptyMenus();
 
-  // Handles the case where we were not empty, but now are.
-  AddEmptyMenus();
+    // Handles the case where we were not empty, but now are.
+    AddEmptyMenus();
 
-  controller->MenuChildrenChanged(this);
+    controller->MenuChildrenChanged(this);
 
-  if (submenu_) {
-    // Force a paint and layout. This handles the case of the top level window's
-    // size remaining the same, resulting in no change to the submenu's size and
-    // no layout.
-    submenu_->Layout();
-    submenu_->SchedulePaint();
+    if (submenu_) {
+      // Force a paint and layout. This handles the case of the top
+      // level window's size remaining the same, resulting in no
+      // change to the submenu's size and no layout.
+      submenu_->Layout();
+      submenu_->SchedulePaint();
+    }
   }
+
+  DeleteRemovedItems();
 }
 
 void MenuItemView::Layout() {
@@ -689,4 +718,10 @@ string16 MenuItemView::GetAcceleratorText() {
       accelerator.GetShortcutText() : string16();
 }
 
+void MenuItemView::DeleteRemovedItems() {
+  for (size_t i = 0; i < removed_items_.size(); ++i)

[sky, 2011/05/06 16:17:00]

STLDeleteElements in each place instead of this method.

[rhashimoto, 2011/05/11 00:44:53]

Done.

+    delete removed_items_[i];
+  removed_items_.clear();
+}
+
 }  // namespace views