Makes bookmark menu lazily create menus and removes limits

chrome/browser/ui/views/toolbar/wrench_menu.cc

 // Copyright 2013 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/toolbar/wrench_menu.h"
 
 #include <algorithm>
 #include <cmath>
 #include <set>
 
@@ skipping 73 matching lines @@
 const int kFullscreenPadding = 74;
 // Padding to left and right of the XX% label.
 const int kZoomLabelHorizontalPadding = kHorizontalPadding;
 #else
 const int kFullscreenPadding = 38;
 const int kZoomLabelHorizontalPadding = 2;
 #endif
 
 // Returns true if |command_id| identifies a bookmark menu item.
 bool IsBookmarkCommand(int command_id) {
-  return command_id >= WrenchMenuModel::kMinBookmarkCommandId &&
-      command_id <= WrenchMenuModel::kMaxBookmarkCommandId;
+  return command_id >= IDC_FIRST_BOOKMARK_MENU;

[Peter Kasting, 2015/03/30 21:35:25]

You should probably add a comment to where this value is defined saying no
Wrench menu command IDs may ever be larger.

[sky, 2015/03/31 15:17:39]

Done.

 }
 
 // Returns true if |command_id| identifies a recent tabs menu item.
 bool IsRecentTabsCommand(int command_id) {
   return command_id >= WrenchMenuModel::kMinRecentTabsCommandId &&
       command_id <= WrenchMenuModel::kMaxRecentTabsCommandId;
 }
 
 // Subclass of ImageButton whose preferred size includes the size of the border.
 class FullscreenButton : public ImageButton {
@@ skipping 704 matching lines @@
   FOR_EACH_OBSERVER(WrenchMenuObserver, observer_list_, WrenchMenuDestroyed());
 }
 
 void WrenchMenu::Init(ui::MenuModel* model) {
   DCHECK(!root_);
   root_ = new MenuItemView(this);
   root_->set_has_icons(true);  // We have checks, radios and icons, set this
                                // so we get the taller menu style.
   PopulateMenu(root_, model);
 
-#if !defined(NDEBUG)
-  // Verify that the reserved command ID's for bookmarks menu are not used.
-  for (int i = WrenchMenuModel::kMinBookmarkCommandId;
-       i <= WrenchMenuModel::kMaxBookmarkCommandId; ++i)
-    DCHECK(command_id_to_entry_.find(i) == command_id_to_entry_.end());
-#endif  // !defined(NDEBUG)
-
   int32 types = views::MenuRunner::HAS_MNEMONICS;
   if (for_drop()) {
     // We add NESTED_DRAG since currently the only operation to open the wrench
     // menu for is an extension action drag, which is controlled by the child
     // BrowserActionsContainer view.
     types |= views::MenuRunner::FOR_DROP | views::MenuRunner::NESTED_DRAG;
   }
   menu_runner_.reset(new views::MenuRunner(root_, types));
 }
 
@@ skipping 211 matching lines @@
     return false;
 
   *accelerator = ui::Accelerator(menu_accelerator.key_code(),
                                  menu_accelerator.modifiers());
   return true;
 }
 
 void WrenchMenu::WillShowMenu(MenuItemView* menu) {
   if (menu == bookmark_menu_)
     CreateBookmarkMenu();
+  else if (bookmark_menu_delegate_)
+    bookmark_menu_delegate_.get()->WillShowMenu(menu);

[Peter Kasting, 2015/03/30 21:35:25]

Nit: .get() probably unnecessary

[sky, 2015/03/31 15:17:39]

Done.

 }
 
 void WrenchMenu::WillHideMenu(MenuItemView* menu) {
   // Turns off the fade out animation of the wrench menus if
   // |feedback_menu_item_| or |screenshot_menu_item_| is selected.  This
   // excludes the wrench menu itself from the screenshot.
   if (menu->HasSubmenu() &&
       ((feedback_menu_item_ && feedback_menu_item_->IsSelected()) ||
        (screenshot_menu_item_ && screenshot_menu_item_->IsSelected()))) {
-    // It's okay to just turn off the animation and no to take care the
-    // animation back because the menu widget will be recreated next time
-    // it's opened. See ToolbarView::RunMenu() and Init() of this class.
+    // It's okay to just turn off the animation and not turn it back on because
+    // the menu widget will be recreated next time it's opened. See
+    // ToolbarView::RunMenu() and Init() of this class.
     menu->GetSubmenu()->GetWidget()->
         SetVisibilityChangedAnimationsEnabled(false);
   }
 }
 
 bool WrenchMenu::ShouldCloseOnDragComplete() {
   return false;
 }
 
 void WrenchMenu::BookmarkModelChanged() {
@@ skipping 110 matching lines @@
 
 MenuItemView* WrenchMenu::AddMenuItem(MenuItemView* parent,
                                       int menu_index,
                                       MenuModel* model,
                                       int model_index,
                                       MenuModel::ItemType menu_type) {
   int command_id = model->GetCommandIdAt(model_index);
   DCHECK(command_id > -1 ||
          (command_id == -1 &&
           model->GetTypeAt(model_index) == MenuModel::TYPE_SEPARATOR));
+  DCHECK_LT(command_id, IDC_FIRST_BOOKMARK_MENU);
 
   if (command_id > -1) {  // Don't add separators to |command_id_to_entry_|.
     // All command ID's should be unique except for IDC_SHOW_HISTORY which is
     // in both wrench menu and RecentTabs submenu,
     if (command_id != IDC_SHOW_HISTORY) {
       DCHECK(command_id_to_entry_.find(command_id) ==
              command_id_to_entry_.end())
           << "command ID " << command_id << " already exists!";
     }
     command_id_to_entry_[command_id].first = model;
@@ skipping 26 matching lines @@
 
 void WrenchMenu::CreateBookmarkMenu() {
   if (bookmark_menu_delegate_.get())
     return;  // Already created the menu.
 
   BookmarkModel* model =
       BookmarkModelFactory::GetForProfile(browser_->profile());
   if (!model->loaded())
     return;
 
+  LOG(WARNING) << model->bookmark_bar_node()->GetTotalNodeCount();

[Peter Kasting, 2015/03/30 21:35:25]

Don't commit this

[sky, 2015/03/31 15:17:39]

Done.

+
   model->AddObserver(this);
 
   // TODO(oshima): Replace with views only API.
   views::Widget* parent = views::Widget::GetWidgetForNativeWindow(
       browser_->window()->GetNativeWindow());
   bookmark_menu_delegate_.reset(
-      new BookmarkMenuDelegate(browser_,
-                               browser_,
-                               parent,
-                               WrenchMenuModel::kMinBookmarkCommandId,
-                               WrenchMenuModel::kMaxBookmarkCommandId));
+      new BookmarkMenuDelegate(browser_, browser_, parent));
   bookmark_menu_delegate_->Init(this,
                                 bookmark_menu_,
                                 model->bookmark_bar_node(),
                                 0,
                                 BookmarkMenuDelegate::SHOW_PERMANENT_FOLDERS,
                                 BOOKMARK_LAUNCH_LOCATION_WRENCH_MENU);
 }
 
 int WrenchMenu::ModelIndexFromCommandId(int command_id) const {
   CommandIDToEntry::const_iterator ix = command_id_to_entry_.find(command_id);
   DCHECK(ix != command_id_to_entry_.end());
   return ix->second.second;
 }