Use a delegation to override FocusRing behavior for checkboxes and radio buttons.

Bug: 706297
Bug: 683858
Change-Id: Ibb51dbacc26164ead556488da18fb322350fddff
Reviewed-on: https://chromium-review.googlesource.com/546697
Commit-Queue: Allen Bauer <kylixrd@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Michael Wasserman <msw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#485963}

ui/views/controls/button/checkbox.cc

@@ -14,9 +14,11 @@
 #include "ui/gfx/canvas.h"
 #include "ui/gfx/color_utils.h"
 #include "ui/gfx/paint_vector_icon.h"
+#include "ui/native_theme/native_theme.h"
 #include "ui/views/animation/ink_drop_impl.h"
 #include "ui/views/animation/ink_drop_ripple.h"
 #include "ui/views/controls/button/label_button_border.h"
+#include "ui/views/controls/focus_ring.h"
 #include "ui/views/layout/layout_provider.h"
 #include "ui/views/painter.h"
 #include "ui/views/resources/grit/views_resources.h"
@@ -25,6 +27,45 @@
 
 namespace views {
 
+// View used to paint the focus ring around the Checkbox icon.
+// The icon is painted separately.
+class IconFocusRing : public View {
+ public:
+  explicit IconFocusRing(Checkbox* checkbox);
+  ~IconFocusRing() override = default;
+
+ private:
+  // View:
+  void Layout() override;
+  void OnPaint(gfx::Canvas* canvas) override;
+
+  Checkbox* checkbox_;
+
+  DISALLOW_COPY_AND_ASSIGN(IconFocusRing);
+};
+
+IconFocusRing::IconFocusRing(Checkbox* checkbox) : checkbox_(checkbox) {
+  FocusRing::InitFocusRing(this);
+}
+
+void IconFocusRing::Layout() {
+  gfx::Rect focus_bounds = checkbox_->image()->bounds();
+  focus_bounds.Inset(gfx::Insets(-2.f));
+  SetBoundsRect(focus_bounds);
+}
+
+void IconFocusRing::OnPaint(gfx::Canvas* canvas) {
+  cc::PaintFlags focus_flags;
+  focus_flags.setAntiAlias(true);
+  focus_flags.setColor(
+      SkColorSetA(GetNativeTheme()->GetSystemColor(
+                      ui::NativeTheme::kColorId_FocusedBorderColor),
+                  0x66));
+  focus_flags.setStyle(cc::PaintFlags::kStroke_Style);
+  focus_flags.setStrokeWidth(2);
+  checkbox_->PaintFocusRing(this, canvas, focus_flags);
+}
+
 // static
 const char Checkbox::kViewClassName[] = "Checkbox";
 
@@ -39,6 +80,9 @@ Checkbox::Checkbox(const base::string16& label)
     set_request_focus_on_press(false);
     SetInkDropMode(InkDropMode::ON);
     set_has_ink_drop_action_on_click(true);
+    focus_ring_ = new IconFocusRing(this);
+    focus_ring_->SetVisible(false);
+    AddChildView(focus_ring_);
   } else {
     std::unique_ptr<LabelButtonBorder> button_border(new LabelButtonBorder());
     // Inset the trailing side by a couple pixels for the focus border.
@@ -128,12 +172,16 @@ void Checkbox::OnFocus() {
   LabelButton::OnFocus();
   if (!UseMd())
     UpdateImage();
+  else
+    focus_ring_->SetVisible(true);
 }
 
 void Checkbox::OnBlur() {
   LabelButton::OnBlur();
   if (!UseMd())
     UpdateImage();
+  else
+    focus_ring_->SetVisible(false);
 }
 
 void Checkbox::OnNativeThemeChanged(const ui::NativeTheme* theme) {
@@ -161,21 +209,6 @@ SkColor Checkbox::GetInkDropBaseColor() const {
       ui::NativeTheme::kColorId_LabelEnabledColor);
 }
 
-void Checkbox::PaintButtonContents(gfx::Canvas* canvas) {
-  if (!UseMd() || !HasFocus())
-    return;
-
-  cc::PaintFlags focus_flags;
-  focus_flags.setAntiAlias(true);
-  focus_flags.setColor(
-      SkColorSetA(GetNativeTheme()->GetSystemColor(
-                      ui::NativeTheme::kColorId_FocusedBorderColor),
-                  0x66));
-  focus_flags.setStyle(cc::PaintFlags::kStroke_Style);
-  focus_flags.setStrokeWidth(2);
-  PaintFocusRing(canvas, focus_flags);
-}
-
 gfx::ImageSkia Checkbox::GetImage(ButtonState for_state) const {
   if (UseMd()) {
     return gfx::CreateVectorIcon(
@@ -212,10 +245,10 @@ void Checkbox::SetCustomImage(bool checked,
   UpdateImage();
 }
 
-void Checkbox::PaintFocusRing(gfx::Canvas* canvas,
+void Checkbox::PaintFocusRing(View* view,
+                              gfx::Canvas* canvas,
                               const cc::PaintFlags& flags) {
-  gfx::RectF focus_rect(image()->bounds());
-  canvas->DrawRoundRect(focus_rect, 2.f, flags);
+  canvas->DrawRoundRect(view->GetLocalBounds(), 2.f, flags);
 }
 
 const gfx::VectorIcon& Checkbox::GetVectorIcon() const {

ui/views/controls/button/checkbox.h

@@ -49,7 +49,6 @@ class VIEWS_EXPORT Checkbox : public LabelButton {
   std::unique_ptr<InkDrop> CreateInkDrop() override;
   std::unique_ptr<InkDropRipple> CreateInkDropRipple() const override;
   SkColor GetInkDropBaseColor() const override;
-  void PaintButtonContents(gfx::Canvas* canvas) override;
   gfx::ImageSkia GetImage(ButtonState for_state) const override;
   std::unique_ptr<LabelButtonBorder> CreateDefaultBorder() const override;
 
@@ -60,13 +59,17 @@ class VIEWS_EXPORT Checkbox : public LabelButton {
                       ButtonState for_state,
                       const gfx::ImageSkia& image);
 
-  // Paints a focus indicator for the view.
-  virtual void PaintFocusRing(gfx::Canvas* canvas, const cc::PaintFlags& flags);
+  // Paints a focus indicator for the view. Overridden in RadioButton.
+  virtual void PaintFocusRing(View* view,
+                              gfx::Canvas* canvas,
+                              const cc::PaintFlags& flags);
 
   // Gets the vector icon to use based on the current state of |checked_|.
   virtual const gfx::VectorIcon& GetVectorIcon() const;
 
  private:
+  friend class IconFocusRing;
+
   // Button:
   void NotifyClick(const ui::Event& event) override;
 
@@ -76,6 +79,9 @@ class VIEWS_EXPORT Checkbox : public LabelButton {
   // True if the checkbox is checked.
   bool checked_;
 
+  // FocusRing used in MD mode
+  View* focus_ring_ = nullptr;
+
   // The images for each button node_data.
   gfx::ImageSkia images_[2][2][STATE_COUNT];
 

ui/views/controls/button/radio_button.cc

@@ -138,9 +138,10 @@ void RadioButton::SetChecked(bool checked) {
   Checkbox::SetChecked(checked);
 }
 
-void RadioButton::PaintFocusRing(gfx::Canvas* canvas,
+void RadioButton::PaintFocusRing(View* view,
+                                 gfx::Canvas* canvas,
                                  const cc::PaintFlags& flags) {
-  canvas->DrawCircle(gfx::RectF(image()->bounds()).CenterPoint(),
+  canvas->DrawCircle(gfx::RectF(view->GetLocalBounds()).CenterPoint(),
                      image()->width() / 2, flags);
 }
 

ui/views/controls/button/radio_button.h

@@ -35,7 +35,8 @@ class VIEWS_EXPORT RadioButton : public Checkbox {
 
   // Overridden from Checkbox:
   void SetChecked(bool checked) override;
-  void PaintFocusRing(gfx::Canvas* canvas,
+  void PaintFocusRing(View* view,
+                      gfx::Canvas* canvas,
                       const cc::PaintFlags& flags) override;
   const gfx::VectorIcon& GetVectorIcon() const override;
 

ui/views/controls/focus_ring.cc

@@ -5,7 +5,6 @@
 #include "ui/views/controls/focus_ring.h"
 
 #include "ui/gfx/canvas.h"
-#include "ui/native_theme/native_theme.h"
 #include "ui/views/controls/focusable_border.h"
 
 namespace views {
@@ -21,7 +20,7 @@ constexpr float kFocusHaloThicknessDp = 2.f;
 constexpr float kFocusHaloCornerRadiusDp =
     FocusableBorder::kCornerRadiusDp + kFocusHaloThicknessDp / 2.f;
 
-FocusRing* GetFocusRing(views::View* parent) {
+FocusRing* GetFocusRing(View* parent) {
   for (int i = 0; i < parent->child_count(); ++i) {
     if (parent->child_at(i)->GetClassName() == FocusRing::kViewClassName)
       return static_cast<FocusRing*>(parent->child_at(i));
@@ -34,7 +33,7 @@ FocusRing* GetFocusRing(views::View* parent) {
 const char FocusRing::kViewClassName[] = "FocusRing";
 
 // static
-views::View* FocusRing::Install(views::View* parent,
+views::View* FocusRing::Install(View* parent,
                                 ui::NativeTheme::ColorId override_color_id) {
   FocusRing* ring = GetFocusRing(parent);
   if (!ring) {
@@ -48,16 +47,21 @@ views::View* FocusRing::Install(views::View* parent,
 }
 
 // static
-void FocusRing::Uninstall(views::View* parent) {
+void FocusRing::Uninstall(View* parent) {
   delete GetFocusRing(parent);
 }
 
-const char* FocusRing::GetClassName() const {
-  return kViewClassName;
+// static
+void FocusRing::InitFocusRing(View* view) {
+  // A layer is necessary to paint beyond the parent's bounds.
+  view->SetPaintToLayer();
+  view->layer()->SetFillsBoundsOpaquely(false);
+  // Don't allow the view to process events.
+  view->set_can_process_events_within_subtree(false);
 }
 
-bool FocusRing::CanProcessEventsWithinSubtree() const {
-  return false;
+const char* FocusRing::GetClassName() const {
+  return kViewClassName;
 }
 
 void FocusRing::Layout() {
@@ -86,9 +90,7 @@ void FocusRing::OnPaint(gfx::Canvas* canvas) {
 
 FocusRing::FocusRing()
     : override_color_id_(ui::NativeTheme::kColorId_NumColors) {
-  // A layer is necessary to paint beyond the parent's bounds.
-  SetPaintToLayer();
-  layer()->SetFillsBoundsOpaquely(false);
+  InitFocusRing(this);
 }
 
 FocusRing::~FocusRing() {}

ui/views/controls/focus_ring.h

@@ -5,7 +5,6 @@
 #ifndef UI_VIEWS_CONTROLS_FOCUS_RING_H_
 #define UI_VIEWS_CONTROLS_FOCUS_RING_H_
 
-#include "base/optional.h"
 #include "ui/native_theme/native_theme.h"
 #include "ui/views/view.h"
 
@@ -21,23 +20,26 @@ class FocusRing : public View {
   // Create a FocusRing and adds it to |parent|, or updates the one that already
   // exists. |override_color_id| will be used in place of the default coloration
   // when provided.
-  static View* Install(views::View* parent,
-                      ui::NativeTheme::ColorId override_color_id =
-                          ui::NativeTheme::kColorId_NumColors);
+  static View* Install(View* parent,
+                       ui::NativeTheme::ColorId override_color_id =
+                           ui::NativeTheme::kColorId_NumColors);
 
   // Removes the FocusRing from |parent|.
-  static void Uninstall(views::View* parent);
+  static void Uninstall(View* parent);
+
+  // Configure |view| for painting focus ring highlights.
+  static void InitFocusRing(View* view);
 
   // View:
   const char* GetClassName() const override;
-  bool CanProcessEventsWithinSubtree() const override;
   void Layout() override;
   void OnPaint(gfx::Canvas* canvas) override;
 
- private:
+ protected:
   FocusRing();
   ~FocusRing() override;
 
+ private:
   ui::NativeTheme::ColorId override_color_id_;
 
   DISALLOW_COPY_AND_ASSIGN(FocusRing);