Omnibox/Views: Refactor layout constants.

Pull out a couple of hard-coded layout constants in LocationBarView and put them
in layout_constants.*. Also audit the uses of these layout constants and update
places that should be using them.

Bug: 801583
Change-Id: Ia157cd313020a17fa4f8ebda662545395c29855b
Reviewed-on: https://chromium-review.googlesource.com/923503Reviewed-by: Trent Apted <tapted@chromium.org>
Commit-Queue: Patti <patricialor@chromium.org>
Cr-Commit-Position: refs/heads/master@{#537572}

chrome/browser/ui/layout_constants.cc

@@ -30,8 +30,14 @@ int GetLayoutConstant(LayoutConstant constant) {
       return hybrid ? 8 : 6;
     case LOCATION_BAR_ELEMENT_PADDING:
       return hybrid ? 3 : 1;
+    case LOCATION_BAR_PADDING:
+      return hybrid ? 3 : 1;
     case LOCATION_BAR_HEIGHT:
       return hybrid ? 32 : 28;
+    case LOCATION_BAR_ICON_SIZE:
+      return 16;
+    case LOCATION_BAR_ICON_INTERIOR_PADDING:
+      return 4;
     case TABSTRIP_NEW_TAB_BUTTON_OVERLAP:
       return hybrid ? 6 : 5;
     case TAB_HEIGHT: {

chrome/browser/ui/layout_constants.h

@@ -26,13 +26,26 @@ enum LayoutConstant {
   // images inside.
   LOCATION_BAR_BUBBLE_ANCHOR_VERTICAL_INSET,
 
-  // The horizontal padding between location bar decorations as well as the
-  // vertical and horizontal padding inside the border.
+  // The horizontal padding between location bar decorations.
   LOCATION_BAR_ELEMENT_PADDING,
 
+  // The padding inside the location bar border (i.e. between the border and the
+  // location bar's children).
+  LOCATION_BAR_PADDING,
+
   // The height to be occupied by the LocationBar.
   LOCATION_BAR_HEIGHT,
 
+  // The size of the icons used inside the LocationBar.
+  LOCATION_BAR_ICON_SIZE,
+
+  // The amount of padding used around the icon inside the LocationBar, i.e. the
+  // full width of a LocationBar icon will be LOCATION_BAR_ICON_SIZE + 2 *
+  // LOCATION_BAR_ICON_INTERIOR_PADDING. Icons may additionally be spaced
+  // horizontally by LOCATION_BAR_ELEMENT_PADDING, but this region is not part
+  // of the icon view (e.g. does not highlight on hover).
+  LOCATION_BAR_ICON_INTERIOR_PADDING,
+
   // The amount of overlap between the last tab and the new tab button.
   TABSTRIP_NEW_TAB_BUTTON_OVERLAP,
 

chrome/browser/ui/views/location_bar/bubble_icon_view.cc

@@ -73,7 +73,8 @@ bool BubbleIconView::GetTooltipText(const gfx::Point& p,
 
 gfx::Size BubbleIconView::CalculatePreferredSize() const {
   gfx::Rect image_rect(image_->GetPreferredSize());
-  image_rect.Inset(-gfx::Insets(LocationBarView::kIconInteriorPadding));
+  image_rect.Inset(
+      -gfx::Insets(GetLayoutConstant(LOCATION_BAR_ICON_INTERIOR_PADDING)));
   DCHECK_EQ(image_rect.height(),
             GetLayoutConstant(LOCATION_BAR_HEIGHT) -
                 2 * (GetLayoutConstant(LOCATION_BAR_ELEMENT_PADDING) +
@@ -232,7 +233,7 @@ void BubbleIconView::UpdateIcon() {
               ui::NativeTheme::kColorId_ProminentButtonColor)
           : GetInkDropBaseColor();
   image_->SetImage(gfx::CreateVectorIcon(
-      GetVectorIcon(), LocationBarView::kIconWidth, icon_color));
+      GetVectorIcon(), GetLayoutConstant(LOCATION_BAR_ICON_SIZE), icon_color));
 }
 
 void BubbleIconView::SetActiveInternal(bool active) {

chrome/browser/ui/views/location_bar/icon_label_bubble_view.cc

@@ -110,7 +110,7 @@ IconLabelBubbleView::IconLabelBubbleView(const gfx::FontList& font_list)
   // |image_| as a separate mouse hover region from |this|.
   image_->set_can_process_events_within_subtree(false);
   image_->SetBorder(views::CreateEmptyBorder(
-      gfx::Insets(LocationBarView::kIconInteriorPadding)));
+      gfx::Insets(GetLayoutConstant(LOCATION_BAR_ICON_INTERIOR_PADDING))));
   AddChildView(image_);
 
   label_->SetHorizontalAlignment(gfx::ALIGN_LEFT);

chrome/browser/ui/views/location_bar/icon_label_bubble_view_unittest.cc

@@ -81,9 +81,10 @@ class TestIconLabelBubbleView : public IconLabelBubbleView {
 
   bool ShouldShowLabel() const override {
     return !IsShrinking() ||
-           (width() > (image()->GetPreferredSize().width() +
-                       2 * LocationBarView::kIconInteriorPadding +
-                       2 * GetLayoutConstant(LOCATION_BAR_ELEMENT_PADDING)));
+           (width() >
+            (image()->GetPreferredSize().width() +
+             2 * GetLayoutConstant(LOCATION_BAR_ICON_INTERIOR_PADDING) +
+             2 * GetLayoutConstant(LOCATION_BAR_ELEMENT_PADDING)));
   }
 
   double WidthMultiplier() const override {

chrome/browser/ui/views/location_bar/keyword_hint_view.cc

@@ -126,7 +126,7 @@ gfx::Size KeywordHintView::CalculatePreferredSize() const {
   return gfx::Size(leading_label_->GetPreferredSize().width() +
                        chip_container_->GetPreferredSize().width() +
                        trailing_label_->GetPreferredSize().width() +
-                       LocationBarView::kIconInteriorPadding,
+                       GetLayoutConstant(LOCATION_BAR_ICON_INTERIOR_PADDING),
                    0);
 }
 

chrome/browser/ui/views/location_bar/location_bar_view.cc

@@ -232,7 +232,8 @@ void LocationBarView::Init() {
     ContentSettingImageView* image_view =
         new ContentSettingImageView(std::move(model), this, font_list);
     content_setting_views_.push_back(image_view);
-    image_view->set_next_element_interior_padding(kIconInteriorPadding);
+    image_view->set_next_element_interior_padding(
+        GetLayoutConstant(LOCATION_BAR_ICON_INTERIOR_PADDING));
     image_view->SetVisible(false);
     AddChildView(image_view);
   }
@@ -431,7 +432,7 @@ gfx::Size LocationBarView::CalculatePreferredSize() const {
         location_icon_view_->GetMinimumSizeForLabelText(GetLocationIconText())
             .width();
   } else {
-    leading_width += GetLayoutConstant(LOCATION_BAR_ELEMENT_PADDING) +
+    leading_width += GetLayoutConstant(LOCATION_BAR_PADDING) +
                      location_icon_view_->GetMinimumSize().width();
   }
 
@@ -454,7 +455,7 @@ gfx::Size LocationBarView::CalculatePreferredSize() const {
   }
 
   min_size.set_width(leading_width + omnibox_view_->GetMinimumSize().width() +
-                     2 * GetLayoutConstant(LOCATION_BAR_ELEMENT_PADDING) -
+                     2 * GetLayoutConstant(LOCATION_BAR_PADDING) -
                      omnibox_view_->GetInsets().width() + trailing_width);
   return min_size;
 }
@@ -710,7 +711,7 @@ int LocationBarView::GetHorizontalEdgeThickness() const {
 
 int LocationBarView::GetTotalVerticalPadding() const {
   return BackgroundWith1PxBorder::kLocationBarBorderThicknessDip +
-         GetLayoutConstant(LOCATION_BAR_ELEMENT_PADDING);
+         GetLayoutConstant(LOCATION_BAR_PADDING);
 }
 
 void LocationBarView::RefreshLocationIcon() {
@@ -726,7 +727,8 @@ void LocationBarView::RefreshLocationIcon() {
                            ? color_utils::DeriveDefaultIconColor(GetColor(TEXT))
                            : GetSecureTextColor(security_level);
   location_icon_view_->SetImage(gfx::CreateVectorIcon(
-      omnibox_view_->GetVectorIcon(), kIconWidth, icon_color));
+      omnibox_view_->GetVectorIcon(), GetLayoutConstant(LOCATION_BAR_ICON_SIZE),
+      icon_color));
   location_icon_view_->Update();
 }
 

chrome/browser/ui/views/location_bar/location_bar_view.h

@@ -98,14 +98,6 @@ class LocationBarView : public LocationBar,
     SECURITY_CHIP_TEXT,
   };
 
-  // Visual width (and height) of icons in location bar.
-  static constexpr int kIconWidth = 16;
-
-  // The amount of padding between the visual edge of an icon and the edge of
-  // its click target, for all all sides of the icon. The total edge length of
-  // each icon view should be kIconWidth + 2 * kIconInteriorPadding.
-  static constexpr int kIconInteriorPadding = 4;
-
   // The location bar view's class name.
   static const char kViewClassName[];
 

chrome/browser/ui/views/location_bar/selected_keyword_view.cc

@@ -7,6 +7,7 @@
 #include "base/logging.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/search_engines/template_url_service_factory.h"
+#include "chrome/browser/ui/layout_constants.h"
 #include "chrome/browser/ui/views/location_bar/location_bar_view.h"
 #include "chrome/grit/generated_resources.h"
 #include "components/search_engines/template_url_service.h"
@@ -33,7 +34,8 @@ SelectedKeywordView::~SelectedKeywordView() {
 
 void SelectedKeywordView::ResetImage() {
   SetImage(gfx::CreateVectorIcon(vector_icons::kSearchIcon,
-                                 LocationBarView::kIconWidth, GetTextColor()));
+                                 GetLayoutConstant(LOCATION_BAR_ICON_SIZE),
+                                 GetTextColor()));
 }
 
 SkColor SelectedKeywordView::GetTextColor() const {

chrome/browser/ui/views/omnibox/omnibox_result_view.cc

@@ -234,8 +234,9 @@ OmniboxResultView::OmniboxResultView(OmniboxPopupContentsView* model,
   CHECK_GE(model_index, 0);
   keyword_icon_->set_owned_by_client();
   keyword_icon_->EnableCanvasFlippingForRTLUI(true);
-  keyword_icon_->SetImage(gfx::CreateVectorIcon(omnibox::kKeywordSearchIcon, 16,
-                                                GetVectorIconColor()));
+  keyword_icon_->SetImage(gfx::CreateVectorIcon(
+      omnibox::kKeywordSearchIcon, GetLayoutConstant(LOCATION_BAR_ICON_SIZE),
+      GetVectorIconColor()));
   keyword_icon_->SizeToPreferredSize();
 }
 
@@ -645,7 +646,8 @@ int OmniboxResultView::GetVerticalMargin() const {
       omnibox::kUIExperimentVerticalMargin,
       OmniboxFieldTrial::kUIVerticalMarginParam,
       Md::GetMode() == Md::MATERIAL_HYBRID ? 8 : 4);
-  const int min_height = LocationBarView::kIconWidth + 2 * kIconVerticalPad;
+  const int min_height =
+      GetLayoutConstant(LOCATION_BAR_ICON_SIZE) + 2 * kIconVerticalPad;
 
   return std::max(kVerticalMargin, (min_height - GetTextHeight()) / 2);
 }
@@ -743,8 +745,8 @@ void OmniboxResultView::SetHovered(bool hovered) {
 
 void OmniboxResultView::Layout() {
   const int horizontal_padding =
-      GetLayoutConstant(LOCATION_BAR_ELEMENT_PADDING) +
-      LocationBarView::kIconInteriorPadding;
+      GetLayoutConstant(LOCATION_BAR_PADDING) +
+      GetLayoutConstant(LOCATION_BAR_ICON_INTERIOR_PADDING);
   // The horizontal bounds we're given are the outside bounds, so we can match
   // the omnibox border outline shape exactly in OnPaint().  We have to inset
   // here to keep the icons lined up.
@@ -761,7 +763,8 @@ void OmniboxResultView::Layout() {
   const int icon_y = GetVerticalMargin() + (row_height - icon.Height()) / 2;
   icon_bounds_.SetRect(start_x, icon_y, icon.Width(), icon.Height());
 
-  const int text_x = start_x + LocationBarView::kIconWidth + horizontal_padding;
+  const int text_x =
+      start_x + GetLayoutConstant(LOCATION_BAR_ICON_SIZE) + horizontal_padding;
   int text_width = end_x - text_x;
 
   if (match_.associated_keyword.get()) {