Recreate ink drop ripple, highlight, and mask when view bounds changes

Some views have ink drop components that are shaped depending on the view size
(eg. IconLabelBubbleView::CreateInkDrop{Ripple,Highlight,Mask}).  For these
views, it's easiest to just recreate the ink drop components instead of creating
special views that adapt their shape depending on the view size.

BUG=899104
R=pkasting,mohsen

Change-Id: Icffd0d703cb49a366810846789cc22aac79f925e
Reviewed-on: https://chromium-review.googlesource.com/c/1308099
Commit-Queue: Thomas Anderson <thomasanderson@chromium.org>
Reviewed-by: Mohsen Izadi <mohsen@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#605078}

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

@@ -229,6 +229,11 @@ gfx::Size IconLabelBubbleView::CalculatePreferredSize() const {
   return GetSizeForLabelWidth(label_->GetPreferredSize().width());
 }
 
+void IconLabelBubbleView::OnBoundsChanged(const gfx::Rect& previous_bounds) {
+  ink_drop_container_->SetBoundsRect(CalculateInkDropContainerBounds());
+  views::Button::OnBoundsChanged(previous_bounds);
+}
+
 void IconLabelBubbleView::Layout() {
   // We may not have horizontal room for both the image and the trailing
   // padding. When the view is expanding (or showing-label steady state), the
@@ -266,12 +271,7 @@ void IconLabelBubbleView::Layout() {
   separator_view_->SetBounds(separator_x, separator_bounds.y(), separator_width,
                              separator_height);
 
-  gfx::Rect ink_drop_bounds = GetLocalBounds();
-  if (ShouldShowSeparator()) {
-    ink_drop_bounds.set_width(ink_drop_bounds.width() -
-                              GetEndPaddingWithSeparator());
-  }
-
+  gfx::Rect ink_drop_bounds = CalculateInkDropContainerBounds();
   ink_drop_container_->SetBoundsRect(ink_drop_bounds);
 
   if (focus_ring() && !ink_drop_bounds.IsEmpty()) {
@@ -563,3 +563,10 @@ void IconLabelBubbleView::HideAnimation() {
   GetInkDrop()->SetShowHighlightOnHover(false);
   GetInkDrop()->SetShowHighlightOnFocus(false);
 }
+
+gfx::Rect IconLabelBubbleView::CalculateInkDropContainerBounds() const {
+  gfx::Rect ink_drop_bounds = GetLocalBounds();
+  if (ShouldShowSeparator())
+    ink_drop_bounds.Inset(0, 0, GetEndPaddingWithSeparator(), 0);
+  return ink_drop_bounds;
+}

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

@@ -132,6 +132,7 @@ class IconLabelBubbleView : public views::InkDropObserver,
 
   // views::Button:
   gfx::Size CalculatePreferredSize() const override;
+  void OnBoundsChanged(const gfx::Rect& previous_bounds) override;
   void Layout() override;
   bool OnMousePressed(const ui::MouseEvent& event) override;
   void GetAccessibleNodeData(ui::AXNodeData* node_data) override;
@@ -230,6 +231,8 @@ class IconLabelBubbleView : public views::InkDropObserver,
   // called directly, use AnimateOut() instead, which handles label visibility.
   void HideAnimation();
 
+  gfx::Rect CalculateInkDropContainerBounds() const;
+
   // The contents of the bubble.
   views::ImageView* image_;
   views::Label* label_;

chrome/browser/ui/views/tabs/new_tab_button.cc

@@ -225,7 +225,6 @@ gfx::Size NewTabButton::CalculatePreferredSize() const {
 void NewTabButton::OnBoundsChanged(const gfx::Rect& previous_bounds) {
   const gfx::Size ink_drop_size = GetContentsBounds().size();
   GetInkDrop()->HostSizeChanged(ink_drop_size);
-  UpdateInkDropMaskLayerSize(ink_drop_size);
 }
 
 bool NewTabButton::GetHitTestMask(gfx::Path* mask) const {

ui/views/animation/flood_fill_ink_drop_ripple.cc

@@ -163,19 +163,6 @@ FloodFillInkDropRipple::~FloodFillInkDropRipple() {
   AbortAllAnimations();
 }
 
-void FloodFillInkDropRipple::HostSizeChanged(const gfx::Size& new_size) {
-  root_layer_.SetBounds(CalculateClipBounds(new_size, clip_insets_));
-  switch (target_ink_drop_state()) {
-    case InkDropState::ACTION_PENDING:
-    case InkDropState::ALTERNATE_ACTION_PENDING:
-    case InkDropState::ACTIVATED:
-      painted_layer_.SetTransform(GetMaxSizeTargetTransform());
-      break;
-    default:
-      break;
-  }
-}
-
 void FloodFillInkDropRipple::SnapToActivated() {
   InkDropRipple::SnapToActivated();
   SetOpacity(visible_opacity_);

ui/views/animation/flood_fill_ink_drop_ripple.h

@@ -66,7 +66,6 @@ class VIEWS_EXPORT FloodFillInkDropRipple : public InkDropRipple {
   ~FloodFillInkDropRipple() override;
 
   // InkDropRipple:
-  void HostSizeChanged(const gfx::Size& new_size) override;
   void SnapToActivated() override;
   ui::Layer* GetRootLayer() override;
 

ui/views/animation/ink_drop_host_view.cc

@@ -209,7 +209,6 @@ void InkDropHostView::ViewHierarchyChanged(
 void InkDropHostView::OnBoundsChanged(const gfx::Rect& previous_bounds) {
   if (ink_drop_)
     ink_drop_->HostSizeChanged(size());
-  UpdateInkDropMaskLayerSize(size());
 }
 
 void InkDropHostView::VisibilityChanged(View* starting_from, bool is_visible) {
@@ -312,11 +311,6 @@ void InkDropHostView::ResetInkDropMask() {
   ink_drop_mask_.reset();
 }
 
-void InkDropHostView::UpdateInkDropMaskLayerSize(const gfx::Size& new_size) {
-  if (ink_drop_mask_)
-    ink_drop_mask_->UpdateLayerSize(new_size);
-}
-
 // static
 gfx::Size InkDropHostView::CalculateLargeInkDropSize(
     const gfx::Size& small_size) {

ui/views/animation/ink_drop_host_view.h

@@ -169,10 +169,6 @@ class VIEWS_EXPORT InkDropHostView : public View {
 
   void ResetInkDropMask();
 
-  // Updates the ink drop mask layer size to |new_size|. It does nothing if
-  // |ink_drop_mask_| is null.
-  void UpdateInkDropMaskLayerSize(const gfx::Size& new_size);
-
   // Returns a large ink drop size based on the |small_size| that works well
   // with the SquareInkDropRipple animation durations.
   static gfx::Size CalculateLargeInkDropSize(const gfx::Size& small_size);

ui/views/animation/ink_drop_impl.cc

@@ -630,8 +630,25 @@ void InkDropImpl::HostSizeChanged(const gfx::Size& new_size) {
   // when a mask layer is applied to it. This will not affect clipping if no
   // mask layer is set.
   root_layer_->SetBounds(gfx::Rect(new_size));
-  if (ink_drop_ripple_)
-    ink_drop_ripple_->HostSizeChanged(new_size);
+
+  const bool create_ink_drop_ripple = !!ink_drop_ripple_;
+  const bool activated =
+      ink_drop_ripple_ &&
+      ink_drop_ripple_->target_ink_drop_state() == InkDropState::ACTIVATED;
+  DestroyInkDropRipple();
+
+  const bool create_ink_drop_highlight = !!highlight_;
+  DestroyInkDropHighlight();
+
+  // Both the ripple and the highlight must have been destroyed before
+  // recreating either of them otherwise the mask will not get recreated.
+  if (create_ink_drop_ripple)
+    CreateInkDropRipple();
+  if (activated)
+    ink_drop_ripple_->SnapToActivated();
+
+  if (create_ink_drop_highlight)
+    CreateInkDropHighlight();
 }
 
 InkDropState InkDropImpl::GetTargetInkDropState() const {

ui/views/animation/ink_drop_impl_unittest.cc

@@ -14,6 +14,7 @@
 #include "ui/compositor/scoped_animation_duration_scale_mode.h"
 #include "ui/gfx/animation/animation.h"
 #include "ui/gfx/animation/animation_test_api.h"
+#include "ui/views/animation/ink_drop_ripple.h"
 #include "ui/views/animation/test/ink_drop_impl_test_api.h"
 #include "ui/views/animation/test/test_ink_drop_host.h"
 #include "ui/views/test/platform_test_helper.h"
@@ -32,6 +33,10 @@ class InkDropImplTest : public testing::Test {
 
   InkDropImpl* ink_drop() { return ink_drop_.get(); }
 
+  InkDropRipple* ink_drop_ripple() { return ink_drop_->ink_drop_ripple_.get(); }
+
+  InkDropHighlight* ink_drop_highlight() { return ink_drop_->highlight_.get(); }
+
   test::InkDropImplTestApi* test_api() { return test_api_.get(); }
 
   // Runs all the pending tasks in |task_runner_|. This can be used to progress
@@ -293,6 +298,30 @@ TEST_F(InkDropImplTest, SuccessfulAnimationEndedDuringDestruction) {
   DestroyInkDrop();
 }
 
+// Make sure the InkDropRipple and InkDropHighlight get recreated when the host
+// size changes (https:://crbug.com/899104).
+TEST_F(InkDropImplTest, RippleAndHighlightRecreatedOnSizeChange) {
+  test_api()->SetShouldHighlight(true);
+  ink_drop()->AnimateToState(InkDropState::ACTIVATED);
+  EXPECT_EQ(1, ink_drop_host()->num_ink_drop_ripples_created());
+  EXPECT_EQ(1, ink_drop_host()->num_ink_drop_highlights_created());
+  EXPECT_EQ(ink_drop_host()->last_ink_drop_ripple(), ink_drop_ripple());
+  EXPECT_EQ(ink_drop_host()->last_ink_drop_highlight(), ink_drop_highlight());
+
+  const gfx::Rect bounds(5, 6, 7, 8);
+  ink_drop_host()->SetBoundsRect(bounds);
+  // SetBoundsRect() calls HostSizeChanged(), but only when
+  // InkDropHostView::ink_drop_ is set, but it's not in testing.  So call this
+  // function manually.
+  ink_drop()->HostSizeChanged(ink_drop_host()->size());
+  EXPECT_EQ(2, ink_drop_host()->num_ink_drop_ripples_created());
+  EXPECT_EQ(2, ink_drop_host()->num_ink_drop_highlights_created());
+  EXPECT_EQ(ink_drop_host()->last_ink_drop_ripple(), ink_drop_ripple());
+  EXPECT_EQ(ink_drop_host()->last_ink_drop_highlight(), ink_drop_highlight());
+  EXPECT_EQ(bounds.size(), ink_drop_ripple()->GetRootLayer()->size());
+  EXPECT_EQ(bounds.size(), ink_drop_highlight()->layer()->size());
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 //
 // Common AutoHighlightMode tests

ui/views/animation/ink_drop_mask.cc

@@ -24,10 +24,6 @@ InkDropMask::~InkDropMask() {
   layer_.set_delegate(nullptr);
 }
 
-void InkDropMask::UpdateLayerSize(const gfx::Size& new_layer_size) {
-  layer_.SetBounds(gfx::Rect(new_layer_size));
-}
-
 void InkDropMask::OnDeviceScaleFactorChanged(float old_device_scale_factor,
                                              float new_device_scale_factor) {}
 

ui/views/animation/ink_drop_mask.h

@@ -24,10 +24,6 @@ class VIEWS_EXPORT InkDropMask : public ui::LayerDelegate {
  public:
   ~InkDropMask() override;
 
-  // Should be called whenever the masked layer is resized so that the mask
-  // layer size always matches that of the layer it is masking.
-  void UpdateLayerSize(const gfx::Size& new_layer_size);
-
   ui::Layer* layer() { return &layer_; }
 
  protected:

ui/views/animation/ink_drop_ripple.cc

@@ -30,8 +30,6 @@ InkDropRipple::InkDropRipple()
 
 InkDropRipple::~InkDropRipple() {}
 
-void InkDropRipple::HostSizeChanged(const gfx::Size& new_size) {}
-
 void InkDropRipple::AnimateToState(InkDropState ink_drop_state) {
   // Does not return early if |target_ink_drop_state_| == |ink_drop_state| for
   // two reasons.

ui/views/animation/ink_drop_ripple.h

@@ -54,10 +54,6 @@ class VIEWS_EXPORT InkDropRipple {
   // AnimationStarted(s2).
   void set_observer(InkDropRippleObserver* observer) { observer_ = observer; }
 
-  // Called by ink drop whenever its host's size is changed in order to give the
-  // ripple an opportunity to handle dynamic host resizes.
-  virtual void HostSizeChanged(const gfx::Size& new_size);
-
   // Animates from the current InkDropState to the new |ink_drop_state|.
   //
   // NOTE: GetTargetInkDropState() should return the new |ink_drop_state| value

ui/views/animation/test/test_ink_drop_host.cc

@@ -77,6 +77,10 @@ class TestInkDropHighlight : public InkDropHighlight {
 TestInkDropHost::TestInkDropHost()
     : num_ink_drop_layers_added_(0),
       num_ink_drop_layers_removed_(0),
+      num_ink_drop_ripples_created_(0),
+      num_ink_drop_highlights_created_(0),
+      last_ink_drop_ripple_(nullptr),
+      last_ink_drop_highlight_(nullptr),
       disable_timers_for_test_(false) {}
 
 TestInkDropHost::~TestInkDropHost() {}
@@ -90,21 +94,24 @@ void TestInkDropHost::RemoveInkDropLayer(ui::Layer* ink_drop_layer) {
 }
 
 std::unique_ptr<InkDropRipple> TestInkDropHost::CreateInkDropRipple() const {
-  gfx::Size size(10, 10);
   std::unique_ptr<InkDropRipple> ripple(new TestInkDropRipple(
-      size, 5, size, 5, gfx::Point(), SK_ColorBLACK, 0.175f));
+      size(), 0, size(), 0, gfx::Point(), SK_ColorBLACK, 0.175f));
   if (disable_timers_for_test_)
     ripple->GetTestApi()->SetDisableAnimationTimers(true);
+  num_ink_drop_ripples_created_++;
+  last_ink_drop_ripple_ = ripple.get();
   return ripple;
 }
 
 std::unique_ptr<InkDropHighlight> TestInkDropHost::CreateInkDropHighlight()
     const {
   std::unique_ptr<InkDropHighlight> highlight;
-  highlight.reset(new TestInkDropHighlight(gfx::Size(10, 10), 4, gfx::PointF(),
-                                           SK_ColorBLACK));
+  highlight.reset(
+      new TestInkDropHighlight(size(), 0, gfx::PointF(), SK_ColorBLACK));
   if (disable_timers_for_test_)
     highlight->GetTestApi()->SetDisableAnimationTimers(true);
+  num_ink_drop_highlights_created_++;
+  last_ink_drop_highlight_ = highlight.get();
   return highlight;
 }
 

ui/views/animation/test/test_ink_drop_host.h

@@ -24,6 +24,20 @@ class TestInkDropHost : public InkDropHostView {
     return num_ink_drop_layers_added_ - num_ink_drop_layers_removed_;
   }
 
+  int num_ink_drop_ripples_created() const {
+    return num_ink_drop_ripples_created_;
+  }
+  int num_ink_drop_highlights_created() const {
+    return num_ink_drop_highlights_created_;
+  }
+
+  const InkDropRipple* last_ink_drop_ripple() const {
+    return last_ink_drop_ripple_;
+  }
+  const InkDropHighlight* last_ink_drop_highlight() const {
+    return last_ink_drop_highlight_;
+  }
+
   void set_disable_timers_for_test(bool disable_timers_for_test) {
     disable_timers_for_test_ = disable_timers_for_test;
   }
@@ -38,6 +52,14 @@ class TestInkDropHost : public InkDropHostView {
   int num_ink_drop_layers_added_;
   int num_ink_drop_layers_removed_;
 
+  // CreateInkDrop{Ripple,Highlight} are const, so these members must be
+  // mutable.
+  mutable int num_ink_drop_ripples_created_;
+  mutable int num_ink_drop_highlights_created_;
+
+  mutable const InkDropRipple* last_ink_drop_ripple_;
+  mutable const InkDropHighlight* last_ink_drop_highlight_;
+
   // When true, the InkDropRipple/InkDropHighlight instances will have their
   // timers disabled after creation.
   bool disable_timers_for_test_;