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

 // Copyright (c) 2012 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 "chrome/browser/ui/views/bookmarks/bookmark_menu_delegate.h"
 
 #include "base/prefs/pref_service.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/bookmarks/bookmark_model.h"
 #include "chrome/browser/bookmarks/bookmark_model_factory.h"
@@ skipping 23 matching lines @@
 using content::UserMetricsAction;
 using views::MenuItemView;
 
 // Max width of a menu. There does not appear to be an OS value for this, yet
 // both IE and FF restrict the max width of a menu.
 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),
       parent_(parent),
       menu_(NULL),
       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) {}
 
 BookmarkMenuDelegate::~BookmarkMenuDelegate() {
   GetBookmarkModel()->RemoveObserver(this);
 }
 
 void BookmarkMenuDelegate::Init(views::MenuDelegate* real_delegate,
                                 MenuItemView* parent,
                                 const BookmarkNode* node,
                                 int start_child_index,
                                 ShowOptions show_options,
                                 BookmarkLaunchLocation location) {
   GetBookmarkModel()->AddObserver(this);
   real_delegate_ = real_delegate;
+  location_ = location;
   if (parent) {
     parent_menu_item_ = parent;
     int initial_count = parent->GetSubmenu() ?
         parent->GetSubmenu()->GetMenuItemCount() : 0;
     if ((start_child_index < node->child_count()) &&
         (initial_count > 0)) {
       parent->AppendSeparator();
     }
     BuildMenu(node, start_child_index, parent, &next_menu_id_);
     if (show_options == SHOW_PERMANENT_FOLDERS)
       BuildMenusForPermanentNodes(parent, &next_menu_id_);
   } else {
     menu_ = CreateMenu(node, start_child_index, show_options);
   }
-
-  location_ = location;
 }
 
 void BookmarkMenuDelegate::SetPageNavigator(PageNavigator* navigator) {
   page_navigator_ = navigator;
   if (context_menu_.get())
     context_menu_->SetPageNavigator(navigator);
 }
 
 BookmarkModel* BookmarkMenuDelegate::GetBookmarkModel() {
   return BookmarkModelFactory::GetForProfile(profile_);
@@ skipping 330 matching lines @@
 }
 
 void BookmarkMenuDelegate::BuildMenuForPermanentNode(
     const BookmarkNode* node,
     MenuItemView* menu,
     int* next_menu_id,
     bool* added_separator) {
   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(
       id, node->GetTitle(), *folder_icon);
   BuildMenu(node, 0, submenu, next_menu_id);
   menu_id_to_node_map_[id] = node;
 }
 
 void BookmarkMenuDelegate::BuildMenu(const BookmarkNode* parent,
                                      int start_child_index,
                                      MenuItemView* menu,
                                      int* next_menu_id) {
   node_to_menu_map_[parent] = menu;
   DCHECK(parent->empty() || start_child_index < parent->child_count());
   ui::ResourceBundle* rb = &ui::ResourceBundle::GetSharedInstance();
   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;
     if (node->is_url()) {
       const gfx::Image& image = GetBookmarkModel()->GetFavicon(node);
       const gfx::ImageSkia* icon = image.IsEmpty() ?
           rb->GetImageSkiaNamed(IDR_DEFAULT_FAVICON) : image.ToImageSkia();
       node_to_menu_map_[node] =
           menu->AppendMenuItemWithIcon(id, node->GetTitle(), *icon);
     } else if (node->is_folder()) {
       gfx::ImageSkia* folder_icon =
           rb->GetImageSkiaNamed(IDR_BOOKMARK_BAR_FOLDER);
       MenuItemView* submenu = menu->AppendSubMenuWithIcon(
           id, node->GetTitle(), *folder_icon);
       BuildMenu(node, 0, submenu, next_menu_id);
     } else {
       NOTREACHED();
     }
   }
 }
+
+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_

+}