Rework somewhat how the folder menu window and main view sizes and positions are calculated. Handle edge case where the top scroll arrow is not showing. Proposed window height was not taking into account the one additional vertical separation.

BUG=46101
TEST=Setup: Create a bookmark folder on the bookmark bar with plenty of bookmarks, more than enough to fill the screen, and so that it is scrollable. 1) Bring up the bookmark folder menu but do not cause it to scroll. 2) Using the contextual menu delete a bookmark midway in the menu. 3) Verify that the top of the menu window and contents do not move relative to the bookmark bar folder button. 4) Bring up the bookmark folder menu again. 5) Cause the menu to scroll up to fill the screen and then scroll it down so that the top scroll arrow is not showing. 6) Using the contextual menu delete a bookmark midway in the menu. 7) Verify that the top of the menu window and contents have not moved in relation to the screen top. 8) Bring up the bookmark folder menu and cause it to scroll such that both the top and bottom scroll arrows are showing. 9) Using the contextual menu, delete a bookmark midway in the menu. 10) Verify that the top scroll arrow, the top-most item showing in the menu, and the menu window have not changed position relative to the top of the window. Note that the contents of the folder menu below the bookmark item being delete should adjust upward to fill the slot originally occupied by the deleted bookmark.

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

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

1 files changed, 42 insertions, 23 deletions

diff --git a/chrome/browser/cocoa/bookmark_bar_folder_controller.mm b/chrome/browser/cocoa/bookmark_bar_folder_controller.mm
index ff556df..179b553 100644
--- a/chrome/browser/cocoa/bookmark_bar_folder_controller.mm
+++ b/chrome/browser/cocoa/bookmark_bar_folder_controller.mm
@@ -377,22 +377,22 @@ const CGFloat kScrollWindowVerticalMargin = 0.0;
       [self adjustButtonWidths] + (2 * bookmarks::kBookmarkVerticalPadding) +
       bookmarks::kScrollViewContentWidthMargin;
   NSPoint newWindowTopLeft = [self windowTopLeftForWidth:windowWidth];
-  NSSize windowSize = [scrollView_ convertSize:NSMakeSize(windowWidth,
-                                                          windowHeight)
-                                        toView:nil];
-  newWindowTopLeft.y -= windowSize.height;
+  NSSize windowSize = NSMakeSize(windowWidth, windowHeight);
+  windowSize = [scrollView_ convertSize:windowSize toView:nil];
+  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 windowFrame = NSMakeRect(newWindowTopLeft.x,
-                                  newWindowTopLeft.y,
-                                  windowSize.width,
-                                  windowSize.height);
-
+      newWindowTopLeft.y - windowHeight, windowSize.width, windowHeight);
   // Make the scrolled content be the right size (full size).
-  NSRect mainViewFrame = NSMakeRect(0, 0,
-                                    windowWidth -
-                                    bookmarks::kScrollViewContentWidthMargin,
-                                    windowHeight);
+  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];
@@ -404,17 +404,33 @@ const CGFloat kScrollWindowVerticalMargin = 0.0;
   } else {
     scrollable_ = NO;
   }
-  NSWindow* window = [self window];
-  [window setFrame:windowFrame display:YES];
-  // If scrollable then offset the view and show the arrows.
+  [window setFrame:windowFrame display:NO];
   if (wasScrollable != scrollable_) {
-    NSSize windowLocalSize = [scrollView_ convertSize:windowFrame.size
-                                             fromView:nil];
-    [mainView_ scrollPoint:NSMakePoint(0, (NSHeight(mainViewFrame) -
-                                           windowLocalSize.height))];
+    // 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)];
   }
+  [window display];
 }
 
 // Determine window size and position.
@@ -429,7 +445,8 @@ const CGFloat kScrollWindowVerticalMargin = 0.0;
   int buttons = std::max(node->GetChildCount() - startingIndex, 1);
 
   // Prelim height of the window.  We'll trim later as needed.
-  int height = buttons * bookmarks::kBookmarkButtonHeight;
+  int height = buttons * bookmarks::kBookmarkButtonHeight +
+      bookmarks::kBookmarkVerticalPadding;
   // We'll need this soon...
   [self window];
 
@@ -1237,7 +1254,8 @@ static BOOL ValueInRangeInclusive(CGFloat low, CGFloat value, CGFloat high) {
   [self closeBookmarkFolder:self];
 
   // Prelim height of the window.  We'll trim later as needed.
-  int height = [buttons_ count] * bookmarks::kBookmarkButtonHeight;
+  int height = [buttons_ count] * bookmarks::kBookmarkButtonHeight +
+      bookmarks::kBookmarkVerticalPadding;
   [self adjustWindowForHeight:height];
 }
 
@@ -1370,7 +1388,8 @@ static BOOL ValueInRangeInclusive(CGFloat low, CGFloat value, CGFloat high) {
   }
 
   // Propose a height for the window.  We'll trim later as needed.
-  int height = buttonCount * bookmarks::kBookmarkButtonHeight;
+  int height = buttonCount * bookmarks::kBookmarkButtonHeight +
+      bookmarks::kBookmarkVerticalPadding;
   [self adjustWindowForHeight:height];
 }