[ash-md] Animates ToggleButton highlight to move it in sync with the thumb

ui/views/controls/button/toggle_button.cc

Index: ui/views/controls/button/toggle_button.cc
diff --git a/ui/views/controls/button/toggle_button.cc b/ui/views/controls/button/toggle_button.cc
index 30bf562a54887ce6a3b62c24af0408b80104c884..1e0f7c7ca5a65d32d59f889097a5c7ee7bce8262 100644
--- a/ui/views/controls/button/toggle_button.cc
+++ b/ui/views/controls/button/toggle_button.cc
@@ -34,8 +34,71 @@ const SkColor kTrackOffColor =
 
 }  // namespace
 
+// Class representing the thumb. When the thumb is clicked it is separated into
+// its own layer and the ink drop layer is made a child of the thumb layer
+// allowing the two to animate in sync.
+class ToggleButton::ThumbView : public views::View {
+ public:
+  ThumbView(views::View* parent) : color_ratio_(0.) {
+    parent->AddChildView(this);
+  }
+  ~ThumbView() override {}
+
+  void AddInkDropLayer(ui::Layer* ink_drop_layer) {
+    SetPaintToLayer(true);
+    layer()->SetFillsBoundsOpaquely(false);
+    layer()->Add(ink_drop_layer);
+  }
+
+  void RemoveInkDropLayer(ui::Layer* ink_drop_layer) {
+    layer()->Remove(ink_drop_layer);
+    SetPaintToLayer(false);
+  }
+
+  void Update(const gfx::Rect& bounds, double color_ratio) {
+    SetBoundsRect(bounds);
+    color_ratio_ = color_ratio;
+    SchedulePaint();
+  }
+
+ private:
+  // views::View:
+  const char* GetClassName() const override {
+    return "ToggleButton::ThumbView";
+  }
+
+  void OnPaint(gfx::Canvas* canvas) override {
+    std::vector<gfx::ShadowValue> shadows;
+    shadows.emplace_back(gfx::Vector2d(0, 1), 4.f,
+                         SkColorSetA(SK_ColorBLACK, 0x99));
+    SkPaint thumb_paint;
+    thumb_paint.setLooper(gfx::CreateShadowDrawLooperCorrectBlur(shadows));
+    thumb_paint.setStyle(SkPaint::kFill_Style);
+    thumb_paint.setAntiAlias(true);
+    const SkColor thumb_on_color = GetNativeTheme()->GetSystemColor(
+        ui::NativeTheme::kColorId_ProminentButtonColor);
+    // TODO(estade): get this color from the theme?
+    const SkColor thumb_off_color = SK_ColorWHITE;
+    const SkAlpha blend = static_cast<SkAlpha>(SK_AlphaOPAQUE * color_ratio_);
+    thumb_paint.setColor(
+        color_utils::AlphaBlend(thumb_on_color, thumb_off_color, blend));
+    gfx::Rect thumb_bounds = GetLocalBounds();
+    thumb_bounds.Inset(gfx::Insets(kThumbVerticalMargin));
+    canvas->DrawCircle(gfx::RectF(thumb_bounds).CenterPoint(),
+                       thumb_bounds.height() / 2.f, thumb_paint);
+  }
+
+  // Color ratio between 0 and 1 that controls the thumb color.
+  double color_ratio_;
+
+  DISALLOW_COPY_AND_ASSIGN(ThumbView);
+};
+
 ToggleButton::ToggleButton(ButtonListener* listener)
