Add test coverage for drag-and-drop of download chips.

Bug: 1124581
Change-Id: I6dead04514935b1a6f5a9abc9f6e4398991776f5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2399455
Commit-Queue: David Black <dmblack@google.com>
Reviewed-by: Ahmed Mehfooz <amehfooz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#805454}

ash/public/cpp/holding_space/holding_space_constants.h

@@ -10,7 +10,7 @@
 
 namespace ash {
 
-// UI constants.
+// Appearance.
 constexpr gfx::Insets kHoldingSpaceContainerPadding(16);
 constexpr int kHoldingSpaceContainerSeparation = 8;
 constexpr gfx::Insets kHoldingSpaceChipPadding(8);
@@ -22,6 +22,10 @@ constexpr int kHoldingSpaceColumnWidth = 160;
 constexpr int kHoldingSpaceRowPadding = 8;
 constexpr gfx::Size kHoldingSpaceScreenshotSize(104, 80);
 
+// View IDs.
+constexpr int kHoldingSpacePinnedFilesContainerId = 1;
+constexpr int kHoldingSpaceRecentFilesContainerId = 2;
+
 }  // namespace ash
 
 #endif  // ASH_PUBLIC_CPP_HOLDING_SPACE_HOLDING_SPACE_CONSTANTS_H_

ash/public/cpp/holding_space/holding_space_test_api.h

@@ -42,6 +42,10 @@ class ASH_EXPORT HoldingSpaceTestApi {
   // Returns true if holding space UI is showing, false otherwise.
   bool IsShowing();
 
+  // Returns the collection of download chips in holding space UI.
+  // If holding space UI is not visible, an empty collection is returned.
+  std::vector<views::View*> GetDownloadChips();
+
   // Returns the collection of pinned file chips in holding space UI.
   // If holding space UI is not visible, an empty collection is returned.
   std::vector<views::View*> GetPinnedFileChips();

ash/system/holding_space/holding_space_test_api.cc

@@ -5,6 +5,7 @@
 #include "ash/public/cpp/holding_space/holding_space_test_api.h"
 
 #include "ash/drag_drop/drag_drop_controller.h"
+#include "ash/public/cpp/holding_space/holding_space_constants.h"
 #include "ash/public/cpp/test/shell_test_api.h"
 #include "ash/shelf/shelf.h"
 #include "ash/shelf/shelf_widget.h"
@@ -88,18 +89,37 @@ bool HoldingSpaceTestApi::IsShowing() {
          holding_space_tray_->GetBubbleView()->GetVisible();
 }
 
+std::vector<views::View*> HoldingSpaceTestApi::GetDownloadChips() {
+  std::vector<views::View*> download_chips;
+  if (holding_space_tray_->GetBubbleView()) {
+    FindDescendentsOfClass<HoldingSpaceItemChipView>(
+        holding_space_tray_->GetBubbleView()->GetViewByID(
+            kHoldingSpaceRecentFilesContainerId),
+        &download_chips);
+  }
+  return download_chips;
+}
+
 std::vector<views::View*> HoldingSpaceTestApi::GetPinnedFileChips() {
-  std::vector<views::View*> chips;
-  FindDescendentsOfClass<HoldingSpaceItemChipView>(
-      holding_space_tray_->GetBubbleView(), &chips);
-  return chips;
+  std::vector<views::View*> pinned_file_chips;
+  if (holding_space_tray_->GetBubbleView()) {
+    FindDescendentsOfClass<HoldingSpaceItemChipView>(
+        holding_space_tray_->GetBubbleView()->GetViewByID(
+            kHoldingSpacePinnedFilesContainerId),
+        &pinned_file_chips);
+  }
+  return pinned_file_chips;
 }
 
 std::vector<views::View*> HoldingSpaceTestApi::GetScreenshotViews() {
-  std::vector<views::View*> screenshots;
-  FindDescendentsOfClass<HoldingSpaceScreenshotView>(
-      holding_space_tray_->GetBubbleView(), &screenshots);
-  return screenshots;
+  std::vector<views::View*> screenshot_views;
+  if (holding_space_tray_->GetBubbleView()) {
+    FindDescendentsOfClass<HoldingSpaceScreenshotView>(
+        holding_space_tray_->GetBubbleView()->GetViewByID(
+            kHoldingSpaceRecentFilesContainerId),
+        &screenshot_views);
+  }
+  return screenshot_views;
 }
 
 }  // namespace ash

