Rework how bookmark bar folder menus and submenus are layed out when scrollin...

chrome/browser/ui/cocoa/bookmarks/bookmark_bar_folder_controller.mm

Index: chrome/browser/ui/cocoa/bookmarks/bookmark_bar_folder_controller.mm
===================================================================
--- chrome/browser/ui/cocoa/bookmarks/bookmark_bar_folder_controller.mm	(revision 70424)
+++ chrome/browser/ui/cocoa/bookmarks/bookmark_bar_folder_controller.mm	(working copy)
@@ -4,7 +4,6 @@
 
 #import "chrome/browser/ui/cocoa/bookmarks/bookmark_bar_folder_controller.h"
 
-#include "app/mac/nsimage_cache.h"
 #include "base/mac/mac_util.h"
 #include "base/sys_string_conversions.h"
 #include "chrome/browser/bookmarks/bookmark_model.h"
@@ -15,10 +14,13 @@
 #import "chrome/browser/ui/cocoa/bookmarks/bookmark_bar_folder_button_cell.h"
 #import "chrome/browser/ui/cocoa/bookmarks/bookmark_bar_folder_hover_state.h"
 #import "chrome/browser/ui/cocoa/bookmarks/bookmark_bar_folder_view.h"
+#import "chrome/browser/ui/cocoa/bookmarks/bookmark_bar_folder_window.h"
 #import "chrome/browser/ui/cocoa/bookmarks/bookmark_folder_target.h"
 #import "chrome/browser/ui/cocoa/browser_window_controller.h"
 #import "chrome/browser/ui/cocoa/event_utils.h"
 
+using bookmarks::kBookmarkBarMenuCornerRadius;
+
 namespace {
 
 // Frequency of the scrolling timer in seconds.
@@ -29,17 +31,10 @@
 const CGFloat kBookmarkBarFolderScrollAmount =
     3 * bookmarks::kBookmarkButtonVerticalSpan;
 
-// Amount to scroll for each scroll wheel delta.
+// Amount to scroll for each scroll wheel roll.
 const CGFloat kBookmarkBarFolderScrollWheelAmount =
     1 * bookmarks::kBookmarkButtonVerticalSpan;
 
-// When constraining a scrolling bookmark bar folder window to the
-// screen, shrink the "constrain" by this much vertically.  Currently
-// this is 0.0 to avoid a problem with tracking areas leaving the
-// window, but should probably be 8.0 or something.
-// TODO(jrg): http://crbug.com/36225
-const CGFloat kScrollWindowVerticalMargin = 0.0;
-
 }  // namespace
 
 @interface BookmarkBarFolderController(Private)
@@ -51,21 +46,37 @@
 
 // Determine the best button width (which will be the widest button or the
 // maximum allowable button width, whichever is less) and resize all buttons.
-// Return the new width (so that the window can be adjusted, if necessary).
+// Return the new width so that the window can be adjusted.
 - (CGFloat)adjustButtonWidths;
 
 // Returns the total menu height needed to display |buttonCount| buttons.
 // Does not do any fancy tricks like trimming the height to fit on the screen.
-- (int)windowHeightForButtonCount:(int)buttonCount;
+- (int)menuHeightForButtonCount:(int)buttonCount;
 
-// Adjust the height and horizontal position of the window such that the
-// scroll arrows are shown as needed and the window appears completely
-// on screen.
-- (void)adjustWindowForHeight:(int)windowHeight;
+// Adjust all of the folder menu window components, showing/hiding the
+// scroll up/down arrows, and resizing as necessary for a proper disaplay.
+// In order to reduce window flicker, all layout changes are deferred until
+// the final stap of the adjustment. To accommodate this deferral, window
+// height and width changes needed by callers to this function pass their
+// desired window changes in |windowSize|. When scrolling is to be performed

[John Grabowski, 2011/01/06 23:30:18]

|windowSize| --> |size|
Add a comment saying who should call this and when.

[mrossetti, 2011/01/08 01:45:38]

Done.

+// any scrolling change is given by |scrollDelta|. The ultimate amount of
+// scrolling may be different from |scrollDelta| in order to accommodate
+// changes in the scroller view layout.
+- (void)adjustWindowLeft:(CGFloat)left
+                    size:(NSSize)size
+             scrollingBy:(CGFloat)scrollDelta;
 
-// Show or hide the scroll arrows at the top/bottom of the window.
-- (void)showOrHideScrollArrows;
+// This function is called when buttons are added or removed from the folder
+// menu and result in the folder menu window layout being changed accordingly.
+// |buttonCount| should contain the updated count of menu buttons.
+- (void)adjustWindowForButtonCount:(NSUInteger)buttonCount;

[John Grabowski, 2011/01/06 23:30:18]

State this modifies window size/position.

[mrossetti, 2011/01/08 01:45:38]

Done.

 
+// A helper function which takes the desired amount to scroll, given by
+// |scrollDelta|, and calculates the actual scrolling change to be applied
+// taking into account the layout of the folder menu window and it's
+// scrollability.
+- (CGFloat)determineFinalScrollDelta:(CGFloat)scrollDelta;

[John Grabowski, 2011/01/06 23:30:18]

If scrollable, when would the return value be different than input?  When you've
"finished" scrolling, yes? Give that as an example.

[mrossetti, 2011/01/08 01:45:38]

