[Mac] Fix the off-the-side chevron menu.

BUG=94323
TEST=Have a lot of bookmarks on the bar. Make the window smaller. The chevron menu appears and only shows items not visible on the bar.

Review URL: http://codereview.chromium.org/7827029

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@99423 0039d316-1c4b-4281-b951-d872f2087c98

8 files changed, 116 insertions, 29 deletions

diff --git a/chrome/browser/ui/cocoa/bookmarks/bookmark_bar_controller.mm b/chrome/browser/ui/cocoa/bookmarks/bookmark_bar_controller.mm
index 32792d2..ecf841a 100644
--- a/chrome/browser/ui/cocoa/bookmarks/bookmark_bar_controller.mm
+++ b/chrome/browser/ui/cocoa/bookmarks/bookmark_bar_controller.mm
@@ -537,9 +537,6 @@ void RecordAppLaunch(Profile* profile, GURL url) {
 
   showFolderMenus_ = !showFolderMenus_;
 
-  if (sender == offTheSideButton_)
-    [[sender cell] setStartingChildIndex:displayedButtonCount_];
-
   // Toggle presentation of bar folder menus.
   [folderTarget_ openBookmarkFolderFromButton:sender];
 }
@@ -768,9 +765,8 @@ void RecordAppLaunch(Profile* profile, GURL url) {
 // Configure the off-the-side button (e.g. specify the node range,
 // check if we should enable or disable it, etc).
 - (void)configureOffTheSideButtonContentsAndVisibility {
-  [[offTheSideButton_ cell] setStartingChildIndex:displayedButtonCount_];
   [[offTheSideButton_ cell]
-   setBookmarkNode:bookmarkModel_->bookmark_bar_node()];
+      setBookmarkNode:bookmarkModel_->bookmark_bar_node()];
   int bookmarkChildren = bookmarkModel_->bookmark_bar_node()->child_count();
   if (bookmarkChildren > displayedButtonCount_) {
     [offTheSideButton_ setHidden:NO];
@@ -2326,6 +2322,11 @@ static BOOL ValueInRangeInclusive(CGFloat low, CGFloat value, CGFloat high) {
                                                   bookmarkModel:bookmarkModel_
                                                   barController:self];
   [folderController_ autorelease];
+
+  // If this is for the off-the-side menu, set the display count.
+  if (parentButton == offTheSideButton_)
+    [folderController_ setOffTheSideNodeStartIndex:displayedButtonCount_];
+
   [folderController_ openMenu];
 
   // No longer need to hold the lock; the folderController_ now owns it.
diff --git a/chrome/browser/ui/cocoa/bookmarks/bookmark_bar_folder_controller.h b/chrome/browser/ui/cocoa/bookmarks/bookmark_bar_folder_controller.h
index aab5c64..a26aa49 100644
--- a/chrome/browser/ui/cocoa/bookmarks/bookmark_bar_folder_controller.h
+++ b/chrome/browser/ui/cocoa/bookmarks/bookmark_bar_folder_controller.h
@@ -49,6 +49,11 @@ class BookmarkModel;
 // Closes the menu.
 - (void)closeMenu;
 
+// For the "Off The Side" chevron menu, this sets the index in the bookmark_bar
+// node at which the folder should start showing menu items. Forwarded to the
+// bridge.
+- (void)setOffTheSideNodeStartIndex:(size_t)index;
+
 @end
 
 @interface BookmarkBarFolderController (ExposedForTesting)
diff --git a/chrome/browser/ui/cocoa/bookmarks/bookmark_bar_folder_controller.mm b/chrome/browser/ui/cocoa/bookmarks/bookmark_bar_folder_controller.mm
index 01648bd..342c30a 100644
--- a/chrome/browser/ui/cocoa/bookmarks/bookmark_bar_folder_controller.mm
+++ b/chrome/browser/ui/cocoa/bookmarks/bookmark_bar_folder_controller.mm
@@ -73,6 +73,10 @@
                  inModes:modes];
 }
 