-    : CustomButton(listener), is_on_(false), slide_animation_(this) {
+    : CustomButton(listener),
+      is_on_(false),
+      slide_animation_(this),
+      thumb_view_(new ToggleButton::ThumbView(this)) {
   slide_animation_.SetSlideDuration(80 /* ms */);
   slide_animation_.SetTweenType(gfx::Tween::LINEAR);
   SetBorder(Border::CreateEmptyBorder(
@@ -44,7 +107,9 @@ ToggleButton::ToggleButton(ButtonListener* listener)
   set_has_ink_drop_action_on_click(true);
 }
 
-ToggleButton::~ToggleButton() {}
+ToggleButton::~ToggleButton() {
+  thumb_view_.reset();
+}
 
 void ToggleButton::SetIsOn(bool is_on, bool animate) {
   if (is_on_ == is_on)
@@ -66,9 +131,13 @@ gfx::Size ToggleButton::GetPreferredSize() const {
   return rect.size();
 }
 
+const char* ToggleButton::GetClassName() const {
+  return "ToggleButton";
+}
+
 void ToggleButton::OnPaint(gfx::Canvas* canvas) {
-  SkAlpha blend =
-      static_cast<SkAlpha>(SK_AlphaOPAQUE * slide_animation_.GetCurrentValue());
+  const double color_ratio = slide_animation_.GetCurrentValue();
+  SkAlpha blend = static_cast<SkAlpha>(SK_AlphaOPAQUE * color_ratio);
 
   // Track.
   gfx::RectF track_rect(GetContentsBounds());
@@ -81,24 +150,6 @@ void ToggleButton::OnPaint(gfx::Canvas* canvas) {
   track_paint.setColor(
       color_utils::AlphaBlend(track_on_color, kTrackOffColor, blend));
   canvas->DrawRoundRect(track_rect, track_rect.height() / 2, track_paint);
-
-  // Thumb.
-  gfx::Rect thumb_bounds = GetThumbBounds();
-  SkPaint thumb_paint;
-  std::vector<gfx::ShadowValue> shadows;
-  shadows.emplace_back(gfx::Vector2d(0, 1), 4.f,
-                       SkColorSetA(SK_ColorBLACK, 0x99));
-  thumb_paint.setLooper(gfx::CreateShadowDrawLooperCorrectBlur(shadows));
-  thumb_paint.setStyle(SkPaint::kFill_Style);
-  thumb_paint.setAntiAlias(true);
-  const SkColor thumb_on_color = GetNativeTheme()->GetSystemColor(
-      ui::NativeTheme::kColorId_ProminentButtonColor);
-  // TODO(estade): get this color from the theme?
-  const SkColor thumb_off_color = SK_ColorWHITE;
-  thumb_paint.setColor(
-      color_utils::AlphaBlend(thumb_on_color, thumb_off_color, blend));
-  canvas->DrawCircle(gfx::RectF(thumb_bounds).CenterPoint(),
-                     thumb_bounds.height() / 2.f, thumb_paint);
 }
 
 void ToggleButton::NotifyClick(const ui::Event& event) {
@@ -106,12 +157,29 @@ void ToggleButton::NotifyClick(const ui::Event& event) {
   CustomButton::NotifyClick(event);
 }
 
+void ToggleButton::OnBoundsChanged(const gfx::Rect& previous_bounds) {
+  UpdateThumb();
+}
+
 void ToggleButton::OnNativeThemeChanged(const ui::NativeTheme* theme) {
   SchedulePaint();
 }
 
+void ToggleButton::AddInkDropLayer(ui::Layer* ink_drop_layer) {
+  CustomButton::AddInkDropLayer(ink_drop_layer);

[bruthig, 2016/10/12 13:38:09]

You shouldn't need to call into the parent here since the layer is actually
being hosted by the ThumbView.

[varkha, 2016/10/12 14:45:24]

Done.

+  thumb_view_->AddInkDropLayer(ink_drop_layer);
+  UpdateThumb();
+  SchedulePaint();
+}
+
+void ToggleButton::RemoveInkDropLayer(ui::Layer* ink_drop_layer) {
+  thumb_view_->RemoveInkDropLayer(ink_drop_layer);

[bruthig, 2016/10/12 13:38:09]

You should maybe check that |thumb_view_| isn't null here.  In case the
ThumbView is destroyed before the InkDrop cleans up the InkDropRipple layer.

[varkha, 2016/10/12 14:45:24]

I think this gets handled auto-magically. When a ink drop host view
(ToggleButton which owns the ThumbView) becomes hidden the ink drop is destroyed
and gets removed from the host. The thumb should not be destroyed before
ToggleView destructor gets called (and if it ever is then you are right we
should add this check).

[bruthig, 2016/10/13 17:09:46]

The |thumb_view_| is being destroyed in ~ToggleButton() on line 111 which is
going to occur before the destruction percolates up to the ~InkDropHostView()
dtor and thus destroying the |ink_drop_| instance.

It should be simple enough to write a test like so:
1. Create ToggleButton
2. Mouse down on thumb to create ripple
3. Destroy the ToggleButton while the ripple is active.

I suspect this is going to raise a use-after-free error.

[varkha, 2016/10/13 23:07:35]

Good call. So the test doesn't crash because the vtable is rolled and the base
class version of InkDropHostView::RemoveInkDropLayer() is called which is
harmless since it checks |destroying_|.
Chrome on the other hand doesn't crash because VisibilityChanged() callback
proactively resets the ink drop layers when a view gets hidden.

We talked about it offline and agreed that this is subtle enough to force the
ink drop to get destroyed explicitly, hence a call to
SetInkDropMode(InkDropMode::OFF) in ~ToggleButton(). I've also added the test
that exercises this sequence.

+  SchedulePaint();
+}
+
 std::unique_ptr<InkDropRipple> ToggleButton::CreateInkDropRipple() const {
-  return CreateDefaultInkDropRipple(GetThumbBounds().CenterPoint());
+  const int radius = (kTrackHeight + kTrackVerticalMargin * 2) / 2;
+  return CreateDefaultInkDropRipple(gfx::Point(radius, radius));
 }
 
 SkColor ToggleButton::GetInkDropBaseColor() const {
@@ -126,10 +194,12 @@ bool ToggleButton::ShouldShowInkDropHighlight() const {
 }
 
 void ToggleButton::AnimationProgressed(const gfx::Animation* animation) {
-  if (animation == &slide_animation_)
+  if (animation == &slide_animation_) {
+    UpdateThumb();
     SchedulePaint();
-  else
-    CustomButton::AnimationProgressed(animation);
+    return;
+  }
+  CustomButton::AnimationProgressed(animation);
 }
 
 gfx::Rect ToggleButton::GetThumbBounds() const {
@@ -144,4 +214,10 @@ gfx::Rect ToggleButton::GetThumbBounds() const {
   return thumb_bounds;
 }
 
+void ToggleButton::UpdateThumb() {
+  gfx::Rect thumb_bounds = GetThumbBounds();
+  thumb_bounds.Inset(gfx::Insets(-kThumbVerticalMargin));
+  thumb_view_->Update(thumb_bounds, slide_animation_.GetCurrentValue());
+}
+
 }  // namespace views