Add textfield internal padding from FocusableBorder.

Add 3px of internal padding for textfield/combobox content.
Remove corresponding unused padding from FocusableBorder.
This better accommodates excessively tall fallback font text.
Update Omnibox, Find Bar, App List, and CookieInfoView.

See before/after pics at http://crbug.com/404999#c51

BUG=404999
TEST=Bookmark bubble/dialog, find bar, etc. don't clip most tall fallback text.
R=sky@chromium.org,pkasting@chromium.org

Review URL: https://codereview.chromium.org/516943003

Cr-Commit-Position: refs/heads/master@{#294016}

chrome/browser/ui/views/cookie_info_view.cc

@@ -26,13 +26,6 @@
 #include "ui/views/layout/layout_constants.h"
 #include "ui/views/window/dialog_delegate.h"
 
-namespace {
-
-// Adjustment to the spacing between subsequent label-field lines.
-const int kExtraLineHeightPadding = 3;
-
-}  // namespace
-
 ///////////////////////////////////////////////////////////////////////////////
 // CookieInfoView, public:
 
@@ -117,22 +110,22 @@ void CookieInfoView::ViewHierarchyChanged(
     Init();
 }
 
-void CookieInfoView::AddLabelRow(int layout_id, views::GridLayout* layout,
+void CookieInfoView::AddLabelRow(int layout_id,
+                                 views::GridLayout* layout,
                                  views::Label* label,
-                                 views::Textfield* text_field) {
+                                 views::Textfield* textfield) {
   layout->StartRow(0, layout_id);
   layout->AddView(label);
-  layout->AddView(text_field, 2, 1, views::GridLayout::FILL,
-                  views::GridLayout::CENTER);
-  layout->AddPaddingRow(0, kExtraLineHeightPadding);
+  layout->AddView(
+      textfield, 2, 1, views::GridLayout::FILL, views::GridLayout::CENTER);
 
   // Now that the Textfield is in the view hierarchy, it can be initialized.
-  text_field->SetReadOnly(true);
-  text_field->SetBorder(views::Border::NullBorder());
+  textfield->SetReadOnly(true);
+  textfield->SetBorder(views::Border::NullBorder());
   // Color these borderless text areas the same as the containing dialog.
-  text_field->SetBackgroundColor(GetNativeTheme()->GetSystemColor(
+  textfield->SetBackgroundColor(GetNativeTheme()->GetSystemColor(
       ui::NativeTheme::kColorId_DialogBackground));
-  text_field->SetTextColor(SkColorSetRGB(0x78, 0x78, 0x78));
+  textfield->SetTextColor(SkColorSetRGB(0x78, 0x78, 0x78));
 }
 
 ///////////////////////////////////////////////////////////////////////////////

chrome/browser/ui/views/cookie_info_view.h

@@ -53,8 +53,10 @@ class CookieInfoView : public views::View {
 
  private:
   // Layout helper routines.
-  void AddLabelRow(int layout_id, views::GridLayout* layout,
-                   views::Label* label, views::Textfield* value);
+  void AddLabelRow(int layout_id,
+                   views::GridLayout* layout,
+                   views::Label* label,
+                   views::Textfield* textfield);
 
   // Sets up the view layout.
   void Init();

chrome/browser/ui/views/find_bar_view.cc

@@ -37,12 +37,13 @@
 
 namespace {
 
-// The margins around the search field, match count label, and the close button.
+// The margins around the UI controls, derived from assets and design specs.
 const int kMarginLeftOfCloseButton = 3;
 const int kMarginRightOfCloseButton = 7;
-const int kMarginLeftOfMatchCountLabel = 2;
+const int kMarginLeftOfMatchCountLabel = 3;
 const int kMarginRightOfMatchCountLabel = 1;
 const int kMarginLeftOfFindTextfield = 12;
+const int kMarginVerticalFindTextfield = 6;
 
 // The margins around the match count label (We add extra space so that the
 // background highlight extends beyond just the text).
@@ -316,24 +317,27 @@ void FindBarView::Layout() {
   // of breathing room (margins around the text).
   sz.Enlarge(kMatchCountExtraWidth, 0);
   sz.SetToMax(gfx::Size(kMatchCountMinWidth, 0));
-  int match_count_x =
+  const int match_count_x =
       find_previous_button_->x() - kMarginRightOfMatchCountLabel - sz.width();
-  int find_text_y = (height() - find_text_->GetPreferredSize().height()) / 2;
+  const int find_text_y = kMarginVerticalFindTextfield;
+  const gfx::Insets find_text_insets(find_text_->GetInsets());
   match_count_text_->SetBounds(match_count_x,
-                               find_text_y + find_text_->GetBaseline() -
+                               find_text_y - find_text_insets.top() +
+                                   find_text_->GetBaseline() -
                                    match_count_text_->GetBaseline(),
                                sz.width(), sz.height());
 
-  // And whatever space is left in between, gets filled up by the find edit box.
-  int find_text_width = std::max(0, match_count_x -
-      kMarginLeftOfMatchCountLabel - kMarginLeftOfFindTextfield);
-  find_text_->SetBounds(kMarginLeftOfFindTextfield, find_text_y,
-      find_text_width, find_text_->GetPreferredSize().height());
+  // Fill the remaining width and available height with the textfield.
+  const int left_margin = kMarginLeftOfFindTextfield - find_text_insets.left();
+  const int find_text_width = std::max(0, match_count_x - left_margin -
+      kMarginLeftOfMatchCountLabel + find_text_insets.right());
+  find_text_->SetBounds(left_margin, find_text_y, find_text_width,
+                        height() - 2 * kMarginVerticalFindTextfield);
 
   // The focus forwarder view is a hidden view that should cover the area
   // between the find text box and the find button so that when the user clicks
   // in that area we focus on the find text box.
-  int find_text_edge = find_text_->x() + find_text_->width();
+  const int find_text_edge = find_text_->x() + find_text_->width();
   focus_forwarder_view_->SetBounds(
       find_text_edge, find_previous_button_->y(),
       find_previous_button_->x() - find_text_edge,
@@ -346,7 +350,8 @@ gfx::Size FindBarView::GetPreferredSize() const {
 
   // Add up all the preferred sizes and margins of the rest of the controls.
   prefsize.Enlarge(kMarginLeftOfCloseButton + kMarginRightOfCloseButton +
-                       kMarginLeftOfFindTextfield,
+                       kMarginLeftOfFindTextfield -
+                       find_text_->GetInsets().width(),
                    0);
   prefsize.Enlarge(find_previous_button_->GetPreferredSize().width(), 0);
   prefsize.Enlarge(find_next_button_->GetPreferredSize().width(), 0);

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

@@ -600,7 +600,6 @@ gfx::Size LocationBarView::GetPreferredSize() const {
     leading_width +=
         kItemPadding + location_icon_view_->GetMinimumSize().width();
   }
-  leading_width += kItemPadding - GetEditLeadingInternalSpace();
 
   // Compute width of omnibox-trailing content.
   int trailing_width = search_button_->visible() ?
@@ -612,7 +611,7 @@ gfx::Size LocationBarView::GetPreferredSize() const {
       IncrementalMinimumWidth(manage_passwords_icon_view_) +
       IncrementalMinimumWidth(zoom_view_) +
       IncrementalMinimumWidth(generated_credit_card_view_) +
-      IncrementalMinimumWidth(mic_search_view_) + kItemPadding;
+      IncrementalMinimumWidth(mic_search_view_);
   for (PageActionViews::const_iterator i(page_action_views_.begin());
        i != page_action_views_.end(); ++i)
     trailing_width += IncrementalMinimumWidth((*i));
@@ -620,8 +619,8 @@ gfx::Size LocationBarView::GetPreferredSize() const {
        i != content_setting_views_.end(); ++i)
     trailing_width += IncrementalMinimumWidth((*i));
 
-  min_size.set_width(
-      leading_width + omnibox_view_->GetMinimumSize().width() + trailing_width);
+  min_size.set_width(leading_width + omnibox_view_->GetMinimumSize().width() +
+      2 * kItemPadding - omnibox_view_->GetInsets().width() + trailing_width);
   return min_size;
 }
 
@@ -637,9 +636,11 @@ void LocationBarView::Layout() {
 
   LocationBarLayout leading_decorations(
       LocationBarLayout::LEFT_EDGE,
-      kItemPadding - GetEditLeadingInternalSpace());
-  LocationBarLayout trailing_decorations(LocationBarLayout::RIGHT_EDGE,
-                                         kItemPadding);
+      kItemPadding - omnibox_view_->GetInsets().left() -
+          GetEditLeadingInternalSpace());
+  LocationBarLayout trailing_decorations(
+      LocationBarLayout::RIGHT_EDGE,
+      kItemPadding - omnibox_view_->GetInsets().right());
 
   const int origin_chip_preferred_width =
       origin_chip_view_->GetPreferredSize().width();
@@ -932,9 +933,8 @@ void LocationBarView::Layout() {
     }
   }
 
-  const gfx::Insets insets = omnibox_view_->GetInsets();
-  omnibox_view_->SetBorder(views::Border::CreateEmptyBorder(
-      insets.top(), insets.left(), insets.bottom(), omnibox_view_margin));
+  omnibox_view_->SetBorder(
+      views::Border::CreateEmptyBorder(0, 0, 0, omnibox_view_margin));
 
   // Layout |ime_inline_autocomplete_view_| next to the user input.
   if (ime_inline_autocomplete_view_->visible()) {

ui/app_list/views/folder_header_view.cc

@@ -167,7 +167,8 @@ void FolderHeaderView::Layout() {
                             : folder_name_placeholder_text_;
   int text_width =
       gfx::Canvas::GetStringWidth(text, folder_name_view_->GetFontList()) +
-      folder_name_view_->GetCaretBounds().width();
+      folder_name_view_->GetCaretBounds().width() +
+      folder_name_view_->GetInsets().width();
   text_width = std::min(text_width, kMaxFolderNameWidth);
   text_bounds.set_x(back_bounds.x() + (rect.width() - text_width) / 2);
   text_bounds.set_width(text_width);

ui/app_list/views/search_box_view.cc

@@ -94,8 +94,11 @@ SearchBoxView::SearchBoxView(SearchBoxViewDelegate* delegate,
     AddChildView(icon_view_);
   }
 
-  views::BoxLayout* layout = new views::BoxLayout(
-      views::BoxLayout::kHorizontal, kPadding, 0, kPadding);
+  views::BoxLayout* layout =
+      new views::BoxLayout(views::BoxLayout::kHorizontal,
+                           kPadding,
+                           0,
+                           kPadding - views::Textfield::kTextPadding);
   SetLayoutManager(layout);
   layout->set_cross_axis_alignment(
       views::BoxLayout::CROSS_AXIS_ALIGNMENT_CENTER);

ui/views/controls/combobox/combobox.cc

@@ -32,6 +32,7 @@
 #include "ui/views/controls/menu/menu_runner_handler.h"
 #include "ui/views/controls/menu/submenu_view.h"
 #include "ui/views/controls/prefix_selector.h"
+#include "ui/views/controls/textfield/textfield.h"
 #include "ui/views/ime/input_method.h"
 #include "ui/views/mouse_constants.h"
 #include "ui/views/painter.h"
@@ -410,6 +411,10 @@ gfx::Size Combobox::GetPreferredSize() const {
   // The preferred size will drive the local bounds which in turn is used to set
   // the minimum width for the dropdown list.
   gfx::Insets insets = GetInsets();
+  insets += gfx::Insets(Textfield::kTextPadding,
+                        Textfield::kTextPadding,
+                        Textfield::kTextPadding,
+                        Textfield::kTextPadding);
   int total_width = std::max(kMinComboboxWidth, content_size_.width()) +
       insets.width() + GetDisclosureArrowLeftPadding() +
       ArrowSize().width() + GetDisclosureArrowRightPadding();
@@ -628,7 +633,7 @@ void Combobox::UpdateFromModel() {
 void Combobox::UpdateBorder() {
   scoped_ptr<FocusableBorder> border(new FocusableBorder());
   if (style_ == STYLE_ACTION)
-    border->SetInsets(8, 13, 8, 13);
+    border->SetInsets(5, 10, 5, 10);
   if (invalid_)
     border->SetColor(kWarningColor);
   SetBorder(border.PassAs<Border>());
@@ -640,6 +645,7 @@ void Combobox::AdjustBoundsForRTLUI(gfx::Rect* rect) const {
 
 void Combobox::PaintText(gfx::Canvas* canvas) {
   gfx::Insets insets = GetInsets();
+  insets += gfx::Insets(0, Textfield::kTextPadding, 0, Textfield::kTextPadding);
 
   gfx::ScopedCanvas scoped_canvas(canvas);
   canvas->ClipRect(GetContentsBounds());

ui/views/controls/focusable_border.cc

@@ -11,23 +11,21 @@
 
 namespace {
 
-// Define the size of the insets
-const int kTopInsetSize = 4;
-const int kLeftInsetSize = 4;
-const int kBottomInsetSize = 4;
-const int kRightInsetSize = 4;
+const int kInsetSize = 1;
 
 }  // namespace
 
 namespace views {
 
 FocusableBorder::FocusableBorder()
-    : insets_(kTopInsetSize, kLeftInsetSize,
-              kBottomInsetSize, kRightInsetSize),
+    : insets_(kInsetSize, kInsetSize, kInsetSize, kInsetSize),
       override_color_(SK_ColorWHITE),
       use_default_color_(true) {
 }
 
+FocusableBorder::~FocusableBorder() {
+}
+
 void FocusableBorder::SetColor(SkColor color) {
   override_color_ = color;
   use_default_color_ = false;

ui/views/controls/focusable_border.h

@@ -21,6 +21,7 @@ namespace views {
 class VIEWS_EXPORT FocusableBorder : public Border {
  public:
   FocusableBorder();
+  virtual ~FocusableBorder();
 
   // Sets the insets of the border.
   void SetInsets(int top, int left, int bottom, int right);

ui/views/controls/textfield/textfield.cc

@@ -238,6 +238,7 @@ int GetViewsCommand(const ui::TextEditCommandAuraLinux& command, bool rtl) {
 
 // static
 const char Textfield::kViewClassName[] = "Textfield";
+const int Textfield::kTextPadding = 3;
 
 // static
 size_t Textfield::GetCaretBlinkMs() {
@@ -550,6 +551,12 @@ bool Textfield::HasTextBeingDragged() {
 ////////////////////////////////////////////////////////////////////////////////
 // Textfield, View overrides:
 
+gfx::Insets Textfield::GetInsets() const {
+  gfx::Insets insets = View::GetInsets();
+  insets += gfx::Insets(kTextPadding, kTextPadding, kTextPadding, kTextPadding);
+  return insets;
+}
+
 int Textfield::GetBaseline() const {
   return GetInsets().top() + GetRenderText()->GetBaseline();
 }
@@ -919,7 +926,15 @@ void Textfield::GetAccessibleState(ui::AXViewState* state) {
 }
 
 void Textfield::OnBoundsChanged(const gfx::Rect& previous_bounds) {
-  GetRenderText()->SetDisplayRect(GetContentsBounds());
+  // Textfield insets include a reasonable amount of whitespace on all sides of
+  // the default font list. Fallback fonts with larger heights may paint over
+  // the vertical whitespace as needed. Alternate solutions involve undesirable
+  // behavior like changing the default font size, shrinking some fallback fonts
+  // beyond their legibility, or enlarging controls dynamically with content.
+  gfx::Rect bounds = GetContentsBounds();
+  // GetContentsBounds() does not actually use the local GetInsets() override.
+  bounds.Inset(gfx::Insets(0, kTextPadding, 0, kTextPadding));
+  GetRenderText()->SetDisplayRect(bounds);
   OnCaretBoundsChanged();
 }
 

ui/views/controls/textfield/textfield.h

@@ -45,6 +45,9 @@ class VIEWS_EXPORT Textfield : public View,
   // The textfield's class name.
   static const char kViewClassName[];
 
+  // The preferred size of the padding to be used around textfield text.
+  static const int kTextPadding;
+
   // Returns the text cursor blink time in milliseconds, or 0 for no blinking.
   static size_t GetCaretBlinkMs();
 
@@ -202,6 +205,7 @@ class VIEWS_EXPORT Textfield : public View,
   bool HasTextBeingDragged();
 
   // View overrides:
+  virtual gfx::Insets GetInsets() const OVERRIDE;
   virtual int GetBaseline() const OVERRIDE;
   virtual gfx::Size GetPreferredSize() const OVERRIDE;
   virtual const char* GetClassName() const OVERRIDE;