ash/system/holding_space/pinned_files_container.cc

@@ -23,6 +23,8 @@
 namespace ash {
 
 PinnedFilesContainer::PinnedFilesContainer() {
+  SetID(kHoldingSpacePinnedFilesContainerId);
+
   SetLayoutManager(std::make_unique<views::BoxLayout>(
       views::BoxLayout::Orientation::kVertical, kHoldingSpaceContainerPadding));
 

ash/system/holding_space/recent_files_container.cc

@@ -22,6 +22,8 @@
 namespace ash {
 
 RecentFilesContainer::RecentFilesContainer() {
+  SetID(kHoldingSpaceRecentFilesContainerId);
+
   SetLayoutManager(std::make_unique<views::BoxLayout>(
       views::BoxLayout::Orientation::kVertical, kHoldingSpaceContainerPadding));
   auto setup_layered_child = [](views::View* child) {

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

@@ -27,6 +27,23 @@ namespace {
 
 // Helpers ---------------------------------------------------------------------
 
+// Adds and returns a holding space item of the specified `type` backed by the
+// file at the specified `file_path`.
+HoldingSpaceItem* AddItem(HoldingSpaceItem::Type type,
+                          const base::FilePath& file_path) {
+  auto item = HoldingSpaceItem::CreateFileBackedItem(
+      type, file_path,
+      /*file_system_url=*/GURL(),
+      /*image=*/
+      std::make_unique<HoldingSpaceImage>(
+          /*placeholder=*/gfx::ImageSkia(),
+          /*async_bitmap_resolver=*/base::DoNothing()));
+
+  auto* item_ptr = item.get();
+  HoldingSpaceController::Get()->model()->AddItem(std::move(item));
+  return item_ptr;
+}
+
 // Returns the path of the downloads mount point for the given `profile`.
 base::FilePath GetDownloadsPath(Profile* profile) {
   base::FilePath result;
@@ -102,40 +119,28 @@ bool HoldingSpaceBrowserTestBase::IsShowing() {
   return test_api_->IsShowing();
 }
 
-HoldingSpaceItem* HoldingSpaceBrowserTestBase::AddPinnedFile() {
-  auto item = HoldingSpaceItem::CreateFileBackedItem(
-      HoldingSpaceItem::Type::kPinnedFile,
-      /*file_path=*/CreateTextFile(GetProfile()),
-      /*file_system_url=*/GURL(),
-      /*image=*/
-      std::make_unique<HoldingSpaceImage>(
-          /*placeholder=*/gfx::ImageSkia(),
-          /*async_bitmap_resolver=*/base::DoNothing()));
+HoldingSpaceItem* HoldingSpaceBrowserTestBase::AddDownloadFile() {
+  return AddItem(HoldingSpaceItem::Type::kDownload,
+                 /*file_path=*/CreateTextFile(GetProfile()));
+}
 
-  auto* item_ptr = item.get();
-  HoldingSpaceController::Get()->model()->AddItem(std::move(item));
-  return item_ptr;
+HoldingSpaceItem* HoldingSpaceBrowserTestBase::AddPinnedFile() {
+  return AddItem(HoldingSpaceItem::Type::kPinnedFile,
+                 /*file_path=*/CreateTextFile(GetProfile()));
 }
 
 HoldingSpaceItem* HoldingSpaceBrowserTestBase::AddScreenshotFile() {
-  auto item = HoldingSpaceItem::CreateFileBackedItem(
-      HoldingSpaceItem::Type::kScreenshot,
-      /*file_path=*/CreateImageFile(GetProfile()),
-      /*file_system_url=*/GURL(),
-      /*image=*/
-      std::make_unique<HoldingSpaceImage>(
-          /*placeholder=*/gfx::ImageSkia(),
-          /*async_bitmap_resolver=*/base::DoNothing()));
+  return AddItem(HoldingSpaceItem::Type::kScreenshot,
+                 /*file_path=*/CreateImageFile(GetProfile()));
+}
 
-  auto* item_ptr = item.get();
-  HoldingSpaceController::Get()->model()->AddItem(std::move(item));
-  return item_ptr;
+std::vector<views::View*> HoldingSpaceBrowserTestBase::GetDownloadChips() {
+  return test_api_->GetDownloadChips();
 }
 
 std::vector<views::View*> HoldingSpaceBrowserTestBase::GetPinnedFileChips() {
   return test_api_->GetPinnedFileChips();
 }
-
 std::vector<views::View*> HoldingSpaceBrowserTestBase::GetScreenshotViews() {
   return test_api_->GetScreenshotViews();
 }

chrome/browser/ui/ash/holding_space/holding_space_browsertest_base.h

@@ -47,12 +47,19 @@ class HoldingSpaceBrowserTestBase : public InProcessBrowserTest {
   // Returns true if holding space UI is showing, false otherwise.
   bool IsShowing();
 
+  // Adds and returns an arbitrary download file to the holding space.
+  HoldingSpaceItem* AddDownloadFile();
+
   // Adds and returns an arbitrary pinned file to the holding space.
   HoldingSpaceItem* AddPinnedFile();
 
   // Adds and returns an arbitrary screenshot file to the holding space.
   HoldingSpaceItem* AddScreenshotFile();
 
+  // Returns the collection of download chips in holding space UI.
+  // If holding space UI is not visible, an empty collection is returned.
+  std::vector<views::View*> GetDownloadChips();
+
   // Returns the collection of pinned file chips in holding space UI.
   // If holding space UI is not visible, an empty collection is returned.
   std::vector<views::View*> GetPinnedFileChips();

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

@@ -90,26 +90,33 @@ using HoldingSpaceUiBrowserTest = HoldingSpaceBrowserTestBase;
 
 // Verifies that drag-and-drop of holding space items works.
 IN_PROC_BROWSER_TEST_F(HoldingSpaceUiBrowserTest, DragAndDrop) {
+  HoldingSpaceItem* const download_file = AddDownloadFile();
   HoldingSpaceItem* const pinned_file = AddPinnedFile();
   HoldingSpaceItem* const screenshot_file = AddScreenshotFile();
 
   Show();
   ASSERT_TRUE(IsShowing());
 
-  std::vector<views::View*> chips = GetPinnedFileChips();
-  ASSERT_EQ(1u, chips.size());
+  std::vector<views::View*> download_chips = GetDownloadChips();
+  ASSERT_EQ(1u, download_chips.size());
 
-  std::vector<views::View*> screenshots = GetScreenshotViews();
-  ASSERT_EQ(1u, screenshots.size());
+  std::vector<views::View*> pinned_file_chips = GetPinnedFileChips();
+  ASSERT_EQ(1u, pinned_file_chips.size());
+
+  std::vector<views::View*> screenshot_views = GetScreenshotViews();
+  ASSERT_EQ(1u, screenshot_views.size());
 
   auto* drop_target_view = DropTargetView::Create(GetRootWindowForNewWindows());
   drop_target_view->GetWidget()->SetBounds(gfx::Rect(0, 0, 100, 100));
   drop_target_view->GetWidget()->ShowInactive();
 
-  MouseDrag(/*from=*/chips[0], /*to=*/drop_target_view);
+  MouseDrag(/*from=*/download_chips[0], /*to=*/drop_target_view);
+  EXPECT_EQ(download_file->file_path(), drop_target_view->copied_file_path());
+
+  MouseDrag(/*from=*/pinned_file_chips[0], /*to=*/drop_target_view);
   EXPECT_EQ(pinned_file->file_path(), drop_target_view->copied_file_path());
 
-  MouseDrag(/*from=*/screenshots[0], /*to=*/drop_target_view);
+  MouseDrag(/*from=*/screenshot_views[0], /*to=*/drop_target_view);
   EXPECT_EQ(screenshot_file->file_path(), drop_target_view->copied_file_path());
 
   drop_target_view->GetWidget()->Close();