Restrict number of downloads/screenshots shown in holding space.

We should show max 2 downloads in holding space UI.
We should show max 3 screenshots in holding space UI.

Bug: 1131263, 1131265
Change-Id: I912b87f77bae136d7fcb3095dc7f0b70a83884f8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2427076
Commit-Queue: David Black <dmblack@google.com>
Reviewed-by: Ahmed Mehfooz <amehfooz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#810094}

ash/public/cpp/holding_space/holding_space_constants.h

@@ -46,6 +46,12 @@ constexpr int kHoldingSpaceRecentFilesContainerId = 2;
 // Note that this is not enforced for pinned items.
 constexpr base::TimeDelta kMaxFileAge = base::TimeDelta::FromDays(1);
 
+// The maximum allowed number of downloads to display in holding space UI.
+constexpr size_t kMaxDownloads = 2u;
+
+// The maximum allowed number of screenshots to display in holding space UI.
+constexpr size_t kMaxScreenshots = 3u;
+
 // Mime type with wildcard which matches all image types.
 constexpr char kMimeTypeImage[] = "image/*";
 

ash/system/holding_space/recent_files_container.cc

@@ -14,11 +14,14 @@
 #include "ash/system/holding_space/holding_space_item_screenshot_view.h"
 #include "ash/system/tray/tray_constants.h"
 #include "ash/system/tray/tray_popup_item_style.h"
+#include "base/containers/adapters.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/views/border.h"
 #include "ui/views/controls/label.h"
 #include "ui/views/controls/separator.h"
 #include "ui/views/layout/box_layout.h"
