8 files changed, 141 insertions, 126 deletions

diff --git a/chrome/browser/cocoa/bookmark_bar_controller.h b/chrome/browser/cocoa/bookmark_bar_controller.h
index dfc51d4..a24f318 100644
--- a/chrome/browser/cocoa/bookmark_bar_controller.h
+++ b/chrome/browser/cocoa/bookmark_bar_controller.h
@@ -28,7 +28,6 @@ class BookmarkModel;
 class BookmarkNode;
 class Browser;
 class GURL;
-@class MenuButton;
 class PrefService;
 class Profile;
 class TabContents;
@@ -178,7 +177,7 @@ willAnimateFromState:(bookmarks::VisualState)oldState
   BOOL watchingForClickOutside_;  // Are watching for a "click outside"?
 
   IBOutlet BookmarkBarView* buttonView_;
-  IBOutlet MenuButton* offTheSideButton_;  // aka the chevron
+  IBOutlet BookmarkButton* offTheSideButton_;  // aka the chevron.
   IBOutlet NSMenu* buttonContextMenu_;
 
   // "Other bookmarks" button on the right side.
@@ -264,6 +263,8 @@ willAnimateFromState:(bookmarks::VisualState)oldState
 // Open a normal bookmark or folder from a button, ...
 - (IBAction)openBookmark:(id)sender;
 - (IBAction)openBookmarkFolderFromButton:(id)sender;
+// From the "off the side" button, ...
+- (IBAction)openOffTheSideFolderFromButton:(id)sender;
 // From a context menu over the button, ...
 - (IBAction)openBookmarkInNewForegroundTab:(id)sender;
 - (IBAction)openBookmarkInNewWindow:(id)sender;
diff --git a/chrome/browser/cocoa/bookmark_bar_controller.mm b/chrome/browser/cocoa/bookmark_bar_controller.mm
index dfffd10..8a6d035 100644
--- a/chrome/browser/cocoa/bookmark_bar_controller.mm
+++ b/chrome/browser/cocoa/bookmark_bar_controller.mm
@@ -295,11 +295,12 @@ const NSTimeInterval kBookmarkBarAnimationDuration = 0.12;
   // expects.  We will resize ourselves open later if needed.
   [[self view] setFrame:NSMakeRect(0, 0, initialWidth_, 0)];
 
+  // Complete init of the "off the side" button, as much as we can.
+  [offTheSideButton_ setDraggable:NO];
+
   // We are enabled by default.
   barIsEnabled_ = YES;
 
-  DCHECK([offTheSideButton_ attachedMenu]);
-
   // To make life happier when the bookmark bar is floating, the chevron is a
   // child of the button view.
   [offTheSideButton_ removeFromSuperview];
@@ -376,10 +377,12 @@ const NSTimeInterval kBookmarkBarAnimationDuration = 0.12;
   }
 }
 
-// Check if we should enable or disable the off-the-side chevron.
-// Assumes that buttons which don't fit in the parent view are removed
-// from it.
-- (void)showOrHideOffTheSideButton {
+// 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:bookmarkBarDisplayedButtons_];
+  [[offTheSideButton_ cell]
+    setBookmarkNode:bookmarkModel_->GetBookmarkBarNode()];
   int bookmarkChildren = bookmarkModel_->GetBookmarkBarNode()->GetChildCount();
   if (bookmarkChildren > bookmarkBarDisplayedButtons_) {
     [offTheSideButton_ setHidden:NO];
@@ -406,7 +409,7 @@ const NSTimeInterval kBookmarkBarAnimationDuration = 0.12;
   }
   [self positionOffTheSideButton];
   [self addButtonsToView];
-  [self showOrHideOffTheSideButton];
+  [self configureOffTheSideButtonContentsAndVisibility];
   [self centerNoItemsLabel];
 }
 
@@ -745,11 +748,13 @@ const NSTimeInterval kBookmarkBarAnimationDuration = 0.12;
     const BookmarkNode* afterNode =
         [buttonToTheRightOfDraggedButton bookmarkNode];
     DCHECK(afterNode);
