OLD: reland "views: change WrenchMenu to use each model's command ID's when creating MenuItemView's"

chrome/browser/ui/views/bookmarks/bookmark_menu_delegate.cc

Index: chrome/browser/ui/views/bookmarks/bookmark_menu_delegate.cc
diff --git a/chrome/browser/ui/views/bookmarks/bookmark_menu_delegate.cc b/chrome/browser/ui/views/bookmarks/bookmark_menu_delegate.cc
index 908be83db3b6ad00544473287be6f39bd79ff8d7..ad6ebc88a636d1246ea28221c39bb64ae599e226 100644
--- a/chrome/browser/ui/views/bookmarks/bookmark_menu_delegate.cc
+++ b/chrome/browser/ui/views/bookmarks/bookmark_menu_delegate.cc
@@ -41,7 +41,8 @@ static const int kMaxMenuWidth = 400;
 BookmarkMenuDelegate::BookmarkMenuDelegate(Browser* browser,
                                            PageNavigator* navigator,
                                            views::Widget* parent,
-                                           int first_menu_id)
+                                           int first_menu_id,
+                                           int max_menu_id)
     : browser_(browser),
       profile_(browser->profile()),
       page_navigator_(navigator),
@@ -50,6 +51,8 @@ BookmarkMenuDelegate::BookmarkMenuDelegate(Browser* browser,
       for_drop_(false),
       parent_menu_item_(NULL),
       next_menu_id_(first_menu_id),
+      min_menu_id_(first_menu_id),
+      max_menu_id_(max_menu_id),
       real_delegate_(NULL),
       is_mutating_model_(false),
       location_(BOOKMARK_LAUNCH_LOCATION_NONE) {}
@@ -66,6 +69,7 @@ void BookmarkMenuDelegate::Init(views::MenuDelegate* real_delegate,
                                 BookmarkLaunchLocation location) {
   GetBookmarkModel()->AddObserver(this);
   real_delegate_ = real_delegate;
+  location_ = location;
   if (parent) {
     parent_menu_item_ = parent;
     int initial_count = parent->GetSubmenu() ?
@@ -80,8 +84,6 @@ void BookmarkMenuDelegate::Init(views::MenuDelegate* real_delegate,
   } else {
     menu_ = CreateMenu(node, start_child_index, show_options);
   }
-
-  location_ = location;
 }
 
 void BookmarkMenuDelegate::SetPageNavigator(PageNavigator* navigator) {
@@ -432,12 +434,17 @@ void BookmarkMenuDelegate::BuildMenuForPermanentNode(
   if (!node->IsVisible() || node->GetTotalNodeCount() == 1)
     return;  // No children, don't create a menu.
 
+  int id = *next_menu_id;
+  // Don't create the submenu if its menu ID will be outside the range allowed.
+  if (IsOutsideMenuIdRange(id))
+    return;
+  (*next_menu_id)++;
+
   if (!*added_separator) {
     *added_separator = true;
     menu->AppendSeparator();
   }
-  int id = *next_menu_id;
-  (*next_menu_id)++;
+
   ui::ResourceBundle* rb = &ui::ResourceBundle::GetSharedInstance();
   gfx::ImageSkia* folder_icon = rb->GetImageSkiaNamed(IDR_BOOKMARK_BAR_FOLDER);
   MenuItemView* submenu = menu->AppendSubMenuWithIcon(
@@ -456,6 +463,10 @@ void BookmarkMenuDelegate::BuildMenu(const BookmarkNode* parent,
   for (int i = start_child_index; i < parent->child_count(); ++i) {
     const BookmarkNode* node = parent->GetChild(i);
     const int id = *next_menu_id;
+    // Don't create the item if its menu ID will be outside the range allowed.
+    if (IsOutsideMenuIdRange(id))
+      break;
+
     (*next_menu_id)++;
 
     menu_id_to_node_map_[id] = node;
@@ -476,3 +487,7 @@ void BookmarkMenuDelegate::BuildMenu(const BookmarkNode* parent,
     }
   }
 }
+
+bool BookmarkMenuDelegate::IsOutsideMenuIdRange(int menu_id) const {
+  return !(menu_id >= min_menu_id_ && menu_id <= max_menu_id_);

[sky, 2013/10/08 15:55:04]

nit: negatives are harder to read, compare to:
menu_id < min_menu_id || menu_id > max_menu_id_

+}