Add textfield internal padding from FocusableBorder.

chrome/browser/ui/views/find_bar_view.cc

Index: chrome/browser/ui/views/find_bar_view.cc
diff --git a/chrome/browser/ui/views/find_bar_view.cc b/chrome/browser/ui/views/find_bar_view.cc
index 7ec9e1a93d6c4e15d57bd1064a3c529d0989229e..19827157adaa3cfb9a126d24a3e81a43379aaeaa 100644
--- a/chrome/browser/ui/views/find_bar_view.cc
+++ b/chrome/browser/ui/views/find_bar_view.cc
@@ -37,12 +37,14 @@
 
 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;

[Peter Kasting, 2014/09/05 22:56:48]

Nit: Should we collapse this and kMarginLeftOfMatchCountLabel together, under
the theory that it's really "kMarginRightOfTextfield"?

[msw, 2014/09/05 23:46:55]

That might be reasonable for follow-up, but I'm not sure that
kMarginLeftOfCloseButton is immediately right of the textfield if the match
count label and the find next/prev buttons are all between the textfield and the
close button, so I'll leave it as-is for now.

 const int kMarginRightOfCloseButton = 7;
-const int kMarginLeftOfMatchCountLabel = 2;
+const int kMarginLeftOfMatchCountLabel = 3;
 const int kMarginRightOfMatchCountLabel = 1;
 const int kMarginLeftOfFindTextfield = 12;
+const int kMarginAboveFindTextfield = 6;

[Peter Kasting, 2014/09/05 22:56:48]

Nit: Maybe this and the next constant should just be one?  (I can't think of why
we'd want to make the vertical spacing uneven.)

[msw, 2014/09/05 23:46:55]

Done. I'm not entirely sure that these values are optimal, since the
IDR_TEXTFIELD assets have some nearly transparent pixels at the top and bottom
of the interior space, but they match the current layout.

+const int kMarginBelowFindTextfield = 6;
 
 // The margins around the match count label (We add extra space so that the
 // background highlight extends beyond just the text).
@@ -318,17 +320,21 @@ void FindBarView::Layout() {
   sz.SetToMax(gfx::Size(kMatchCountMinWidth, 0));
   int match_count_x =
       find_previous_button_->x() - kMarginRightOfMatchCountLabel - sz.width();
-  int find_text_y = (height() - find_text_->GetPreferredSize().height()) / 2;
+  int find_text_y = kMarginAboveFindTextfield;

[Peter Kasting, 2014/09/05 22:56:49]

Interesting, is it no longer possible to use the preferred height to compute
this?

[msw, 2014/09/05 23:46:55]

This approach is preferable to the current code for several reasons:
(1) The textfield is given the entire vertical space available to it, which may
be needed if the fallback text exceeds the additional 6px that this change
provides. (unlikely, but possible)
(2) This lets the textfield center via the baseline technique, rather than
combining that with some additional naive control centering logic within the
vertical space.
(3) The find bar textfield currentlt exceeds the available height with
excessively tall system fonts (eg. Segoe UI 15pt), which causes the textfield
control's white background to spill over the find bar assets. This change fixes
that defect (but obviously we'll need to expand the control or clamp the font
height [further] in the long term).
(4) Avoiding potentially expensice GetPreferredSize calls is always good when
the preferred size isn't actually needed.

   match_count_text_->SetBounds(match_count_x,
                                find_text_y + 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.
+  int margin_a = kMarginLeftOfFindTextfield - views::Textfield::kTextPadding;
+  int margin_b = kMarginLeftOfMatchCountLabel - views::Textfield::kTextPadding;

[Peter Kasting, 2014/09/05 22:56:48]

Nit: I'm not in love with "margin_a" and "margin_b" as names... maybe
"left_margin" and roll the other temp into the width calculation below?

[msw, 2014/09/05 23:46:55]

Done.

+  int find_text_width = std::max(0, match_count_x - margin_a - margin_b);
+  find_text_->SetBounds(
+      margin_a,
+      find_text_y,
+      find_text_width,
+      height() - kMarginAboveFindTextfield - kMarginBelowFindTextfield);
 
   // 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
@@ -346,7 +352,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 -
+                       2 * views::Textfield::kTextPadding,
                    0);
   prefsize.Enlarge(find_previous_button_->GetPreferredSize().width(), 0);
   prefsize.Enlarge(find_next_button_->GetPreferredSize().width(), 0);