+- (void)setOffTheSideNodeStartIndex:(size_t)index {
+  menuBridge_->set_off_the_side_node_start_index(index);
+}
+
 - (void)bookmarkMenuDidClose:(BookmarkMenuCocoaController*)controller {
   // Inform the bookmark bar that the folder has closed on the next iteration
   // of the event loop. If the menu was closed via a click event on a folder
diff --git a/chrome/browser/ui/cocoa/bookmarks/bookmark_bar_folder_controller_unittest.mm b/chrome/browser/ui/cocoa/bookmarks/bookmark_bar_folder_controller_unittest.mm
index 16c08ad..f496f20 100644
--- a/chrome/browser/ui/cocoa/bookmarks/bookmark_bar_folder_controller_unittest.mm
+++ b/chrome/browser/ui/cocoa/bookmarks/bookmark_bar_folder_controller_unittest.mm
@@ -135,3 +135,54 @@ TEST_F(BookmarkBarFolderControllerTest, NestedFolder) {
   EXPECT_NSEQ(@"subitem 1", [[subitems objectAtIndex:0] title]);
   EXPECT_NSEQ(@"subitem 2", [[subitems objectAtIndex:1] title]);
 }
+
+TEST_F(BookmarkBarFolderControllerTest, OffTheSide) {
+  // Create the model.
+  BookmarkModel* model = GetModel();
+  const BookmarkNode* root = model->bookmark_bar_node();
+
+  model->AddURL(root, root->child_count(), ASCIIToUTF16("item 1"),
+      GURL("http://example.com"));
+  model->AddURL(root, root->child_count(), ASCIIToUTF16("item 2"),
+      GURL("http://www.google.com/"));
+  model->AddURL(root, root->child_count(), ASCIIToUTF16("item 3"),
+      GURL("http://www.chromium.org/"));
+  model->AddURL(root, root->child_count(), ASCIIToUTF16("item 4"),
+      GURL("http://build.chromium.org/"));
+  model->AddURL(root, root->child_count(), ASCIIToUTF16("item 5"),
+      GURL("http://example2.com"));
+
+  // Create the controller and menu.
+  scoped_nsobject<BookmarkBarFolderController> bbfc(CreateController(root));
+  CloseFolderAfterDelay(bbfc, 0.1);
+  [bbfc openMenu];
+
+  NSArray* items = [GetMenu(bbfc) itemArray];
+  ASSERT_EQ(5u, [items count]);
+
+  EXPECT_NSEQ(@"item 1", [[items objectAtIndex:0] title]);
+  EXPECT_NSEQ(@"item 2", [[items objectAtIndex:1] title]);
+  EXPECT_NSEQ(@"item 3", [[items objectAtIndex:2] title]);
+  EXPECT_NSEQ(@"item 4", [[items objectAtIndex:3] title]);
+  EXPECT_NSEQ(@"item 5", [[items objectAtIndex:4] title]);
+
+  [bbfc setOffTheSideNodeStartIndex:1];
+  CloseFolderAfterDelay(bbfc, 0.1);
+  [bbfc openMenu];
+  items = [GetMenu(bbfc) itemArray];
+  ASSERT_EQ(4u, [items count]);
+  EXPECT_NSEQ(@"item 2", [[items objectAtIndex:0] title]);
+
+  [bbfc setOffTheSideNodeStartIndex:4];
+  CloseFolderAfterDelay(bbfc, 0.1);
+  [bbfc openMenu];
+  items = [GetMenu(bbfc) itemArray];
+  ASSERT_EQ(1u, [items count]);
+  EXPECT_NSEQ(@"item 5", [[items objectAtIndex:0] title]);
+
+  [bbfc setOffTheSideNodeStartIndex:0];
+  CloseFolderAfterDelay(bbfc, 0.1);
+  [bbfc openMenu];
+  items = [GetMenu(bbfc) itemArray];
+  EXPECT_EQ(5u, [items count]);
+}
diff --git a/chrome/browser/ui/cocoa/bookmarks/bookmark_button_cell.h b/chrome/browser/ui/cocoa/bookmarks/bookmark_button_cell.h
index b702548..e9cc012 100644
--- a/chrome/browser/ui/cocoa/bookmarks/bookmark_button_cell.h
+++ b/chrome/browser/ui/cocoa/bookmarks/bookmark_button_cell.h
@@ -19,15 +19,11 @@ class BookmarkNode;
  @private
   BOOL empty_;  // is this an "empty" button placeholder button cell?
 
-  // Starting index of bookmarkFolder children that we care to use.
-  int startingChildIndex_;
-
   // Text color for title.
   scoped_nsobject<NSColor> textColor_;
 }
 
 @property(nonatomic, readwrite, assign) const BookmarkNode* bookmarkNode;
