Fixing the height of the bookmark bar and buttons on the bookmark bar.

Bug: 876690, 876682, 879154
Change-Id: Ia72f26e9d7e0402868dd628fa2e10492b46a2cee
Reviewed-on: https://chromium-review.googlesource.com/1194728
Commit-Queue: Charlene Yan <cyan@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Bret Sepulveda <bsep@chromium.org>
Cr-Commit-Position: refs/heads/master@{#587664}

chrome/browser/ui/layout_constants.cc

@@ -28,13 +28,19 @@ int GetCocoaLayoutConstant(LayoutConstant constant) {
 
 int GetLayoutConstant(LayoutConstant constant) {
   const int mode = ui::MaterialDesignController::GetMode();
+  static const int kBookmarkBarVerticalMargin = 4;
   const bool hybrid = mode == ui::MaterialDesignController::MATERIAL_HYBRID;
   const bool touch_optimized_material =
       ui::MaterialDesignController::IsTouchOptimizedUiEnabled();
   const bool newer_material = ui::MaterialDesignController::IsNewerMaterialUi();
   switch (constant) {
     case BOOKMARK_BAR_HEIGHT:
-      return touch_optimized_material ? 40 : 32;
+      // The fixed margin ensures the bookmark buttons appear centered relative
+      // to the white space above and below.
+      return GetLayoutConstant(BOOKMARK_BAR_BUTTON_HEIGHT) +
+             kBookmarkBarVerticalMargin;
+    case BOOKMARK_BAR_BUTTON_HEIGHT:
+      return touch_optimized_material ? 36 : 28;
     case BOOKMARK_BAR_NTP_HEIGHT:
       return touch_optimized_material ? GetLayoutConstant(BOOKMARK_BAR_HEIGHT)
                                       : 39;

chrome/browser/ui/layout_constants.h

@@ -10,11 +10,14 @@
 #include "ui/gfx/geometry/size.h"
 
 enum LayoutConstant {
-  // The minimum height of Bookmarks Bar, when attached to the toolbar. The
-  // height of the toolbar may grow to more than this value if the embedded
-  // views need more space, for example, when the font is larger than normal.
+  // The height of Bookmarks Bar when attached to the toolbar. The height of the
+  // Bookmarks Bar is larger than the BOOKMARK_BAR_BUTTON_HEIGHT by a fixed
+  // amount.
   BOOKMARK_BAR_HEIGHT,
 
+  // The height of a button within the Bookmarks Bar.
+  BOOKMARK_BAR_BUTTON_HEIGHT,
+
 #if defined(OS_MACOSX)
   // This is a little smaller than the bookmarkbar height because of the visual
   // overlap with the main toolbar. This height should not be used when

chrome/browser/ui/views/bookmarks/bookmark_bar_view.cc

@@ -857,24 +857,9 @@ int BookmarkBarView::GetToolbarOverlap() const {
   return gfx::ToRoundedInt(kOverlap * size_animation_.GetCurrentValue());
 }
 
-int BookmarkBarView::GetPreferredHeight() const {
-  return std::max(GetTallestButtonHeight(),
-                  GetLayoutConstant(BOOKMARK_BAR_HEIGHT));
-}
-
-int BookmarkBarView::GetTallestButtonHeight() const {
-  int height = 0;
-  for (int i = 0; i < child_count(); ++i) {
-    const views::View* view = child_at(i);
-    if (view->visible())
-      height = std::max(view->GetPreferredSize().height(), height);
-  }
-  return height;
-}
-
 gfx::Size BookmarkBarView::CalculatePreferredSize() const {
   gfx::Size prefsize;
-  int preferred_height = GetPreferredHeight();
+  int preferred_height = GetLayoutConstant(BOOKMARK_BAR_HEIGHT);
   if (IsDetached()) {
     prefsize.set_height(
         preferred_height +
@@ -914,7 +899,7 @@ gfx::Size BookmarkBarView::GetMinimumSize() const {
   // It should also contain the Managed Bookmarks folder, if it is visible.
   int width = kBookmarkBarHorizontalMargin;
 
-  int height = GetPreferredHeight();
+  int height = GetLayoutConstant(BOOKMARK_BAR_HEIGHT);
   if (IsDetached()) {
     double current_state = 1 - size_animation_.GetCurrentValue();
     height += static_cast<int>(
@@ -956,7 +941,7 @@ void BookmarkBarView::Layout() {
   int x = kBookmarkBarHorizontalMargin;
   int width = View::width() - 2 * kBookmarkBarHorizontalMargin;
 
-  int height = GetTallestButtonHeight();
+  int height = GetLayoutConstant(BOOKMARK_BAR_BUTTON_HEIGHT);
 
   int y = (GetContentsBounds().height() - height) / 2;
 

chrome/browser/ui/views/bookmarks/bookmark_bar_view.h

@@ -396,13 +396,6 @@ class BookmarkBarView : public views::AccessiblePaneView,
   // or -1 if |button| is not a bookmark button from this bar.
   int GetIndexForButton(views::View* button);
 
-  // Gets the preferred height of the bookmark bar which is the max of either
-  // the bookmark bar constant or the tallest icon.
-  int GetPreferredHeight() const;
-
-  // Gets the height of the tallest visible button on the bookmark bar.
-  int GetTallestButtonHeight() const;
-
   // Needed to react to kShowAppsShortcutInBookmarkBar changes.
   PrefChangeRegistrar profile_pref_registrar_;