Last round of cleaning up MD layout constants.

chrome/browser/ui/views/toolbar/toolbar_button.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_button.h"
 
 #include "base/bind.h"
 #include "base/location.h"
 #include "base/single_thread_task_runner.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "build/build_config.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/themes/theme_service.h"
 #include "chrome/browser/themes/theme_service_factory.h"
-#include "chrome/browser/ui/layout_constants.h"
 #include "chrome/browser/ui/views/location_bar/location_bar_view.h"
 #include "ui/accessibility/ax_node_data.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/models/menu_model.h"
 #include "ui/display/display.h"
 #include "ui/display/screen.h"
 #include "ui/strings/grit/ui_strings.h"
 #include "ui/views/controls/button/label_button_border.h"
 #include "ui/views/controls/menu/menu_item_view.h"
 #include "ui/views/controls/menu/menu_model_adapter.h"
@@ skipping 25 matching lines @@
 void ToolbarButton::ClearPendingMenu() {
   show_menu_factory_.InvalidateWeakPtrs();
 }
 
 bool ToolbarButton::IsMenuShowing() const {
   return menu_showing_;
 }
 
 gfx::Size ToolbarButton::GetPreferredSize() const {
   gfx::Size size(image()->GetPreferredSize());
+  // TODO(estade): do we ever actually set text on a ToolbarButton?

[Peter Kasting, 2016/12/08 20:57:50]

Maybe for extensions?

[Evan Stade, 2016/12/13 01:31:19]

I don't see how extensions could reach this because ToolbarButton has only three
overrides: Back, Home and Reload, plus a direct instantiation for Forward.

   gfx::Size label_size = label()->GetPreferredSize();
+  CHECK_EQ(0, label_size.width());

[Peter Kasting, 2016/12/08 20:57:50]

I'd kinda rather add something like this in a separate CL, just so you don't get
this whole one auto-reverted if this breaks something.

(Might be clearer to do as a CHECK(FALSE) inside the next conditional as well.)

[Evan Stade, 2016/12/13 01:31:19]

oops --- this was just meant to be debug code I sent by the trybots to make sure
they were OK with removing this code. Between the trybots OK'ing this and my own
inability to find where something sets a label, I think we can simplify this
code.

   if (label_size.width() > 0) {
-    size.Enlarge(
-        label_size.width() + GetLayoutConstant(LOCATION_BAR_HORIZONTAL_PADDING),
-        0);
+    size.Enlarge(label_size.width() /* + LocationBarView::kHorizontalPadding*/,

[Peter Kasting, 2016/12/08 20:57:50]

Commenting this out seems wrong?

[Evan Stade, 2016/12/13 01:31:19]

yea, removed.

+                 0);
   }
-  const int pad = GetLayoutConstant(TOOLBAR_BUTTON_PADDING);
-  size.Enlarge(2 * pad, 2 * pad);
-  return size;
+  gfx::Rect rect(size);
+  rect.Inset(gfx::Insets(-kInteriorPadding));
+  return rect.size();
 }
 
 bool ToolbarButton::OnMousePressed(const ui::MouseEvent& event) {
   if (IsTriggerableEvent(event) && enabled() && ShouldShowMenu() &&
       HitTestPoint(event.location())) {
     // Store the y pos of the mouse coordinates so we can use them later to
     // determine if the user dragged the mouse down (which should pop up the
     // drag down menu immediately, instead of waiting for the timer)
     y_position_on_lbuttondown_ = event.y();
 
@@ skipping 63 matching lines @@
       ui::AX_ATTR_ACTION, l10n_util::GetStringUTF8(IDS_APP_ACCACTION_PRESS));
   node_data->AddStateFlag(ui::AX_STATE_HASPOPUP);
 }
 
 std::unique_ptr<views::LabelButtonBorder> ToolbarButton::CreateDefaultBorder()
     const {
   std::unique_ptr<views::LabelButtonBorder> border =
       views::LabelButton::CreateDefaultBorder();
 
   if (ThemeServiceFactory::GetForProfile(profile_)->UsingSystemTheme())
-    border->set_insets(gfx::Insets(GetLayoutConstant(TOOLBAR_BUTTON_PADDING)));
+    border->set_insets(gfx::Insets(kInteriorPadding));
 
   return border;
 }
 
 void ToolbarButton::ShowContextMenuForView(View* source,
                                            const gfx::Point& point,
                                            ui::MenuSourceType source_type) {
   if (!enabled())
     return;
 
@@ skipping 74 matching lines @@
   if (state() != STATE_DISABLED)
     SetState(STATE_NORMAL);
 
   menu_runner_.reset();
   menu_model_adapter_.reset();
 }
 
 const char* ToolbarButton::GetClassName() const {
   return "ToolbarButton";
 }