linux_aura: Fixes a couple of issues with the button border integration.

chrome/browser/ui/libgtk2ui/gtk2_border.cc

Index: chrome/browser/ui/libgtk2ui/gtk2_border.cc
diff --git a/chrome/browser/ui/libgtk2ui/gtk2_border.cc b/chrome/browser/ui/libgtk2ui/gtk2_border.cc
index 57b6f5da5c0aa506a4d56bac684d4b3cb1d8e5d3..fbe7c6225095ed86eadc68f1ef7da9e8d5beeef0 100644
--- a/chrome/browser/ui/libgtk2ui/gtk2_border.cc
+++ b/chrome/browser/ui/libgtk2ui/gtk2_border.cc
@@ -7,34 +7,54 @@
 #include <gtk/gtk.h>
 
 #include "chrome/browser/ui/libgtk2ui/gtk2_ui.h"
+#include "third_party/skia/include/effects/SkLerpXfermode.h"
+#include "ui/gfx/animation/animation.h"
 #include "ui/gfx/canvas.h"
 #include "ui/gfx/image/image_skia_source.h"
-#include "ui/views/controls/button/custom_button.h"
+#include "ui/gfx/rect.h"
+#include "ui/gfx/skia_util.h"
+#include "ui/views/controls/button/label_button.h"
+#include "ui/views/controls/button/label_button_border.h"
+#include "ui/views/native_theme_delegate.h"
+
+using views::Button;
+using views::NativeThemeDelegate;
 
 namespace libgtk2ui {
 
 namespace {
 
-GtkStateType ViewsToGtkState(views::Button::ButtonState state) {
+Button::ButtonState GetViewsButtonState(ui::NativeTheme::State state) {

[msw, 2014/01/24 00:22:37]

nit: Rather than duplicate this label_button_border.cc function, make it a
Button class static helper function or similar.

[Elliot Glaysher, 2014/01/24 21:56:00]

Done.

   switch (state) {
-    case views::Button::STATE_HOVERED:
-      return GTK_STATE_PRELIGHT;
-    case views::Button::STATE_PRESSED:
-      return GTK_STATE_ACTIVE;
-    case views::Button::STATE_DISABLED:
-      return GTK_STATE_INSENSITIVE;
-    default:
-      return GTK_STATE_NORMAL;
+    case ui::NativeTheme::kDisabled: return Button::STATE_DISABLED;
+    case ui::NativeTheme::kHovered:  return Button::STATE_HOVERED;
+    case ui::NativeTheme::kNormal:   return Button::STATE_NORMAL;
+    case ui::NativeTheme::kPressed:  return Button::STATE_PRESSED;
+    case ui::NativeTheme::kMaxState: NOTREACHED() << "Unknown state: " << state;
   }
+  return Button::STATE_NORMAL;
+}
+
+GtkStateType GetGtkState(ui::NativeTheme::State state) {
+  switch (state) {
+    case ui::NativeTheme::kDisabled: return GTK_STATE_INSENSITIVE;
+    case ui::NativeTheme::kHovered:  return GTK_STATE_PRELIGHT;
+    case ui::NativeTheme::kNormal:   return GTK_STATE_NORMAL;
+    case ui::NativeTheme::kPressed:  return GTK_STATE_ACTIVE;
+    case ui::NativeTheme::kMaxState: NOTREACHED() << "Unknown state: " << state;
+  }
+  return GTK_STATE_NORMAL;
 }
 
 class ButtonImageSkiaSource : public gfx::ImageSkiaSource {
  public:
   ButtonImageSkiaSource(const Gtk2UI* gtk2_ui,
-                        GtkStateType state,
+                        const GtkStateType state,
+                        const bool focused,
                         const gfx::Size& size)
       : gtk2_ui_(gtk2_ui),
         state_(state),
+        focused_(focused),
         size_(size) {
   }
 
@@ -45,13 +65,14 @@ class ButtonImageSkiaSource : public gfx::ImageSkiaSource {
     int w = size_.width() * scale;
     int h = size_.height() * scale;
     return gfx::ImageSkiaRep(
-        gtk2_ui_->DrawGtkButtonBorder(state_, w, h), scale);
+        gtk2_ui_->DrawGtkButtonBorder(state_, focused_, w, h), scale);
   }
 
  private:
   const Gtk2UI* gtk2_ui_;
-  GtkStateType state_;
-  gfx::Size size_;
+  const GtkStateType state_;
+  const bool focused_;
+  const gfx::Size size_;
 
   DISALLOW_COPY_AND_ASSIGN(ButtonImageSkiaSource);
 };
@@ -59,8 +80,8 @@ class ButtonImageSkiaSource : public gfx::ImageSkiaSource {
 }  // namespace
 
 Gtk2Border::Gtk2Border(Gtk2UI* gtk2_ui,
-                       views::CustomButton* owning_button,
-                       views::Border* border)
+                       views::LabelButton* owning_button,
+                       views::LabelButtonBorder* border)
     : gtk2_ui_(gtk2_ui),
       use_gtk_(gtk2_ui_->GetUseSystemTheme()),
       owning_button_(owning_button),
@@ -73,8 +94,9 @@ Gtk2Border::~Gtk2Border() {
 }
 
 void Gtk2Border::InvalidateAndSetUsesGtk(bool use_gtk) {
-  hovered_ = gfx::ImageSkia();
-  pressed_ = gfx::ImageSkia();
+  for (int i = 0; i < 2; ++i)
+    for (int j = 0; j < views::Button::STATE_COUNT; ++j)
+      button_images_[i][j] = gfx::ImageSkia();
 
   // Our owning view must have its layout invalidated because the insets could
   // have changed.
@@ -90,22 +112,32 @@ void Gtk2Border::Paint(const views::View& view, gfx::Canvas* canvas) {
   }
 
   DCHECK_EQ(&view, owning_button_);
-  GtkStateType state = ViewsToGtkState(owning_button_->state());
-
-  if (state == GTK_STATE_PRELIGHT) {
-    if (hovered_.isNull() || hovered_.size() != view.size()) {
-      hovered_ = gfx::ImageSkia(
-          new ButtonImageSkiaSource(gtk2_ui_, state, view.size()), view.size());
-    }
-
-    canvas->DrawImageInt(hovered_, 0, 0);
-  } else if (state == GTK_STATE_ACTIVE) {
-    if (pressed_.isNull() || pressed_.size() != view.size()) {
-      pressed_ = gfx::ImageSkia(
-          new ButtonImageSkiaSource(gtk2_ui_, state, view.size()), view.size());
-    }
-
-    canvas->DrawImageInt(pressed_, 0, 0);
+  const NativeThemeDelegate* native_theme_delegate = owning_button_;
+  gfx::Rect rect(native_theme_delegate->GetThemePaintRect());

[msw, 2014/01/24 00:22:37]

Rather than copy this from LabelButtonBorder, does it make any sense to add a
virtual LabelButtonBorder::PaintState, and then make this a LabelButtonBorder
subclass which overloads that function (or similar)?

[Elliot Glaysher, 2014/01/24 21:56:00]

I don't think so. Several people working on views have a strong preference for
composition over inheritance, and I'm not sure that a Gtk2Border is-a
LabelButtonBorder.

+  ui::NativeTheme::ExtraParams extra;
+  ui::NativeTheme::State state = native_theme_delegate->GetThemeState(&extra);
+
+  const gfx::Animation* animation = native_theme_delegate->GetThemeAnimation();
+  if (animation && animation->is_animating()) {
+    // Linearly interpolate background and foreground painters during
+    // animation.

[msw, 2014/01/24 00:22:37]

nit: fits on line above, just like where this is copied from.

+    const SkRect sk_rect = gfx::RectToSkRect(rect);
+    canvas->sk_canvas()->saveLayer(&sk_rect, NULL);
+    state = native_theme_delegate->GetBackgroundThemeState(&extra);
+    PaintState(state, extra, rect, canvas);
+
+    SkPaint paint;
+    skia::RefPtr<SkXfermode> sk_lerp_xfer =
+        skia::AdoptRef(SkLerpXfermode::Create(animation->GetCurrentValue()));
+    paint.setXfermode(sk_lerp_xfer.get());
+    canvas->sk_canvas()->saveLayer(&sk_rect, &paint);
+    state = native_theme_delegate->GetForegroundThemeState(&extra);
+    PaintState(state, extra, rect, canvas);
+    canvas->sk_canvas()->restore();
+
+    canvas->sk_canvas()->restore();
+  } else {
+    PaintState(state, extra, rect, canvas);
   }
 }
 
@@ -124,4 +156,27 @@ gfx::Size Gtk2Border::GetMinimumSize() const {
   return gfx::Size(insets.width(), insets.height());
 }
 
+void Gtk2Border::PaintState(const ui::NativeTheme::State state,
+                            const ui::NativeTheme::ExtraParams& extra,
+                            const gfx::Rect& rect,
+                            gfx::Canvas* canvas) {
+  bool focused = extra.button.is_focused;
+  Button::ButtonState views_state = GetViewsButtonState(state);
+
+  // If the LabelButtonBorder has a painter for our focused/state combination,

[msw, 2014/01/24 00:22:37]

Why do you check this? Perhaps just check the button's style (STYLE_BUTTON for
bubbles/dialogs, and STYLE_TEXTBUTTON for back/forward/reload) to know what
focused/state images to skip? But doesn't GTK draw something for
back/forward/reload when they are unfocused and normal (not hovered or pressed),
while views doesn't? It seems like you should just always try to get the GTK
appearance.

[Elliot Glaysher, 2014/01/24 21:56:00]

Taking these two questions out of order...

On 2014/01/24 00:22:37, msw wrote:
Not on toolbars. On toolbars, buttons don't have borders in the normal state.
My recollection was that there were cases where this was set outside of the
LabelButtonBorder constructor; this patch started as a fix for crbug.com/337081,
before I realized that the real fix was to never show GTK-ish rendering in the
content area. However, I can't find any places where it would actually make a
difference.

I've changed this, but that said, this feels fragile. It seems to me to be
slightly more robust to ask the LabelButtonBorder about whether a border would
be drawn in this case.

[msw, 2014/01/24 23:57:16]

To clarify, will back/forward/reload behave differently than bookmarks? Views
just has the two button styles. I like the new explicit ShouldDrawBorder code.

[Elliot Glaysher, 2014/01/25 00:38:19]

In the current GTK port, there is no visual difference between the
back/forward/reload buttons and the bookmark buttons. My hope is that in the gtk
mode of linux_aura, there will be no visual difference either.

+  // that means it was configured to draw a border then, which means that we
+  // should draw a GTKish border.
+  if (border_->GetPainter(focused, views_state)) {
+    gfx::ImageSkia* image = &button_images_[focused][views_state];
+
+    if (image->isNull() || image->size() != rect.size()) {
+      GtkStateType gtk_state = GetGtkState(state);
+      *image = gfx::ImageSkia(
+          new ButtonImageSkiaSource(gtk2_ui_, gtk_state, focused, rect.size()),
+          rect.size());
+    }
+    canvas->DrawImageInt(*image, rect.x(), rect.y());
+  }
+}
+
 }  // namespace libgtk2ui