Implement copy image to clipboard for holding space.

Per change in product requirements, the "Copy to clipboard" context menu
command is now "Copy image" and will only be shown for holding space
items which are backed by an image file.

When executed, "copy image" will read, decode, and write the backing
file's image contents to the clipboard buffer so as to be available to
the user for pasting.

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

ash/ash_strings.grd

@@ -958,8 +958,8 @@ This file contains the strings for ash.
       <message name="IDS_ASH_HOLDING_SPACE_SCREENSHOTS_TITLE" desc="Title of the screenshots area in the holding space bubble.">
         Screenshots
       </message>
-      <message name="IDS_ASH_HOLDING_SPACE_CONTEXT_MENU_COPY_TO_CLIPBOARD" desc="Title of the option to copy items to clipboard.">
-        Copy to clipboard
+      <message name="IDS_ASH_HOLDING_SPACE_CONTEXT_MENU_COPY_IMAGE_TO_CLIPBOARD" desc="Title of the option to copy an image to the clipboard in the holding space item context menu.">
+        Copy image
       </message>
       <message name="IDS_ASH_HOLDING_SPACE_CONTEXT_MENU_PIN" desc="Title of the pin option in the holding space item context menu.">
         Pin

ash/ash_strings_grd/IDS_ASH_HOLDING_SPACE_CONTEXT_MENU_COPY_IMAGE_TO_CLIPBOARD.png.sha1

+eec68acaa531698a5002d91e43ba07bea5162df7
\ No newline at end of file

ash/ash_strings_grd/IDS_ASH_HOLDING_SPACE_CONTEXT_MENU_COPY_TO_CLIPBOARD.png.sha1

-b1012145df7abf2c183a8b9f8fc6f3fdf8efd6f8
\ No newline at end of file

ash/public/cpp/holding_space/holding_space_client.h