+#include "ui/views/layout/flex_layout.h"
+#include "ui/views/view_class_properties.h"
 
 namespace ash {
 
@@ -44,10 +47,13 @@ RecentFilesContainer::RecentFilesContainer(
   style.SetupLabel(screenshots_label);
 
   screenshots_container_ = AddChildView(std::make_unique<views::View>());
-  screenshots_container_->SetLayoutManager(std::make_unique<views::BoxLayout>(
-      views::BoxLayout::Orientation::kHorizontal,
-      kHoldingSpaceScreenshotsContainerPadding,
-      kHoldingSpaceScreenshotSpacing));
+  screenshots_container_
+      ->SetLayoutManager(std::make_unique<views::FlexLayout>())
+      ->SetOrientation(views::LayoutOrientation::kHorizontal)
+      .SetInteriorMargin(kHoldingSpaceScreenshotsContainerPadding)
+      .SetDefault(views::kMarginsKey,
+                  gfx::Insets(/*top=*/0, /*left=*/0, /*bottom=*/0,
+                              /*right=*/kHoldingSpaceScreenshotSpacing));
 
   auto* recent_downloads_label = AddChildView(std::make_unique<views::Label>(
       l10n_util::GetStringUTF16(IDS_ASH_HOLDING_SPACE_RECENT_DOWNLOADS_TITLE)));
@@ -65,16 +71,10 @@ RecentFilesContainer::~RecentFilesContainer() = default;
 
 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>(delegate_, item),
-        /*index=*/0);
-  } else if (item->type() == HoldingSpaceItem::Type::kDownload) {
-    views_by_item_id_[item->id()] = recent_downloads_container_->AddChildViewAt(
-        std::make_unique<HoldingSpaceItemChipView>(delegate_, item),
-        /*index=*/0);
-  }
+  if (item->type() == HoldingSpaceItem::Type::kScreenshot)
+    AddHoldingSpaceScreenshotView(item);
+  else if (item->type() == HoldingSpaceItem::Type::kDownload)
+    AddHoldingSpaceDownloadView(item);
 }
 
 void RecentFilesContainer::RemoveAllHoldingSpaceItemViews() {
@@ -85,17 +85,110 @@ void RecentFilesContainer::RemoveAllHoldingSpaceItemViews() {
 
 void RecentFilesContainer::RemoveHoldingSpaceItemView(
     const HoldingSpaceItem* item) {
+  if (item->type() == HoldingSpaceItem::Type::kScreenshot)
+    RemoveHoldingSpaceScreenshotView(item);
+  else if (item->type() == HoldingSpaceItem::Type::kDownload)
+    RemoveHoldingSpaceDownloadView(item);
+}
+
+void RecentFilesContainer::AddHoldingSpaceScreenshotView(
+    const HoldingSpaceItem* item) {
+  DCHECK_EQ(item->type(), HoldingSpaceItem::Type::kScreenshot);
+  DCHECK(!base::Contains(views_by_item_id_, item->id()));
+
+  // Remove the last screenshot view if we are already at max capacity.
+  if (screenshots_container_->children().size() == kMaxScreenshots) {
+    std::unique_ptr<views::View> view =
+        screenshots_container_->RemoveChildViewT(
+            screenshots_container_->children().back());
+    views_by_item_id_.erase(
+        HoldingSpaceItemView::Cast(view.get())->item()->id());
+  }
+
+  // Add the screenshot view to the front in order to sort by recency.
+  views_by_item_id_[item->id()] = screenshots_container_->AddChildViewAt(
+      std::make_unique<HoldingSpaceItemScreenshotView>(delegate_, item),
+      /*index=*/0);
+}
+
+void RecentFilesContainer::RemoveHoldingSpaceScreenshotView(
+    const HoldingSpaceItem* item) {
+  DCHECK_EQ(item->type(), HoldingSpaceItem::Type::kScreenshot);
+
   auto it = views_by_item_id_.find(item->id());
   if (it == views_by_item_id_.end())
     return;
 
-  if (item->type() == HoldingSpaceItem::Type::kScreenshot) {
-    screenshots_container_->RemoveChildViewT(it->second);
-  } else if (item->type() == HoldingSpaceItem::Type::kDownload) {
-    recent_downloads_container_->RemoveChildViewT(it->second);
+  // Remove the screenshot view associated with `item`.
+  screenshots_container_->RemoveChildViewT(it->second);
+  views_by_item_id_.erase(it);
+
+  // Verify that we are *not* at max capacity.
+  DCHECK_LT(screenshots_container_->children().size(), kMaxScreenshots);
+
+  // Since we are under max capacity, we can add at most one screenshot 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->type() == HoldingSpaceItem::Type::kScreenshot &&
+        !base::Contains(views_by_item_id_, candidate->id())) {
+      views_by_item_id_[candidate->id()] = screenshots_container_->AddChildView(
+          std::make_unique<HoldingSpaceItemScreenshotView>(delegate_,
+                                                           candidate.get()));
+      return;
+    }
   }
+}
 
+void RecentFilesContainer::AddHoldingSpaceDownloadView(
+    const HoldingSpaceItem* item) {
+  DCHECK_EQ(item->type(), HoldingSpaceItem::Type::kDownload);
+  DCHECK(!base::Contains(views_by_item_id_, item->id()));
+
+  // Remove the last download view if we are already at max capacity.
+  if (recent_downloads_container_->children().size() == kMaxDownloads) {
+    std::unique_ptr<views::View> view =
+        recent_downloads_container_->RemoveChildViewT(
+            recent_downloads_container_->children().back());
+    views_by_item_id_.erase(
+        HoldingSpaceItemView::Cast(view.get())->item()->id());
+  }
+
+  // Add the download view to the front in order to sort by recency.
+  views_by_item_id_[item->id()] = recent_downloads_container_->AddChildViewAt(
+      std::make_unique<HoldingSpaceItemChipView>(delegate_, item), /*index=*/0);
+}
+
+void RecentFilesContainer::RemoveHoldingSpaceDownloadView(
+    const HoldingSpaceItem* item) {
+  DCHECK_EQ(item->type(), HoldingSpaceItem::Type::kDownload);
+
+  auto it = views_by_item_id_.find(item->id());
+  if (it == views_by_item_id_.end())
+    return;
+
+  // Remove the download view associated with `item`.
+  recent_downloads_container_->RemoveChildViewT(it->second);
   views_by_item_id_.erase(it);
+
+  // Verify that we are *not* at max capacity.
+  DCHECK_LT(recent_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->type() == HoldingSpaceItem::Type::kDownload &&
+        !base::Contains(views_by_item_id_, candidate->id())) {
+      views_by_item_id_[candidate->id()] =
+          recent_downloads_container_->AddChildView(
+              std::make_unique<HoldingSpaceItemChipView>(delegate_,
+                                                         candidate.get()));
+      return;
+    }
+  }
 }
 
 }  // namespace ash
\ No newline at end of file

ash/system/holding_space/recent_files_container.h

@@ -28,6 +28,11 @@ class RecentFilesContainer : public HoldingSpaceItemViewsContainer {
   void RemoveHoldingSpaceItemView(const HoldingSpaceItem* item) override;
 
  private:
+  void AddHoldingSpaceScreenshotView(const HoldingSpaceItem* item);
+  void RemoveHoldingSpaceScreenshotView(const HoldingSpaceItem* item);
+  void AddHoldingSpaceDownloadView(const HoldingSpaceItem* item);
+  void RemoveHoldingSpaceDownloadView(const HoldingSpaceItem* item);
+
   HoldingSpaceItemViewDelegate* const delegate_;
   views::View* screenshots_container_ = nullptr;
   HoldingSpaceItemChipsContainer* recent_downloads_container_ = nullptr;