linux_aura: Fix the insets on LabelButtons.

Starting in r266097, we always used the insets provided by Gtk2Border. We had previously had a hacky solution where we tried to determine whether the button was a toolbar button by digging in its style, but that hack was removed because it overreached. We had this hack to deal with slightly larger icon assets that GTK handed us.

In r269180, there was a subtle timing change, which then meant  that buttons outside the toolbar started using the GTK insets, such as buttons in the extension removal dialog, the javascript alert dialog, etc.

Now, Gtk2Border always uses the insets provided by the views::LabelButtonBorder (and we no longer have a concept of "GTK insets"), and we explicitly set smaller insets that don't cause the clipping issues only in ToolbarButton. This lets both bugs be fixed at the same time.

BUG=372305
TEST=Using GTK+ theme mode, the left edge of the back button shouldn't be visually chopped off.
TEST=Using GTK+ theme mode, The OK button on a javascript alert shouldn't be vertically squashed.
TEST=Using the default theme, both of the above should look normal.
R=msw@chromium.org, sky@chromium.org

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@272326 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/ui/libgtk2ui/gtk2_border.cc

@@ -72,9 +72,11 @@ class ButtonImageSkiaSource : public gfx::ImageSkiaSource {
 }  // namespace
 
 Gtk2Border::Gtk2Border(Gtk2UI* gtk2_ui,
-                       views::LabelButton* owning_button)
+                       views::LabelButton* owning_button,
+                       scoped_ptr<views::Border> border)
     : gtk2_ui_(gtk2_ui),
       owning_button_(owning_button),
+      border_(border.Pass()),
       observer_manager_(this) {
   observer_manager_.Add(NativeThemeGtk2::instance());
 }
