Implement animate in/out of HoldingSpaceItemView.

HoldingSpaceItemViews fade in and translate up into place. When being
removed, they fade out and also translate up.

Demo: http://shortn/_UrRIQF1CXm

Still TODO:
- Animate container grow/shrink
- Animate pinned section empty label
- Animate section labels (when sections are going away)
- Animate screen captures and downloads sections independently of each
  other so that adding/removing items from one section does not impact
  the other unnecessarily.

Bug: 1154998
Change-Id: Ib28c35b79b20e265059c496c2c7c8ceb23defe6c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2575322
Commit-Queue: David Black <dmblack@google.com>
Reviewed-by: David Black <dmblack@google.com>
Reviewed-by: Toni Baržić <tbarzic@chromium.org>
Cr-Commit-Position: refs/heads/master@{#834301}

ash/system/holding_space/holding_space_item_view.cc

@@ -16,6 +16,10 @@
 #include "components/vector_icons/vector_icons.h"
 #include "ui/base/class_property.h"
 #include "ui/base/dragdrop/drag_drop_types.h"
+#include "ui/compositor/layer_animation_element.h"
+#include "ui/compositor/layer_animation_observer.h"
+#include "ui/compositor/layer_animation_sequence.h"
+#include "ui/compositor/layer_animator.h"
 #include "ui/gfx/canvas.h"
 #include "ui/gfx/color_palette.h"
 #include "ui/gfx/paint_vector_icon.h"
@@ -33,11 +37,36 @@ namespace ash {
 
 namespace {
 
+// Animation.
+constexpr base::TimeDelta kAnimationDuration =
+    base::TimeDelta::FromMilliseconds(167);
+constexpr SkScalar kAnimationTranslationY = 20;
+
 // A UI class property used to identify if a view is an instance of
 // `HoldingSpaceItemView`. Class name is not an adequate identifier as it may be
 // overridden by subclasses.
 DEFINE_UI_CLASS_PROPERTY_KEY(bool, kIsHoldingSpaceItemViewProperty, false)
 
+// Helpers ---------------------------------------------------------------------
+
+// Creates a `ui::LayerAnimationSequence` for the specified `element` observed
+// by the specified `observer`.
+std::unique_ptr<ui::LayerAnimationSequence> CreateObservedSequence(
+    std::unique_ptr<ui::LayerAnimationElement> element,
+    ui::LayerAnimationObserver* observer) {
+  auto sequence = std::make_unique<ui::LayerAnimationSequence>();
+  sequence->AddElement(std::move(element));
+  sequence->AddObserver(observer);
+  return sequence;
+}
+
+// Creates a `gfx:Transform` for the specified `x` and `y` offsets.
+gfx::Transform CreateTransformFromOffset(SkScalar x, SkScalar y) {
+  gfx::Transform transform;
+  transform.Translate(x, y);
+  return transform;
+}
+
 // CallbackPainter -------------------------------------------------------------
 
 // A painter which delegates painting to a callback.
@@ -98,6 +127,15 @@ HoldingSpaceItemView::HoldingSpaceItemView(
   SetPaintToLayer();
   layer()->SetFillsBoundsOpaquely(false);
 
+  // This view will be animated in. Set initial opacity and transform so that
+  // the next call to `AnimateIn()` will fade in and translate the view into
+  // place. Note that preemption strategy is also set to cause any calls to
+  // `AnimateIn()`/`AnimateOut()` to preempt any in-progress animation.
+  layer()->SetOpacity(0.f);
+  layer()->SetTransform(CreateTransformFromOffset(0, kAnimationTranslationY));
+  layer()->GetAnimator()->set_preemption_strategy(
+      ui::LayerAnimator::PreemptionStrategy::IMMEDIATELY_ANIMATE_TO_NEW_TARGET);
+
   // Focus.
   SetFocusBehavior(FocusBehavior::ALWAYS);
   focused_layer_owner_ =
@@ -200,6 +238,16 @@ void HoldingSpaceItemView::OnMouseReleased(const ui::MouseEvent& event) {
   delegate_->OnHoldingSpaceItemViewMouseReleased(this, event);
 }
 
+void HoldingSpaceItemView::AnimateIn(ui::LayerAnimationObserver* observer) {
+  AnimateImmediatelyTo(/*opacity=*/1.f, gfx::Transform(), observer);
+}
+
+void HoldingSpaceItemView::AnimateOut(ui::LayerAnimationObserver* observer) {
+  AnimateImmediatelyTo(/*opacity=*/0.f,
+                       CreateTransformFromOffset(0, -kAnimationTranslationY),
+                       observer);
+}
+
 void HoldingSpaceItemView::StartDrag(const ui::LocatedEvent& event,
                                      ui::mojom::DragEventSource source) {
   int drag_operations = GetDragOperations(event.location());
@@ -350,6 +398,28 @@ void HoldingSpaceItemView::UpdatePin() {
   OnPinVisiblityChanged(true);
 }
 
+void HoldingSpaceItemView::AnimateImmediatelyTo(
+    float opacity,
+    const gfx::Transform& transform,
+    ui::LayerAnimationObserver* observer) {
+  // Opacity animation.
+  auto opacity_element = ui::LayerAnimationElement::CreateOpacityElement(
+      opacity, kAnimationDuration);
+  opacity_element->set_tween_type(gfx::Tween::Type::LINEAR);
+
+  // Transform animation.
+  auto transform_element = ui::LayerAnimationElement::CreateTransformElement(
+      transform, kAnimationDuration);
+  transform_element->set_tween_type(gfx::Tween::Type::EASE_OUT_3);
+
+  // Note that the `ui::LayerAnimator` takes ownership of any animation
+  // sequences so they need to be released.
+  layer()->GetAnimator()->StartTogether(
+      {CreateObservedSequence(std::move(opacity_element), observer).release(),
+       CreateObservedSequence(std::move(transform_element), observer)
+           .release()});
+}
+
 BEGIN_METADATA(HoldingSpaceItemView, views::InkDropHostView)
 END_METADATA
 

ash/system/holding_space/holding_space_item_view.h

@@ -12,6 +12,10 @@
 #include "ui/views/controls/image_view.h"
 #include "ui/views/metadata/metadata_header_macros.h"
 
+namespace ui {
+class LayerAnimationObserver;
+}  // namespace ui
+
 namespace views {
 class ToggleImageButton;
 }  // namespace views
@@ -53,6 +57,11 @@ class ASH_EXPORT HoldingSpaceItemView : public views::InkDropHostView {
   bool OnMousePressed(const ui::MouseEvent& event) override;
   void OnMouseReleased(const ui::MouseEvent& event) override;
 
+  // Invoked to initiate animate in/out of this view. Any animations created
+  // will be associated with the specified `observer`.
+  void AnimateIn(ui::LayerAnimationObserver* observer);
+  void AnimateOut(ui::LayerAnimationObserver* observer);
+
   // Starts a drag from this view at the location specified by the given `event`
   // and with the specified `source`. Note that this method copies the logic of
   // `views::View::DoDrag()` as a workaround to that API being private.
@@ -80,6 +89,13 @@ class ASH_EXPORT HoldingSpaceItemView : public views::InkDropHostView {
   void OnPinPressed();
   void UpdatePin();
 
+  // Animates this view to the specified `opacity` and `transform`, preempting
+  // any in-progress animations. Any animations created will be associated with
+  // the specified `observer`.
+  void AnimateImmediatelyTo(float opacity,
+                            const gfx::Transform& transform,
+                            ui::LayerAnimationObserver* observer);
+
   HoldingSpaceItemViewDelegate* const delegate_;
   const HoldingSpaceItem* const item_;
 

ash/system/holding_space/holding_space_item_views_container.cc

@@ -5,49 +5,16 @@
 #include "ash/system/holding_space/holding_space_item_views_container.h"
 
 #include "ash/public/cpp/holding_space/holding_space_item.h"
-#include "ash/system/holding_space/holding_space_item_view.h"
 #include "ash/system/holding_space/holding_space_item_view_delegate.h"
-#include "ui/compositor/layer_animation_observer.h"
-#include "ui/compositor/scoped_layer_animation_settings.h"
+#include "ui/compositor/callback_layer_animation_observer.h"
 
 namespace ash {
 
 namespace {
 
-using AnimatableProperty = ui::LayerAnimationElement::AnimatableProperty;
-
-// CallbackAnimationObserver ---------------------------------------------------
-
-// An implicit animation observer which invokes a `callback` on animation
-// completion. The `callback` will be notified whether the animation completed
-// due to abort or if the animation completed normally.
-class CallbackAnimationObserver : public ui::ImplicitAnimationObserver {
- public:
-  using Callback = base::RepeatingCallback<void(bool aborted)>;
-
-  explicit CallbackAnimationObserver(Callback callback) : callback_(callback) {}
-  CallbackAnimationObserver(const CallbackAnimationObserver&) = delete;
-  CallbackAnimationObserver& operator=(const CallbackAnimationObserver&) =
-      delete;
-  ~CallbackAnimationObserver() override = default;
-
- private:
-  // ui::ImplicitAnimationObserver:
-  void OnImplicitAnimationsCompleted() override {
-    bool aborted = false;
-    for (int i = AnimatableProperty::FIRST_PROPERTY;
-         i < AnimatableProperty::SENTINEL; ++i) {
-      const AnimatableProperty property = static_cast<AnimatableProperty>(i);
-      if (WasAnimationAbortedForProperty(property)) {
-        aborted = true;
-        break;
-      }
-    }
-    callback_.Run(aborted);
-  }
-
-  Callback callback_;
-};
+// Value returned during notification of animation completion events in order to
+// delete the observer which provided notification.
+constexpr bool kDeleteObserver = true;
 
 }  // namespace
 
@@ -55,23 +22,8 @@ class CallbackAnimationObserver : public ui::ImplicitAnimationObserver {
 
 HoldingSpaceItemViewsContainer::HoldingSpaceItemViewsContainer(
     HoldingSpaceItemViewDelegate* delegate)
-    : delegate_(delegate),
-      animate_in_observer_(
-          std::make_unique<CallbackAnimationObserver>(base::BindRepeating(
-              &HoldingSpaceItemViewsContainer::OnAnimateInCompleted,
-              base::Unretained(this)))),
-      animate_out_observer_(
-          std::make_unique<CallbackAnimationObserver>(base::BindRepeating(
-              &HoldingSpaceItemViewsContainer::OnAnimateOutCompleted,
-              base::Unretained(this)))) {
+    : delegate_(delegate) {
   controller_observer_.Add(HoldingSpaceController::Get());
-
-  // The holding space views container will attach `animate_in_observer_` and
-  // `animate_out_observer_` to a `ui::ScopedLayerAnimationSettings` associated
-  // with itself in order to determine when animations are completed. To do so,
-  // the holding space item views container must have a layer.
-  SetPaintToLayer();
-  layer()->SetFillsBoundsOpaquely(false);
 }
 
 HoldingSpaceItemViewsContainer::~HoldingSpaceItemViewsContainer() = default;
@@ -131,13 +83,14 @@ void HoldingSpaceItemViewsContainer::MaybeAnimateIn() {
 
   animation_state_ |= AnimationState::kAnimatingIn;
 
-  // In the event that the call to `AnimateIn()` did not result in an animation
-  // being scheduled, `OnAnimateInCompleted()` should still be called. To ensure
-  // this occurs, add the animation observer to a scoped settings doing nothing.
-  ui::ScopedLayerAnimationSettings animation_settings(layer()->GetAnimator());
-  animation_settings.AddObserver(animate_in_observer_.get());
+  // NOTE: `animate_in_observer` is deleted after `OnAnimateInCompleted()`.
+  ui::CallbackLayerAnimationObserver* animate_in_observer =
+      new ui::CallbackLayerAnimationObserver(base::BindRepeating(
+          &HoldingSpaceItemViewsContainer::OnAnimateInCompleted,
+          base::Unretained(this)));
 
-  AnimateIn(animate_in_observer_.get());
+  AnimateIn(animate_in_observer);
+  animate_in_observer->SetActive();
 }
 
 void HoldingSpaceItemViewsContainer::MaybeAnimateOut() {
@@ -151,22 +104,23 @@ void HoldingSpaceItemViewsContainer::MaybeAnimateOut() {
   // so should not be acted upon by the user during this time.
   SetCanProcessEventsWithinSubtree(false);
 
-  // In the event that the call to `AnimateOut()` did not result in an animation
-  // being scheduled, `OnAnimateOutCompleted()` should still be called. To
-  // ensure this occurs, add the animation observer to a scoped settings doing
-  // nothing.
-  ui::ScopedLayerAnimationSettings animation_settings(layer()->GetAnimator());
-  animation_settings.AddObserver(animate_out_observer_.get());
+  // NOTE: `animate_out_observer` is deleted after `OnAnimateOutCompleted()`.
+  ui::CallbackLayerAnimationObserver* animate_out_observer =
+      new ui::CallbackLayerAnimationObserver(base::BindRepeating(
+          &HoldingSpaceItemViewsContainer::OnAnimateOutCompleted,
+          base::Unretained(this)));
 
-  AnimateOut(animate_out_observer_.get());
+  AnimateOut(animate_out_observer);
+  animate_out_observer->SetActive();
 }
 
-void HoldingSpaceItemViewsContainer::OnAnimateInCompleted(bool aborted) {
+bool HoldingSpaceItemViewsContainer::OnAnimateInCompleted(
+    const ui::CallbackLayerAnimationObserver& observer) {
   DCHECK(animation_state_ & AnimationState::kAnimatingIn);
   animation_state_ &= ~AnimationState::kAnimatingIn;
 
-  if (aborted)
-    return;
+  if (observer.aborted_count())
+    return kDeleteObserver;
 
   DCHECK_EQ(animation_state_, AnimationState::kNotAnimating);
 
@@ -174,14 +128,17 @@ void HoldingSpaceItemViewsContainer::OnAnimateInCompleted(bool aborted) {
   // that have been animated in should all be associated with holding space
   // items that exist in the model.
   SetCanProcessEventsWithinSubtree(true);
+
+  return kDeleteObserver;
 }
 
-void HoldingSpaceItemViewsContainer::OnAnimateOutCompleted(bool aborted) {
+bool HoldingSpaceItemViewsContainer::OnAnimateOutCompleted(
+    const ui::CallbackLayerAnimationObserver& observer) {
   DCHECK(animation_state_ & AnimationState::kAnimatingOut);
   animation_state_ &= ~AnimationState::kAnimatingOut;
 
-  if (aborted)
-    return;
+  if (observer.aborted_count())
+    return kDeleteObserver;
 
   DCHECK_EQ(animation_state_, AnimationState::kNotAnimating);
 
@@ -196,7 +153,7 @@ void HoldingSpaceItemViewsContainer::OnAnimateOutCompleted(bool aborted) {
 
   HoldingSpaceModel* model = HoldingSpaceController::Get()->model();
   if (!model)
-    return;
+    return kDeleteObserver;
 
   bool is_empty = true;
 
@@ -209,6 +166,8 @@ void HoldingSpaceItemViewsContainer::OnAnimateOutCompleted(bool aborted) {
 
   if (!is_empty)
     MaybeAnimateIn();
+
+  return kDeleteObserver;
 }
 
 }  // namespace ash

ash/system/holding_space/holding_space_item_views_container.h

@@ -14,7 +14,8 @@
 #include "ui/views/view.h"
 
 namespace ui {
-class ImplicitAnimationObserver;
+class CallbackLayerAnimationObserver;
+class LayerAnimationObserver;
 }  // namespace ui
 
 namespace ash {
@@ -79,11 +80,11 @@ class HoldingSpaceItemViewsContainer : public views::View,
 
   // Invoked to initiate animate in of the contents of this holding space item
   // views container. Any animations created must be associated with `observer`.
-  virtual void AnimateIn(ui::ImplicitAnimationObserver* observer) = 0;
+  virtual void AnimateIn(ui::LayerAnimationObserver* observer) = 0;
 
   // Invoked to initiate animate out of the contents of this holding space item
   // views container. Any animations created must be associated with `observer`.
-  virtual void AnimateOut(ui::ImplicitAnimationObserver* observer) = 0;
+  virtual void AnimateOut(ui::LayerAnimationObserver* observer) = 0;
 
   HoldingSpaceItemViewDelegate* delegate() { return delegate_; }
 
@@ -103,16 +104,13 @@ class HoldingSpaceItemViewsContainer : public views::View,
   void MaybeAnimateOut();
 
   // Invoked when an animate in/out of the contents of this holding space item
-  // views container has been completed. If `aborted` is true, the animation
-  // completed due to abort, otherwise the animation completed normally.
-  void OnAnimateInCompleted(bool aborted);
-  void OnAnimateOutCompleted(bool aborted);
+  // views container has been completed. These methods always return true to
+  // delete the observer which notified the event.
+  bool OnAnimateInCompleted(const ui::CallbackLayerAnimationObserver&);
+  bool OnAnimateOutCompleted(const ui::CallbackLayerAnimationObserver&);
 
   HoldingSpaceItemViewDelegate* const delegate_;
 
-  std::unique_ptr<ui::ImplicitAnimationObserver> animate_in_observer_;
-  std::unique_ptr<ui::ImplicitAnimationObserver> animate_out_observer_;
-
   // Bit flag representation of current `AnimationState`. Note that it is
   // briefly possible to be both `kAnimatingIn` and `kAnimatingOut` when one
   // animation is preempting another.

ash/system/holding_space/pinned_files_container.cc

@@ -180,14 +180,16 @@ void PinnedFilesContainer::RemoveAllHoldingSpaceItemViews() {
   item_chips_container_->RemoveAllChildViews(true);
 }
 
-// TODO(dmblack): Implement.
-void PinnedFilesContainer::AnimateIn(ui::ImplicitAnimationObserver* observer) {
-  NOTIMPLEMENTED();
+// TODO(dmblack): Handle grow/shrink of container.
+void PinnedFilesContainer::AnimateIn(ui::LayerAnimationObserver* observer) {
+  for (auto& view_by_item_id : views_by_item_id_)
+    view_by_item_id.second->AnimateIn(observer);
 }
 
-// TODO(dmblack): Implement.
-void PinnedFilesContainer::AnimateOut(ui::ImplicitAnimationObserver* observer) {
-  NOTIMPLEMENTED();
+// TODO(dmblack): Handle animate out of `empty_prompt_label_`.
+void PinnedFilesContainer::AnimateOut(ui::LayerAnimationObserver* observer) {
+  for (auto& view_by_item_id : views_by_item_id_)
+    view_by_item_id.second->AnimateOut(observer);
 }
 
 }  // namespace ash

ash/system/holding_space/pinned_files_container.h

@@ -16,6 +16,7 @@ class Label;
 namespace ash {
 
 class HoldingSpaceItemChipsContainer;
+class HoldingSpaceItemView;
 
 // Container for pinned files that the user adds to the holding space bubble.
 class PinnedFilesContainer : public HoldingSpaceItemViewsContainer {
@@ -32,15 +33,15 @@ class PinnedFilesContainer : public HoldingSpaceItemViewsContainer {
   bool WillAddHoldingSpaceItemView(const HoldingSpaceItem* item) override;
   void AddHoldingSpaceItemView(const HoldingSpaceItem* item) override;
   void RemoveAllHoldingSpaceItemViews() override;
-  void AnimateIn(ui::ImplicitAnimationObserver* observer) override;
-  void AnimateOut(ui::ImplicitAnimationObserver* observer) override;
+  void AnimateIn(ui::LayerAnimationObserver* observer) override;
+  void AnimateOut(ui::LayerAnimationObserver* observer) override;
 
  private:
   // Owned by view hierarchy.
   views::Label* empty_prompt_label_ = nullptr;
   HoldingSpaceItemChipsContainer* item_chips_container_ = nullptr;
 
-  std::map<std::string, views::View*> views_by_item_id_;
+  std::map<std::string, HoldingSpaceItemView*> views_by_item_id_;
 };
 
 }  // namespace ash

ash/system/holding_space/recent_files_container.cc

@@ -194,14 +194,23 @@ void RecentFilesContainer::RemoveAllHoldingSpaceItemViews() {
   downloads_container_->RemoveAllChildViews(true);
 }
 
-// TODO(dmblack): Implement.
-void RecentFilesContainer::AnimateIn(ui::ImplicitAnimationObserver* observer) {
-  NOTIMPLEMENTED();
+// TODO(dmblack): Handle grow/shrink of container.
+// TODO(dmblack): Animate the screen captures section separately from the
+// downloads section so that adding/removing an item from one section does not
+// impact the other.
+void RecentFilesContainer::AnimateIn(ui::LayerAnimationObserver* observer) {
+  for (auto& view_by_item_id : views_by_item_id_)
+    view_by_item_id.second->AnimateIn(observer);
 }
 
-// TODO(dmblack): Implement.
-void RecentFilesContainer::AnimateOut(ui::ImplicitAnimationObserver* observer) {
-  NOTIMPLEMENTED();
+// TODO(dmblack): Handle animate out of screen captures and downloads labels if
+// those sections are going away permanently.
+// TODO(dmblack): Animate the screen captures section separately from the
+// downloads section so that adding/removing an item from one section does not
+// impact the other.
+void RecentFilesContainer::AnimateOut(ui::LayerAnimationObserver* observer) {
+  for (auto& view_by_item_id : views_by_item_id_)
+    view_by_item_id.second->AnimateOut(observer);
 }
 
 void RecentFilesContainer::AddHoldingSpaceScreenCaptureView(

ash/system/holding_space/recent_files_container.h

@@ -16,6 +16,7 @@ class Label;
 namespace ash {
 
 class HoldingSpaceItemChipsContainer;
+class HoldingSpaceItemView;
 
 // Container for the recent files (e.g. screen captures, downloads, etc).
 class RecentFilesContainer : public HoldingSpaceItemViewsContainer {
@@ -33,8 +34,8 @@ class RecentFilesContainer : public HoldingSpaceItemViewsContainer {
   bool WillAddHoldingSpaceItemView(const HoldingSpaceItem* item) override;
   void AddHoldingSpaceItemView(const HoldingSpaceItem* item) override;
   void RemoveAllHoldingSpaceItemViews() override;
-  void AnimateIn(ui::ImplicitAnimationObserver* observer) override;
-  void AnimateOut(ui::ImplicitAnimationObserver* observer) override;
+  void AnimateIn(ui::LayerAnimationObserver* observer) override;
+  void AnimateOut(ui::LayerAnimationObserver* observer) override;
 
  private:
   void AddHoldingSpaceScreenCaptureView(const HoldingSpaceItem* item);
@@ -50,7 +51,7 @@ class RecentFilesContainer : public HoldingSpaceItemViewsContainer {
   HoldingSpaceItemChipsContainer* downloads_container_ = nullptr;
   views::View* downloads_header_ = nullptr;
 
-  std::map<std::string, views::View*> views_by_item_id_;
+  std::map<std::string, HoldingSpaceItemView*> views_by_item_id_;
 };
 
 }  // namespace ash

chrome/browser/ui/ash/holding_space/holding_space_ui_browsertest.cc

@@ -250,6 +250,9 @@ class HoldingSpaceUiDragAndDropBrowserTest
 
 // Verifies that drag-and-drop of holding space items works.
 IN_PROC_BROWSER_TEST_P(HoldingSpaceUiDragAndDropBrowserTest, DragAndDrop) {
+  ui::ScopedAnimationDurationScaleMode scoped_animation_duration_scale_mode(
+      ui::ScopedAnimationDurationScaleMode::ZERO_DURATION);
+
   // Verify drag-and-drop of download items.
   HoldingSpaceItem* const download_file = AddDownloadFile();
 
@@ -320,6 +323,9 @@ IN_PROC_BROWSER_TEST_F(HoldingSpaceUiBrowserTest, LockScreen) {
 
 // Verifies that opening holding space items works.
 IN_PROC_BROWSER_TEST_F(HoldingSpaceUiBrowserTest, OpenItem) {
+  ui::ScopedAnimationDurationScaleMode scoped_animation_duration_scale_mode(
+      ui::ScopedAnimationDurationScaleMode::ZERO_DURATION);
+
   auto* const activation_client = wm::GetActivationClient(
       HoldingSpaceBrowserTestBase::GetRootWindowForNewWindows());