Change toolbar height for material design on Ash

chrome/browser/ui/views/toolbar/toolbar_view.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/ui/views/toolbar/toolbar_view.h"
 
 #include <algorithm>
 
 #include "base/command_line.h"
 #include "base/i18n/number_formatting.h"
@@ skipping 84 matching lines @@
 #if !defined(OS_CHROMEOS)
 bool HasAshShell() {
 #if defined(USE_ASH)
   return ash::Shell::HasInstance();
 #else
   return false;
 #endif  // USE_ASH
 }
 #endif  // OS_CHROMEOS
 
+// Returns the y-position that will center an element of height
+// |child_height| inside an element of height |parent_height|. For
+// material design excess padding is placed below, for non-material
+// it is placed above.
+int CenteredChildY(int parent_height, int child_height) {
+  int roundoff_amount = ui::MaterialDesignController::IsModeMaterial() ? 0 : 1;
+  return (parent_height - child_height + roundoff_amount) / 2;
+}
+
 }  // namespace
 
 // static
 const char ToolbarView::kViewClassName[] = "ToolbarView";
 
 ////////////////////////////////////////////////////////////////////////////////
 // ToolbarView, public:
 
 ToolbarView::ToolbarView(Browser* browser)
     : back_(NULL),
@@ skipping 431 matching lines @@
   if (back_ == NULL)
     return;
 
   if (!is_display_mode_normal()) {
     location_bar_->SetBounds(0, PopupTopSpacing(), width(),
                              location_bar_->GetPreferredSize().height());
     return;
   }
 
   // We assume all child elements except the location bar are the same height.
-  // Set child_y such that buttons appear vertically centered. We put any excess
-  // padding above the buttons.
+  // Set child_y such that buttons appear vertically centered.
   int child_height =
       std::min(back_->GetPreferredSize().height(), height());
-  int child_y = (height() - child_height + 1) / 2;
+  int child_y = CenteredChildY(height(), child_height);
 
   // If the window is maximized, we extend the back button to the left so that
   // clicking on the left-most pixel will activate the back button.
   // TODO(abarth):  If the window becomes maximized but is not resized,
   //                then Layout() might not be called and the back button
   //                will be slightly the wrong size.  We should force a
   //                Layout() in this case.
   //                http://crbug.com/5540
   bool maximized = browser_->window() && browser_->window()->IsMaximized();
   int back_width = back_->GetPreferredSize().width();
@@ skipping 38 matching lines @@
   const int right_edge_spacing = theme_provider->GetDisplayProperty(
       ThemeProperties::PROPERTY_TOOLBAR_VIEW_RIGHT_EDGE_SPACING);
   const int location_bar_right_padding = theme_provider->GetDisplayProperty(
       ThemeProperties::PROPERTY_TOOLBAR_VIEW_LOCATION_BAR_RIGHT_PADDING);
   int available_width = std::max(
       0, width() - right_edge_spacing - app_menu_width - browser_actions_width -
              (browser_actions_width > 0 ? element_padding : 0) -
              location_bar_right_padding - next_element_x);
 
   int location_height = location_bar_->GetPreferredSize().height();
-  int location_y = (height() - location_height + 1) / 2;
+  int location_y = CenteredChildY(height(), location_height);
+
   location_bar_->SetBounds(next_element_x, location_y,
                            std::max(available_width, 0), location_height);
   next_element_x = location_bar_->bounds().right() + location_bar_right_padding;
 
   browser_actions_->SetBounds(
       next_element_x, child_y, browser_actions_width, child_height);
   next_element_x = browser_actions_->bounds().right();
   if (browser_actions_width > 0)
     next_element_x += element_padding;
 
@@ skipping 126 matching lines @@
 }
 
 int ToolbarView::PopupTopSpacing() const {
   const int kPopupTopSpacingNonGlass = 3;
   return GetWidget()->ShouldWindowContentsBeTransparent() ?
       0 : kPopupTopSpacingNonGlass;
 }
 
 gfx::Size ToolbarView::SizeForContentSize(gfx::Size size) const {
   if (is_display_mode_normal()) {
-    // For Material Design the size of ToolbarView is encoded as a constant in
-    // ThemeProperties. For non-material the size is based on the provided
-    // assets.
+    // For Material Design the size of the toolbar is computed using the size
+    // of the location bar and constant padding values. For non-material the
+    // size is based on the provided assets.
     if (ui::MaterialDesignController::IsModeMaterial()) {
       int content_height = std::max(back_->GetPreferredSize().height(),
                                     location_bar_->GetPreferredSize().height());
-      int padding = GetThemeProvider()->GetDisplayProperty(
-          ThemeProperties::PROPERTY_TOOLBAR_VIEW_VERTICAL_PADDING);
-      size.SetToMax(gfx::Size(0, content_height + (padding * 2)));
+      int top_padding = GetThemeProvider()->GetDisplayProperty(
+          ThemeProperties::PROPERTY_TOOLBAR_VIEW_TOP_VERTICAL_PADDING);
+      int bottom_padding = GetThemeProvider()->GetDisplayProperty(
+          ThemeProperties::PROPERTY_TOOLBAR_VIEW_BOTTOM_VERTICAL_PADDING);

[Peter Kasting, 2015/09/09 22:41:53]

If this is the only place you use this constant you might as well just define
the value locally here instead of in the theme provider, unless you'll need this
elsewhere later.

[tdanderson, 2015/09/10 19:48:25]

I'm not sure whether or not I'd need it later, but regardless I'd prefer to keep
it in the theme provider so that the top padding and bottom padding are defined
in the same place. But I can define it locally if you have strong feelings
otherwise.

+      size.SetToMax(
+          gfx::Size(0, content_height + (top_padding + bottom_padding)));

[Peter Kasting, 2015/09/09 22:41:53]

Nit: No need for parens around final addition

[tdanderson, 2015/09/10 19:48:25]

Done.

     } else {
       gfx::ImageSkia* normal_background =
           GetThemeProvider()->GetImageSkiaNamed(IDR_CONTENT_TOP_CENTER);
       size.SetToMax(
           gfx::Size(0, normal_background->height() - content_shadow_height()));
     }
   } else if (size.height() == 0) {
     // Location mode with a 0 height location bar. If on ash, expand by one
     // pixel to show a border in the title bar, otherwise leave the size as zero
     // height.
@@ skipping 55 matching lines @@
   SchedulePaint();
 }
 
 int ToolbarView::content_shadow_height() const {
   ui::ThemeProvider* theme_provider = GetThemeProvider();
   return theme_provider->GetDisplayProperty(
       browser_->host_desktop_type() == chrome::HOST_DESKTOP_TYPE_ASH
           ? ThemeProperties::PROPERTY_TOOLBAR_VIEW_CONTENT_SHADOW_HEIGHT_ASH
           : ThemeProperties::PROPERTY_TOOLBAR_VIEW_CONTENT_SHADOW_HEIGHT);
 }