-@property(nonatomic, readwrite, assign) int startingChildIndex;
 
 // Create a button cell which draws with a theme.
 + (id)buttonCellForNode:(const BookmarkNode*)node
diff --git a/chrome/browser/ui/cocoa/bookmarks/bookmark_button_cell.mm b/chrome/browser/ui/cocoa/bookmarks/bookmark_button_cell.mm
index be6b3cf..21db378 100644
--- a/chrome/browser/ui/cocoa/bookmarks/bookmark_button_cell.mm
+++ b/chrome/browser/ui/cocoa/bookmarks/bookmark_button_cell.mm
@@ -23,8 +23,6 @@
 
 @implementation BookmarkButtonCell
 
-@synthesize startingChildIndex = startingChildIndex_;
-
 + (id)buttonCellForNode:(const BookmarkNode*)node
             contextMenu:(NSMenu*)contextMenu
                cellText:(NSString*)cellText
diff --git a/chrome/browser/ui/cocoa/bookmarks/bookmark_menu_bridge.h b/chrome/browser/ui/cocoa/bookmarks/bookmark_menu_bridge.h
index a45205e..cda5392 100644
--- a/chrome/browser/ui/cocoa/bookmarks/bookmark_menu_bridge.h
+++ b/chrome/browser/ui/cocoa/bookmarks/bookmark_menu_bridge.h
@@ -78,6 +78,13 @@ class BookmarkMenuBridge : public BookmarkModelObserver,
   // Rebuilds a bookmark menu that's a submenu of another menu.
   void UpdateSubMenu(NSMenu* bookmark_menu);
 
+  // If this bridge is managing a menu for the "Off the Side" chevron button,
+  // this sets the index in the menu of the first node to display.
+  void set_off_the_side_node_start_index(size_t index) {
+    off_the_side_node_start_index_ = index;
+    InvalidateMenu();
+  }
+
   // I wish I had a "friend @class" construct.
   BookmarkModel* GetBookmarkModel();
   Profile* GetProfile();
@@ -107,7 +114,8 @@ class BookmarkMenuBridge : public BookmarkModelObserver,
   // If |add_extra_items| is true, also adds extra menu items at bottom of
   // menu, such as "Open All Bookmarks".
   // TODO(jrg): add a counter to enforce maximum nodes added