-    return afterNode->GetParent()->IndexOfChild(afterNode);
+    int index = afterNode->GetParent()->IndexOfChild(afterNode);
+    // Make sure we don't get confused by buttons which aren't visible.
+    return std::min(index, bookmarkBarDisplayedButtons_);
   }
 
   // If nothing is to my right I am at the end!
-  return [buttons_ count];
+  return bookmarkBarDisplayedButtons_;
 }
 
 - (BOOL)dragButton:(BookmarkButton*)sourceButton
@@ -777,10 +782,14 @@ const NSTimeInterval kBookmarkBarAnimationDuration = 0.12;
     destIndex = [self indexForDragOfButton:sourceButton toPoint:point];
   }
 
-  if (copy)
-    bookmarkModel_->Copy(sourceNode, destParent, destIndex);
-  else
-    bookmarkModel_->Move(sourceNode, destParent, destIndex);
+  // Be sure we don't try and drop a folder into itself.
+  if (sourceNode != destParent) {
+    if (copy) {
+      bookmarkModel_->Copy(sourceNode, destParent, destIndex);
+    } else {
+      bookmarkModel_->Move(sourceNode, destParent, destIndex);
+    }
+  }
 
   [self closeAllBookmarkFolders];  // For a hover open, if needed.
 
@@ -805,7 +814,8 @@ static BOOL ValueInRangeInclusive(CGFloat low, CGFloat value, CGFloat high) {
                                     fromArray:(NSArray*)array {
   for (BookmarkButton* button in array) {
     // Break early if we've gone too far.
-    if (NSMinX([button frame]) > point.x)
+    if ((NSMinX([button frame]) > point.x) ||
+        (![button superview]))
       return nil;
     // Careful -- this only applies to the bar with horiz buttons.
     // Intentionally NOT using NSPointInRect() so that scrolling into
@@ -814,7 +824,7 @@ static BOOL ValueInRangeInclusive(CGFloat low, CGFloat value, CGFloat high) {
                               point.x,
                               NSMaxX([button frame]))) {
       // Over a button but let's be a little more specific (make sure
-      // it's over the middle half, not just over it.)
+      // it's over the middle half, not just over it).
       NSRect frame = [button frame];
       NSRect middleHalfOfButton = NSInsetRect(frame, frame.size.width / 4, 0);
       if (ValueInRangeInclusive(NSMinX(middleHalfOfButton),
@@ -841,10 +851,13 @@ static BOOL ValueInRangeInclusive(CGFloat low, CGFloat value, CGFloat high) {
 - (BookmarkButton*)buttonForDroppingOnAtPoint:(NSPoint)point {
   BookmarkButton* button = [self buttonForDroppingOnAtPoint:point
                                                   fromArray:buttons_.get()];
-  // One more chance -- try "Other Bookmarks".
+  // One more chance -- try "Other Bookmarks" and "off the side" (if visible).
   // This is different than BookmarkBarFolderController.
   if (!button) {
-    NSArray* array = [NSArray arrayWithObject:otherBookmarksButton_];
+    NSMutableArray* array = [NSMutableArray array];
+    if (![self offTheSideButtonIsHidden])
+      [array addObject:offTheSideButton_];
+    [array addObject:otherBookmarksButton_];
     button = [self buttonForDroppingOnAtPoint:point
                                     fromArray:array];
   }
@@ -911,7 +924,7 @@ static BOOL ValueInRangeInclusive(CGFloat low, CGFloat value, CGFloat high) {
                                toPoint:(NSPoint)point {
   CGFloat x = 0;
   int destIndex = [self indexForDragOfButton:sourceButton toPoint:point];
-  int numButtons = static_cast<int>([buttons_ count]);
+  int numButtons = bookmarkBarDisplayedButtons_;
 
   // If it's a drop strictly between existing buttons ...
   if (destIndex >= 0 && destIndex < numButtons) {
@@ -1105,6 +1118,16 @@ static BOOL ValueInRangeInclusive(CGFloat low, CGFloat value, CGFloat high) {
   [folderTarget_ openBookmarkFolderFromButton:sender];
 }
 
+// The button that sends this one is special; the "off the side"
+// button (chevron) opens like a folder button but isn't exactly a
+// parent folder.
+- (IBAction)openOffTheSideFolderFromButton:(id)sender {
+  DCHECK([sender isKindOfClass:[BookmarkButton class]]);
+  DCHECK([[sender cell] isKindOfClass:[BookmarkButtonCell class]]);
+  [[sender cell] setStartingChildIndex:bookmarkBarDisplayedButtons_];
+  [folderTarget_ openBookmarkFolderFromButton:sender];
+}
+
 // Recursively add the given bookmark node and all its children to
 // menu, one menu item per node.
 - (void)addNode:(const BookmarkNode*)child toMenu:(NSMenu*)menu {
@@ -1185,45 +1208,6 @@ static BOOL ValueInRangeInclusive(CGFloat low, CGFloat value, CGFloat high) {
   [self watchForClickOutside:YES];
 }
 
-// Rebuild the off-the-side menu.
-- (void)buildOffTheSideMenuIfNeeded {
-  NSMenu* menu = [self offTheSideMenu];
-  DCHECK(menu);
-
-  // Only rebuild if needed.  We determine we need a rebuild when the
-  // bookmark bar is cleared of buttons.
-  if (!needToRebuildOffTheSideMenu_)
-    return;
-  needToRebuildOffTheSideMenu_ = YES;
-
-  // Remove old menu items (backwards order is as good as any); leave the
-  // blank one at position 0 (see menu_button.h).
-  for (NSInteger i = [menu numberOfItems] - 1; i >= 1 ; i--)
-    [menu removeItemAtIndex:i];
-
-  // Add items corresponding to buttons which aren't displayed.  Since
-  // we build the buttons in the same order as the bookmark bar child
-  // count, we have a clear hint as to where to begin.
-  const BookmarkNode* barNode = bookmarkModel_->GetBookmarkBarNode();
-  for (int i = bookmarkBarDisplayedButtons_;
-       i < barNode->GetChildCount(); i++) {
-    const BookmarkNode* child = barNode->GetChild(i);
-    [self addNode:child toMenu:menu];
-  }
-}
-
-// Get the off-the-side menu.
-- (NSMenu*)offTheSideMenu {
-  return [offTheSideButton_ attachedMenu];
-}
-
-// Called by any menus which have set us as their delegate (right now just the
-// off-the-side menu).  This is the trigger for a delayed rebuild.
-- (void)menuNeedsUpdate:(NSMenu*)menu {
-  if (menu == [self offTheSideMenu])
-    [self buildOffTheSideMenuIfNeeded];
-}
-
 // As a convention we set the menu's delegate to be the button's cell
 // so we can easily obtain bookmark info.  Convention applied in
 // -[BookmarkButtonCell menu].
@@ -1507,7 +1491,7 @@ static BOOL ValueInRangeInclusive(CGFloat low, CGFloat value, CGFloat high) {
   }
   // We may have just crossed a threshold to enable the off-the-side
   // button.
-  [self showOrHideOffTheSideButton];
+  [self configureOffTheSideButtonContentsAndVisibility];
 }
 
 - (IBAction)openBookmarkMenuItem:(id)sender {
@@ -1643,7 +1627,7 @@ static BOOL ValueInRangeInclusive(CGFloat low, CGFloat value, CGFloat high) {
   [self positionOffTheSideButton];
   [self addNonBookmarkButtonsToView];
   [self addButtonsToView];
-  [self showOrHideOffTheSideButton];
+  [self configureOffTheSideButtonContentsAndVisibility];
   [self setNodeForBarMenu];
 }
 
diff --git a/chrome/browser/cocoa/bookmark_bar_controller_unittest.mm b/chrome/browser/cocoa/bookmark_bar_controller_unittest.mm
index da9cfc3..7b35f97e 100644
--- a/chrome/browser/cocoa/bookmark_bar_controller_unittest.mm
+++ b/chrome/browser/cocoa/bookmark_bar_controller_unittest.mm
@@ -830,51 +830,6 @@ TEST_F(BookmarkBarControllerTest, CommandClickOnFolder) {
   NSApp = oldApp;
 }
 
-TEST_F(BookmarkBarControllerTest, TestBuildOffTheSideMenu) {
-  BookmarkModel* model = helper_.profile()->GetBookmarkModel();
-  NSMenu* menu = [bar_ offTheSideMenu];
-  ASSERT_TRUE(menu);
-
-  // The bookmark bar should start out with nothing.
-  EXPECT_EQ(0U, [[bar_ buttons] count]);
-
-  // Make sure things work when there's nothing. Note that there should always
-  // be a blank first menu item.
-  [bar_ buildOffTheSideMenuIfNeeded];
-  EXPECT_EQ(1, [menu numberOfItems]);
-
-  // We add lots of bookmarks. At first, we expect nothing to be added to the
-  // off-the-side menu. But once they start getting added, we expect the
-  // remaining ones to be added too. We expect a reasonably substantial number
-  // of items to be added by the end.
-  int num_off_the_side = 0;
-  for (int i = 0; i < 50; i++) {
-    const BookmarkNode* parent = model->GetBookmarkBarNode();
-    model->AddURL(parent, parent->GetChildCount(),
-                  L"very wide title",
-                  GURL("http://www.foobar.com/"));
-    [bar_ buildOffTheSideMenuIfNeeded];
-
-    if (num_off_the_side) {
-      num_off_the_side++;
-      EXPECT_EQ(1 + num_off_the_side, [menu numberOfItems]);
-    } else {
-      EXPECT_TRUE([menu numberOfItems] == 1 || [menu numberOfItems] == 2);
-      if ([menu numberOfItems] == 2)
-        num_off_the_side++;
-    }
-  }
-  EXPECT_GE(num_off_the_side, 20);
-
-  // Reset, and check that the built menu is "empty" again.
-  const BookmarkNode* parent = model->GetBookmarkBarNode();
-  while (parent->GetChildCount())
-    model->Remove(parent, 0);
-  EXPECT_EQ(0U, [[bar_ buttons] count]);
-  [bar_ buildOffTheSideMenuIfNeeded];
-  EXPECT_EQ(1, [menu numberOfItems]);
-}
-
 TEST_F(BookmarkBarControllerTest, DisplaysHelpMessageOnEmpty) {
   BookmarkModel* model = helper_.profile()->GetBookmarkModel();
   [bar_ loaded:model];
@@ -1224,6 +1179,45 @@ TEST_F(BookmarkBarControllerTest, TestFolders) {
   [bar_ closeBookmarkFolder:nil];
 }
 
+// Make sure the "off the side" folder looks like a bookmark folder
+// but only contains "off the side" items.
+TEST_F(BookmarkBarControllerTest, OffTheSideFolder) {
+
+  // It starts hidden.
+  EXPECT_TRUE([bar_ offTheSideButtonIsHidden]);
+
+  // Create some buttons.
+  BookmarkModel* model = helper_.profile()->GetBookmarkModel();
+  const BookmarkNode* parent = model->GetBookmarkBarNode();
+  for (int x = 0; x < 30; x++) {
+    model->AddURL(parent, parent->GetChildCount(),
+                  L"medium-size-title", GURL("http://framma-lamma.com"));
+  }
+
+  // Should no longer be hidden.
+  EXPECT_FALSE([bar_ offTheSideButtonIsHidden]);
+
+  // Open it; make sure we have a folder controller.
+  EXPECT_FALSE([bar_ folderController]);
+  [bar_ openOffTheSideFolderFromButton:[bar_ offTheSideButton]];
+  BookmarkBarFolderController* bbfc = [bar_ folderController];
+  EXPECT_TRUE(bbfc);
+
+  // Confirm the contents are only buttons which fell off the side by
+  // making sure that none of the nodes in the off-the-side folder are
+  // found in bar buttons.  Be careful since not all the bar buttons
+  // may be currently displayed.
+  NSArray* folderButtons = [bbfc buttons];
+  NSArray* barButtons = [bar_ buttons];
+  for (BookmarkButton* folderButton in folderButtons) {
+    for (BookmarkButton* barButton in barButtons) {
+      if ([barButton superview]) {
+        EXPECT_NE([folderButton bookmarkNode], [barButton bookmarkNode]);
+      }
+    }
+  }
+}
+
 TEST_F(BookmarkBarControllerTest, ClickOutsideCheck) {
   NSEvent* event = test_event_utils::MakeMouseEvent(NSMouseMoved, 0);
   EXPECT_FALSE([bar_ isEventAClickOutside:event]);
diff --git a/chrome/browser/cocoa/bookmark_bar_folder_controller.mm b/chrome/browser/cocoa/bookmark_bar_folder_controller.mm
index c334ff8..d3ac6a3 100644
--- a/chrome/browser/cocoa/bookmark_bar_folder_controller.mm
+++ b/chrome/browser/cocoa/bookmark_bar_folder_controller.mm
@@ -220,9 +220,10 @@ const CGFloat kBookmarkBarFolderScrollAmount =
   NSPoint newWindowTopLeft = [self windowTopLeft];
   const BookmarkNode* node = [parentButton_ bookmarkNode];
   DCHECK(node);
-  int buttons = node->GetChildCount();
-  if (buttons == 0)
-    buttons = 1;  // the "empty" button
+  int startingIndex = [[parentButton_ cell] startingChildIndex];
+  DCHECK_LE(startingIndex, node->GetChildCount());
+  // Must have at least 1 button (for "empty")
+  int buttons = std::max(node->GetChildCount() - startingIndex, 1);
 
   int height = buttons * bookmarks::kBookmarkButtonHeight;
 
@@ -258,7 +259,9 @@ const CGFloat kBookmarkBarFolderScrollAmount =
     [buttons_ addObject:button];
     [mainView_ addSubview:button];
   } else {
-    for (int i = 0; i < node->GetChildCount(); i++) {
+    for (int i = startingIndex;
+         i < node->GetChildCount();
+         i++) {
       const BookmarkNode* child = node->GetChild(i);
       BookmarkButton* button = [self makeButtonForNode:child
                                                  frame:buttonsOuterFrame];
@@ -648,7 +651,9 @@ static BOOL ValueInRangeInclusive(CGFloat low, CGFloat value, CGFloat high) {
   const BookmarkNode* beforeNode = [buttonToTheTopOfDraggedButton
                                        bookmarkNode];
   DCHECK(beforeNode);
-  return beforeNode->GetParent()->IndexOfChild(beforeNode) + 1;
+  // Be careful if the number of buttons != number of nodes.
+  return ((beforeNode->GetParent()->IndexOfChild(beforeNode) + 1) -
+          [[parentButton_ cell] startingChildIndex]);
 }
 
 - (BookmarkModel*)bookmarkModel {
@@ -680,6 +685,8 @@ static BOOL ValueInRangeInclusive(CGFloat low, CGFloat value, CGFloat high) {
     // Else we're dropping somewhere in the folder, so find the right spot.
     destParent = [parentButton_ bookmarkNode];
     destIndex = [self indexForDragOfButton:sourceButton toPoint:point];
+    // Be careful if the number of buttons != number of nodes.
+    destIndex += [[parentButton_ cell] startingChildIndex];
   }
 
   if (copy)
diff --git a/chrome/browser/cocoa/bookmark_button.h b/chrome/browser/cocoa/bookmark_button.h
index 5050315..10c1bc4 100644
--- a/chrome/browser/cocoa/bookmark_button.h
+++ b/chrome/browser/cocoa/bookmark_button.h
@@ -122,7 +122,7 @@ class ThemeProvider;
 // Class for bookmark bar buttons that can be drag sources.
 @interface BookmarkButton : DraggableButton {
  @private
-  NSObject<BookmarkButtonDelegate>* delegate_;  // weak like all delegates
+  IBOutlet NSObject<BookmarkButtonDelegate>* delegate_;  // Weak.
 
   // Saved pointer to the BWC for the browser window that contains this button.
   // Used to lock and release bar visibility during a drag.  The pointer is
diff --git a/chrome/browser/cocoa/bookmark_button_cell.h b/chrome/browser/cocoa/bookmark_button_cell.h
index 8a0c13d..a2f072a 100644
--- a/chrome/browser/cocoa/bookmark_button_cell.h
+++ b/chrome/browser/cocoa/bookmark_button_cell.h
@@ -17,9 +17,13 @@ class BookmarkNode;
 @interface BookmarkButtonCell : GradientButtonCell<NSMenuDelegate> {
  @private
   BOOL empty_;  // is this an "empty" button placeholder button cell?
+
+  // Starting index of bookmarkFolder children that we care to use.
+  int startingChildIndex_;
 }
 
 @property (readwrite, assign) const BookmarkNode* bookmarkNode;
+@property (readwrite, assign) int startingChildIndex;
 
 - (id)initTextCell:(NSString*)string;  // Designated initializer
 - (BOOL)empty;  // returns YES if empty.
diff --git a/chrome/browser/cocoa/bookmark_button_cell.mm b/chrome/browser/cocoa/bookmark_button_cell.mm
index e7f7656..7125a5c 100644
--- a/chrome/browser/cocoa/bookmark_button_cell.mm
+++ b/chrome/browser/cocoa/bookmark_button_cell.mm
@@ -7,28 +7,46 @@
 #import "chrome/browser/cocoa/bookmark_button_cell.h"
 #import "chrome/browser/cocoa/bookmark_menu.h"
 
+
+@interface BookmarkButtonCell(Private)
+- (void)configureBookmarkButtonCell;
+@end
+
+
 @implementation BookmarkButtonCell
 
+@synthesize startingChildIndex = startingChildIndex_;
+
 - (id)initTextCell:(NSString*)string {
   if ((self = [super initTextCell:string])) {
-    [self setButtonType:NSMomentaryPushInButton];
-    [self setBezelStyle:NSShadowlessSquareBezelStyle];
-    [self setShowsBorderOnlyWhileMouseInside:YES];
-    [self setControlSize:NSSmallControlSize];
-    [self setAlignment:NSLeftTextAlignment];
-    [self setFont:[NSFont systemFontOfSize:[NSFont smallSystemFontSize]]];
-    [self setWraps:NO];
-    // NSLineBreakByTruncatingMiddle seems more common on OSX but let's
-    // try to match Windows for a bit to see what happens.
-    [self setLineBreakMode:NSLineBreakByTruncatingTail];
-
-    // Theming doesn't work for bookmark buttons yet (text chucked).
-    [super setShouldTheme:NO];
-
+    [self configureBookmarkButtonCell];
   }
   return self;
 }
 
+// Used by the off-the-side menu, the only case where a
+// BookmarkButtonCell is loaded from a nib.
+- (void)awakeFromNib {
+  [self configureBookmarkButtonCell];
+}
+
+// Perform all normal init routines specific to the BookmarkButtonCell.
+- (void)configureBookmarkButtonCell {
+  [self setButtonType:NSMomentaryPushInButton];
+  [self setBezelStyle:NSShadowlessSquareBezelStyle];
+  [self setShowsBorderOnlyWhileMouseInside:YES];
+  [self setControlSize:NSSmallControlSize];
+  [self setAlignment:NSLeftTextAlignment];
+  [self setFont:[NSFont systemFontOfSize:[NSFont smallSystemFontSize]]];
+  [self setWraps:NO];
+  // NSLineBreakByTruncatingMiddle seems more common on OSX but let's
+  // try to match Windows for a bit to see what happens.
+  [self setLineBreakMode:NSLineBreakByTruncatingTail];
+
+  // Theming doesn't work for bookmark buttons yet (cell text is chucked).
+  [super setShouldTheme:NO];
+}
+
 - (BOOL)empty {
   return empty_;
 }
diff --git a/chrome/browser/cocoa/bookmark_button_cell_unittest.mm b/chrome/browser/cocoa/bookmark_button_cell_unittest.mm
index cb467f5..63c043b 100644
--- a/chrome/browser/cocoa/bookmark_button_cell_unittest.mm
+++ b/chrome/browser/cocoa/bookmark_button_cell_unittest.mm
@@ -111,4 +111,11 @@ TEST_F(BookmarkButtonCellTest, BookmarkMouseForwarding) {
   EXPECT_EQ(button.get()->exits_, 3);
 }
 
+// Confirms a cell created in a nib is initialized properly
+TEST_F(BookmarkButtonCellTest, Awake) {
+  scoped_nsobject<BookmarkButtonCell> cell([[BookmarkButtonCell alloc] init]);
+  [cell awakeFromNib];
+  EXPECT_EQ(NSLeftTextAlignment, [cell alignment]);
+}
+
 }  // namespace