The close button for bubbles should be an ImageButton, not a LabelButton.

ui/views/bubble/bubble_frame_view.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/bubble/bubble_frame_view.h"
 
 #include <algorithm>
 #include <utility>
 
 #include "build/build_config.h"
 #include "ui/base/default_style.h"
 #include "ui/base/hit_test.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/material_design/material_design_controller.h"
 #include "ui/base/resource/resource_bundle.h"
 #include "ui/compositor/paint_context.h"
 #include "ui/compositor/paint_recorder.h"
 #include "ui/display/display.h"
 #include "ui/display/screen.h"
 #include "ui/gfx/geometry/vector2d.h"
 #include "ui/gfx/path.h"
 #include "ui/gfx/skia_util.h"
 #include "ui/gfx/vector_icons_public.h"
 #include "ui/native_theme/native_theme.h"
 #include "ui/resources/grit/ui_resources.h"
 #include "ui/strings/grit/ui_strings.h"
 #include "ui/views/bubble/bubble_border.h"
-#include "ui/views/controls/button/label_button.h"
 #include "ui/views/controls/button/vector_icon_button.h"
 #include "ui/views/controls/image_view.h"
+#include "ui/views/controls/label.h"
 #include "ui/views/layout/box_layout.h"
 #include "ui/views/layout/layout_constants.h"
 #include "ui/views/resources/grit/views_resources.h"
 #include "ui/views/widget/widget.h"
 #include "ui/views/widget/widget_delegate.h"
 #include "ui/views/window/client_view.h"
 
 namespace views {
 
 namespace {
 
 // Background color of the footnote view.
 const SkColor kFootnoteBackgroundColor = SkColorSetRGB(245, 245, 245);
 
 // Color of the top border of the footnote.
 const SkColor kFootnoteBorderColor = SkColorSetRGB(229, 229, 229);
 
 constexpr int kClosePaddingRight = 7;
-constexpr int kClosePaddingTop = 6;
+constexpr int kClosePaddingTop = 7;
 
 // The MD spec states that the center of the "x" should be 16x16 from the top
 // right of the dialog.
 constexpr int kClosePaddingRightMd = 4;
 constexpr int kClosePaddingTopMd = 5;
 
 // Get the |vertical| or horizontal amount that |available_bounds| overflows
 // |window_bounds|.
 int GetOffScreenLength(const gfx::Rect& available_bounds,
                        const gfx::Rect& window_bounds,
@@ skipping 49 matching lines @@
   // the HTCLOSE result from NonClientHitTest().
   close_->SetTooltipText(base::string16());
 #endif
   AddChildView(close_);
 }
 
 BubbleFrameView::~BubbleFrameView() {}
 
 // static
 Button* BubbleFrameView::CreateCloseButton(VectorIconButtonDelegate* delegate) {
-  Button* close_button = nullptr;
+  ImageButton* close_button = nullptr;
   if (ui::MaterialDesignController::IsSecondaryUiMaterial()) {
     VectorIconButton* close = new VectorIconButton(delegate);
     close->SetIcon(gfx::VectorIconId::BAR_CLOSE);
-    close->SetSize(close->GetPreferredSize());
     close_button = close;
   } else {
-    ui::ResourceBundle& rb = ui::ResourceBundle::GetSharedInstance();
-    LabelButton* close = new LabelButton(delegate, base::string16());
-    close->SetImage(CustomButton::STATE_NORMAL,
-                    *rb.GetImageNamed(IDR_CLOSE_DIALOG).ToImageSkia());
-    close->SetImage(CustomButton::STATE_HOVERED,
-                    *rb.GetImageNamed(IDR_CLOSE_DIALOG_H).ToImageSkia());
-    close->SetImage(CustomButton::STATE_PRESSED,
-                    *rb.GetImageNamed(IDR_CLOSE_DIALOG_P).ToImageSkia());
-    close->SetBorder(nullptr);
-    close->SetSize(close->GetPreferredSize());
-    close_button = close;
+    ui::ResourceBundle* rb = &ui::ResourceBundle::GetSharedInstance();
+    close_button = new ImageButton(delegate);
+    close_button->SetImage(CustomButton::STATE_NORMAL,
+                           *rb->GetImageNamed(IDR_CLOSE_DIALOG).ToImageSkia());
+    close_button->SetImage(
+        CustomButton::STATE_HOVERED,
+        *rb->GetImageNamed(IDR_CLOSE_DIALOG_H).ToImageSkia());
+    close_button->SetImage(
+        CustomButton::STATE_PRESSED,
+        *rb->GetImageNamed(IDR_CLOSE_DIALOG_P).ToImageSkia());
   }
   close_button->SetTooltipText(l10n_util::GetStringUTF16(IDS_APP_CLOSE));
+  close_button->SizeToPreferredSize();
   return close_button;
 }
 
 gfx::Rect BubbleFrameView::GetBoundsForClientView() const {
   gfx::Rect client_bounds = GetContentsBounds();
   client_bounds.Inset(GetInsets());
   if (footnote_container_) {
     client_bounds.set_height(client_bounds.height() -
                              footnote_container_->height());
   }
@@ skipping 159 matching lines @@
   // preferred size.
   return GetPreferredSize();
 #endif
 }
 
 void BubbleFrameView::Layout() {
   // The title margins may not be set, but make sure that's only the case when
   // there's no title.
   DCHECK(!title_margins_.IsEmpty() || !title_->visible());
 
-  gfx::Rect bounds(GetContentsBounds());
+  const gfx::Rect contents_bounds = GetContentsBounds();
+  gfx::Rect bounds = contents_bounds;
   bounds.Inset(title_margins_);
   if (bounds.IsEmpty())
     return;
 
   // The close button is positioned somewhat closer to the edge of the bubble.
-  gfx::Point close_position = GetContentsBounds().top_right();
+  gfx::Point close_position = contents_bounds.top_right();
   if (ui::MaterialDesignController::IsSecondaryUiMaterial()) {

[msw, 2017/01/31 17:53:16]

q: did you remember to test MD and non-MD?

[Peter Kasting, 2017/01/31 21:31:01]

The Harmony close button is a VectorIconButton instead, so it's not actually
affected by this change.

     close_position += gfx::Vector2d(-close_->width() - kClosePaddingRightMd,
                                     kClosePaddingTopMd);
   } else {
     close_position +=
         gfx::Vector2d(-close_->width() - kClosePaddingRight, kClosePaddingTop);
   }
   close_->SetPosition(close_position);
 
   gfx::Size title_icon_pref_size(title_icon_->GetPreferredSize());
   int padding = 0;
@@ skipping 10 matching lines @@
     title_->SetPosition(gfx::Point(
         title_label_x, bounds.y() + (title_height - title_->height()) / 2));
   }
 
   title_icon_->SetBounds(bounds.x(), bounds.y(), title_icon_pref_size.width(),
                          title_height);
   bounds.set_width(title_->bounds().right() - bounds.x());
   bounds.set_height(title_height);
 
   if (footnote_container_) {
-    gfx::Rect local_bounds = GetContentsBounds();
-    int height = footnote_container_->GetHeightForWidth(local_bounds.width());
-    footnote_container_->SetBounds(local_bounds.x(),
-                                   local_bounds.bottom() - height,
-                                   local_bounds.width(), height);
+    const int width = contents_bounds.width();
+    const int height = footnote_container_->GetHeightForWidth(width);
+    footnote_container_->SetBounds(
+        contents_bounds.x(), contents_bounds.bottom() - height, width, height);
   }
 }
 
 void BubbleFrameView::OnThemeChanged() {
   UpdateWindowTitle();
   ResetWindowControls();
   UpdateWindowIcon();
 }
 
 void BubbleFrameView::OnNativeThemeChanged(const ui::NativeTheme* theme) {
@@ skipping 171 matching lines @@
   size.Enlarge(client_insets.width(), client_insets.height());
   size.SetToMax(gfx::Size(title_bar_width, 0));
 
   if (footnote_container_)
     size.Enlarge(0, footnote_container_->GetHeightForWidth(size.width()));
 
   return size;
 }
 
 }  // namespace views