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

views/controls/menu/menu_item_view.cc

 // Copyright (c) 2011 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 "views/controls/menu/menu_item_view.h"
 
 #include "base/utf_string_conversions.h"
 #include "grit/app_strings.h"
 #include "ui/base/accessibility/accessible_view_state.h"
 #include "ui/base/l10n/l10n_util.h"
@@ skipping 83 matching lines @@
   Init(NULL, 0, SUBMENU, delegate);
 }
 
 MenuItemView::~MenuItemView() {
   // TODO(sky): ownership is bit wrong now. In particular if a nested message
   // loop is running deletion can't be done, otherwise the stack gets
   // thoroughly screwed. The destructor should be made private, and
   // 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) {
   *tooltip = UTF16ToWideHack(tooltip_);
   if (!tooltip->empty())
     return true;
   if (GetType() != SEPARATOR) {
     gfx::Point location(p);
     ConvertPointToScreen(this, &location);
     *tooltip = GetDelegate()->GetTooltipText(command_, location);
@@ skipping 129 matching lines @@
   controller_->Run(parent, NULL, this, bounds, anchor, NULL);
 }
 
 void MenuItemView::Cancel() {
   if (controller_ && !canceled_) {
     canceled_ = true;
     controller_->Cancel(MenuController::EXIT_ALL);
   }
 }
 
+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) {
   SkBitmap icon;
   bool has_icon = false;
   std::wstring label;
   MenuItemView::Type type;
   ui::MenuModel::ItemType menu_type = model->GetTypeAt(index);
   switch (menu_type) {
     case ui::MenuModel::TYPE_COMMAND:
@@ skipping 22 matching lines @@
       break;
   }
 
   return AppendMenuItemImpl(id, label, has_icon ? icon : SkBitmap(), type);
 }
 
 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() {
   if (!submenu_)
     submenu_ = new SubmenuView(this);
   return submenu_;
 }
 
 bool MenuItemView::HasSubmenu() const {
   return (submenu_ != NULL);
@@ skipping 92 matching lines @@
           GetMenuItemByID(id);
       if (result)
         return result;
     }
   }
   return NULL;
 }
 
 void MenuItemView::ChildrenChanged() {
   MenuController* controller = GetMenuController();
-  if (!controller)
-    return;  // We're not showing, nothing to do.
+  if (controller) {
+    // Handles the case where we were empty and are no longer empty.
+    RemoveEmptyMenus();
 
-  // 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() {
   if (!has_children())
     return;
 
   // Child views are laid out right aligned and given the full height. To right
   // align start with the last view and progress to the first.
   for (int i = child_count() - 1, x = width() - item_right_margin_; i >= 0;
        --i) {
@@ skipping 231 matching lines @@
   return width;
 }
 
 string16 MenuItemView::GetAcceleratorText() {
   Accelerator accelerator;
   return (GetDelegate() &&
           GetDelegate()->GetAccelerator(GetCommand(), &accelerator)) ?
       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