Implement PinItem/UnpinItem in HoldingSpaceClient.

These APIs will eventually be wired up to Holding Space UI.

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

chrome/browser/ui/ash/holding_space/BUILD.gn

@@ -46,6 +46,7 @@ source_set("test_support") {
     "//chrome/browser",
     "//chrome/browser/chromeos",
     "//chrome/browser/extensions",
+    "//chrome/browser/ui",
     "//chrome/test:test_support_ui",
     "//ui/views",
   ]

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

@@ -17,6 +17,7 @@
 #include "chrome/browser/chromeos/file_manager/path_util.h"
 #include "chrome/browser/extensions/component_loader.h"
 #include "chrome/browser/profiles/profile_manager.h"
+#include "chrome/browser/ui/ash/holding_space/holding_space_util.h"
 #include "storage/browser/file_system/external_mount_points.h"
 #include "ui/gfx/image/image_skia.h"
 #include "ui/views/view.h"
@@ -29,11 +30,12 @@ namespace {
 
 // 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,
+HoldingSpaceItem* AddItem(Profile* profile,
+                          HoldingSpaceItem::Type type,
                           const base::FilePath& file_path) {
   auto item = HoldingSpaceItem::CreateFileBackedItem(
       type, file_path,
-      /*file_system_url=*/GURL(),
+      holding_space_util::ResolveFileSystemUrl(profile, file_path),
       /*image=*/
       std::make_unique<HoldingSpaceImage>(
           /*placeholder=*/gfx::ImageSkia(),
@@ -120,17 +122,17 @@ bool HoldingSpaceBrowserTestBase::IsShowing() {
 }
 
 HoldingSpaceItem* HoldingSpaceBrowserTestBase::AddDownloadFile() {
-  return AddItem(HoldingSpaceItem::Type::kDownload,
+  return AddItem(GetProfile(), HoldingSpaceItem::Type::kDownload,
                  /*file_path=*/CreateTextFile(GetProfile()));
 }
 
 HoldingSpaceItem* HoldingSpaceBrowserTestBase::AddPinnedFile() {
-  return AddItem(HoldingSpaceItem::Type::kPinnedFile,
+  return AddItem(GetProfile(), HoldingSpaceItem::Type::kPinnedFile,
                  /*file_path=*/CreateTextFile(GetProfile()));
 }
 
 HoldingSpaceItem* HoldingSpaceBrowserTestBase::AddScreenshotFile() {
-  return AddItem(HoldingSpaceItem::Type::kScreenshot,
+  return AddItem(GetProfile(), HoldingSpaceItem::Type::kScreenshot,
                  /*file_path=*/CreateImageFile(GetProfile()));
 }
 
@@ -141,6 +143,7 @@ std::vector<views::View*> HoldingSpaceBrowserTestBase::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_client_impl.cc

@@ -7,7 +7,12 @@
 #include "ash/public/cpp/holding_space/holding_space_item.h"
 #include "base/bind.h"
 #include "base/notreached.h"
+#include "chrome/browser/chromeos/file_manager/app_id.h"
+#include "chrome/browser/chromeos/file_manager/fileapi_util.h"
 #include "chrome/browser/chromeos/file_manager/open_util.h"
+#include "chrome/browser/ui/ash/holding_space/holding_space_keyed_service.h"
+#include "chrome/browser/ui/ash/holding_space/holding_space_keyed_service_factory.h"
+#include "storage/browser/file_system/file_system_context.h"
 
 namespace ash {
 
@@ -17,6 +22,11 @@ using SuccessCallback = HoldingSpaceClient::SuccessCallback;
 
 // Helpers ---------------------------------------------------------------------
 
+// Returns the `HoldingSpaceKeyedService` associated with the given `profile`.
+HoldingSpaceKeyedService* GetHoldingSpaceKeyedService(Profile* profile) {
+  return HoldingSpaceKeyedServiceFactory::GetInstance()->GetService(profile);
+}
+
 // Attempts to open the file or folder at the specified `file_path`.
 // Success is returned via the supplied `callback`.
 void OpenFileOrFolder(Profile* profile,
@@ -66,14 +76,22 @@ void HoldingSpaceClientImpl::OpenItemInFolder(const HoldingSpaceItem& item,
                    platform_util::OPEN_FOLDER, std::move(callback));
 }
 
-// TODO(crbug/1126274): Implement.
 void HoldingSpaceClientImpl::PinItem(const HoldingSpaceItem& item) {
-  NOTIMPLEMENTED();
+  DCHECK_NE(item.type(), HoldingSpaceItem::Type::kPinnedFile);
+  const storage::FileSystemURL& file_system_url =
+      file_manager::util::GetFileSystemContextForExtensionId(
+          profile_, file_manager::kFileManagerAppId)
+          ->CrackURL(item.file_system_url());
+  GetHoldingSpaceKeyedService(profile_)->AddPinnedFile(file_system_url);
 }
 
-// TODO(crbug/1126274): Implement.
 void HoldingSpaceClientImpl::UnpinItem(const HoldingSpaceItem& item) {
-  NOTIMPLEMENTED();
+  DCHECK_EQ(item.type(), HoldingSpaceItem::Type::kPinnedFile);
+  const storage::FileSystemURL& file_system_url =
+      file_manager::util::GetFileSystemContextForExtensionId(
+          profile_, file_manager::kFileManagerAppId)
+          ->CrackURL(item.file_system_url());
+  GetHoldingSpaceKeyedService(profile_)->RemovePinnedFile(file_system_url);
 }
 
 }  // namespace ash

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

@@ -8,6 +8,7 @@
 #include "ash/public/cpp/holding_space/holding_space_controller.h"
 #include "ash/public/cpp/holding_space/holding_space_image.h"
 #include "ash/public/cpp/holding_space/holding_space_item.h"
+#include "ash/public/cpp/holding_space/holding_space_model.h"
 #include "base/bind_helpers.h"
 #include "base/files/file_path.h"
 #include "base/run_loop.h"
@@ -118,4 +119,45 @@ IN_PROC_BROWSER_TEST_F(HoldingSpaceClientImplTest, OpenItemInFolder) {
   }
 }
 
+// Verifies that `HoldingSpaceClient::PinItem()` works as intended.
+IN_PROC_BROWSER_TEST_F(HoldingSpaceClientImplTest, PinItem) {
+  ASSERT_TRUE(HoldingSpaceController::Get());
+
+  auto* holding_space_client = HoldingSpaceController::Get()->client();
+  auto* holding_space_model = HoldingSpaceController::Get()->model();
+  ASSERT_TRUE(holding_space_client && holding_space_model);
+
+  // Create a download holding space item.
+  HoldingSpaceItem* download_item = AddDownloadFile();
+  ASSERT_EQ(1u, holding_space_model->items().size());
+
+  // Attempt to pin the download holding space item.
+  holding_space_client->PinItem(*download_item);
+  ASSERT_EQ(2u, holding_space_model->items().size());
+
+  // The pinned holding space item should have type `kPinnedFile` but share the
+  // same text and file path as the original download holding space item.
+  HoldingSpaceItem* pinned_file_item = holding_space_model->items()[1].get();
+  EXPECT_EQ(pinned_file_item->type(), HoldingSpaceItem::Type::kPinnedFile);
+  EXPECT_EQ(download_item->text(), pinned_file_item->text());
+  EXPECT_EQ(download_item->file_path(), pinned_file_item->file_path());
+}
+
+// Verifies that `HoldingSpaceClient::UnpinItem()` works as intended.
+IN_PROC_BROWSER_TEST_F(HoldingSpaceClientImplTest, UnpinItem) {
+  ASSERT_TRUE(HoldingSpaceController::Get());
+
+  auto* holding_space_client = HoldingSpaceController::Get()->client();
+  auto* holding_space_model = HoldingSpaceController::Get()->model();
+  ASSERT_TRUE(holding_space_client && holding_space_model);
+
+  // Create a pinned file holding space item.
+  HoldingSpaceItem* pinned_file_item = AddPinnedFile();
+  ASSERT_EQ(1u, holding_space_model->items().size());
+
+  // Attempt to unpin the pinned file holding space item.
+  holding_space_client->UnpinItem(*pinned_file_item);
+  ASSERT_EQ(0u, holding_space_model->items().size());
+}
+
 }  // namespace ash