Polish on relocating the pinned app icon by mouse/gesture

When relocating the pinned app across pages, a proxy image view is
created in ScrollableShelfView. The proxy image view has the identical
bounds with ShelfView::|drag_view_|. However, |drag_view_|'s bounds
are adjusted to fit into the targeting icon slot. It is why the
dragged icon may move under the finger by itself. To fix the issue,
the proxy image's bounds should rely on the mouse/gesture event's
location.

This CL also adds the animation to move the drag icon from event
location to the ideal place after drag ends.

Bug: 1031367
Change-Id: I539abc1b97e061a50c2a53b1cc6b784e78c14983
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1990565
Commit-Queue: Andrew Xu <andrewxu@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#729963}

ash/shelf/scrollable_shelf_view.cc

@@ -9,6 +9,7 @@
 #include "ash/public/cpp/presentation_time_recorder.h"
 #include "ash/public/cpp/shelf_config.h"
 #include "ash/screen_util.h"
+#include "ash/shelf/shelf_app_button.h"
 #include "ash/shelf/shelf_focus_cycler.h"
 #include "ash/shelf/shelf_navigation_widget.h"
 #include "ash/shelf/shelf_tooltip_manager.h"
@@ -130,6 +131,58 @@ int GetAppIconEndPadding() {
 
 }  // namespace
 
+////////////////////////////////////////////////////////////////////////////////
+// DragIconDropAnimationDelegate
+
+class ScrollableShelfView::DragIconDropAnimationDelegate
+    : public ui::ImplicitAnimationObserver {
+ public:
+  DragIconDropAnimationDelegate(views::View* original_view,
+                                const gfx::Rect& target_bounds,
+                                std::unique_ptr<DragImageView> proxy_view)
+      : original_view_(original_view),
+        target_bounds_(target_bounds),
+        proxy_view_(std::move(proxy_view)) {}
+  ~DragIconDropAnimationDelegate() override = default;
+
+  DragIconDropAnimationDelegate(const DragIconDropAnimationDelegate&) = delete;
+  DragIconDropAnimationDelegate& operator=(
+      const DragIconDropAnimationDelegate&) = delete;
+
+  void StartAnimation() {
+    ui::ScopedLayerAnimationSettings animation_settings(
+        proxy_view_->layer()->GetAnimator());
+    animation_settings.SetTweenType(gfx::Tween::FAST_OUT_LINEAR_IN);
+    animation_settings.SetPreemptionStrategy(
+        ui::LayerAnimator::IMMEDIATELY_SET_NEW_TARGET);
+    animation_settings.AddObserver(this);
+
+    proxy_view_->layer()->SetBounds(target_bounds_);
+  }
+
+  // ui::ImplicitAnimationObserver:
+  void OnImplicitAnimationsCompleted() override {
+    StopObserving();
+
+    // Destructs the proxy image view and shows the original drag view at the
+    // end of animation.
+    original_view_->layer()->SetOpacity(1.0f);
+    proxy_view_.reset();
+  }
+
+ private:
+  // Original app icon being dragged in ShelfView.
+  views::View* original_view_ = nullptr;
+
+  // The target bounds after icon is dropped in |proxy_view_| parent's
+  // coordinates.
+  gfx::Rect target_bounds_;
+
+  // Placeholder icon representing |original_icon_| that moves with the pointer
+  // while being dragged.
+  std::unique_ptr<DragImageView> proxy_view_;
+};
+
 ////////////////////////////////////////////////////////////////////////////////
 // GradientLayerDelegate
 
