Prep for holding space container animations.

Moving forward, when an add or removal of a holding space item occurs,
the current container contents will animate out after which the
container will be updated and its new contents animated in.

This CL does not implement the animations but stubs in places for them.
There should be no user visible change as a result of this CL.

Note that this CL also attempts to simplify logic by removing all
holding space item views when they are animated out and re-adding any
views necessary before animating them in. This is slightly less
performant but since the view count is small, the code is much simpler
than would otherwise be if needing to diff the view state with the model
state.

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

ash/system/holding_space/holding_space_item_view.cc

@@ -75,7 +75,7 @@ class CallbackPainter : public views::Painter {
 HoldingSpaceItemView::HoldingSpaceItemView(
     HoldingSpaceItemViewDelegate* delegate,
     const HoldingSpaceItem* item)
-    : delegate_(delegate), item_(item) {
+    : delegate_(delegate), item_(item), item_id_(item->id()) {
   SetProperty(kIsHoldingSpaceItemViewProperty, true);
 
   set_context_menu_controller(delegate_);

ash/system/holding_space/holding_space_item_view.h

@@ -20,8 +20,10 @@ namespace ash {
 class HoldingSpaceItem;
 class HoldingSpaceItemViewDelegate;
 
-// Base class for HoldingSpaceItemChipView and
-// HoldingSpaceItemScreenCaptureView.
+// Base class for `HoldingSpaceItemChipView` and
+// `HoldingSpaceItemScreenCaptureView`. Note that `HoldingSpaceItemView` may
+// temporarily outlive its associated `HoldingSpaceItem` when it is being
+// animated out.
 class ASH_EXPORT HoldingSpaceItemView : public views::InkDropHostView {
  public:
   METADATA_HEADER(HoldingSpaceItemView);
@@ -57,6 +59,7 @@ class ASH_EXPORT HoldingSpaceItemView : public views::InkDropHostView {
                  ui::mojom::DragEventSource source);
 
   const HoldingSpaceItem* item() const { return item_; }
+  const std::string& item_id() const { return item_id_; }
 
   void SetSelected(bool selected);
   bool selected() const { return selected_; }
@@ -73,7 +76,13 @@ class ASH_EXPORT HoldingSpaceItemView : public views::InkDropHostView {
 
   HoldingSpaceItemViewDelegate* const delegate_;
   const HoldingSpaceItem* const item_;
-  views::ToggleImageButton* pin_ = nullptr;
+
+  // Cache the id of the associated holding space item so that it can be
+  // accessed even after `item_` has been destroyed. Note that `item_` may be
+  // destroyed if this view is in the process of animating out.
+  const std::string item_id_;
+
+  views::ToggleImageButton* pin_ = nullptr;  // Owned by view hierarchy.
 
   // Owners for the layers used to paint focused and selected states.
   std::unique_ptr<ui::LayerOwner> selected_layer_owner_;

ash/system/holding_space/holding_space_item_view_delegate.cc

@@ -56,6 +56,20 @@ void OpenItems(const std::vector<const HoldingSpaceItemView*>& views) {
 
 }  // namespace
 
+// HoldingSpaceItemViewDelegate::ScopedSelectionRestore ------------------------
+
+HoldingSpaceItemViewDelegate::ScopedSelectionRestore::ScopedSelectionRestore(
+    HoldingSpaceItemViewDelegate* delegate)
+    : delegate_(delegate) {
+  for (const HoldingSpaceItemView* view : delegate_->GetSelection())
+    selected_item_ids_.push_back(view->item_id());
+}
+
+HoldingSpaceItemViewDelegate::ScopedSelectionRestore::
+    ~ScopedSelectionRestore() {
+  delegate_->SetSelection(selected_item_ids_);
+}
+
 // HoldingSpaceItemViewDelegate ------------------------------------------------
 
 HoldingSpaceItemViewDelegate::HoldingSpaceItemViewDelegate() {
@@ -391,4 +405,10 @@ void HoldingSpaceItemViewDelegate::SetSelection(views::View* selection) {
     view->SetSelected(view == selection);
 }
 
+void HoldingSpaceItemViewDelegate::SetSelection(
+    const std::vector<std::string>& item_ids) {
+  for (HoldingSpaceItemView* view : views_)
+    view->SetSelected(base::Contains(item_ids, view->item_id()));
+}
+
 }  // namespace ash

ash/system/holding_space/holding_space_item_view_delegate.h

@@ -41,6 +41,20 @@ class ASH_EXPORT HoldingSpaceItemViewDelegate
       public views::ViewObserver,
       public ui::SimpleMenuModel::Delegate {
  public:
+  // A class which caches the current selection of holding space item views on
+  // creation and restores that selection on destruction.
+  class ScopedSelectionRestore {
+   public:
+    explicit ScopedSelectionRestore(HoldingSpaceItemViewDelegate* delegate);
+    ScopedSelectionRestore(const ScopedSelectionRestore&) = delete;
+    ScopedSelectionRestore& operator=(const ScopedSelectionRestore&) = delete;
+    ~ScopedSelectionRestore();
+
+   private:
+    HoldingSpaceItemViewDelegate* const delegate_;
+    std::vector<std::string> selected_item_ids_;
+  };
+
   HoldingSpaceItemViewDelegate();
   HoldingSpaceItemViewDelegate(const HoldingSpaceItemViewDelegate&) = delete;
   HoldingSpaceItemViewDelegate& operator=(const HoldingSpaceItemViewDelegate&) =
@@ -103,6 +117,10 @@ class ASH_EXPORT HoldingSpaceItemViewDelegate
   // Marks `view` as selected. All other `views_` are marked unselected.
   void SetSelection(views::View* view);
 
+  // Marks any `views_` whose associated holding space items are contained in
+  // `item_ids` as selected. All other `views_` are marked unselected.
+  void SetSelection(const std::vector<std::string>& item_ids);
+
   std::unique_ptr<ui::SimpleMenuModel> context_menu_model_;
   std::unique_ptr<views::MenuRunner> context_menu_runner_;
 

ash/system/holding_space/holding_space_item_views_container.cc

@@ -5,11 +5,73 @@
 #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"
 
 namespace ash {
 
-HoldingSpaceItemViewsContainer::HoldingSpaceItemViewsContainer() {
+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_;
+};
+
+}  // namespace
+
+// HoldingSpaceItemViewsContainer ----------------------------------------------
+
+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)))) {
   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;
@@ -32,34 +94,121 @@ void HoldingSpaceItemViewsContainer::ChildVisibilityChanged(
 void HoldingSpaceItemViewsContainer::OnHoldingSpaceModelAttached(
     HoldingSpaceModel* model) {
   model_observer_.Add(model);
-  for (const auto& item : model->items()) {
-    if (item->IsFinalized())
-      AddHoldingSpaceItemView(item.get(), /*due_to_finalization=*/false);
-  }
+  for (const auto& item : model->items())
+    OnHoldingSpaceItemAdded(item.get());
 }
 
 void HoldingSpaceItemViewsContainer::OnHoldingSpaceModelDetached(
     HoldingSpaceModel* model) {
   model_observer_.Remove(model);
-  RemoveAllHoldingSpaceItemViews();
+  if (ContainsHoldingSpaceItemViews())
+    MaybeAnimateOut();
 }
 
 void HoldingSpaceItemViewsContainer::OnHoldingSpaceItemAdded(
     const HoldingSpaceItem* item) {
   if (!item->IsFinalized())
     return;
-
-  AddHoldingSpaceItemView(item, /*due_to_finalization=*/false);
+  if (WillAddHoldingSpaceItemView(item))
+    MaybeAnimateOut();
 }
 
 void HoldingSpaceItemViewsContainer::OnHoldingSpaceItemRemoved(
     const HoldingSpaceItem* item) {
-  RemoveHoldingSpaceItemView(item);
+  if (ContainsHoldingSpaceItemView(item))
+    MaybeAnimateOut();
 }
 
 void HoldingSpaceItemViewsContainer::OnHoldingSpaceItemFinalized(
     const HoldingSpaceItem* item) {
-  AddHoldingSpaceItemView(item, /*due_to_finalization=*/true);
+  if (WillAddHoldingSpaceItemView(item))
+    MaybeAnimateOut();
+}
+
+void HoldingSpaceItemViewsContainer::MaybeAnimateIn() {
+  if (animation_state_ & AnimationState::kAnimatingIn)
+    return;
+
+  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());
+
+  AnimateIn(animate_in_observer_.get());
+}
+
+void HoldingSpaceItemViewsContainer::MaybeAnimateOut() {
+  if (animation_state_ & AnimationState::kAnimatingOut)
+    return;
+
+  animation_state_ |= AnimationState::kAnimatingOut;
+
+  // Don't allow event processing while animating out. The views being animated
+  // out may be associated with holding space items that no longer exist and
+  // 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());
+
+  AnimateOut(animate_out_observer_.get());
+}
+
+void HoldingSpaceItemViewsContainer::OnAnimateInCompleted(bool aborted) {
+  DCHECK(animation_state_ & AnimationState::kAnimatingIn);
+  animation_state_ &= ~AnimationState::kAnimatingIn;
+
+  if (aborted)
+    return;
+
+  DCHECK_EQ(animation_state_, AnimationState::kNotAnimating);
+
+  // Restore event processing that was disabled while animating out. The views
+  // that have been animated in should all be associated with holding space
+  // items that exist in the model.
+  SetCanProcessEventsWithinSubtree(true);
+}
+
+void HoldingSpaceItemViewsContainer::OnAnimateOutCompleted(bool aborted) {
+  DCHECK(animation_state_ & AnimationState::kAnimatingOut);
+  animation_state_ &= ~AnimationState::kAnimatingOut;
+
+  if (aborted)
+    return;
+
+  DCHECK_EQ(animation_state_, AnimationState::kNotAnimating);
+
+  // All holding space item views are going to be removed after which views will
+  // be re-added for those items which still exist. A `ScopedSelectionRestore`
+  // will serve to persist the current selection during this modification.
+  HoldingSpaceItemViewDelegate::ScopedSelectionRestore scoped_selection_restore(
+      delegate_);
+
+  if (ContainsHoldingSpaceItemViews())
+    RemoveAllHoldingSpaceItemViews();
+
+  HoldingSpaceModel* model = HoldingSpaceController::Get()->model();
+  if (!model)
+    return;
+
+  bool is_empty = true;
+
+  for (const auto& item : model->items()) {
+    if (item->IsFinalized() && WillAddHoldingSpaceItemView(item.get())) {
+      AddHoldingSpaceItemView(item.get());
+      is_empty = false;
+    }
+  }
+
+  if (!is_empty)
+    MaybeAnimateIn();
 }
 
 }  // namespace ash

ash/system/holding_space/holding_space_item_views_container.h

@@ -13,15 +13,20 @@
 #include "base/scoped_observer.h"
 #include "ui/views/view.h"
 
+namespace ui {
+class ImplicitAnimationObserver;
+}  // namespace ui
+
 namespace ash {
 
 class HoldingSpaceItem;
+class HoldingSpaceItemViewDelegate;
 
 class HoldingSpaceItemViewsContainer : public views::View,
                                        public HoldingSpaceControllerObserver,
                                        public HoldingSpaceModelObserver {
  public:
-  HoldingSpaceItemViewsContainer();
+  explicit HoldingSpaceItemViewsContainer(HoldingSpaceItemViewDelegate*);
   HoldingSpaceItemViewsContainer(const HoldingSpaceItemViewsContainer& other) =
       delete;
   HoldingSpaceItemViewsContainer& operator=(
@@ -33,11 +38,6 @@ class HoldingSpaceItemViewsContainer : public views::View,
   // items are created while the bubble widget is being asynchronously closed.
   void Reset();
 
-  virtual void AddHoldingSpaceItemView(const HoldingSpaceItem* item,
-                                       bool due_to_finalization) = 0;
-  virtual void RemoveAllHoldingSpaceItemViews() = 0;
-  virtual void RemoveHoldingSpaceItemView(const HoldingSpaceItem* item) = 0;
-
   // views::View:
   void ChildPreferredSizeChanged(views::View* child) override;
   void ChildVisibilityChanged(views::View* child) override;
@@ -51,7 +51,73 @@ class HoldingSpaceItemViewsContainer : public views::View,
   void OnHoldingSpaceItemRemoved(const HoldingSpaceItem* item) override;
   void OnHoldingSpaceItemFinalized(const HoldingSpaceItem* item) override;
 
+ protected:
+  // Returns whether a view for the specified `item` exists in this holding
+  // space item views container. Note that returning true will result in a call
+  // to `RemoveAllHoldingSpaceItemViews()` after which views for existing
+  // holding space items will be re-added via call to
+  // `AddHoldingSpaceItemView()`.
+  virtual bool ContainsHoldingSpaceItemView(const HoldingSpaceItem* item) = 0;
+
+  // Returns whether any views associated with holding space items exist which
+  // in this holding space item views container. Note that returning true will
+  // result in a call to `RemoveAllHoldingSpaceItemViews()`.
+  virtual bool ContainsHoldingSpaceItemViews() = 0;
+
+  // Returns whether a view for the specified `item` will be added to this
+  // holding space item views container. Note that `AddHoldingSpaceItemView()`
+  // will only be invoked if this method returns true for the given `item`.
+  virtual bool WillAddHoldingSpaceItemView(const HoldingSpaceItem* item) = 0;
+
+  // Invoked to add a view to this holding space item views container for the
+  // specified `item`.
+  virtual void AddHoldingSpaceItemView(const HoldingSpaceItem* item) = 0;
+
+  // Invoked to remove all views associated with holding space items from this
+  // holding space item views container.
+  virtual void RemoveAllHoldingSpaceItemViews() = 0;
+
+  // 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;
+
+  // 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;
+
+  HoldingSpaceItemViewDelegate* delegate() { return delegate_; }
+
  private:
+  enum AnimationState : uint32_t {
+    kNotAnimating = 0,
+    kAnimatingIn = 1 << 1,
+    kAnimatingOut = 1 << 2,
+  };
+
+  // Invoke to start animating in the contents of this holding space item views
+  // container. No-ops if animate in is already in progress.
+  void MaybeAnimateIn();
+
+  // Invoke to start animating out the contents of this holding space item views
+  // container. No-ops if animate out is already in progress.
+  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);
+
+  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.
+  uint32_t animation_state_ = AnimationState::kNotAnimating;
+
   ScopedObserver<HoldingSpaceController, HoldingSpaceControllerObserver>
       controller_observer_{this};
   ScopedObserver<HoldingSpaceModel, HoldingSpaceModelObserver> model_observer_{

ash/system/holding_space/pinned_files_container.cc

@@ -82,7 +82,7 @@ class HoldingSpaceScrollView : public views::ScrollView,
 
 PinnedFilesContainer::PinnedFilesContainer(
     HoldingSpaceItemViewDelegate* delegate)
-    : delegate_(delegate) {
+    : HoldingSpaceItemViewsContainer(delegate) {
   SetID(kHoldingSpacePinnedFilesContainerId);
 
   SetLayoutManager(std::make_unique<views::BoxLayout>(
@@ -151,31 +151,28 @@ void PinnedFilesContainer::ViewHierarchyChanged(
   SetVisible(details.is_add);
 }
 
-void PinnedFilesContainer::AddHoldingSpaceItemView(const HoldingSpaceItem* item,
-                                                   bool due_to_finalization) {
-  DCHECK(!base::Contains(views_by_item_id_, item->id()));
-  DCHECK(item->IsFinalized());
+bool PinnedFilesContainer::ContainsHoldingSpaceItemView(
+    const HoldingSpaceItem* item) {
+  return base::Contains(views_by_item_id_, item->id());
+}
 
-  if (item->type() != HoldingSpaceItem::Type::kPinnedFile)
-    return;
+bool PinnedFilesContainer::ContainsHoldingSpaceItemViews() {
+  return !views_by_item_id_.empty();
+}
 
-  size_t index = 0;
-
-  if (due_to_finalization) {
-    // Find the position at which the view should be added.
-    for (const auto& candidate :
-         base::Reversed(HoldingSpaceController::Get()->model()->items())) {
-      if (candidate->id() == item->id())
-        break;
-      if (candidate->IsFinalized() &&
-          candidate->type() == HoldingSpaceItem::Type::kPinnedFile) {
-        ++index;
-      }
-    }
-  }
+bool PinnedFilesContainer::WillAddHoldingSpaceItemView(
+    const HoldingSpaceItem* item) {
+  return item->type() == HoldingSpaceItem::Type::kPinnedFile;
+}
+
+void PinnedFilesContainer::AddHoldingSpaceItemView(
+    const HoldingSpaceItem* item) {
+  DCHECK(item->IsFinalized());
+  DCHECK_EQ(item->type(), HoldingSpaceItem::Type::kPinnedFile);
+  DCHECK(!base::Contains(views_by_item_id_, item->id()));
 
   views_by_item_id_[item->id()] = item_chips_container_->AddChildViewAt(
-      std::make_unique<HoldingSpaceItemChipView>(delegate_, item), index);
+      std::make_unique<HoldingSpaceItemChipView>(delegate(), item), 0);
 }
 
 void PinnedFilesContainer::RemoveAllHoldingSpaceItemViews() {
@@ -183,14 +180,14 @@ void PinnedFilesContainer::RemoveAllHoldingSpaceItemViews() {
   item_chips_container_->RemoveAllChildViews(true);
 }
 
-void PinnedFilesContainer::RemoveHoldingSpaceItemView(
-    const HoldingSpaceItem* item) {
-  auto it = views_by_item_id_.find(item->id());
-  if (it == views_by_item_id_.end())
-    return;
+// TODO(dmblack): Implement.
+void PinnedFilesContainer::AnimateIn(ui::ImplicitAnimationObserver* observer) {
+  NOTIMPLEMENTED();
+}
 
-  item_chips_container_->RemoveChildViewT(it->second);
-  views_by_item_id_.erase(it->first);
+// TODO(dmblack): Implement.
+void PinnedFilesContainer::AnimateOut(ui::ImplicitAnimationObserver* observer) {
+  NOTIMPLEMENTED();
 }
 
 }  // namespace ash

ash/system/holding_space/pinned_files_container.h

@@ -16,7 +16,6 @@ class Label;
 namespace ash {
 
 class HoldingSpaceItemChipsContainer;
-class HoldingSpaceItemViewDelegate;
 
 // Container for pinned files that the user adds to the holding space bubble.
 class PinnedFilesContainer : public HoldingSpaceItemViewsContainer {
@@ -28,14 +27,16 @@ class PinnedFilesContainer : public HoldingSpaceItemViewsContainer {
 
   // HoldingSpaceItemViewsContainer:
   void ViewHierarchyChanged(const views::ViewHierarchyChangedDetails&) override;
-  void AddHoldingSpaceItemView(const HoldingSpaceItem* item,
-                               bool due_to_finalization) override;
+  bool ContainsHoldingSpaceItemView(const HoldingSpaceItem* item) override;
+  bool ContainsHoldingSpaceItemViews() override;
+  bool WillAddHoldingSpaceItemView(const HoldingSpaceItem* item) override;
+  void AddHoldingSpaceItemView(const HoldingSpaceItem* item) override;
   void RemoveAllHoldingSpaceItemViews() override;
-  void RemoveHoldingSpaceItemView(const HoldingSpaceItem* item) override;
+  void AnimateIn(ui::ImplicitAnimationObserver* observer) override;
+  void AnimateOut(ui::ImplicitAnimationObserver* observer) override;
 
  private:
-  HoldingSpaceItemViewDelegate* const delegate_;
-
+  // Owned by view hierarchy.
   views::Label* empty_prompt_label_ = nullptr;
   HoldingSpaceItemChipsContainer* item_chips_container_ = nullptr;
 

ash/system/holding_space/recent_files_container.cc

@@ -61,15 +61,6 @@ bool BelongsToScreenCaptureSection(HoldingSpaceItem::Type type) {
          type == HoldingSpaceItem::Type::kScreenRecording;
 }
 
-// Returns if items of the specified types belong to the same section.
-bool BelongToSameSection(HoldingSpaceItem::Type type,
-                         HoldingSpaceItem::Type other_type) {
-  return (BelongsToScreenCaptureSection(type) &&
-          BelongsToScreenCaptureSection(other_type)) ||
-         (BelongsToDownloadsSection(type) &&
-          BelongsToDownloadsSection(other_type));
-}
-
 // DownloadsHeader--------------------------------------------------------------
 
 class DownloadsHeader : public views::Button {
@@ -114,7 +105,7 @@ class DownloadsHeader : public views::Button {
 
 RecentFilesContainer::RecentFilesContainer(
     HoldingSpaceItemViewDelegate* delegate)
-    : delegate_(delegate) {
+    : HoldingSpaceItemViewsContainer(delegate) {
   SetID(kHoldingSpaceRecentFilesContainerId);
   SetVisible(false);
 
@@ -170,34 +161,31 @@ void RecentFilesContainer::ViewHierarchyChanged(
     OnDownloadsContainerViewHierarchyChanged(details);
 }
 
-void RecentFilesContainer::AddHoldingSpaceItemView(const HoldingSpaceItem* item,
-                                                   bool due_to_finalization) {
-  DCHECK(item->IsFinalized());
+bool RecentFilesContainer::ContainsHoldingSpaceItemView(
+    const HoldingSpaceItem* item) {
+  return base::Contains(views_by_item_id_, item->id());
+}
 
-  if (!BelongsToScreenCaptureSection(item->type()) &&
-      !BelongsToDownloadsSection(item->type())) {
-    return;
-  }
+bool RecentFilesContainer::ContainsHoldingSpaceItemViews() {
+  return !views_by_item_id_.empty();
+}
 
-  size_t index = 0;
-
-  if (due_to_finalization) {
-    // Find the position at which the view should be added.
-    for (const auto& candidate :
-         base::Reversed(HoldingSpaceController::Get()->model()->items())) {
-      if (candidate->id() == item->id())
-        break;
-      if (candidate->IsFinalized() &&
-          BelongToSameSection(candidate->type(), item->type())) {
-        ++index;
-      }
-    }
-  }
+bool RecentFilesContainer::WillAddHoldingSpaceItemView(
+    const HoldingSpaceItem* item) {
+  return BelongsToDownloadsSection(item->type()) ||
+         BelongsToScreenCaptureSection(item->type());
+}
+
+void RecentFilesContainer::AddHoldingSpaceItemView(
+    const HoldingSpaceItem* item) {
+  DCHECK(item->IsFinalized());
 
   if (BelongsToScreenCaptureSection(item->type()))
-    AddHoldingSpaceScreenCaptureView(item, index);
+    AddHoldingSpaceScreenCaptureView(item);
   else if (BelongsToDownloadsSection(item->type()))
-    AddHoldingSpaceDownloadView(item, index);
+    AddHoldingSpaceDownloadView(item);
+  else
+    NOTREACHED();
 }
 
 void RecentFilesContainer::RemoveAllHoldingSpaceItemViews() {
@@ -206,23 +194,21 @@ void RecentFilesContainer::RemoveAllHoldingSpaceItemViews() {
   downloads_container_->RemoveAllChildViews(true);
 }
 
-void RecentFilesContainer::RemoveHoldingSpaceItemView(
-    const HoldingSpaceItem* item) {
-  if (BelongsToScreenCaptureSection(item->type()))
-    RemoveHoldingSpaceScreenCaptureView(item);
-  else if (BelongsToDownloadsSection(item->type()))
-    RemoveHoldingSpaceDownloadView(item);
+// TODO(dmblack): Implement.
+void RecentFilesContainer::AnimateIn(ui::ImplicitAnimationObserver* observer) {
+  NOTIMPLEMENTED();
+}
+
+// TODO(dmblack): Implement.
+void RecentFilesContainer::AnimateOut(ui::ImplicitAnimationObserver* observer) {
+  NOTIMPLEMENTED();
 }
 
 void RecentFilesContainer::AddHoldingSpaceScreenCaptureView(
-    const HoldingSpaceItem* item,
-    size_t index) {
+    const HoldingSpaceItem* item) {
   DCHECK(BelongsToScreenCaptureSection(item->type()));
   DCHECK(!base::Contains(views_by_item_id_, item->id()));
 
-  if (index >= kMaxScreenCaptures)
-    return;
-
   // Remove the last screen capture view if we are already at max capacity.
   if (screen_captures_container_->children().size() == kMaxScreenCaptures) {
     std::unique_ptr<views::View> view =
@@ -234,51 +220,14 @@ void RecentFilesContainer::AddHoldingSpaceScreenCaptureView(
 
   // Add the screen capture view to the front in order to sort by recency.
   views_by_item_id_[item->id()] = screen_captures_container_->AddChildViewAt(
-      std::make_unique<HoldingSpaceItemScreenCaptureView>(delegate_, item),
-      index);
-}
-
-void RecentFilesContainer::RemoveHoldingSpaceScreenCaptureView(
-    const HoldingSpaceItem* item) {
-  DCHECK(BelongsToScreenCaptureSection(item->type()));
-
-  auto it = views_by_item_id_.find(item->id());
-  if (it == views_by_item_id_.end())
-    return;
-
-  // Remove the screen capture view associated with `item`.
-  screen_captures_container_->RemoveChildViewT(it->second);
-  views_by_item_id_.erase(it);
-
-  // Verify that we are *not* at max capacity.
-  DCHECK_LT(screen_captures_container_->children().size(), kMaxScreenCaptures);
-
-  // Since we are under max capacity, we can add at most one screen capture view
-  // to replace the view we just removed. Note that we add the replacement to
-  // the back in order to maintain sort by recency.
-  for (const auto& candidate :
-       base::Reversed(HoldingSpaceController::Get()->model()->items())) {
-    if (candidate->IsFinalized() &&
-        BelongsToScreenCaptureSection(item->type()) &&
-        !base::Contains(views_by_item_id_, candidate->id())) {
-      views_by_item_id_[candidate->id()] =
-          screen_captures_container_->AddChildView(
-              std::make_unique<HoldingSpaceItemScreenCaptureView>(
-                  delegate_, candidate.get()));
-      return;
-    }
-  }
+      std::make_unique<HoldingSpaceItemScreenCaptureView>(delegate(), item), 0);
 }
 
 void RecentFilesContainer::AddHoldingSpaceDownloadView(
-    const HoldingSpaceItem* item,
-    size_t index) {
+    const HoldingSpaceItem* item) {
   DCHECK(BelongsToDownloadsSection(item->type()));
   DCHECK(!base::Contains(views_by_item_id_, item->id()));
 
-  if (index >= kMaxDownloads)
-    return;
-
   // Remove the last download view if we are already at max capacity.
   if (downloads_container_->children().size() == kMaxDownloads) {
     std::unique_ptr<views::View> view = downloads_container_->RemoveChildViewT(
@@ -289,37 +238,7 @@ void RecentFilesContainer::AddHoldingSpaceDownloadView(
 
   // Add the download view to the front in order to sort by recency.
   views_by_item_id_[item->id()] = downloads_container_->AddChildViewAt(
-      std::make_unique<HoldingSpaceItemChipView>(delegate_, item), index);
-}
-
-void RecentFilesContainer::RemoveHoldingSpaceDownloadView(
-    const HoldingSpaceItem* item) {
-  DCHECK(BelongsToDownloadsSection(item->type()));
-
-  auto it = views_by_item_id_.find(item->id());
-  if (it == views_by_item_id_.end())
-    return;
-
-  // Remove the download view associated with `item`.
-  downloads_container_->RemoveChildViewT(it->second);
-  views_by_item_id_.erase(it);
-
-  // Verify that we are *not* at max capacity.
-  DCHECK_LT(downloads_container_->children().size(), kMaxDownloads);
-
-  // Since we are under max capacity, we can add at most one download view to
-  // replace the view we just removed. Note that we add the replacement to the
-  // back in order to maintain sort by recency.
-  for (const auto& candidate :
-       base::Reversed(HoldingSpaceController::Get()->model()->items())) {
-    if (candidate->IsFinalized() && BelongsToDownloadsSection(item->type()) &&
-        !base::Contains(views_by_item_id_, candidate->id())) {
-      views_by_item_id_[candidate->id()] = downloads_container_->AddChildView(
-          std::make_unique<HoldingSpaceItemChipView>(delegate_,
-                                                     candidate.get()));
-      return;
-    }
-  }
+      std::make_unique<HoldingSpaceItemChipView>(delegate(), item), 0);
 }
 
 void RecentFilesContainer::OnScreenCapturesContainerViewHierarchyChanged(

ash/system/holding_space/recent_files_container.h

@@ -16,7 +16,6 @@ class Label;
 namespace ash {
 
 class HoldingSpaceItemChipsContainer;
-class HoldingSpaceItemViewDelegate;
 
 // Container for the recent files (e.g. screen captures, downloads, etc).
 class RecentFilesContainer : public HoldingSpaceItemViewsContainer {
@@ -29,24 +28,23 @@ class RecentFilesContainer : public HoldingSpaceItemViewsContainer {
   // HoldingSpaceItemViewsContainer:
   void ChildVisibilityChanged(views::View* child) override;
   void ViewHierarchyChanged(const views::ViewHierarchyChangedDetails&) override;
-  void AddHoldingSpaceItemView(const HoldingSpaceItem* item,
-                               bool due_to_finalization) override;
+  bool ContainsHoldingSpaceItemView(const HoldingSpaceItem* item) override;
+  bool ContainsHoldingSpaceItemViews() override;
+  bool WillAddHoldingSpaceItemView(const HoldingSpaceItem* item) override;
+  void AddHoldingSpaceItemView(const HoldingSpaceItem* item) override;
   void RemoveAllHoldingSpaceItemViews() override;
-  void RemoveHoldingSpaceItemView(const HoldingSpaceItem* item) override;
+  void AnimateIn(ui::ImplicitAnimationObserver* observer) override;
+  void AnimateOut(ui::ImplicitAnimationObserver* observer) override;
 
  private:
-  void AddHoldingSpaceScreenCaptureView(const HoldingSpaceItem* item,
-                                        size_t index);
-  void RemoveHoldingSpaceScreenCaptureView(const HoldingSpaceItem* item);
-  void AddHoldingSpaceDownloadView(const HoldingSpaceItem* item, size_t index);
-  void RemoveHoldingSpaceDownloadView(const HoldingSpaceItem* item);
+  void AddHoldingSpaceScreenCaptureView(const HoldingSpaceItem* item);
+  void AddHoldingSpaceDownloadView(const HoldingSpaceItem* item);
   void OnScreenCapturesContainerViewHierarchyChanged(
       const views::ViewHierarchyChangedDetails& details);
   void OnDownloadsContainerViewHierarchyChanged(
       const views::ViewHierarchyChangedDetails& details);
 
-  HoldingSpaceItemViewDelegate* const delegate_;
-
+  // Owned by view hierarchy.
   views::View* screen_captures_container_ = nullptr;
   views::Label* screen_captures_label_ = nullptr;
   HoldingSpaceItemChipsContainer* downloads_container_ = nullptr;