-  void AddNodeToMenu(const BookmarkNode* node, NSMenu* menu,
+  void AddNodeToMenu(const BookmarkNode* node,
+                     NSMenu* menu,
                      bool add_extra_items);
 
   // Helper for adding an item to our bookmark menu. An item which has a
@@ -140,12 +148,22 @@ class BookmarkMenuBridge : public BookmarkModelObserver,
  private:
   friend class BookmarkMenuBridgeTest;
 
+  // Performs the actual work for AddNodeToMenu(), keeping count of the
+  // recursion depth.
+  void AddNodeToMenuRecursive(const BookmarkNode* node,
+                              NSMenu* menu,
+                              bool add_extra_items,
+                              int recursion_depth);
+
   // True iff the menu is up-to-date with the actual BookmarkModel.
   bool menu_is_valid_;
 
   // The root node of the menu.
   const BookmarkNode* root_node_;
 
+  // Index from which to start adding children from the model.
+  size_t off_the_side_node_start_index_;
+
   Profile* profile_;  // Weak.
   scoped_nsobject<BookmarkMenuCocoaController> controller_;
 
diff --git a/chrome/browser/ui/cocoa/bookmarks/bookmark_menu_bridge.mm b/chrome/browser/ui/cocoa/bookmarks/bookmark_menu_bridge.mm
index 69fdddb..c3d681e 100644
--- a/chrome/browser/ui/cocoa/bookmarks/bookmark_menu_bridge.mm
+++ b/chrome/browser/ui/cocoa/bookmarks/bookmark_menu_bridge.mm
@@ -26,6 +26,7 @@ BookmarkMenuBridge::BookmarkMenuBridge(Profile* profile,
                                        NSMenu *menu)
     : menu_is_valid_(false),
       root_node_(NULL),
+      off_the_side_node_start_index_(0),
       profile_(profile),
       controller_([[BookmarkMenuCocoaController alloc] initWithBridge:this
                                                               andMenu:menu]) {
@@ -38,6 +39,7 @@ BookmarkMenuBridge::BookmarkMenuBridge(const BookmarkNode* root_node,
                                        NSMenu* menu)
     : menu_is_valid_(false),
       root_node_(root_node),
+      off_the_side_node_start_index_(0),
       profile_(profile),
       controller_([[BookmarkMenuCocoaController alloc] initWithBridge:this
                                                               andMenu:menu]) {
@@ -226,9 +228,16 @@ void BookmarkMenuBridge::AddNodeAsSubmenu(NSMenu* menu,
   AddNodeToMenu(node, other_submenu, add_extra_items);
 }
 
-// TODO(jrg): limit the number of bookmarks in the menubar?
-void BookmarkMenuBridge::AddNodeToMenu(const BookmarkNode* node, NSMenu* menu,
+void BookmarkMenuBridge::AddNodeToMenu(const BookmarkNode* node,
+                                       NSMenu* menu,
                                        bool add_extra_items) {
+  AddNodeToMenuRecursive(node, menu, add_extra_items, 0);
+}
+
+void BookmarkMenuBridge::AddNodeToMenuRecursive(const BookmarkNode* node,
+                                                NSMenu* menu,
+                                                bool add_extra_items,
+                                                int recursion_depth) {
   int child_count = node->child_count();
   if (!child_count) {
     NSString* empty_string = l10n_util::GetNSString(IDS_MENU_EMPTY_SUBMENU);
@@ -236,21 +245,26 @@ void BookmarkMenuBridge::AddNodeToMenu(const BookmarkNode* node, NSMenu* menu,
                                                    action:nil
                                             keyEquivalent:@""] autorelease];
     [menu addItem:item];
-  } else for (int i = 0; i < child_count; i++) {
-    const BookmarkNode* child = node->GetChild(i);
-    NSString* title = [BookmarkMenuCocoaController menuTitleForNode:child];
-    NSMenuItem* item = [[[NSMenuItem alloc] initWithTitle:title
-                                                   action:nil
-                                            keyEquivalent:@""] autorelease];
-    [menu addItem:item];
-    bookmark_nodes_[child] = item;
-    if (child->is_folder()) {
-      [item setImage:folder_image_];
-      NSMenu* submenu = [[[NSMenu alloc] initWithTitle:title] autorelease];
-      [menu setSubmenu:submenu forItem:item];
-      AddNodeToMenu(child, submenu, add_extra_items);  // recursive call
-    } else {
-      ConfigureMenuItem(child, item, false);
+  } else {
+    // If this is the first recursive pass, start adding children from the
+    // start index, which defaults to 0.
+    const int start = recursion_depth == 0 ? off_the_side_node_start_index_ : 0;
+    for (int i = start; i < child_count; i++) {
+      const BookmarkNode* child = node->GetChild(i);
+      NSString* title = [BookmarkMenuCocoaController menuTitleForNode:child];
+      NSMenuItem* item = [[[NSMenuItem alloc] initWithTitle:title
+                                                     action:nil
+                                              keyEquivalent:@""] autorelease];
+      [menu addItem:item];
+      bookmark_nodes_[child] = item;
+      if (child->is_folder()) {
+        [item setImage:folder_image_];
+        NSMenu* submenu = [[[NSMenu alloc] initWithTitle:title] autorelease];
+        [menu setSubmenu:submenu forItem:item];
+        AddNodeToMenu(child, submenu, add_extra_items);  // recursive call
+      } else {
+        ConfigureMenuItem(child, item, false);
+      }
     }
   }