@@ -24,10 +24,12 @@ class ASH_PUBLIC_EXPORT HoldingSpaceClient {
   // Adds a screenshot item backed by the provided `file_path`.
   virtual void AddScreenshot(const base::FilePath& file_path) = 0;
 
-  // Attempts to copy the specified holding space `item` to the clipboard.
-  // Success is returned via the supplied `callback`.
-  virtual void CopyToClipboard(const HoldingSpaceItem& item,
-                               SuccessCallback callback) = 0;
+  // Attempts to copy the contents of the image file backing the specified
+  // holding space `item` to the clipboard. If the backing file is not suspected
+  // to contain image data, this method will abort early. Success is returned
+  // via the supplied `callback`.
+  virtual void CopyImageToClipboard(const HoldingSpaceItem& item,
+                                    SuccessCallback callback) = 0;
 
   // Attempts to open the specified holding space `item`.
   // Success is returned via the supplied `callback`.

ash/public/cpp/holding_space/holding_space_constants.h

@@ -11,15 +11,6 @@
 
 namespace ash {
 
-// Context menu commands.
-enum HoldingSpaceCommandId {
-  kPinItem,
-  kCopyToClipboard,
-  kShowInFolder,
-  kUnpinItem,
-  kMaxValue = kUnpinItem
-};
-
 // Appearance.
 constexpr gfx::Insets kHoldingSpaceContainerPadding(16);
 constexpr int kHoldingSpaceContainerChildSpacing = 16;
@@ -38,13 +29,25 @@ constexpr int kHoldingSpaceScreenshotSpacing = 8;
 constexpr gfx::Size kHoldingSpaceScreenshotSize(104, 80);
 constexpr gfx::Insets kHoldingSpaceScreenshotsContainerPadding(8, 0);
 
+// Context menu commands.
+enum HoldingSpaceCommandId {
+  kPinItem,
+  kCopyImageToClipboard,
+  kShowInFolder,
+  kUnpinItem,
+  kMaxValue = kUnpinItem
+};
+
 // View IDs.
 constexpr int kHoldingSpacePinnedFilesContainerId = 1;
 constexpr int kHoldingSpaceRecentFilesContainerId = 2;
 
-// The maximum allowed time age for files restored into holding space model.
-// This is not enforced for pinned items.
-const base::TimeDelta kMaxFileAge = base::TimeDelta::FromDays(1);
+// The maximum allowed age for files restored into the holding space model.
+// Note that this is not enforced for pinned items.
+constexpr base::TimeDelta kMaxFileAge = base::TimeDelta::FromDays(1);
+
+// Mime type with wildcard which matches all image types.
+constexpr char kMimeTypeImage[] = "image/*";
 
 }  // namespace ash
 

ash/system/holding_space/holding_space_item_context_menu.cc

@@ -12,6 +12,7 @@
 #include "ash/strings/grit/ash_strings.h"
 #include "ash/system/holding_space/holding_space_item_view.h"
 #include "base/bind.h"
+#include "net/base/mime_util.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/models/simple_menu_model.h"
 #include "ui/views/controls/menu/menu_runner.h"
@@ -46,8 +47,8 @@ void HoldingSpaceItemContextMenu::ShowContextMenuForViewImpl(
 void HoldingSpaceItemContextMenu::ExecuteCommand(int command_id,
                                                  int event_flags) {
   switch (command_id) {
-    case HoldingSpaceCommandId::kCopyToClipboard:
-      HoldingSpaceController::Get()->client()->CopyToClipboard(
+    case HoldingSpaceCommandId::kCopyImageToClipboard:
+      HoldingSpaceController::Get()->client()->CopyImageToClipboard(
           *item_, base::DoNothing());
       break;
     case HoldingSpaceCommandId::kPinItem:
@@ -70,13 +71,21 @@ ui::SimpleMenuModel* HoldingSpaceItemContextMenu::BuildMenuModel() {
       l10n_util::GetStringUTF16(
           IDS_ASH_HOLDING_SPACE_CONTEXT_MENU_SHOW_IN_FOLDER),
       ui::ImageModel::FromVectorIcon(kFolderIcon));
-  context_menu_model_->AddItemWithIcon(
-      HoldingSpaceCommandId::kCopyToClipboard,
-      l10n_util::GetStringUTF16(
-          IDS_ASH_HOLDING_SPACE_CONTEXT_MENU_COPY_TO_CLIPBOARD),
-      ui::ImageModel::FromVectorIcon(kCopyIcon));
 
-  bool is_pinned = HoldingSpaceController::Get()->model()->GetItem(
+  std::string mime_type;
+  const bool is_image =
+      net::GetMimeTypeFromFile(item_->file_path(), &mime_type) &&
+      net::MatchesMimeType(kMimeTypeImage, mime_type);
+
+  if (is_image) {
+    context_menu_model_->AddItemWithIcon(
+        HoldingSpaceCommandId::kCopyImageToClipboard,
+        l10n_util::GetStringUTF16(
+            IDS_ASH_HOLDING_SPACE_CONTEXT_MENU_COPY_IMAGE_TO_CLIPBOARD),
+        ui::ImageModel::FromVectorIcon(kCopyIcon));
+  }
+
+  const bool is_pinned = HoldingSpaceController::Get()->model()->GetItem(
       HoldingSpaceItem::GetFileBackedItemId(HoldingSpaceItem::Type::kPinnedFile,
                                             item_->file_path()));
   if (!is_pinned) {
@@ -90,6 +99,7 @@ ui::SimpleMenuModel* HoldingSpaceItemContextMenu::BuildMenuModel() {
         l10n_util::GetStringUTF16(IDS_ASH_HOLDING_SPACE_CONTEXT_MENU_UNPIN),
         ui::ImageModel::FromVectorIcon(views::kUnpinIcon));
   }
+
   return context_menu_model_.get();
 }
 

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

@@ -4,14 +4,19 @@
 
 #include "chrome/browser/ui/ash/holding_space/holding_space_client_impl.h"
 
+#include "ash/public/cpp/holding_space/holding_space_constants.h"
 #include "ash/public/cpp/holding_space/holding_space_item.h"
 #include "base/bind.h"
 #include "base/notreached.h"
+#include "base/task/task_traits.h"
+#include "base/task/thread_pool.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/clipboard_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 "net/base/mime_util.h"
 #include "storage/browser/file_system/file_system_context.h"
 
 namespace ash {
@@ -38,11 +43,30 @@ void HoldingSpaceClientImpl::AddScreenshot(const base::FilePath& file_path) {
   GetHoldingSpaceKeyedService(profile_)->AddScreenshot(file_path);
 }
 
-// TODO(crbug/1126274): Implement.
-void HoldingSpaceClientImpl::CopyToClipboard(const HoldingSpaceItem& item,
-                                             SuccessCallback callback) {
-  NOTIMPLEMENTED();
-  std::move(callback).Run(/*success=*/false);
+void HoldingSpaceClientImpl::CopyImageToClipboard(const HoldingSpaceItem& item,
+                                                  SuccessCallback callback) {
+  std::string mime_type;
+  if (item.file_path().empty() ||
+      !net::GetMimeTypeFromFile(item.file_path(), &mime_type) ||
+      !net::MatchesMimeType(kMimeTypeImage, mime_type)) {
+    std::move(callback).Run(/*success=*/false);
+    return;
+  }
+  // Reading and decoding of the image file needs to be done on an I/O thread.
+  base::ThreadPool::PostTaskAndReply(
+      FROM_HERE,
+      {base::MayBlock(), base::TaskPriority::BEST_EFFORT,
+       base::TaskShutdownBehavior::SKIP_ON_SHUTDOWN},
+      base::BindOnce(&clipboard_util::ReadFileAndCopyToClipboardLocal,
+                     item.file_path()),
+      base::BindOnce(
+          [](SuccessCallback callback) {
+            // We don't currently receive a signal regarding whether image
+            // decoding was successful or not. For the time being, assume
+            // success when the task runs until proven otherwise.
+            std::move(callback).Run(/*success=*/true);
+          },
+          std::move(callback)));
 }
 
 void HoldingSpaceClientImpl::OpenItem(const HoldingSpaceItem& item,

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

@@ -23,7 +23,7 @@ class HoldingSpaceClientImpl : public HoldingSpaceClient {
 
   // HoldingSpaceClient:
   void AddScreenshot(const base::FilePath& file_path) override;
-  void CopyToClipboard(const HoldingSpaceItem&, SuccessCallback) override;
+  void CopyImageToClipboard(const HoldingSpaceItem&, SuccessCallback) override;
   void OpenItem(const HoldingSpaceItem&, SuccessCallback) override;
   void ShowItemInFolder(const HoldingSpaceItem&, SuccessCallback) override;
   void PinItem(const HoldingSpaceItem& item) override;

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

@@ -4,6 +4,8 @@
 
 #include "chrome/browser/ui/ash/holding_space/holding_space_client_impl.h"
 
+#include <string>
+
 #include "ash/public/cpp/ash_features.h"
 #include "ash/public/cpp/holding_space/holding_space_controller.h"
 #include "ash/public/cpp/holding_space/holding_space_image.h"
@@ -11,6 +13,7 @@
 #include "ash/public/cpp/holding_space/holding_space_model.h"
 #include "base/bind_helpers.h"
 #include "base/files/file_path.h"
+#include "base/path_service.h"
 #include "base/run_loop.h"
 #include "base/test/bind_test_util.h"
 #include "base/test/scoped_feature_list.h"
@@ -25,8 +28,78 @@
 
 namespace ash {
 
+namespace {
+
+// File paths for test data.
+constexpr char kTestDataDir[] = "chrome/test/data/chromeos/file_manager/";
+constexpr char kImageFilePath[] = "image.png";
+constexpr char kTextFilePath[] = "text.txt";
+
+// Helpers ---------------------------------------------------------------------
+
+// Returns the file for the `relative_path` in the test data directory.
+base::FilePath TestFile(const std::string& relative_path) {
+  base::FilePath source_root;
+  EXPECT_TRUE(base::PathService::Get(base::DIR_SOURCE_ROOT, &source_root));
+  return source_root.AppendASCII(kTestDataDir).AppendASCII(relative_path);
+}
+
+}  // namespace
+
+// Tests -----------------------------------------------------------------------
+
 using HoldingSpaceClientImplTest = HoldingSpaceBrowserTestBase;
 
+// Verifies that `HoldingSpaceClient::CopyImageToClipboard()` works as intended
+// when attempting to copy both image backed and non-image backed holding space
+// items.
+IN_PROC_BROWSER_TEST_F(HoldingSpaceClientImplTest, CopyImageToClipboard) {
+  ASSERT_TRUE(HoldingSpaceController::Get());
+
+  auto* holding_space_client = HoldingSpaceController::Get()->client();
+  ASSERT_TRUE(holding_space_client);
+
+  {
+    // Create a holding space item backed by a non-image file.
+    auto holding_space_item = HoldingSpaceItem::CreateFileBackedItem(
+        HoldingSpaceItem::Type::kDownload, TestFile(kTextFilePath), GURL(),
+        std::make_unique<HoldingSpaceImage>(
+            /*placeholder=*/gfx::ImageSkia(),
+            /*async_bitmap_resolver=*/base::DoNothing()));
+
+    // We expect `HoldingSpaceClient::CopyImageToClipboard()` to fail when the
+    // backing file for `holding_space_item` is not an image file.
+    base::RunLoop run_loop;
+    holding_space_client->CopyImageToClipboard(
+        *holding_space_item,
+        base::BindLambdaForTesting([&run_loop](bool success) {
+          EXPECT_FALSE(success);
+          run_loop.Quit();
+        }));
+    run_loop.Run();
+  }
+
+  {
+    // Create a holding space item backed by an image file.
+    auto holding_space_item = HoldingSpaceItem::CreateFileBackedItem(
+        HoldingSpaceItem::Type::kDownload, TestFile(kImageFilePath), GURL(),
+        std::make_unique<HoldingSpaceImage>(
+            /*placeholder=*/gfx::ImageSkia(),
+            /*async_bitmap_resolver=*/base::DoNothing()));
+
+    // We expect `HoldingSpaceClient::CopyImageToClipboard()` to succeed when
+    // the backing file for `holding_space_item` is an image file.
+    base::RunLoop run_loop;
+    holding_space_client->CopyImageToClipboard(
+        *holding_space_item,
+        base::BindLambdaForTesting([&run_loop](bool success) {
+          EXPECT_TRUE(success);
+          run_loop.Quit();
+        }));
+    run_loop.Run();
+  }
+}
+
 // Verifies that `HoldingSpaceClient::OpenItem()` works as intended when
 // attempting to open holding space items backed by both non-existing and
 // existing files.