Done.

+
 // |point| is in the base coordinate system of the destination window;
 // it comes from an id<NSDraggingInfo>. |copy| is YES if a copy is to be
 // made and inserted into the new location while leaving the bookmark in
@@ -131,9 +142,6 @@
     barController_ = barController;  // WEAK
     buttons_.reset([[NSMutableArray alloc] init]);
     folderTarget_.reset([[BookmarkFolderTarget alloc] initWithController:self]);
-    NSImage* image = app::mac::GetCachedImageWithName(@"menu_overflow_up.pdf");
-    DCHECK(image);
-    verticalScrollArrowHeight_ = [image size].height;
     [self configureWindow];
     hoverState_.reset([[BookmarkBarFolderHoverState alloc] init]);
   }
@@ -163,6 +171,15 @@
   [super dealloc];
 }
 
+- (void)awakeFromNib {
+  NSRect windowFrame = [[self window] frame];
+  NSRect buttonViewFrame = [scrollView_ frame];
+  leftPadding_ = buttonViewFrame.origin.x;
+  rightPadding_ =
+      NSWidth(windowFrame) - NSWidth(buttonViewFrame) - leftPadding_;
+  verticalScrollArrowHeight_ = NSHeight([scrollUpArrowView_ frame]);
+}
+
 // Overriden from NSWindowController to call childFolderWillShow: before showing
 // the window.
 - (void)showWindow:(id)sender {
@@ -276,11 +293,6 @@
   return button;
 }
 
-// Exposed for testing.
-- (NSView*)mainView {
-  return mainView_;
-}
-
 - (id)folderTarget {
   return folderTarget_.get();
 }
@@ -354,7 +366,8 @@
             convertBaseToScreen:[[parentButton_ superview]
                                     convertPoint:NSZeroPoint toView:nil]];
     newWindowTopLeft = NSMakePoint(buttonBottomLeftInScreen.x,
-                                   bookmarkBarBottomLeftInScreen.y);
+                                   bookmarkBarBottomLeftInScreen.y +
+                                      bookmarks::kBookmarkBarMenuOffset);
     // Make sure the window is on-screen; if not, push left.  It is
     // intentional that top level folders "push left" slightly
     // different than subfolders.
@@ -367,12 +380,12 @@
   } else {
     // Parent is a folder; grow right/left.
     newWindowTopLeft.x = [self childFolderWindowLeftForWidth:windowWidth];
-    NSPoint top = NSMakePoint(0, (NSMaxY([parentButton_ frame]) +
-                                  bookmarks::kBookmarkVerticalPadding));
-    NSPoint topOfWindow =
-        [[parentButton_ window]
-            convertBaseToScreen:[[parentButton_ superview]
-                                    convertPoint:top toView:nil]];
+    NSPoint topOfWindow = NSMakePoint(0,
+                                      (NSMaxY([parentButton_ frame]) +
+                                          bookmarks::kBookmarkVerticalPadding));
+    topOfWindow = [[parentButton_ window]
+                   convertBaseToScreen:[[parentButton_ superview]
+                                        convertPoint:topOfWindow toView:nil]];
     newWindowTopLeft.y = topOfWindow.y;
   }
   return newWindowTopLeft;
@@ -384,74 +397,199 @@
   [[self window] setLevel:NSPopUpMenuWindowLevel];
 }
 
