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

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

the original cl (hhttps://codereview.chromium.org/25667005/) was reverted because of crash in bug 304756, this cl relands it with the fixes.

this is in preparation for dynamic recent tabs model and menu:

* WrenchMenu:
- for each menu item, MenuItemView::GetCommand() used to be separate and different from MenuModel::GetCommandIdAt(model_index); they are the same now.
- different range of command id's are reserved for recent tabs and bookmarks.
- command id's of all items in the wrench menu and all its submenus can't clash, except for separator and IDC_SHOW_HISTORY, the latter of which is in both wrench menu and recent tabs submenu.

* RecentTabsSubMenuModel:
- make all command ids within range, use different ids among device name headers.

* BookmarkMenuDelegate
- makes sure only maximum number of menu items allowed is created.
- add test.

BUG=256750
TEST=wrench menu and all its submenus still work.

[kuan, 2013-10-08 09:53:31]

hi scott,

this relands the cl with fixes for the crash triggered from
BookmarkMenuDelegate.  the new changes are those in
bookmark_menu_delegate.h/cc/test and 2 other files with annotations.

ptal.  thx.

https://codereview.chromium.org/26350003/diff/1/chrome/browser/ui/views/bookm...
File chrome/browser/ui/views/bookmarks/bookmark_menu_controller_views.cc
(right):

https://codereview.chromium.org/26350003/diff/1/chrome/browser/ui/views/bookm...
chrome/browser/ui/views/bookmarks/bookmark_menu_controller_views.cc:38:
kint32max)),
pass in max menu id.

https://codereview.chromium.org/26350003/diff/1/chrome/browser/ui/views/wrenc...
File chrome/browser/ui/views/wrench_menu.cc (right):

https://codereview.chromium.org/26350003/diff/1/chrome/browser/ui/views/wrenc...
chrome/browser/ui/views/wrench_menu.cc:1262:
WrenchMenuModel::kMaxBookmarkCommandId));
pass in max menu id.

[kuan, 2013-10-08 15:02:39]

hi scott, the cl is now ready for review; the new changes (as compared to the cl
b4 it was reverted) are as i've detailed earlier.

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

Two comments, only looked at diffs.

https://codereview.chromium.org/26350003/diff/5001/chrome/browser/ui/views/bo...
File chrome/browser/ui/views/bookmarks/bookmark_menu_delegate.cc (right):

https://codereview.chromium.org/26350003/diff/5001/chrome/browser/ui/views/bo...
chrome/browser/ui/views/bookmarks/bookmark_menu_delegate.cc:492: return
!(menu_id >= min_menu_id_ && menu_id <= max_menu_id_);
nit: negatives are harder to read, compare to:
menu_id < min_menu_id || menu_id > max_menu_id_

https://codereview.chromium.org/26350003/diff/5001/chrome/browser/ui/views/bo...
File chrome/browser/ui/views/bookmarks/bookmark_menu_delegate.h (right):

https://codereview.chromium.org/26350003/diff/5001/chrome/browser/ui/views/bo...
chrome/browser/ui/views/bookmarks/bookmark_menu_delegate.h:202: int
min_menu_id_;
Make both of these (min/max) const.