[Views] Update ink drop for omnibox icons

chrome/browser/ui/views/location_bar/icon_label_bubble_view.cc

Index: chrome/browser/ui/views/location_bar/icon_label_bubble_view.cc
diff --git a/chrome/browser/ui/views/location_bar/icon_label_bubble_view.cc b/chrome/browser/ui/views/location_bar/icon_label_bubble_view.cc
index 38e36e746a84a6d9b330689fb76cc94cde13d89a..06c1bc3379923d450c0e4935a72a9ed7554e1cde 100644
--- a/chrome/browser/ui/views/location_bar/icon_label_bubble_view.cc
+++ b/chrome/browser/ui/views/location_bar/icon_label_bubble_view.cc
@@ -8,11 +8,16 @@
 #include "chrome/browser/ui/views/location_bar/background_with_1_px_border.h"
 #include "chrome/browser/ui/views/location_bar/location_bar_view.h"
 #include "ui/accessibility/ax_node_data.h"
+#include "ui/compositor/layer_animator.h"
+#include "ui/compositor/scoped_layer_animation_settings.h"
 #include "ui/gfx/canvas.h"
 #include "ui/gfx/color_utils.h"
 #include "ui/gfx/scoped_canvas.h"
 #include "ui/native_theme/native_theme.h"
+#include "ui/views/animation/flood_fill_ink_drop_ripple.h"
 #include "ui/views/animation/ink_drop_highlight.h"
+#include "ui/views/animation/ink_drop_impl.h"
+#include "ui/views/animation/ink_drop_ripple.h"
 #include "ui/views/border.h"
 #include "ui/views/controls/image_view.h"
 #include "ui/views/widget/widget.h"
@@ -22,12 +27,95 @@ namespace {
 // Amount of space on either side of the separator that appears after the label.
 constexpr int kSpaceBesideSeparator = 8;
 
+// The length of the separator's fade animation.
+constexpr int kFadeInDurationMs = 250;
+constexpr int kFadeOutDurationMs = 175;
+
 }  // namespace
 
+//////////////////////////////////////////////////////////////////
+// SeparatorView class
+
+// A view that draws the separator.
+class SeparatorView : public views::View, public gfx::AnimationDelegate {
+ public:
+  explicit SeparatorView(IconLabelBubbleView* owner);
+
+  // View:
+  void OnPaint(gfx::Canvas* canvas) override;
+  bool CanProcessEventsWithinSubtree() const override;
+
+  // Updates the opacity based on the ink drop's state.
+  void UpdateOpacity();
+
+ private:
+  // Weak.
+  IconLabelBubbleView* owner_;
+};
+
+SeparatorView::SeparatorView(IconLabelBubbleView* owner) {
+  DCHECK(owner);
+  owner_ = owner;
+
+  SetPaintToLayer();
+  layer()->SetFillsBoundsOpaquely(false);
+}
+
+void SeparatorView::OnPaint(gfx::Canvas* canvas) {
+  if (!owner_->ShouldShowLabel())
+    return;
+
+  const SkColor plain_text_color = owner_->GetNativeTheme()->GetSystemColor(
+      ui::NativeTheme::kColorId_TextfieldDefaultColor);
+  const SkColor separator_color = SkColorSetA(
+      plain_text_color, color_utils::IsDark(plain_text_color) ? 0x59 : 0xCC);
+
+  gfx::Rect bounds(owner_->label()->bounds());
+  const int kSeparatorHeight = 16;
+  bounds.Inset(0, (bounds.height() - kSeparatorHeight) / 2);
+
+  // Draw the 1 px separator.
+  gfx::ScopedCanvas scoped_canvas(canvas);
+  const float scale = canvas->UndoDeviceScaleFactor();
+  // Keep the separator aligned on a pixel center.
+  const gfx::RectF pixel_aligned_bounds =
+      gfx::ScaleRect(gfx::RectF(bounds), scale) - gfx::Vector2dF(0.5f, 0);
+  canvas->DrawLine(pixel_aligned_bounds.top_right(),
+                   pixel_aligned_bounds.bottom_right(), separator_color);
+}
+
+bool SeparatorView::CanProcessEventsWithinSubtree() const {
+  return false;
+}
+
+void SeparatorView::UpdateOpacity() {
+  views::InkDrop* ink_drop = owner_->GetInkDrop();
+  DCHECK(ink_drop);
+
+  views::InkDropState state = ink_drop->GetTargetInkDropState();
+  float opacity = 0.0f;
+  float duration = kFadeOutDurationMs;
+  if (!ink_drop->ShouldHighlight() &&

[bruthig, 2017/03/08 18:58:24]

Should this really care about the logical hover state of the ink drop or should
it care about whether the ripple or the highlight is visible?
These are not the same thing, see AutoHighlightMode::HIDE_ON_RIPPLE behavior.
The tl;dr is that the highlight is hidden for a period of time after a ripple
animates to hidden.

Put another way, if that mode changed to HIDE_ON_RIPPLE would you get the
desired/reasonable behavior for the separator visibility?

[spqchan, 2017/03/14 00:07:52]

The separator needs to be invisible when the ripple or highlight is visible. If
the mode changed to HIDE_ON_RIPPLE, I would get the desired behaviour. Is there
a better a way to detect if either the ripple or highlight is visible? I
searched around in the codebase but couldn't find one.

+      (state == views::InkDropState::HIDDEN ||
+       state == views::InkDropState::DEACTIVATED)) {
+    opacity = 1.0f;
+    duration = kFadeInDurationMs;
+  }
+
+  ui::ScopedLayerAnimationSettings animation(layer()->GetAnimator());
+  animation.SetTransitionDuration(base::TimeDelta::FromMilliseconds(duration));
+  animation.SetTweenType(gfx::Tween::Type::EASE_IN);
+  layer()->SetOpacity(opacity);
+}
+
+//////////////////////////////////////////////////////////////////
+// IconLabelBubbleView class
+
 IconLabelBubbleView::IconLabelBubbleView(const gfx::FontList& font_list,
                                          bool elide_in_middle)
     : image_(new views::ImageView()),