-- (int)windowHeightForButtonCount:(int)buttonCount {
+- (int)menuHeightForButtonCount:(int)buttonCount {
+  // This does not take into account any padding which may be required at the
+  // top and/or bottom of the window.
   return (buttonCount * bookmarks::kBookmarkButtonVerticalSpan) +
       bookmarks::kBookmarkVerticalPadding;
 }
 
-- (void)adjustWindowForHeight:(int)windowHeight {
-  // Adjust all button widths to be consistent, determine the best size for
-  // the window, and set the window frame.
-  CGFloat windowWidth =
-      [self adjustButtonWidths] +
-      (2 * bookmarks::kBookmarkSubMenuHorizontalPadding);
-  NSPoint newWindowTopLeft = [self windowTopLeftForWidth:windowWidth];
-  NSSize windowSize = NSMakeSize(windowWidth, windowHeight);
-  windowSize = [scrollView_ convertSize:windowSize toView:nil];
+- (void)adjustWindowLeft:(CGFloat)left
+                    size:(NSSize)size
+             scrollingBy:(CGFloat)scrollDelta {
+  // Callers of this function should make adjustments to the vertical
+  // attributes of the folder view only (height, scroll position).
+  // This function will then make appropriate layout adjustments in order
+  // to accommodate screen/dock margins, scroll-up and scroll-down arrow
+  // presentation, etc.
+  // The 4 views whose vertical height and origins may be adjusted
+  // by this function are:
+  //  1) window, 2) visible content view, 3) scroller view, 4) folder view.
+
+  // Note: Yes, this is a quite long function, but breaking it up makes little
+  // sense, complicates the logic, and obscures the overall algorithm.
+
+  // Values which are used to calculate new frames and origins.
   NSWindow* window = [self window];
-  // If the window is already visible then make sure its top remains stable.
-  BOOL windowAlreadyShowing = [window isVisible];
-  CGFloat deltaY = windowHeight - NSHeight([mainView_ frame]);
-  if (windowAlreadyShowing) {
-    NSRect oldFrame = [window frame];
-    newWindowTopLeft.y = oldFrame.origin.y + NSHeight(oldFrame);
+  NSRect oldWindowFrame = [window frame];

[John Grabowski, 2011/01/06 23:30:18]

Maybe create structs/classes to reflect window/frame state.  Then write methods
to compare them.  Might help encapsulate logic and reduce number of variables.

I don't think that will make things more complicated; I think it'll make this
easier to read.

[mrossetti, 2011/01/08 01:45:38]

I refactored this a bit but it doesn't really lend itself to class-ification as
there are so many interdependencies.  Let me know what you think about the new
approach.

+  CGFloat oldWindowY = NSMinY(oldWindowFrame);
+  CGFloat oldWindowTop = NSMaxY(oldWindowFrame);
+  CGFloat deltaWindowHeight = 0.0;
+  CGFloat deltaWindowY = 0.0;
+
+  NSRect oldVisibleFrame = [visibleView_ frame];
+  CGFloat oldVisibleY = oldVisibleFrame.origin.y;
+  CGFloat deltaVisibleHeight = 0.0;
+  CGFloat deltaVisibleY = 0.0;
+
+  NSRect oldScrollerFrame = [scrollView_ frame];
+  CGFloat oldScrollerY = oldScrollerFrame.origin.y;
+  CGFloat deltaScrollerHeight = 0.0;
+  CGFloat deltaScrollerY = 0.0;
+
+  NSPoint oldScrollPoint = [scrollView_ documentVisibleRect].origin;
+  oldScrollPoint.y -= scrollDelta;
+  NSRect folderFrame = [folderView_ frame];
+  CGFloat folderHeight = NSHeight(folderFrame);
+  CGFloat folderY = oldScrollerY + oldVisibleY + oldWindowY - oldScrollPoint.y;
+  CGFloat folderTop = folderY + folderHeight;
+
+  BOOL couldScrollUp = ![scrollUpArrowView_ isHidden];
+  BOOL couldScrollDown = ![scrollDownArrowView_ isHidden];
+  NSScreen* screen = [window screen];
+  NSRect screenFrame =  [screen frame];
+  CGFloat minimumY =
+      NSMinY([screen visibleFrame]) + bookmarks::kScrollWindowVerticalMargin;
+  CGFloat maximumY =
+      NSMaxY(screenFrame) - bookmarks::kScrollWindowVerticalMargin;
+  CGFloat effectiveFolderY = folderY;
+  if (!couldScrollUp && !couldScrollDown) effectiveFolderY -= size.height;
+  BOOL canScrollUp = effectiveFolderY < minimumY;
+  BOOL canScrollDown = folderTop > maximumY;
+
+  // Accommodate changes in the bottom of the menu.
+  if (canScrollUp) {
+    if (!couldScrollUp) {
+      // Couldn't -> Can
+      deltaWindowY = -oldWindowY;
+      deltaWindowHeight = -deltaWindowY;
+      deltaVisibleY = minimumY;
+      deltaVisibleHeight = -deltaVisibleY;
+      deltaScrollerY = verticalScrollArrowHeight_;
+      deltaScrollerHeight = -deltaScrollerY;
+      // Adjust the scroll delta if we've grown the window and it is
+      // now scroll-up-able, but don't adjust it factor if we've
+      // scrolled down and it wasn't scroll-up-able but now is.
+      if (canScrollDown == couldScrollDown) {
+        CGFloat deltaScroll = deltaWindowY + deltaScrollerY + deltaVisibleY;
+        oldScrollPoint.y += deltaScroll + size.height;
+      }
+    } else {
+      if (!canScrollDown && size.height > 0.0) {
+        oldScrollPoint.y += size.height;
+      }
+    }
+  } else {
+    if (couldScrollUp) {
+      // Could -> Can't
+      CGFloat newWindowY = folderY;
+      deltaWindowY = newWindowY - oldWindowY;
+      deltaWindowHeight = -deltaWindowY;
+      deltaVisibleY = -bookmarks::kScrollWindowVerticalMargin;
+      deltaVisibleHeight = -deltaVisibleY;
+      deltaScrollerY = -verticalScrollArrowHeight_;
+      deltaScrollerHeight = -deltaScrollerY;
+      // Adjust the scroll delta if we are no longer scroll-up-able
+      // and the scroll-down-able-ness hasn't changed.
+      if (canScrollDown == couldScrollDown) {
+        CGFloat deltaScroll = deltaWindowY + deltaScrollerY + deltaVisibleY;
+        oldScrollPoint.y += deltaScroll;
+      }
+    } else {
+      // Couldn't -> Can't
+      // Check for menu height change by looking at the relative tops of the
+      // menu folder and the window folder, which previously would have been
+      // the same.
+      deltaWindowY = oldWindowTop - folderTop;
+      deltaWindowHeight = -deltaWindowY;
+    }
   }
-  NSRect windowFrame = NSMakeRect(newWindowTopLeft.x,
-      newWindowTopLeft.y - windowHeight, windowSize.width, windowHeight);
-  // Make the scrolled content be the right size (full size).
-  NSRect mainViewFrame = NSMakeRect(0, 0, NSWidth(windowFrame) -
-      bookmarks::kScrollViewContentWidthMargin, NSHeight(windowFrame));
-  [mainView_ setFrame:mainViewFrame];
-  // Make sure the window fits on the screen.  If not, constrain.
-  // We'll scroll to allow the user to see all the content.
-  NSRect screenFrame = [[[self window] screen] frame];
-  screenFrame = NSInsetRect(screenFrame, 0, kScrollWindowVerticalMargin);
-  BOOL wasScrollable = scrollable_;
-  if (!NSContainsRect(screenFrame, windowFrame)) {
-    scrollable_ = YES;
-    windowFrame = NSIntersectionRect(screenFrame, windowFrame);
+
+  // Accommodate changes in the top of the menu.
+  if (canScrollDown == couldScrollDown) {
+    if (!canScrollDown) {
+      // Not scroll-down-able but the menu top has changed.
+      deltaWindowHeight += scrollDelta;
+    }
   } else {
-    scrollable_ = NO;
+    if (canScrollDown) {
+      // Couldn't -> Can
+      deltaWindowHeight += (NSMaxY(screenFrame) - oldWindowTop);
+      deltaVisibleHeight -= bookmarks::kScrollWindowVerticalMargin;
+      deltaScrollerHeight -= verticalScrollArrowHeight_;
+    } else {
+      // Could -> Can't
+      deltaWindowHeight -= bookmarks::kScrollWindowVerticalMargin;
+      deltaVisibleHeight += bookmarks::kScrollWindowVerticalMargin;
+      deltaScrollerHeight += verticalScrollArrowHeight_;
+    }
   }
-  [window setFrame:windowFrame display:NO];
-  if (wasScrollable != scrollable_) {
-    // If scrollability changed then rework visibility of the scroll arrows
-    // and the scroll offset of the menu view.
-    NSSize windowLocalSize =
-        [scrollView_ convertSize:windowFrame.size fromView:nil];
-    CGFloat scrollPointY = NSHeight(mainViewFrame) - windowLocalSize.height +
-        bookmarks::kBookmarkVerticalPadding;
-    [mainView_ scrollPoint:NSMakePoint(0, scrollPointY)];
-    [self showOrHideScrollArrows];
-    [self addOrUpdateScrollTracking];
-  } else if (scrollable_ && windowAlreadyShowing) {
-    // If the window was already showing and is still scrollable then make
-    // sure the main view moves upward, not downward so that the content
-    // at the bottom of the menu, not the top, appears to move.
-    // The edge case is when the menu is scrolled all the way to top (hence
-    // the test of scrollDownArrowShown_) - don't scroll then.
-    NSView* superView = [mainView_ superview];
-    DCHECK([superView isKindOfClass:[NSClipView class]]);
-    NSClipView* clipView = static_cast<NSClipView*>(superView);
-    CGFloat scrollPointY = [clipView bounds].origin.y +
-        bookmarks::kBookmarkVerticalPadding;
-    if (scrollDownArrowShown_ || deltaY > 0.0)
-      scrollPointY += deltaY;
-    [mainView_ scrollPoint:NSMakePoint(0, scrollPointY)];
+
+  // Hide or show the scroll arrows.
+  if (canScrollUp != couldScrollUp)
+    [scrollUpArrowView_ setHidden:couldScrollUp];
+  if (canScrollDown != couldScrollDown)
+    [scrollDownArrowView_ setHidden:couldScrollDown];
+
+  // Adjust the geometry. The order is important because of sizer dependencies.
+  oldScrollerFrame.origin.y += deltaScrollerY;
+  oldScrollerFrame.size.height += deltaScrollerHeight;
+  [scrollView_ setFrame:oldScrollerFrame];
+  oldVisibleFrame.origin.y += deltaVisibleY;
+  oldVisibleFrame.size.height += deltaVisibleHeight;
+  [visibleView_ setFrame:oldVisibleFrame];
+  BOOL scrolled = NO;
+  // This little bit of trickery detects the one special case where
+  // the window is now scroll-up-able _and_ going to be resized -- scroll
+  // first in order to prevent flashing.
+  if (canScrollUp && !couldScrollUp &&
+      scrollDelta == 0.0 && deltaWindowHeight >= 0.0) {
+    [[scrollView_ documentView] scrollPoint:oldScrollPoint];
+    scrolled = YES;
   }
-  [window display];
+
+  oldWindowFrame.origin.y += deltaWindowY;
+  oldWindowFrame.origin.x = left;
+  oldWindowFrame.size.height += deltaWindowHeight;
+  oldWindowFrame.size.width = size.width;
+  [window setFrame:oldWindowFrame display:YES];
+  if (!scrolled) {
+    // In all other cases we defer scrolling until the window has been resized
+    // in order to prevent flashing.
+    [[scrollView_ documentView] scrollPoint:oldScrollPoint];
+  }
+
+  if (canScrollUp != couldScrollUp || canScrollDown != couldScrollDown ||
+      scrollDelta != 0.0) {
+    if (canScrollUp || canScrollDown)
+      [self addOrUpdateScrollTracking];
+    else
+      [self removeScrollTracking];
+  }
 }
 
+- (void)adjustWindowForButtonCount:(NSUInteger)buttonCount {
+  NSRect folderFrame = [folderView_ frame];
+  CGFloat newMenuHeight =
+      (CGFloat)[self menuHeightForButtonCount:[buttons_ count]];
+  CGFloat deltaMenuHeight = newMenuHeight - NSHeight(folderFrame);
+  // If the height has changed then also change the origin, and adjust the
+  // scroll (if scrolling).
+  if ([self canScrollUp]) {
+    NSPoint scrollPoint = [scrollView_ documentVisibleRect].origin;
+    scrollPoint.y += deltaMenuHeight;
+    [[scrollView_ documentView] scrollPoint:scrollPoint];
+  }
+  folderFrame.size.height += deltaMenuHeight;
+  [folderView_ setFrameSize:folderFrame.size];
+  CGFloat windowWidth =
+      [self adjustButtonWidths] + leftPadding_ + rightPadding_;
+  NSPoint newWindowTopLeft = [self windowTopLeftForWidth:windowWidth];
+  CGFloat left = newWindowTopLeft.x;
+  NSSize newSize = NSMakeSize(windowWidth, deltaMenuHeight);
+  [self adjustWindowLeft:left size:newSize scrollingBy:0.0];
+}
+
 // Determine window size and position.
 // Create buttons for all our nodes.
 // TODO(jrg): break up into more and smaller routines for easier unit testing.
@@ -464,17 +602,17 @@
   int buttons = std::max(node->GetChildCount() - startingIndex, 1);
 
   // Prelim height of the window.  We'll trim later as needed.
-  int height = [self windowHeightForButtonCount:buttons];
+  int height = [self menuHeightForButtonCount:buttons];
   // We'll need this soon...
   [self window];
 
   // TODO(jrg): combine with frame code in bookmark_bar_controller.mm
   // http://crbug.com/35966
   NSRect buttonsOuterFrame = NSMakeRect(
-    bookmarks::kBookmarkSubMenuHorizontalPadding,
-    (height - bookmarks::kBookmarkButtonVerticalSpan),
-    bookmarks::kDefaultBookmarkWidth,
-    bookmarks::kBookmarkButtonHeight);
+      0,
+      (height - bookmarks::kBookmarkButtonVerticalSpan),
+      bookmarks::kDefaultBookmarkWidth,
+      bookmarks::kBookmarkButtonHeight);
 
   // TODO(jrg): combine with addNodesToButtonList: code from
   // bookmark_bar_controller.mm (but use y offset)
@@ -484,7 +622,7 @@
     BookmarkButton* button = [self makeButtonForNode:nil
                                                frame:buttonsOuterFrame];
     [buttons_ addObject:button];
-    [mainView_ addSubview:button];
+    [folderView_ addSubview:button];
   } else {
     for (int i = startingIndex;
          i < node->GetChildCount();
@@ -493,13 +631,33 @@
       BookmarkButton* button = [self makeButtonForNode:child
                                                  frame:buttonsOuterFrame];
       [buttons_ addObject:button];
-      [mainView_ addSubview:button];
+      [folderView_ addSubview:button];
       buttonsOuterFrame.origin.y -= bookmarks::kBookmarkButtonVerticalSpan;
     }
   }
 
-  [self adjustWindowForHeight:height];
-  // Finally pop me up.
+  // Newly showing window.  Lay out the window.

[John Grabowski, 2011/01/06 23:30:18]

Move all this into a configureWindowLayout method

[mrossetti, 2011/01/08 01:45:38]

Done.

+  // Adjust all button widths to be consistent, then base the window width
+  // on this ideal button width.
+  CGFloat buttonWidth = [self adjustButtonWidths];
+  CGFloat windowWidth = buttonWidth + leftPadding_ + rightPadding_;
+  NSPoint newWindowTopLeft = [self windowTopLeftForWidth:windowWidth];
+  // Make sure as much of a submenu is exposed (which otherwise would be a
+  // problem if the parent button is close to the bottom of the screen).
+  if ([parentController_ isKindOfClass:[self class]]) {
+    newWindowTopLeft.y = MAX(newWindowTopLeft.y,
+                             height + bookmarks::kScrollWindowVerticalMargin);
+  }
+  NSWindow* window = [self window];
+  NSRect windowFrame = NSMakeRect(newWindowTopLeft.x,
+                                  newWindowTopLeft.y - height,
+                                  windowWidth, height);
+  [window setFrame:windowFrame display:NO];
+  NSRect folderFrame = NSMakeRect(0, 0, windowWidth, height);
+  [folderView_ setFrame:folderFrame];
+  NSSize newSize = NSMakeSize(windowWidth, 0.0);
+  [self adjustWindowLeft:newWindowTopLeft.x size:newSize scrollingBy:0.0];
+  [window display];
   [self configureWindowLevel];
 }
 
@@ -525,85 +683,6 @@
   return width;
 }
 
-- (BOOL)canScrollUp {
-  // If removal of an arrow would make things "finished", state as
-  // such.
-  CGFloat scrollY = [scrollView_ documentVisibleRect].origin.y;
-  if (scrollUpArrowShown_)
-    scrollY -= verticalScrollArrowHeight_;
-
-  if (scrollY <= 0)
-    return NO;
-  return YES;
-}
-
-- (BOOL)canScrollDown {
-  CGFloat arrowAdjustment = 0.0;
-
-  // We do NOT adjust based on the scrollDOWN arrow.  This keeps
-  // things from "jumping"; if removal of the down arrow (at the top
-  // of the window) would cause a scroll to end, we'll end.
-  if (scrollUpArrowShown_)
-    arrowAdjustment += verticalScrollArrowHeight_;
-
-  NSPoint scrollPosition = [scrollView_ documentVisibleRect].origin;
-  NSRect documentRect = [[scrollView_ documentView] frame];
-
-  // If we are exactly the right height, return no.  We need this
-  // extra conditional in the case where we've just scrolled/grown
-  // into position.
-  if (NSHeight([[self window] frame]) == NSHeight(documentRect))
-    return NO;
-
-  if ((scrollPosition.y + NSHeight([[self window] frame])) >=
-      (NSHeight(documentRect) + arrowAdjustment)) {
-    return NO;
-  }
-  return YES;
-}
-
-- (void)showOrHideScrollArrows {
-  NSRect frame = [scrollView_ frame];
-  CGFloat scrollDelta = 0.0;
-  BOOL canScrollDown = [self canScrollDown];
-  BOOL canScrollUp = [self canScrollUp];
-
-  if (canScrollUp != scrollUpArrowShown_) {
-    if (scrollUpArrowShown_) {
-      frame.origin.y -= verticalScrollArrowHeight_;
-      frame.size.height += verticalScrollArrowHeight_;
-      scrollDelta = verticalScrollArrowHeight_;
-    } else {
-      frame.origin.y += verticalScrollArrowHeight_;
-      frame.size.height -= verticalScrollArrowHeight_;
-      scrollDelta = -verticalScrollArrowHeight_;
-    }
-  }
-  if (canScrollDown != scrollDownArrowShown_) {
-    if (scrollDownArrowShown_) {
-      frame.size.height += verticalScrollArrowHeight_;
-    } else {
-      frame.size.height -= verticalScrollArrowHeight_;
-    }
-  }
-  scrollUpArrowShown_ = canScrollUp;
-  scrollDownArrowShown_ = canScrollDown;
-  [scrollView_ setFrame:frame];
-
-  // Adjust scroll based on new frame.  For example, if we make room
-  // for an arrow at the bottom, adjust the scroll so the topmost item
-  // is still fully visible.
-  if (scrollDelta) {
-    NSPoint scrollPosition = [scrollView_ documentVisibleRect].origin;
-    scrollPosition.y -= scrollDelta;
-    [[scrollView_ documentView] scrollPoint:scrollPosition];
-  }
-}
-
-- (BOOL)scrollable {
-  return scrollable_;
-}
-
 // Start a "scroll up" timer.
 - (void)beginScrollWindowUp {
   [self addScrollTimerWithDelta:kBookmarkBarFolderScrollAmount];
@@ -624,67 +703,55 @@
 }
 
 // Perform a single scroll of the specified amount.
-// Scroll up:
-// Scroll the documentView by the growth amount.
-// If we cannot grow the window, simply scroll the documentView.
-// If we can grow the window up without falling off the screen, do it.
-// Scroll down:
-// Never change the window size; only scroll the documentView.
 - (void)performOneScroll:(CGFloat)delta {
-  NSRect windowFrame = [[self window] frame];
-  NSRect screenFrame = [[[self window] screen] frame];
-
-  // First scroll the "document" area.
-  NSPoint scrollPosition = [scrollView_ documentVisibleRect].origin;
-  scrollPosition.y -= delta;
-  [[scrollView_ documentView] scrollPoint:scrollPosition];
-
-  if (buttonThatMouseIsIn_)
-    [buttonThatMouseIsIn_ toggleButtonBorderingWhileMouseInside];
-
-  // We update the window size after shifting the scroll to avoid a race.
-  CGFloat screenHeightMinusMargin = (NSHeight(screenFrame) -
-                                     (2 * kScrollWindowVerticalMargin));
-  if (delta) {
-    // If we can, grow the window (up).
-    if (NSHeight(windowFrame) < screenHeightMinusMargin) {
-      CGFloat growAmount = delta;
-      // Don't scroll more than enough to "finish".
-      if (scrollPosition.y < 0)
-        growAmount += scrollPosition.y;
-      windowFrame.size.height += growAmount;
-      windowFrame.size.height = std::min(NSHeight(windowFrame),
-                                         screenHeightMinusMargin);
-      // Watch out for a finish that isn't the full height of the screen.
-      // We get here if using the scroll wheel to scroll by small amounts.
-      windowFrame.size.height = std::min(NSHeight(windowFrame),
-                                         NSHeight([mainView_ frame]));
-      // Don't allow scrolling to make the window smaller, ever.  This
-      // conditional is important when processing scrollWheel events.
-      if (windowFrame.size.height > [[self window] frame].size.height) {
-        [[self window] setFrame:windowFrame display:YES];
-        [self addOrUpdateScrollTracking];
-      }
-    }
+  CGFloat finalDelta = [self determineFinalScrollDelta:delta];
+  if (finalDelta > 0.0 || finalDelta < 0.0) {
+    if (buttonThatMouseIsIn_)
+      [buttonThatMouseIsIn_ toggleButtonBorderingWhileMouseInside];
+    NSRect windowFrame = [[self window] frame];
+    NSSize newSize = NSMakeSize(NSWidth(windowFrame), 0.0);
+    [self adjustWindowLeft:windowFrame.origin.x
+                      size:newSize
+               scrollingBy:finalDelta];
   }
+}
 
-  // If we're at either end, happiness.
-  if ((scrollPosition.y <= 0) ||
-      ((scrollPosition.y + NSHeight(windowFrame) >=
-        NSHeight([mainView_ frame])) &&
-       (windowFrame.size.height == screenHeightMinusMargin))) {
-    [self endScroll];
+- (CGFloat)determineFinalScrollDelta:(CGFloat)delta {
+  if (delta > 0.0 && ![scrollUpArrowView_ isHidden] ||
+      delta < 0.0 && ![scrollDownArrowView_ isHidden]) {
+    NSWindow* window = [self window];
+    NSRect windowFrame = [window frame];
+    NSScreen* screen = [window screen];
+    NSPoint scrollPosition = [scrollView_ documentVisibleRect].origin;
+    CGFloat scrollY = scrollPosition.y;
+    NSRect scrollerFrame = [scrollView_ frame];
+    CGFloat scrollerY = NSMinY(scrollerFrame);
+    NSRect visibleFrame = [visibleView_ frame];
+    CGFloat visibleY = NSMinY(visibleFrame);
+    CGFloat windowY = NSMinY(windowFrame);
+    CGFloat offset = scrollerY + visibleY + windowY;
 
-    // If we can't scroll either up or down we are completely done.
-    // For example, perhaps we've scrolled a little and grown the
-    // window on-screen until there is now room for everything.
-    if (![self canScrollUp] && ![self canScrollDown]) {
-      scrollable_ = NO;
-      [self removeScrollTracking];
+    if (delta > 0.0) {
+      // Scrolling up.
+      CGFloat minimumY = NSMinY([screen visibleFrame]) +
+                         bookmarks::kScrollWindowVerticalMargin;
+      CGFloat maxUpDelta = scrollY - offset + minimumY;
+      delta = MIN(delta, maxUpDelta);
+    } else {
+      // Scrolling down.
+      NSRect screenFrame =  [screen frame];
+      CGFloat topOfScreen = NSMaxY(screenFrame);
+      NSRect folderFrame = [folderView_ frame];
+      CGFloat folderHeight = NSHeight(folderFrame);
+      CGFloat folderTop = folderHeight - scrollY + offset;
+      CGFloat maxDownDelta =
+          topOfScreen - folderTop - bookmarks::kScrollWindowVerticalMargin;
+      delta = MAX(delta, maxDownDelta);
     }
+  } else {
+    delta = 0.0;
   }
-
-  [self showOrHideScrollArrows];
+  return delta;
 }
 
 // Perform a scroll of the window on the screen.
@@ -716,22 +783,28 @@
 // menubar (to be fixed by setting the proper window level; see
 // initializer).
 - (void)mouseMoved:(NSEvent*)theEvent {
-  DCHECK([theEvent window] == [self window]);
+  NSWindow* window = [theEvent window];
+  DCHECK(window == [self window]);
 
   NSPoint eventScreenLocation =
-      [[theEvent window] convertBaseToScreen:[theEvent locationInWindow]];
+      [window convertBaseToScreen:[theEvent locationInWindow]];
 
-  // We use frame (not visibleFrame) since our bookmark folder is on
-  // TOP of the menubar.
-  NSRect visibleRect = [[[self window] screen] frame];
-  CGFloat closeToTopOfScreen = NSMaxY(visibleRect) -
-      verticalScrollArrowHeight_;
-  CGFloat closeToBottomOfScreen = NSMinY(visibleRect) +
-      verticalScrollArrowHeight_;
+  // Base hot spot calculations on the positions of the scroll arrow views.
+  NSRect testRect = [scrollDownArrowView_ frame];
+  NSPoint testPoint = [visibleView_ convertPoint:testRect.origin
+                                                  toView:nil];
+  testPoint = [window convertBaseToScreen:testPoint];
+  CGFloat closeToTopOfScreen = testPoint.y;
 
-  if (eventScreenLocation.y <= closeToBottomOfScreen) {
+  testRect = [scrollUpArrowView_ frame];
+  testPoint = [visibleView_ convertPoint:testRect.origin toView:nil];
+  testPoint = [window convertBaseToScreen:testPoint];
+  CGFloat closeToBottomOfScreen = testPoint.y + testRect.size.height;
+  if (eventScreenLocation.y <= closeToBottomOfScreen &&
+      ![scrollUpArrowView_ isHidden]) {
     [self beginScrollWindowUp];
-  } else if (eventScreenLocation.y > closeToTopOfScreen) {
+  } else if (eventScreenLocation.y > closeToTopOfScreen &&
+      ![scrollDownArrowView_ isHidden]) {
     [self beginScrollWindowDown];
   } else {
     [self endScroll];
@@ -766,17 +839,6 @@
   scrollTrackingArea_.reset();
 }
 
-// Delegate callback.
-- (void)windowWillClose:(NSNotification*)notification {
-  // If a "hover open" is pending when the bookmark bar folder is
-  // closed, be sure it gets cancelled.
-  [NSObject cancelPreviousPerformRequestsWithTarget:self];
-
-  [barController_ childFolderWillClose:self];
-  [self closeBookmarkFolder:self];
-  [self autorelease];
-}
-
 // Close the old hover-open bookmark folder, and open a new one.  We
 // do both in one step to allow for a delay in closing the old one.
 // See comments above kDragHoverCloseDelay (bookmark_bar_controller.h)
@@ -800,7 +862,7 @@
 }
 
 - (void)scrollWheel:(NSEvent *)theEvent {
-  if (scrollable_) {
+  if (![scrollUpArrowView_ isHidden] || ![scrollDownArrowView_ isHidden]) {
     // We go negative since an NSScrollView has a flipped coordinate frame.
     CGFloat amt = kBookmarkBarFolderScrollWheelAmount * -[theEvent deltaY];
     [self performOneScroll:amt];
@@ -926,7 +988,7 @@
   // dropLocation is in bar local coordinates.
   // http://crbug.com/36276
   NSPoint dropLocation =
-      [mainView_ convertPoint:point
+      [folderView_ convertPoint:point
                      fromView:[[self window] contentView]];
   BookmarkButton* buttonToTheTopOfDraggedButton = nil;
   // Buttons are laid out in this array from top to bottom (screen
@@ -1000,6 +1062,19 @@
   return wasCopiedOrMoved;
 }
 
+#pragma mark NSWindowDelegate Functions
+
+- (void)windowWillClose:(NSNotification*)notification {
+  // If a "hover open" is pending when the bookmark bar folder is
+  // closed, be sure it gets cancelled.
+  [NSObject cancelPreviousPerformRequestsWithTarget:self];
+
+  [self endScroll];  // Just in case we were scrolling.
+  [barController_ childFolderWillClose:self];
+  [self closeBookmarkFolder:self];
+  [self autorelease];
+}
+
 #pragma mark BookmarkButtonDelegate Protocol
 
 - (void)fillPasteboard:(NSPasteboard*)pboard
@@ -1280,14 +1355,12 @@
   BookmarkButton* newButton = [self makeButtonForNode:node
                                                 frame:newButtonFrame];
   [buttons_ insertObject:newButton atIndex:buttonIndex];
-  [mainView_ addSubview:newButton];
+  [folderView_ addSubview:newButton];
 
   // Close any child folder(s) which may still be open.
   [self closeBookmarkFolder:self];
 
-  // Prelim height of the window.  We'll trim later as needed.
-  int height = [self windowHeightForButtonCount:[buttons_ count]];
-  [self adjustWindowForHeight:height];
+  [self adjustWindowForButtonCount:[buttons_ count]];
 }
 
 // More code which essentially duplicates that of BookmarkBarController.
@@ -1394,9 +1467,6 @@
 
   [oldButton setDelegate:nil];
   [oldButton removeFromSuperview];
-  if (animate && !ignoreAnimations_)
-    NSShowAnimationEffect(NSAnimationEffectDisappearingItemDefault, poofPoint,
-                          NSZeroSize, nil, nil, nil);
   [buttons_ removeObjectAtIndex:buttonIndex];
   for (NSInteger i = 0; i < buttonIndex; ++i) {
     BookmarkButton* button = [buttons_ objectAtIndex:i];
@@ -1419,22 +1489,25 @@
     // If all nodes have been removed from this folder then add in the
     // 'empty' placeholder button.
     NSRect buttonFrame =
-        NSMakeRect(bookmarks::kBookmarkSubMenuHorizontalPadding,
+        NSMakeRect(0,
                    bookmarks::kBookmarkButtonHeight -
-                   (bookmarks::kBookmarkBarHeight -
-                    bookmarks::kBookmarkVerticalPadding),
+                       (bookmarks::kBookmarkBarHeight -
+                        bookmarks::kBookmarkVerticalPadding),
                    bookmarks::kDefaultBookmarkWidth,
                    (bookmarks::kBookmarkBarHeight -
-                    2 * bookmarks::kBookmarkVerticalPadding));
+                       2 * bookmarks::kBookmarkVerticalPadding));
     BookmarkButton* button = [self makeButtonForNode:nil
                                                frame:buttonFrame];
     [buttons_ addObject:button];
-    [mainView_ addSubview:button];
+    [folderView_ addSubview:button];
     buttonCount = 1;
   }
 
-  // Propose a height for the window.  We'll trim later as needed.
-  [self adjustWindowForHeight:[self windowHeightForButtonCount:buttonCount]];
+  [self adjustWindowForButtonCount:buttonCount];
+
+  if (animate && !ignoreAnimations_)
+    NSShowAnimationEffect(NSAnimationEffectDisappearingItemDefault, poofPoint,
+                          NSZeroSize, nil, nil, nil);
 }
 
 - (id<BookmarkButtonControllerProtocol>)controllerForNode:
@@ -1448,6 +1521,30 @@
 
 #pragma mark TestingAPI Only
 
+- (BOOL)canScrollUp {
+  return ![scrollUpArrowView_ isHidden];
+}
+
+- (BOOL)canScrollDown {
+  return ![scrollDownArrowView_ isHidden];
+}
+
+- (CGFloat)verticalScrollArrowHeight {
+  return verticalScrollArrowHeight_;
+}
+
+- (NSView*)visibleView {
+  return visibleView_;
+}
+
+- (NSView*)scrollView {
+  return scrollView_;
+}
+
+- (NSView*)folderView {
+  return folderView_;
+}
+
 - (void)setIgnoreAnimations:(BOOL)ignore {
   ignoreAnimations_ = ignore;
 }