Add support to add and remove item views

This CL allows the holding space bubble to dynamically
add / remove items and resize as required when that
happens.

Bug: 1129321

Change-Id: I8b072de15628ca0fc56f23d6286026b0f6aeda69
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2412639
Commit-Queue: Ahmed Mehfooz <amehfooz@chromium.org>
Reviewed-by: David Black <dmblack@google.com>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#809164}

ash/BUILD.gn

@@ -932,6 +932,8 @@ component("ash") {
     "system/holding_space/holding_space_item_screenshot_view.h",
     "system/holding_space/holding_space_item_view.cc",
     "system/holding_space/holding_space_item_view.h",
+    "system/holding_space/holding_space_item_views_container.cc",
+    "system/holding_space/holding_space_item_views_container.h",
     "system/holding_space/holding_space_tray.cc",
     "system/holding_space/holding_space_tray.h",
     "system/holding_space/pinned_files_container.cc",

ash/system/holding_space/holding_space_item_chip_view.cc

@@ -99,8 +99,6 @@ void HoldingSpaceItemChipView::ButtonPressed(views::Button* sender,
     HoldingSpaceController::Get()->client()->UnpinItem(*item());
   else
     HoldingSpaceController::Get()->client()->PinItem(*item());
-
-  UpdatePin();
 }
 
 void HoldingSpaceItemChipView::AddPinButton() {

ash/system/holding_space/holding_space_item_chips_container.cc

@@ -12,7 +12,6 @@
 #include "ui/views/layout/proposed_layout.h"
 
 namespace ash {
-
 namespace {
 
 // Need a custom grid layout to facilitate removal of views from the grid,
@@ -41,7 +40,7 @@ class SimpleGridLayout : public views::LayoutManagerBase {
     int total_children = 0;
     for (auto* child : host_view()->children()) {
       if (IsChildIncludedInLayout(child))
-        total_children++;
+        ++total_children;
     }
     // Equivalent to `ceil(children().size() / column_count_)`.
     int number_of_rows = (total_children + column_count_ - 1) / column_count_;
@@ -96,6 +95,12 @@ class SimpleGridLayout : public views::LayoutManagerBase {
     return proposed_layout;
   }
 
+  void OnLayoutChanged() override {
+    LayoutManagerBase::OnLayoutChanged();
+    cached_child_preferred_size_.reset();
+    host_view()->SetPreferredSize(CalculatePreferredSize());
+  }
+
  private:
   mutable base::Optional<gfx::Size> cached_child_preferred_size_;
 

ash/system/holding_space/holding_space_item_views_container.cc

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "ash/system/holding_space/holding_space_item_views_container.h"
+
+#include "ash/public/cpp/holding_space/holding_space_item.h"
+
+namespace ash {
+
+HoldingSpaceItemViewsContainer::HoldingSpaceItemViewsContainer() {
+  controller_observer_.Add(HoldingSpaceController::Get());
+}
+
+HoldingSpaceItemViewsContainer::~HoldingSpaceItemViewsContainer() = default;
+
+void HoldingSpaceItemViewsContainer::ChildPreferredSizeChanged(
+    views::View* child) {
+  PreferredSizeChanged();
+}
+
+void HoldingSpaceItemViewsContainer::OnHoldingSpaceModelAttached(
+    HoldingSpaceModel* model) {
+  model_observer_.Add(model);
+  for (const auto& item : model->items())
+    AddHoldingSpaceItemView(item.get());
+}
+
+void HoldingSpaceItemViewsContainer::OnHoldingSpaceModelDetached(
+    HoldingSpaceModel* model) {
+  model_observer_.Remove(model);
+  RemoveAllHoldingSpaceItemViews();
+}
+
+void HoldingSpaceItemViewsContainer::OnHoldingSpaceItemAdded(
+    const HoldingSpaceItem* item) {
+  AddHoldingSpaceItemView(item);
+}
+
+void HoldingSpaceItemViewsContainer::OnHoldingSpaceItemRemoved(
+    const HoldingSpaceItem* item) {
+  RemoveHoldingSpaceItemView(item);
+}
+
+}  // namespace ash
\ No newline at end of file

ash/system/holding_space/holding_space_item_views_container.h

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef ASH_SYSTEM_HOLDING_SPACE_HOLDING_SPACE_ITEM_VIEWS_CONTAINER_H_
+#define ASH_SYSTEM_HOLDING_SPACE_HOLDING_SPACE_ITEM_VIEWS_CONTAINER_H_
+
+#include "ash/ash_export.h"
+#include "ash/public/cpp/holding_space/holding_space_controller.h"
+#include "ash/public/cpp/holding_space/holding_space_controller_observer.h"
+#include "ash/public/cpp/holding_space/holding_space_model.h"
+#include "ash/public/cpp/holding_space/holding_space_model_observer.h"
+#include "base/scoped_observer.h"
+#include "ui/views/view.h"
+
+namespace ash {
+
+class HoldingSpaceItem;
+
+class HoldingSpaceItemViewsContainer : public views::View,
+                                       public HoldingSpaceControllerObserver,
+                                       public HoldingSpaceModelObserver {
+ public:
+  HoldingSpaceItemViewsContainer();
+  HoldingSpaceItemViewsContainer(const HoldingSpaceItemViewsContainer& other) =
+      delete;
+  HoldingSpaceItemViewsContainer& operator=(
+      const HoldingSpaceItemViewsContainer& other) = delete;
+  ~HoldingSpaceItemViewsContainer() override;
+
+  virtual void AddHoldingSpaceItemView(const HoldingSpaceItem* item) = 0;
+  virtual void RemoveAllHoldingSpaceItemViews() = 0;
+  virtual void RemoveHoldingSpaceItemView(const HoldingSpaceItem* item) = 0;
+
+  // views::View:
+  void ChildPreferredSizeChanged(views::View* child) override;
+
+  // HoldingSpaceControllerObserver:
+  void OnHoldingSpaceModelAttached(HoldingSpaceModel* model) override;
+  void OnHoldingSpaceModelDetached(HoldingSpaceModel* model) override;
+
+  // HoldingSpaceModelObserver:
+  void OnHoldingSpaceItemAdded(const HoldingSpaceItem* item) override;
+  void OnHoldingSpaceItemRemoved(const HoldingSpaceItem* item) override;
+
+ private:
+  ScopedObserver<HoldingSpaceController, HoldingSpaceControllerObserver>
+      controller_observer_{this};
+  ScopedObserver<HoldingSpaceModel, HoldingSpaceModelObserver> model_observer_{
+      this};
+};
+
+}  // namespace ash
+
+#endif  // ASH_SYSTEM_HOLDING_SPACE_HOLDING_SPACE_ITEM_VIEWS_CONTAINER_H_

ash/system/holding_space/pinned_files_container.cc

@@ -40,20 +40,35 @@ PinnedFilesContainer::PinnedFilesContainer() {
   item_chips_container_ =
       AddChildView(std::make_unique<HoldingSpaceItemChipsContainer>());
 
-  // TODO(crbug.com/1125254): Populate containers if and when holding space
-  // model is attached, below is a temporary solution.
-  for (const auto& item : HoldingSpaceController::Get()->model()->items()) {
-    if (item->type() == HoldingSpaceItem::Type::kPinnedFile) {
-      item_chips_container_->AddChildView(
-          std::make_unique<HoldingSpaceItemChipView>(item.get()));
-    }
-  }
+  if (HoldingSpaceController::Get()->model())
+    OnHoldingSpaceModelAttached(HoldingSpaceController::Get()->model());
 }
 
 PinnedFilesContainer::~PinnedFilesContainer() = default;
 
-const char* PinnedFilesContainer::GetClassName() const {
-  return "PinnedFilesContainer";
+void PinnedFilesContainer::AddHoldingSpaceItemView(
+    const HoldingSpaceItem* item) {
+  DCHECK(!base::Contains(views_by_item_id_, item->id()));
+
+  if (item->type() == HoldingSpaceItem::Type::kPinnedFile) {
+    views_by_item_id_[item->id()] = item_chips_container_->AddChildViewAt(
+        std::make_unique<HoldingSpaceItemChipView>(item), 0 /*index*/);
+  }
+}
+
+void PinnedFilesContainer::RemoveAllHoldingSpaceItemViews() {
+  views_by_item_id_.clear();
+  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;
+
+  item_chips_container_->RemoveChildViewT(it->second);
+  views_by_item_id_.erase(it->first);
 }
 
 }  // namespace ash

ash/system/holding_space/pinned_files_container.h

@@ -5,25 +5,31 @@
 #ifndef ASH_SYSTEM_HOLDING_SPACE_PINNED_FILES_CONTAINER_H_
 #define ASH_SYSTEM_HOLDING_SPACE_PINNED_FILES_CONTAINER_H_
 
-#include "ui/views/view.h"
+#include <map>
+
+#include "ash/system/holding_space/holding_space_item_views_container.h"
 
 namespace ash {
 
 class HoldingSpaceItemChipsContainer;
 
 // Container for pinned files that the user adds to the holding space bubble.
-class PinnedFilesContainer : public views::View {
+class PinnedFilesContainer : public HoldingSpaceItemViewsContainer {
  public:
   PinnedFilesContainer();
   PinnedFilesContainer(const PinnedFilesContainer& other) = delete;
   PinnedFilesContainer& operator=(const PinnedFilesContainer& other) = delete;
   ~PinnedFilesContainer() override;
 
-  // views::View:
-  const char* GetClassName() const override;
+  // HoldingSpaceItemViewsContainer:
+  void AddHoldingSpaceItemView(const HoldingSpaceItem* item) override;
+  void RemoveAllHoldingSpaceItemViews() override;
+  void RemoveHoldingSpaceItemView(const HoldingSpaceItem* item) override;
 
  private:
   HoldingSpaceItemChipsContainer* item_chips_container_ = nullptr;
+
+  std::map<std::string, views::View*> views_by_item_id_;
 };
 
 }  // namespace ash

ash/system/holding_space/recent_files_container.cc

@@ -47,15 +47,6 @@ RecentFilesContainer::RecentFilesContainer() {
       kHoldingSpaceScreenshotsContainerPadding,
       kHoldingSpaceScreenshotSpacing));
 
-  // TODO(crbug.com/1125254): Populate containers if and when holding space
-  // model is attached, below is a temporary solution.
-  for (const auto& item : HoldingSpaceController::Get()->model()->items()) {
-    if (item->type() == HoldingSpaceItem::Type::kScreenshot) {
-      screenshots_container_->AddChildView(
-          std::make_unique<HoldingSpaceItemScreenshotView>(item.get()));
-    }
-  }
-
   auto* recent_downloads_label = AddChildView(std::make_unique<views::Label>(
       l10n_util::GetStringUTF16(IDS_ASH_HOLDING_SPACE_RECENT_DOWNLOADS_TITLE)));
   setup_layered_child(recent_downloads_label);
@@ -64,20 +55,43 @@ RecentFilesContainer::RecentFilesContainer() {
   recent_downloads_container_ =
       AddChildView(std::make_unique<HoldingSpaceItemChipsContainer>());
 
-  // TODO(crbug.com/1125254): Populate containers if and when holding space
-  // model is attached, below is a temporary solution.
-  for (const auto& item : HoldingSpaceController::Get()->model()->items()) {
-    if (item->type() == HoldingSpaceItem::Type::kDownload) {
-      recent_downloads_container_->AddChildView(
-          std::make_unique<HoldingSpaceItemChipView>(item.get()));
-    }
-  }
+  if (HoldingSpaceController::Get()->model())
+    OnHoldingSpaceModelAttached(HoldingSpaceController::Get()->model());
 }
 
 RecentFilesContainer::~RecentFilesContainer() = default;
 
-const char* RecentFilesContainer::GetClassName() const {
-  return "RecentFilesContainer";
+void RecentFilesContainer::AddHoldingSpaceItemView(
+    const HoldingSpaceItem* item) {
+  DCHECK(!base::Contains(views_by_item_id_, item->id()));
+  if (item->type() == HoldingSpaceItem::Type::kScreenshot) {
+    views_by_item_id_[item->id()] = screenshots_container_->AddChildViewAt(
+        std::make_unique<HoldingSpaceItemScreenshotView>(item), 0 /*index*/);
+  } else if (item->type() == HoldingSpaceItem::Type::kDownload) {
+    views_by_item_id_[item->id()] = recent_downloads_container_->AddChildViewAt(
+        std::make_unique<HoldingSpaceItemChipView>(item), 0 /*index*/);
+  }
+}
+
+void RecentFilesContainer::RemoveAllHoldingSpaceItemViews() {
+  views_by_item_id_.clear();
+  screenshots_container_->RemoveAllChildViews(true);
+  recent_downloads_container_->RemoveAllChildViews(true);
+}
+
+void RecentFilesContainer::RemoveHoldingSpaceItemView(
+    const HoldingSpaceItem* item) {
+  auto it = views_by_item_id_.find(item->id());
+  if (it == views_by_item_id_.end())
+    return;
+  views::View* view = it->second;
+
+  if (item->type() == HoldingSpaceItem::Type::kScreenshot) {
+    screenshots_container_->RemoveChildViewT(view);
+  } else if (item->type() == HoldingSpaceItem::Type::kDownload) {
+    recent_downloads_container_->RemoveChildViewT(view);
+  }
+  views_by_item_id_.erase(it);
 }
 
 }  // namespace ash
\ No newline at end of file

ash/system/holding_space/recent_files_container.h

@@ -5,26 +5,32 @@
 #ifndef ASH_SYSTEM_HOLDING_SPACE_RECENT_FILES_CONTAINER_H_
 #define ASH_SYSTEM_HOLDING_SPACE_RECENT_FILES_CONTAINER_H_
 
-#include "ui/views/view.h"
+#include <map>
+
+#include "ash/system/holding_space/holding_space_item_views_container.h"
 
 namespace ash {
 
 class HoldingSpaceItemChipsContainer;
 
 // Container for the recent files (Screenshots, downloads etc).
-class RecentFilesContainer : public views::View {
+class RecentFilesContainer : public HoldingSpaceItemViewsContainer {
  public:
   RecentFilesContainer();
   RecentFilesContainer(const RecentFilesContainer& other) = delete;
   RecentFilesContainer& operator=(const RecentFilesContainer& other) = delete;
   ~RecentFilesContainer() override;
 
-  // views::View:
-  const char* GetClassName() const override;
+  // HoldingSpaceItemViewsContainer:
+  void AddHoldingSpaceItemView(const HoldingSpaceItem* item) override;
+  void RemoveAllHoldingSpaceItemViews() override;
+  void RemoveHoldingSpaceItemView(const HoldingSpaceItem* item) override;
 
  private:
   views::View* screenshots_container_ = nullptr;
   HoldingSpaceItemChipsContainer* recent_downloads_container_ = nullptr;
+
+  std::map<std::string, views::View*> views_by_item_id_;
 };
 
 }  // namespace ash