@@ -1045,6 +1098,8 @@ void ScrollableShelfView::CreateDragIconProxyByLocationWithNoAnimation(
   drag_icon_->GetWidget()->SetVisibilityAnimationTransition(
       views::Widget::ANIMATE_NONE);
   drag_icon_->SetWidgetVisible(true);
+  drag_icon_->SetPaintToLayer();
+  drag_icon_->layer()->SetFillsBoundsOpaquely(false);
 }
 
 void ScrollableShelfView::UpdateDragIconProxy(
@@ -1063,10 +1118,47 @@ void ScrollableShelfView::UpdateDragIconProxy(
 }
 
 void ScrollableShelfView::DestroyDragIconProxy() {
-  drag_icon_.reset();
-
   if (page_flip_timer_.IsRunning())
     page_flip_timer_.AbandonAndStop();
+
+  views::View* drag_view = shelf_view_->drag_view();
+
+  const bool should_start_animation =
+      drag_view && !shelf_view_->dragged_off_shelf() && drag_icon_.get();
+  if (!should_start_animation) {
+    drag_icon_.reset();
+    return;
+  }
+
+  // The ideal bounds stored in view model are in |shelf_view_|'s coordinates.
+  views::ViewModel* shelf_view_model = shelf_view_->view_model();
+  const gfx::Rect target_bounds = shelf_view_model->ideal_bounds(
+      shelf_view_model->GetIndexOfView(drag_view));
+  const gfx::Rect mirrored_target_bounds =
+      shelf_view_->GetMirroredRect(target_bounds);
+
+  // No animation is created if the target slot for the drag icon is not on the
+  // current page. This edge case may be triggered by trying to move the icon of
+  // a running app to the area exclusively for pinned apps.
+  gfx::RectF target_bounds_in_local(mirrored_target_bounds);
+  ConvertRectToTarget(shelf_view_, this, &target_bounds_in_local);
+  if (!visible_space_.Contains(gfx::ToEnclosedRect(target_bounds_in_local))) {
+    drag_icon_.reset();
+    drag_view->layer()->SetOpacity(1.0f);
+    return;
+  }
+
+  // Converts the ideal bounds to |drag_icon_|'s coordinates. Notes that
+  // |drag_icon_| and |shelf_view_| are in different widgets.
+  gfx::Point origin_point = mirrored_target_bounds.origin();
+  ConvertPointToScreen(shelf_view_, &origin_point);
+  ConvertPointFromScreen(drag_icon_->parent(), &origin_point);
+
+  drag_icon_drop_animation_delegate_ =
+      std::make_unique<DragIconDropAnimationDelegate>(
+          drag_view, gfx::Rect(origin_point, target_bounds.size()),
+          std::move(drag_icon_));
+  drag_icon_drop_animation_delegate_->StartAnimation();
 }
 
 bool ScrollableShelfView::StartDrag(
@@ -1849,7 +1941,15 @@ bool ScrollableShelfView::IsDragIconWithinVisibleSpace() const {
   gfx::Rect visible_space_in_screen = visible_space_;
   views::View::ConvertRectToScreen(this, &visible_space_in_screen);
 
-  return visible_space_in_screen.Contains(drag_icon_->GetBoundsInScreen());
+  const gfx::Rect drag_icon_screen_bounds = drag_icon_->GetBoundsInScreen();
+
+  if (GetShelf()->IsHorizontalAlignment()) {
+    return drag_icon_screen_bounds.x() >= visible_space_in_screen.x() &&
+           drag_icon_screen_bounds.right() <= visible_space_in_screen.right();
+  }
+
+  return drag_icon_screen_bounds.y() >= visible_space_in_screen.y() &&
+         drag_icon_screen_bounds.bottom() <= visible_space_in_screen.bottom();
 }
 
 bool ScrollableShelfView::ShouldDelegateScrollToShelf(

ash/shelf/scrollable_shelf_view.h

@@ -95,6 +95,8 @@ class ASH_EXPORT ScrollableShelfView : public views::AccessiblePaneView,
   LayoutStrategy layout_strategy_for_test() const { return layout_strategy_; }
   gfx::Vector2dF scroll_offset_for_test() const { return scroll_offset_; }
 
+  const DragImageView* drag_icon_for_test() const { return drag_icon_.get(); }
+
   int first_tappable_app_index() { return first_tappable_app_index_; }
   int last_tappable_app_index() { return last_tappable_app_index_; }
 
@@ -117,6 +119,7 @@ class ASH_EXPORT ScrollableShelfView : public views::AccessiblePaneView,
  private:
   class GradientLayerDelegate;
   class ScrollableShelfArrowView;
+  class DragIconDropAnimationDelegate;
 
   struct FadeZone {
     // Bounds of the fade in/out zone.
@@ -437,6 +440,11 @@ class ASH_EXPORT ScrollableShelfView : public views::AccessiblePaneView,
   // app icon can be dragged out of the shelf view.
   std::unique_ptr<DragImageView> drag_icon_;
 
+  // The delegate to create the animation of moving the dropped icon to the
+  // ideal place after drag release.
+  std::unique_ptr<DragIconDropAnimationDelegate>
+      drag_icon_drop_animation_delegate_;
+
   base::OneShotTimer page_flip_timer_;
 
   // Metric reporter for scrolling animations.

ash/shelf/scrollable_shelf_view_unittest.cc

@@ -4,6 +4,7 @@
 
 #include "ash/shelf/scrollable_shelf_view.h"
 
+#include "ash/drag_drop/drag_image_view.h"
 #include "ash/public/cpp/shelf_config.h"
 #include "ash/root_window_controller.h"
 #include "ash/shelf/shelf_test_util.h"
@@ -385,7 +386,9 @@ TEST_F(ScrollableShelfViewTest, ShowTooltipForArrowButtons) {
   EXPECT_TRUE(tooltip_manager->IsVisible());
 }
 
-// Verifies that dragging an app icon to a new shelf page works well.
+// Verifies that dragging an app icon to a new shelf page works well. In
+// addition, the dragged icon moves with mouse before mouse release (see
+// https://crbug.com/1031367).
 TEST_F(ScrollableShelfViewTest, DragIconToNewPage) {
   scrollable_shelf_view_->set_page_flip_time_threshold(
       base::TimeDelta::FromMilliseconds(10));
@@ -401,8 +404,13 @@ TEST_F(ScrollableShelfViewTest, DragIconToNewPage) {
       view_model->view_at(scrollable_shelf_view_->last_tappable_app_index());
   const gfx::Point drag_start_point =
       dragged_view->GetBoundsInScreen().CenterPoint();
+
+  // Ensures that the app icon is not dragged to the ideal bounds directly.
+  // It helps to construct a more complex scenario that the animation
+  // is created to move the dropped icon to the target place after drag release.
   const gfx::Point drag_end_point =
-      scrollable_shelf_view_->left_arrow()->GetBoundsInScreen().CenterPoint();
+      scrollable_shelf_view_->left_arrow()->GetBoundsInScreen().origin() -
+      gfx::Vector2d(10, 0);
 
   ASSERT_NE(0, view_model->GetIndexOfView(dragged_view));
 
@@ -415,7 +423,18 @@ TEST_F(ScrollableShelfViewTest, DragIconToNewPage) {
     PageFlipWaiter waiter(scrollable_shelf_view_);
     waiter.Wait();
   }
+
+  // Expects that the drag icon moves with drag pointer before mouse release.
+  const gfx::Rect intermediate_bounds =
+      scrollable_shelf_view_->drag_icon_for_test()->GetBoundsInScreen();
+  EXPECT_EQ(drag_end_point, intermediate_bounds.CenterPoint());
+
   GetEventGenerator()->ReleaseLeftButton();
+  ASSERT_NE(intermediate_bounds.CenterPoint(),
+            dragged_view->GetBoundsInScreen().CenterPoint());
+
+  // Expects that the proxy icon is deleted after mouse release.
+  EXPECT_EQ(nullptr, scrollable_shelf_view_->drag_icon_for_test());
 
   // Verifies that:
   // (1) Scrollable shelf view has the expected layout strategy.

ash/shelf/shelf_view.cc

@@ -1490,14 +1490,9 @@ void ShelfView::PointerReleasedOnButton(views::View* view,
   if (drag_pointer_ != NONE)
     return;
 
-  if (chromeos::switches::ShouldShowScrollableShelf()) {
+  if (chromeos::switches::ShouldShowScrollableShelf())
     drag_and_drop_host_->DestroyDragIconProxy();
 
-    // |drag_view_| is reset already when being removed from the shelf view.
-    if (drag_view_)
-      drag_view_->layer()->SetOpacity(1.0f);
-  }
-
   // If the drag pointer is NONE, no drag operation is going on and the
   // drag_view can be released.
   drag_view_ = nullptr;
@@ -1658,13 +1653,17 @@ void ShelfView::ContinueDrag(const ui::LocatedEvent& event) {
     }
   }
 
+  // Calculates the drag point in screen before MoveDragViewTo is called.
+  gfx::Point drag_point_in_screen(event.location());
+  ConvertPointToScreen(drag_view_, &drag_point_in_screen);
+
   gfx::Point drag_point(event.location());
   ConvertPointToTarget(drag_view_, this, &drag_point);
   MoveDragViewTo(shelf_->PrimaryAxisValue(drag_point.x() - drag_origin_.x(),
                                           drag_point.y() - drag_origin_.y()));
   if (chromeos::switches::ShouldShowScrollableShelf()) {
-    drag_and_drop_host_->UpdateDragIconProxy(
-        drag_view_->GetBoundsInScreen().origin());
+    drag_and_drop_host_->UpdateDragIconProxy(drag_point_in_screen -
+                                             drag_origin_.OffsetFromOrigin());
   }
 }
 
@@ -1873,12 +1872,13 @@ bool ShelfView::HandleRipOffDrag(const ui::LocatedEvent& event) {
                         drag_view_, gfx::Vector2d(0, 0),
                         kDragAndDropProxyScale);
 
+    dragged_off_shelf_ = true;
+
     if (chromeos::switches::ShouldShowScrollableShelf())
       drag_and_drop_host_->DestroyDragIconProxy();
     else
       drag_view_->layer()->SetOpacity(0.0f);
 
-    dragged_off_shelf_ = true;
     if (RemovableByRipOff(current_index) == REMOVABLE) {
       // Move the item to the back and hide it. ShelfItemMoved() callback will
       // handle the |view_model_| update and call AnimateToIdealBounds().

ash/shelf/shelf_view.h

@@ -332,7 +332,7 @@ class ASH_EXPORT ShelfView : public views::AccessiblePaneView,
     app_icons_layout_offset_ = app_icons_layout_offset;
   }
 
-  const ShelfAppButton* drag_view() const { return drag_view_; }
+  ShelfAppButton* drag_view() { return drag_view_; }
 
   // Returns true when this ShelfView is used for Overflow Bubble.
   // In this mode, it does not show app list and overflow button.
@@ -356,6 +356,7 @@ class ASH_EXPORT ShelfView : public views::AccessiblePaneView,
   ShelfWidget* shelf_widget() const { return shelf_->shelf_widget(); }
   OverflowBubble* overflow_bubble() { return overflow_bubble_.get(); }
   views::ViewModel* view_model() { return view_model_.get(); }
+  bool dragged_off_shelf() const { return dragged_off_shelf_; }
 
  private:
   friend class ShelfViewTestAPI;