@@ -113,14 +115,11 @@ void Gtk2Border::Paint(const views::View& view, gfx::Canvas* canvas) {
 }
 
 gfx::Insets Gtk2Border::GetInsets() const {
-  // On STYLE_TEXTUBTTON, we want the smaller insets so we can fit the GTK icon
-  // in the toolbar without cutting off the edges of the GTK image.
-  return gtk2_ui_->GetButtonInsets();
+  return border_->GetInsets();
 }
 
 gfx::Size Gtk2Border::GetMinimumSize() const {
-  gfx::Insets insets = GetInsets();
-  return gfx::Size(insets.width(), insets.height());
+  return border_->GetMinimumSize();
 }
 
 void Gtk2Border::OnNativeThemeUpdated(ui::NativeTheme* observed_theme) {

chrome/browser/ui/libgtk2ui/gtk2_border.h

@@ -28,7 +28,8 @@ class Gtk2UI;
 class Gtk2Border : public views::Border, public ui::NativeThemeObserver {
  public:
   Gtk2Border(Gtk2UI* gtk2_ui,
-             views::LabelButton* owning_button);
+             views::LabelButton* owning_button,
+             scoped_ptr<views::Border> border);
   virtual ~Gtk2Border();
 
   // Overridden from views::Border:
@@ -55,6 +56,10 @@ class Gtk2Border : public views::Border, public ui::NativeThemeObserver {
   // force invalidate the layout on theme changes.
   views::LabelButton* owning_button_;
 
+  // The views::Border that we are replacing in native mode. We keep track of
+  // this for inset information.
+  scoped_ptr<views::Border> border_;
+
   ScopedObserver<ui::NativeTheme, ui::NativeThemeObserver> observer_manager_;
 
   DISALLOW_COPY_AND_ASSIGN(Gtk2Border);

chrome/browser/ui/libgtk2ui/gtk2_ui.cc

@@ -559,7 +559,8 @@ scoped_ptr<views::Border> Gtk2UI::CreateNativeBorder(
   if (owning_button->GetNativeTheme() != NativeThemeGtk2::instance())
     return border.Pass();
 
-  return scoped_ptr<views::Border>(new Gtk2Border(this, owning_button));
+  return scoped_ptr<views::Border>(
+      new Gtk2Border(this, owning_button, border.Pass()));
 }
 
 void Gtk2UI::AddWindowButtonOrderObserver(
@@ -940,9 +941,6 @@ void Gtk2UI::LoadGtkValues() {
       GdkColorToSkColor(entry_style->base[GTK_STATE_ACTIVE]);
   inactive_selection_fg_color_ =
       GdkColorToSkColor(entry_style->text[GTK_STATE_ACTIVE]);
-
-  // Update the insets that we hand to Gtk2Border.
-  UpdateButtonInsets();
 }
 
 GdkColor Gtk2UI::BuildFrameColors(GtkStyle* frame_style) {
@@ -1359,34 +1357,6 @@ SkBitmap Gtk2UI::DrawGtkButtonBorder(int gtk_state,
   return border;
 }
 
-gfx::Insets Gtk2UI::GetButtonInsets() const {
-  return button_insets_;
-}
-
-void Gtk2UI::UpdateButtonInsets() {
-  GtkWidget* window = gtk_offscreen_window_new();
-  GtkWidget* button = gtk_button_new();
-  gtk_container_add(GTK_CONTAINER(window), button);
-
-  GtkBorder* border = NULL;
-  gtk_widget_style_get(GTK_WIDGET(button),
-                       "default-border",
-                       &border,
-                       NULL);
-
-  gfx::Insets insets;
-  if (border) {
-    button_insets_ = gfx::Insets(border->top, border->left,
-                                 border->bottom, border->right);
-    gtk_border_free(border);
-  } else {
-    // Defined in gtkbutton.c:
-    button_insets_ = gfx::Insets(1, 1, 1, 1);
-  }
-
-  gtk_widget_destroy(window);
-}
-
 void Gtk2UI::ClearAllThemeData() {
   gtk_images_.clear();
 }

chrome/browser/ui/libgtk2ui/gtk2_ui.h

@@ -17,7 +17,6 @@
 #include "chrome/browser/ui/libgtk2ui/owned_widget_gtk2.h"
 #include "ui/events/linux/text_edit_key_bindings_delegate_auralinux.h"
 #include "ui/gfx/color_utils.h"
-#include "ui/gfx/geometry/insets.h"
 #include "ui/views/linux_ui/linux_ui.h"
 #include "ui/views/window/frame_buttons.h"
 
@@ -60,9 +59,6 @@ class Gtk2UI : public views::LinuxUI {
                                int width,
                                int height) const;
 
-  // Returns the current insets for a button.
-  gfx::Insets GetButtonInsets() const;
-
   // ui::LinuxInputMethodContextFactory:
   virtual scoped_ptr<ui::LinuxInputMethodContext> CreateInputMethodContext(
       ui::LinuxInputMethodContextDelegate* delegate) const OVERRIDE;
@@ -190,10 +186,6 @@ class Gtk2UI : public views::LinuxUI {
   // entry.
   void GetSelectedEntryForegroundHSL(color_utils::HSL* tint) const;
 
-  // Create a GTK window and button and queries what "default-border" is, which
-  // corresponds with our insets.
-  void UpdateButtonInsets();
-
   // Frees all calculated images and color data.
   void ClearAllThemeData();
 
@@ -229,8 +221,6 @@ class Gtk2UI : public views::LinuxUI {
   SkColor inactive_selection_bg_color_;
   SkColor inactive_selection_fg_color_;
 
-  gfx::Insets button_insets_;
-
 #if defined(USE_GCONF)
   // Currently, the only source of window button configuration. This will
   // change if we ever have to support XFCE's configuration system or KDE's.

chrome/browser/ui/views/toolbar/back_button.cc

@@ -38,14 +38,16 @@ void BackButton::SetLeadingMargin(int margin) {
   InvalidateLayout();
 }
 
-scoped_ptr<views::Border> BackButton::CreateDefaultBorder() const {
+scoped_ptr<views::LabelButtonBorder> BackButton::CreateDefaultBorder() const {
+  scoped_ptr<views::LabelButtonBorder> border =
+      ToolbarButton::CreateDefaultBorder();
+
   // Adjust border insets to follow the margin change,
   // which will be reflected in where the border is painted
   // through |GetThemePaintRect|.
-  scoped_ptr<views::LabelButtonBorder> border(
-      new views::LabelButtonBorder(style()));
   const gfx::Insets insets(border->GetInsets());
   border->set_insets(gfx::Insets(insets.top(), insets.left() + margin_leading_,
                                  insets.bottom(), insets.right()));
-  return border.PassAs<views::Border>();
+
+  return border.Pass();
 }

chrome/browser/ui/views/toolbar/back_button.h

@@ -31,7 +31,8 @@ class BackButton : public ToolbarButton {
 
  protected:
   virtual gfx::Rect GetThemePaintRect() const OVERRIDE;
-  virtual scoped_ptr<views::Border> CreateDefaultBorder() const OVERRIDE;
+  virtual scoped_ptr<views::LabelButtonBorder> CreateDefaultBorder() const
+      OVERRIDE;
 
  private:
   // Any leading margin to be applied. Used when the back button is in

chrome/browser/ui/views/toolbar/toolbar_button.cc

@@ -11,6 +11,7 @@
 #include "ui/accessibility/ax_view_state.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/models/menu_model.h"
+#include "ui/base/theme_provider.h"
 #include "ui/gfx/display.h"
 #include "ui/gfx/screen.h"
 #include "ui/views/controls/button/label_button_border.h"
@@ -128,6 +129,21 @@ void ToolbarButton::GetAccessibleState(ui::AXViewState* state) {
   state->AddStateFlag(ui::AX_STATE_HASPOPUP);
 }
 
+scoped_ptr<views::LabelButtonBorder>
+ToolbarButton::CreateDefaultBorder() const {
+  scoped_ptr<views::LabelButtonBorder> border =
+      LabelButton::CreateDefaultBorder();
+
+  ui::ThemeProvider* provider = GetThemeProvider();
+  if (provider && provider->UsingSystemTheme()) {
+    // We set smaller insets here to accommodate the slightly larger GTK+
+    // icons.
+    border->set_insets(gfx::Insets(2, 2, 2, 2));
+  }
+
+  return border.Pass();
+}
+
 void ToolbarButton::ShowContextMenuForView(View* source,
                                            const gfx::Point& point,
                                            ui::MenuSourceType source_type) {

chrome/browser/ui/views/toolbar/toolbar_button.h

@@ -45,6 +45,8 @@ class ToolbarButton : public views::LabelButton,
   virtual void OnMouseExited(const ui::MouseEvent& event) OVERRIDE;
   virtual void OnGestureEvent(ui::GestureEvent* event) OVERRIDE;
   virtual void GetAccessibleState(ui::AXViewState* state) OVERRIDE;
+  virtual scoped_ptr<views::LabelButtonBorder> CreateDefaultBorder() const
+      OVERRIDE;
 
   // views::ContextMenuController:
   virtual void ShowContextMenuForView(View* source,

ui/views/controls/button/blue_button.cc

@@ -45,7 +45,7 @@ const char* BlueButton::GetClassName() const {
   return BlueButton::kViewClassName;
 }
 
-scoped_ptr<Border> BlueButton::CreateDefaultBorder() const {
+scoped_ptr<LabelButtonBorder> BlueButton::CreateDefaultBorder() const {
   // Insets for splitting the images.
   const gfx::Insets insets(5, 5, 5, 5);
   scoped_ptr<LabelButtonBorder> button_border(new LabelButtonBorder(style()));
@@ -66,7 +66,7 @@ scoped_ptr<Border> BlueButton::CreateDefaultBorder() const {
       *rb.GetImageSkiaNamed(IDR_BLUE_BUTTON_FOCUSED_PRESSED), insets));
   button_border->SetPainter(true, STATE_DISABLED, Painter::CreateImagePainter(
       *rb.GetImageSkiaNamed(IDR_BLUE_BUTTON_DISABLED), insets));
-  return button_border.PassAs<Border>();
+  return button_border.Pass();
 }
 
 }  // namespace views

ui/views/controls/button/blue_button.h

@@ -22,7 +22,7 @@ class VIEWS_EXPORT BlueButton : public LabelButton {
   // Overridden from LabelButton:
   virtual void ResetColorsFromNativeTheme() OVERRIDE;
   virtual const char* GetClassName() const OVERRIDE;
-  virtual scoped_ptr<Border> CreateDefaultBorder() const OVERRIDE;
+  virtual scoped_ptr<LabelButtonBorder> CreateDefaultBorder() const OVERRIDE;
 
   DISALLOW_COPY_AND_ASSIGN(BlueButton);
 };

ui/views/controls/button/label_button.cc

@@ -266,8 +266,8 @@ const char* LabelButton::GetClassName() const {
   return kViewClassName;
 }
 
-scoped_ptr<Border> LabelButton::CreateDefaultBorder() const {
-  return scoped_ptr<Border>(new LabelButtonBorder(style_));
+scoped_ptr<LabelButtonBorder> LabelButton::CreateDefaultBorder() const {
+  return scoped_ptr<LabelButtonBorder>(new LabelButtonBorder(style_));
 }
 
 void LabelButton::SetBorder(scoped_ptr<Border> border) {
@@ -360,16 +360,17 @@ void LabelButton::UpdateThemedBorder() {
   if (!border_is_themed_border_)
     return;
 
-  scoped_ptr<Border> label_button_border = CreateDefaultBorder();
+  scoped_ptr<LabelButtonBorder> label_button_border = CreateDefaultBorder();
 
 #if defined(OS_LINUX) && !defined(OS_CHROMEOS)
   views::LinuxUI* linux_ui = views::LinuxUI::instance();
   if (linux_ui) {
-    SetBorder(linux_ui->CreateNativeBorder(this, label_button_border.Pass()));
+    SetBorder(linux_ui->CreateNativeBorder(
+        this, label_button_border.PassAs<Border>()));
   } else
 #endif
   {
-    SetBorder(label_button_border.Pass());
+    SetBorder(label_button_border.PassAs<Border>());
   }
 
   border_is_themed_border_ = true;

ui/views/controls/button/label_button.h

@@ -97,7 +97,7 @@ class VIEWS_EXPORT LabelButton : public CustomButton,
 
   // Creates the default border for this button. This can be overridden by
   // subclasses or by LinuxUI.
-  virtual scoped_ptr<Border> CreateDefaultBorder() const;
+  virtual scoped_ptr<LabelButtonBorder> CreateDefaultBorder() const;
 
   // Updates the image view to contain the appropriate button state image.
   void UpdateImage();