Mac: Fix bookmark bar buttons not being layed out correctly on startup.

BUG=17625

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

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

3 files changed, 41 insertions, 16 deletions

diff --git a/chrome/browser/cocoa/bookmark_bar_controller.h b/chrome/browser/cocoa/bookmark_bar_controller.h
index b8986f1..2965b6f 100644
--- a/chrome/browser/cocoa/bookmark_bar_controller.h
+++ b/chrome/browser/cocoa/bookmark_bar_controller.h
@@ -29,6 +29,15 @@ class PrefService;
 @class ToolbarController;
 @protocol ViewResizer;
 
+namespace bookmarks {
+
+// Magic numbers from Cole
+const CGFloat kDefaultBookmarkWidth = 150.0;
+const CGFloat kBookmarkVerticalPadding = 2.0;
+const CGFloat kBookmarkHorizontalPadding = 1.0;
+
+}  // namespace
+
 // The interface for an object which can open URLs for a bookmark.
 @protocol BookmarkURLOpener
 - (void)openBookmarkURL:(const GURL&)url
diff --git a/chrome/browser/cocoa/bookmark_bar_controller.mm b/chrome/browser/cocoa/bookmark_bar_controller.mm
index f8286de..03248f6 100644
--- a/chrome/browser/cocoa/bookmark_bar_controller.mm
+++ b/chrome/browser/cocoa/bookmark_bar_controller.mm
@@ -55,15 +55,6 @@
 - (int)preferredHeight;
 @end
 
-namespace {
-
-// Magic numbers from Cole
-const CGFloat kDefaultBookmarkWidth = 150.0;
-const CGFloat kBookmarkVerticalPadding = 2.0;
-const CGFloat kBookmarkHorizontalPadding = 1.0;
-
-}  // namespace
-
 @implementation BookmarkBarController
 
 - (id)initWithBrowser:(Browser*)browser
@@ -552,7 +543,7 @@ const CGFloat kBookmarkHorizontalPadding = 1.0;
 // For a bookmark named "SFGateFooWoo", it is just fine.
 - (CGFloat)widthForBookmarkButtonCell:(NSCell*)cell {
   CGFloat desired = [cell cellSize].width + 2;
-  return std::min(desired, kDefaultBookmarkWidth);
+  return std::min(desired, bookmarks::kDefaultBookmarkWidth);
 }
 
 // Returns a frame appropriate for the given bookmark cell, suitable
@@ -561,13 +552,17 @@ const CGFloat kBookmarkHorizontalPadding = 1.0;
 // for the next button.
 - (NSRect)frameForBookmarkButtonFromCell:(NSCell*)cell
                                  xOffset:(int*)xOffset {
-  NSRect bounds = [[self view] bounds];
-  // TODO: be smarter about this; the animator delays the right height
-  if (bounds.size.height == 0)
-    bounds.size.height = bookmarks::kBookmarkBarHeight;
+  NSRect bounds = [buttonView_ bounds];
+  // TODO(erg,jrg): There used to be an if statement here, comparing the height
+  // to 0. This essentially broke sizing because we're dealing with floats and
+  // the height wasn't precisely zero. The previous author wrote that they were
+  // doing this because of an animator, but we are not doing animations for beta
+  // so we do not care.
+  bounds.size.height = bookmarks::kBookmarkBarHeight;
+
   NSRect frame = NSInsetRect(bounds,
-                             kBookmarkHorizontalPadding,
-                             kBookmarkVerticalPadding);
+                             bookmarks::kBookmarkHorizontalPadding,
+                             bookmarks::kBookmarkVerticalPadding);
   frame.size.width = [self widthForBookmarkButtonCell:cell];
 
   // Add an X offset based on what we've already done
diff --git a/chrome/browser/cocoa/bookmark_bar_controller_unittest.mm b/chrome/browser/cocoa/bookmark_bar_controller_unittest.mm
index 5da3d0a..985e96f 100644
--- a/chrome/browser/cocoa/bookmark_bar_controller_unittest.mm
+++ b/chrome/browser/cocoa/bookmark_bar_controller_unittest.mm
@@ -6,6 +6,7 @@
 
 #include "base/basictypes.h"
 #include "base/scoped_nsobject.h"
+#import "chrome/browser/cocoa/bookmark_bar_constants.h"
 #import "chrome/browser/cocoa/bookmark_bar_controller.h"
 #import "chrome/browser/cocoa/bookmark_bar_view.h"
 #include "chrome/browser/cocoa/browser_test_helper.h"
@@ -648,6 +649,26 @@ TEST_F(BookmarkBarControllerTest, HidesHelpMessageWithBookmark) {
   EXPECT_TRUE([[[bar_ buttonView] noItemTextfield] isHidden]);
 }
 
+TEST_F(BookmarkBarControllerTest, BookmarkButtonSizing) {
+  BookmarkModel* model = helper_.profile()->GetBookmarkModel();
+
+  const BookmarkNode* parent = model->GetBookmarkBarNode();
+  model->AddURL(parent, parent->GetChildCount(),
+                L"title", GURL("http://one.com"));
+
+  [bar_ loaded:model];
+
+  // Make sure the internal bookmark button also is the correct height.
+  NSArray* buttons = [bar_ buttons];
+  EXPECT_GT([buttons count], 0u);
+  for (NSButton* button in buttons) {
+    EXPECT_FLOAT_EQ(
+        bookmarks::kBookmarkBarHeight - 2 *
+                    bookmarks::kBookmarkVerticalPadding,
+        [button frame].size.height);
+  }
+}
+
 // Cannot test these methods since they simply call a single static
 // method, BookmarkEditor::Show(), which is impossible to mock.
 // editBookmark:, addPage: