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 child_y = (parent_height - child_height + 1) / 2;

[Peter Kasting, 2015/09/08 19:38:34]

Nit: Why not just:

  int roundoff_amount = ui::MaterialDesignController::IsModeMaterial() ? 0 : 1;
  return (parent_height - child_height + roundoff_amount ) / 2;

[tdanderson, 2015/09/09 17:23:06]

Changed.

+  if (!ui::MaterialDesignController::IsModeMaterial() ||
+      parent_height - child_height % 2 == 0) {
+    return child_y;
+  }
+  return child_y - 1;
+}
+
 }  // namespace
 
 // static
 const char ToolbarView::kViewClassName[] = "ToolbarView";
 
 ////////////////////////////////////////////////////////////////////////////////
 // ToolbarView, public:
 
 ToolbarView::ToolbarView(Browser* browser)
     : back_(NULL),
@@ skipping 432 matching lines @@
     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.

[Peter Kasting, 2015/09/08 19:38:34]

Nit: Delete the last sentence here

[tdanderson, 2015/09/09 17:23:06]

Done.

   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 127 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

[Peter Kasting, 2015/09/08 19:38:33]

This description seems somewhat inaccurate.  We don't encode the size as a
constant, we compute the size based on given padding amounts.

[tdanderson, 2015/09/09 17:23:06]

Re-worded.

     // 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);
+      size.SetToMax(
+          gfx::Size(0, content_height + (top_padding + bottom_padding)));
     } 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);
 }