Modified OverviewMode's LabelButton bounds to cover the entire item.

ui/views/controls/button/label_button.cc

 // Copyright (c) 2012 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 "ui/views/controls/button/label_button.h"
 
 #include "base/lazy_instance.h"
 #include "base/logging.h"
 #include "ui/gfx/animation/throb_animation.h"
 #include "ui/gfx/canvas.h"
@@ skipping 52 matching lines @@
       image_(new ImageView()),
       label_(new Label()),
       cached_normal_font_list_(GetDefaultNormalFontList()),
       cached_bold_font_list_(GetDefaultBoldFontList()),
       button_state_images_(),
       button_state_colors_(),
       explicitly_set_colors_(),
       is_default_(false),
       style_(STYLE_TEXTBUTTON),
       border_is_themed_border_(true),
-      image_label_spacing_(kSpacing) {
+      image_label_spacing_(kSpacing),
+      ignore_vertical_overlap_(true) {
   SetAnimationDuration(kHoverAnimationDurationMs);
   SetText(text);
 
   AddChildView(image_);
   image_->set_interactive(false);
 
   AddChildView(label_);
   label_->SetFontList(cached_normal_font_list_);
   label_->SetAutoColorReadabilityEnabled(false);
   label_->SetHorizontalAlignment(gfx::ALIGN_LEFT);
@@ skipping 206 matching lines @@
   gfx::Rect child_area(GetChildAreaBounds());
   child_area.Inset(GetInsets());
 
   gfx::Size image_size(image_->GetPreferredSize());
   image_size.SetToMin(child_area.size());
 
   // The label takes any remaining width after sizing the image, unless both
   // views are centered. In that case, using the tighter preferred label width
   // avoids wasted space within the label that would look like awkward padding.
   // Labels can paint over the full button height, including the border height.
-  gfx::Size label_size(child_area.width(), height());
+  gfx::Size label_size(child_area.width(), ignore_vertical_overlap_
+                                               ? height()
+                                               : child_area.height());
   if (!image_size.IsEmpty() && !label_size.IsEmpty()) {
     label_size.set_width(std::max(child_area.width() -
         image_size.width() - image_label_spacing_, 0));
     if (adjusted_alignment == gfx::ALIGN_CENTER) {
       // Ensure multi-line labels paired with images use their available width.
       label_size.set_width(
           std::min(label_size.width(), label_->GetPreferredSize().width()));
     }
   }
 
   gfx::Point image_origin(child_area.origin());
   image_origin.Offset(0, (child_area.height() - image_size.height()) / 2);
   if (adjusted_alignment == gfx::ALIGN_CENTER) {
     const int spacing = (image_size.width() > 0 && label_size.width() > 0) ?
         image_label_spacing_ : 0;
     const int total_width = image_size.width() + label_size.width() +
         spacing;
     image_origin.Offset((child_area.width() - total_width) / 2, 0);
   } else if (adjusted_alignment == gfx::ALIGN_RIGHT) {
     image_origin.Offset(child_area.width() - image_size.width(), 0);
   }
 
-  gfx::Point label_origin(child_area.x(), 0);
+  gfx::Point label_origin(child_area.x(),
+                          ignore_vertical_overlap_ ? 0 : child_area.y());

[flackr, 2015/01/30 15:34:04]

Out of curiosity, can you tell from the git history if ignoring the border was
added on purpose? I wonder if it was just overlooked, or if it's only required
for a particular view perhaps we could respect the border overlap by default and
only ignore it for that particular case.

[Nina, 2015/01/30 15:44:32]

It's required by the bookmark bar:
https://codereview.chromium.org/424663008
https://code.google.com/p/chromium/issues/detail?id=397995#c3
So the original behaviour was respecting the border and it was changed later.
Wouldn't ignoring it for a particular case require repetition of most code on
Layout()?

[sky, 2015/02/03 21:40:02]

This seems suspicious to me. If BookmarkBar wants custom behavior it should be
tweaking something, not everyone else.

[msw, 2015/02/03 23:50:19]

Ignoring the vertical insets for text rendering was done to match the legacy
behavior of view::TextButton, it wasn't just for bookmark bar buttons. The
bookmark bar was just a prominent case where this change was needed (because it
clamps buttons to a fixed short height). Keeping the default as true for
|ignore_vertical_overlap_| seems okay to me; otherwise we'll need to audit all
the LabelButton instances to ensure that this wouldn't cut off tall text (for
example when fallback font heights exceed the system font height, which seems to
often be the case for Hindi text on Windows 7, etc.).

[Nina, 2015/02/04 16:20:11]

Acknowledged.

[flackr, 2015/02/04 16:39:08]

So it sounds like if we ignore borders for the purpose of clipping (i.e.
label_size) but respect them for positioning we could do away with
ignore_vertical_overlap_ right?

   if (!image_size.IsEmpty() && adjusted_alignment != gfx::ALIGN_RIGHT) {
     label_origin.set_x(image_origin.x() + image_size.width() +
         image_label_spacing_);
   }
 
   image_->SetBoundsRect(gfx::Rect(image_origin, image_size));
   label_->SetBoundsRect(gfx::Rect(label_origin, label_size));
 }
 
 const char* LabelButton::GetClassName() const {
@@ skipping 175 matching lines @@
   GetExtraParams(params);
   return ui::NativeTheme::kHovered;
 }
 
 void LabelButton::ResetCachedPreferredSize() {
   cached_preferred_size_valid_ = false;
   cached_preferred_size_= gfx::Size();
 }
 
 }  // namespace views