-      label_(new views::Label(base::string16(), font_list)) {
+      label_(new views::Label(base::string16(), font_list)),
+      ink_drop_container_(new views::InkDropContainerView()) {
   // Disable separate hit testing for |image_|.  This prevents views treating
   // |image_| as a separate mouse hover region from |this|.
   image_->set_interactive(false);
@@ -41,6 +129,14 @@ IconLabelBubbleView::IconLabelBubbleView(const gfx::FontList& font_list,
     label_->SetElideBehavior(gfx::ELIDE_MIDDLE);
   AddChildView(label_);
 
+  separator_view_.reset(new SeparatorView(this));
+  AddChildView(separator_view_.get());
+
+  AddChildView(ink_drop_container_);
+  ink_drop_container_->SetPaintToLayer();

[bruthig, 2017/03/08 18:58:24]

Why does the |ink_drop_container_| have to paint to it's own layer when the ink
drop is not active?

[spqchan, 2017/03/14 00:07:52]

That's a good point. I just realized that the container will SetPaintToLayer()
when you call AddInkDropLayer(). Removed it

+  ink_drop_container_->layer()->SetFillsBoundsOpaquely(false);
+  ink_drop_container_->SetVisible(false);
+
   // Bubbles are given the full internal height of the location bar so that all
   // child views in the location bar have the same height. The visible height of
   // the bubble should be smaller, so use an empty border to shrink down the
@@ -75,6 +171,10 @@ bool IconLabelBubbleView::IsShrinking() const {
   return false;
 }
 
+bool IconLabelBubbleView::IsInkDropEnabled() const {
+  return false;
+}
+
 bool IconLabelBubbleView::OnActivate(const ui::Event& event) {
   return false;
 }
@@ -120,6 +220,24 @@ void IconLabelBubbleView::Layout() {
   const int label_width =
       std::max(0, width() - label_x - bubble_trailing_padding);
   label_->SetBounds(label_x, 0, label_width, height());
+  separator_view_->SetBoundsRect(GetLocalBounds());
+  ink_drop_container_->SetBoundsRect(GetLocalBounds());
+}
+
+bool IconLabelBubbleView::OnMousePressed(const ui::MouseEvent& event) {
+  if (event.IsOnlyLeftMouseButton() && IsInkDropEnabled())
+    AnimateInkDrop(views::InkDropState::ACTION_PENDING, &event);
+  return true;
+}
+
+void IconLabelBubbleView::OnMouseReleased(const ui::MouseEvent& event) {
+  if (!event.IsLeftMouseButton() || !IsInkDropEnabled())
+    return;
+
+  const bool activated = HitTestPoint(event.location());
+  AnimateInkDrop(
+      activated ? views::InkDropState::ACTIVATED : views::InkDropState::HIDDEN,
+      &event);
 }
 
 void IconLabelBubbleView::GetAccessibleNodeData(ui::AXNodeData* node_data) {
@@ -134,25 +252,80 @@ void IconLabelBubbleView::OnNativeThemeChanged(
 }
 
 void IconLabelBubbleView::AddInkDropLayer(ui::Layer* ink_drop_layer) {
-  image()->SetPaintToLayer();
-  image()->layer()->SetFillsBoundsOpaquely(false);
-  InkDropHostView::AddInkDropLayer(ink_drop_layer);
+  ink_drop_container_->AddInkDropLayer(ink_drop_layer);
+  SchedulePaint();

[bruthig, 2017/03/08 18:58:24]

Why is this SchedulePaint() necessary?

[spqchan, 2017/03/14 00:07:52]

Whoops. Thanks for catching it, this should be removed. This was code from my
experiments.

 }
 
 void IconLabelBubbleView::RemoveInkDropLayer(ui::Layer* ink_drop_layer) {
-  InkDropHostView::RemoveInkDropLayer(ink_drop_layer);
-  image()->DestroyLayer();
+  ink_drop_container_->RemoveInkDropLayer(ink_drop_layer);
+  SchedulePaint();

[bruthig, 2017/03/08 18:58:24]

Why is this SchedulePaint() necessary?

[spqchan, 2017/03/14 00:07:52]

Whoops. Thanks for catching it, this should be removed. This was code from my
experiments.

+}
+
+std::unique_ptr<views::InkDrop> IconLabelBubbleView::CreateInkDrop() {
+  std::unique_ptr<views::InkDropImpl> ink_drop = CreateDefaultInkDropImpl();
+  ink_drop->SetShowHighlightOnFocus(true);

[bruthig, 2017/03/08 18:58:24]

|ink_drop| doesn't appear to be used?

[spqchan, 2017/03/14 00:07:52]

Whoops, thanks for the catch!

+  return CreateDefaultFloodFillInkDropImpl();
 }
 
 std::unique_ptr<views::InkDropHighlight>
 IconLabelBubbleView::CreateInkDropHighlight() const {
-  // Only show a highlight effect when the label is empty/invisible.
-  return label()->visible() ? nullptr
-                            : InkDropHostView::CreateInkDropHighlight();
+  if (!IsInkDropEnabled())
+    return nullptr;
+
+  gfx::PointF ink_drop_center = gfx::RectF(GetLocalBounds()).CenterPoint();
+  gfx::Size ink_drop_size = size();
+  if (ink_drop_size.IsEmpty())
+    ink_drop_size = gfx::Size(kDefaultInkDropSize, kDefaultInkDropSize);
+
+  if (ShouldShowLabel()) {
+    int padding_offset = -GetPostSeparatorPadding();
+    ink_drop_center.Offset(padding_offset / 2, 0);
+    ink_drop_size.Enlarge(padding_offset, 0);
+  }
+
+  return InkDropHostView::CreateDefaultInkDropHighlight(ink_drop_center,
+                                                        ink_drop_size);
+}
+
+std::unique_ptr<views::InkDropRipple> IconLabelBubbleView::CreateInkDropRipple()
+    const {
+  if (!IsInkDropEnabled())
+    return nullptr;
+
+  gfx::Point ink_drop_center = GetLocalBounds().CenterPoint();
+  gfx::Size ink_drop_size = size();
+  if (ink_drop_size.IsEmpty())
+    return CreateDefaultInkDropRipple(ink_drop_center);
+
+  if (ShouldShowLabel()) {
+    int padding_offset = -GetPostSeparatorPadding();
+    ink_drop_center.Offset(padding_offset / 2, 0);
+    ink_drop_size.Enlarge(padding_offset, 0);
+  }
+
+  return base::MakeUnique<views::FloodFillInkDropRipple>(
+      ink_drop_size, ink_drop_center, GetInkDropBaseColor(),
+      ink_drop_visible_opacity());
 }
 
 SkColor IconLabelBubbleView::GetInkDropBaseColor() const {
-  return color_utils::DeriveDefaultIconColor(GetTextColor());
+  return color_utils::DeriveDefaultIconColor(GetNativeTheme()->GetSystemColor(
+      ui::NativeTheme::kColorId_TextfieldDefaultColor));
+}
+
+void IconLabelBubbleView::InkDropAnimationStarted() {
+  separator_view_->UpdateOpacity();
+}
+
+void IconLabelBubbleView::OnWidgetDestroying(views::Widget* widget) {
+  widget->RemoveObserver(this);
+}
+
+void IconLabelBubbleView::OnWidgetVisibilityChanged(views::Widget* widget,
+                                                    bool visible) {
+  // |widget| is a bubble that has just got shown / hidden.
+  if (!visible)
+    AnimateInkDrop(views::InkDropState::DEACTIVATED, nullptr /* event */);
 }
 
 SkColor IconLabelBubbleView::GetParentBackgroundColor() const {
@@ -216,26 +389,3 @@ float IconLabelBubbleView::GetScaleFactor() const {
 const char* IconLabelBubbleView::GetClassName() const {
   return "IconLabelBubbleView";
 }
-
-void IconLabelBubbleView::OnPaint(gfx::Canvas* canvas) {
-  if (!ShouldShowLabel())
-    return;
-
-  const SkColor plain_text_color = GetNativeTheme()->GetSystemColor(
-      ui::NativeTheme::kColorId_TextfieldDefaultColor);
-  const SkColor separator_color = SkColorSetA(
-      plain_text_color, color_utils::IsDark(plain_text_color) ? 0x59 : 0xCC);
-
-  gfx::Rect bounds(label_->bounds());
-  const int kSeparatorHeight = 16;
-  bounds.Inset(0, (bounds.height() - kSeparatorHeight) / 2);
-
-  // Draw the 1 px separator.
-  gfx::ScopedCanvas scoped_canvas(canvas);
-  const float scale = canvas->UndoDeviceScaleFactor();
-  // Keep the separator aligned on a pixel center.
-  const gfx::RectF pixel_aligned_bounds =
-      gfx::ScaleRect(gfx::RectF(bounds), scale) - gfx::Vector2dF(0.5f, 0);
-  canvas->DrawLine(pixel_aligned_bounds.top_right(),
-                   pixel_aligned_bounds.bottom_right(), separator_color);
-}