Appending images to an existing clipboard.

This implements an append option so we can append images to an existing
clipboard data object. This allows us to preserve existing clipboard
metadata and copy images to the system clipboard.

Bug: 827333
Change-Id: I89062055d21f252274c6a77cc72cfaf2eed3598a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2463851Reviewed-by: Darwin Huang <huangdarwin@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Reviewed-by: Luciano Pacheco <lucmult@chromium.org>
Commit-Queue: Alexander Nohe <nohe@chromium.org>
Cr-Commit-Position: refs/heads/master@{#821176}

chrome/browser/chromeos/extensions/file_manager/private_api_file_system.cc

@@ -70,6 +70,8 @@
 #include "storage/common/file_system/file_system_types.h"
 #include "storage/common/file_system/file_system_util.h"
 #include "third_party/cros_system_api/constants/cryptohome.h"
+#include "ui/base/clipboard/clipboard_buffer.h"
+#include "ui/base/clipboard/clipboard_non_backed.h"
 
 using chromeos::disks::DiskMountManager;
 using content::BrowserThread;
@@ -970,6 +972,9 @@ FileManagerPrivateInternalCopyImageToClipboardFunction::Run() {
     return RespondNow(Error("Image file URL was invalid"));
   }
 
+  clipboard_sequence_ =
+      ui::ClipboardNonBacked::GetForCurrentThread()->GetSequenceNumber(
+          ui::ClipboardBuffer::kCopyPaste);
   std::unique_ptr<storage::FileStreamReader> reader =
       file_system_context->CreateFileStreamReader(
           file_system_url, 0, storage::kMaximumLength, base::Time());
@@ -979,7 +984,7 @@ FileManagerPrivateInternalCopyImageToClipboardFunction::Run() {
           &CopyImageRespondOnUIThread,
           base::BindOnce(
               &FileManagerPrivateInternalCopyImageToClipboardFunction::
-                  RespondWith,
+                  MoveBytesToClipboard,
               this));
 
   content::GetIOThreadTaskRunner({})->PostTask(
@@ -992,12 +997,22 @@ FileManagerPrivateInternalCopyImageToClipboardFunction::Run() {
   return RespondLater();
 }
 
+void FileManagerPrivateInternalCopyImageToClipboardFunction::
+    MoveBytesToClipboard(scoped_refptr<base::RefCountedString> bytes) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+
+  clipboard_util::DecodeImageFileAndCopyToClipboard(
+      clipboard_sequence_, /*maintain_clipboard=*/true,
+      /*png_data=*/std::move(bytes),
+      base::BindOnce(
+          &FileManagerPrivateInternalCopyImageToClipboardFunction::RespondWith,
+          this));
+}
+
 void FileManagerPrivateInternalCopyImageToClipboardFunction::RespondWith(
-    scoped_refptr<base::RefCountedString> bytes) {
+    bool is_on_clipboard) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
-  clipboard_util::DecodeImageFileAndCopyToClipboard(std::move(bytes));
-  // Copy image to clipboard is async, this responds before copy is finished.
-  Respond(NoArguments());
+  Respond(OneArgument(base::Value(is_on_clipboard)));
 }
 
 ExtensionFunction::ResponseAction FileManagerPrivateCancelCopyFunction::Run() {

chrome/browser/chromeos/extensions/file_manager/private_api_file_system.h

@@ -274,10 +274,16 @@ class FileManagerPrivateInternalCopyImageToClipboardFunction
   ResponseAction Run() override;
 
  private:
-  void RespondWith(scoped_refptr<base::RefCountedString> bytes);
+  // `is_on_clipboard` specifies whether or not the image was copied to the
+  // clipboard.
+  void RespondWith(bool is_on_clipboard);
+  void MoveBytesToClipboard(scoped_refptr<base::RefCountedString> bytes);
 
   const ChromeExtensionFunctionDetails chrome_details_;
   std::unique_ptr<storage::FileStreamStringConverter> converter_;
+  // Stores the clipboard copy sequence number to validate the clipboard did not
+  // change during an async operation.
+  uint64_t clipboard_sequence_ = 0;
 };
 
 // Implements the chrome.fileManagerPrivate.startCopy method.

chrome/browser/ui/ash/clipboard_util.cc

@@ -4,9 +4,14 @@
 
 #include "chrome/browser/ui/ash/clipboard_util.h"
 
+#include <stdint.h>
+#include <memory>
+
 #include "base/base64.h"
+#include "base/callback.h"
 #include "base/files/file_util.h"
 #include "base/memory/ref_counted_memory.h"
+#include "base/memory/scoped_refptr.h"
 #include "base/metrics/user_metrics.h"
 #include "base/strings/strcat.h"
 #include "base/threading/scoped_blocking_call.h"
@@ -14,16 +19,44 @@
 #include "content/public/browser/browser_thread.h"
 #include "services/data_decoder/public/cpp/decode_image.h"
 #include "ui/base/clipboard/clipboard.h"
+#include "ui/base/clipboard/clipboard_buffer.h"
+#include "ui/base/clipboard/clipboard_data.h"
+#include "ui/base/clipboard/clipboard_non_backed.h"
 #include "ui/base/clipboard/scoped_clipboard_writer.h"
 
 namespace clipboard_util {
 namespace {
 
-void CopyImageToClipboard(scoped_refptr<base::RefCountedString> png_data,
+void CopyAndMaintainClipboard(
+    std::unique_ptr<ui::ClipboardData> data_with_image,
+    const std::string& markup_content,
+    scoped_refptr<base::RefCountedString> png_data,
     const SkBitmap& decoded_image) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 
-  ui::ScopedClipboardWriter clipboard_writer(ui::ClipboardBuffer::kCopyPaste);
+  data_with_image->set_markup_data(markup_content);
+  data_with_image->SetBitmapData(decoded_image);
+  ui::ClipboardNonBacked::GetForCurrentThread()->WriteClipboardData(
+      std::move(data_with_image));
+}
+
+/*
+ * `maintain_clipboard` indicates whether the clipboard state should attempt to
+ * be maintained.
+ * `clipboard_sequence` is the versioning of the clipboard when we start our
+ * copy operation.
+ * `callback` alerts whether or not the image was copied to the clipboard while
+ * meeting the `maintain_clipboard` state. If the image is copied it will return
+ * true, otherwise if the image is not copied because the `clipboard_sequence`
+ * does not match, it will return false.
+ * `decoded_image` is the image we are attempting to copy to the clipboard.
+ */
+void CopyImageToClipboard(bool maintain_clipboard,
+                          uint64_t clipboard_sequence,
+                          base::OnceCallback<void(bool)> callback,
+                          scoped_refptr<base::RefCountedString> png_data,
+                          const SkBitmap& decoded_image) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 
   // Send both HTML and and Image formats to clipboard. HTML format is needed
   // by ARC, while Image is needed by Hangout.
@@ -35,8 +68,30 @@ void CopyImageToClipboard(scoped_refptr<base::RefCountedString> png_data,
       base::Base64Encode(base::as_bytes(base::make_span(png_data->data())));
   std::string html = base::StrCat(
       {kImageClipboardFormatPrefix, encoded, kImageClipboardFormatSuffix});
+
+  if (!maintain_clipboard) {
+    ui::ScopedClipboardWriter clipboard_writer(ui::ClipboardBuffer::kCopyPaste);
     clipboard_writer.WriteHTML(base::UTF8ToUTF16(html), std::string());
     clipboard_writer.WriteImage(decoded_image);
+    std::move(callback).Run(true);
+    return;
+  }
+
+  uint64_t current_sequence =
+      ui::ClipboardNonBacked::GetForCurrentThread()->GetSequenceNumber(
+          ui::ClipboardBuffer::kCopyPaste);
+  if (current_sequence != clipboard_sequence) {
+    // Clipboard data changed and this copy operation is no longer relevant.
+    std::move(callback).Run(false);
+    return;
+  }
+  std::unique_ptr<ui::ClipboardData> current_data =
+      std::make_unique<ui::ClipboardData>(
+          *ui::ClipboardNonBacked::GetForCurrentThread()->GetClipboardData(
+              nullptr));
+  CopyAndMaintainClipboard(std::move(current_data), html, png_data,
+                           decoded_image);
+  std::move(callback).Run(true);
 }
 
 }  // namespace
@@ -52,11 +107,17 @@ void ReadFileAndCopyToClipboardLocal(const base::FilePath& local_file) {
   }
 
   content::GetUIThreadTaskRunner({})->PostTask(
-      FROM_HERE, base::BindOnce(&DecodeImageFileAndCopyToClipboard, png_data));
+      FROM_HERE, base::BindOnce(&DecodeImageFileAndCopyToClipboard,
+                                /*clipboard_sequence=*/0,
+                                /*maintain_clipboard=*/false, png_data,
+                                base::DoNothing::Once<bool>()));
 }
 
 void DecodeImageFileAndCopyToClipboard(
-    scoped_refptr<base::RefCountedString> png_data) {
+    uint64_t clipboard_sequence,
+    bool maintain_clipboard,
+    scoped_refptr<base::RefCountedString> png_data,
+    base::OnceCallback<void(bool)> callback) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 
   // Decode the image in sandboxed process because |png_data| comes from
@@ -65,6 +126,7 @@ void DecodeImageFileAndCopyToClipboard(
       std::vector<uint8_t>(png_data->data().begin(), png_data->data().end()),
       data_decoder::mojom::ImageCodec::DEFAULT, false,
       data_decoder::kDefaultMaxSizeInBytes, gfx::Size(),
-      base::BindOnce(&CopyImageToClipboard, png_data));
+      base::BindOnce(&CopyImageToClipboard, maintain_clipboard,
+                     clipboard_sequence, std::move(callback), png_data));
 }
 }  // namespace clipboard_util

chrome/browser/ui/ash/clipboard_util.h

@@ -5,6 +5,9 @@
 #ifndef CHROME_BROWSER_UI_ASH_CLIPBOARD_UTIL_H_
 #define CHROME_BROWSER_UI_ASH_CLIPBOARD_UTIL_H_
 
+#include <stdint.h>
+
+#include "base/callback_forward.h"
 #include "base/memory/ref_counted_memory.h"
 #include "base/memory/scoped_refptr.h"
 namespace base {
@@ -18,8 +21,21 @@ namespace clipboard_util {
 void ReadFileAndCopyToClipboardLocal(const base::FilePath& local_file);
 
 // Takes an image file as a string and copies it to the system clipboard.
+//
+// `clipboard_sequence` - Clipboard version to determine whether the clipboard
+// state has changed. A sequence of 0 is used to specify an invalid sequence.
+// `maintain_clipboard` - Used to determine whether or not we care about
+// maintaining the clipboard state or not. If this value is false, it is okay to
+// pass a `clipboard_sequence` of 0.
+// `png_data` - The image we want to copy to the clipboard as a string.
+// `callback` - Reports if the copy was successful. Reasons that this could
+// return false include that the sequence numbers do not match and when
+// `maintain_clipboard` is true.
 void DecodeImageFileAndCopyToClipboard(
-    scoped_refptr<base::RefCountedString> png_data);
+    uint64_t clipboard_sequence,
+    bool maintain_clipboard,
+    scoped_refptr<base::RefCountedString> png_data,
+    base::OnceCallback<void(bool)> callback);
 
 }  // namespace clipboard_util
 

chrome/common/extensions/api/file_manager_private.idl

@@ -1089,7 +1089,7 @@ interface Functions {
   // |entry| Entry of the image to copy to the system clipboard.
   [nocompile]
   static void copyImageToClipboard([instanceof=Entry] object entry,
-                                   SimpleCallback callback);
+                                   BooleanCallback callback);
 
   // Starts to copy an entry. If the source is a directory, the copy is done
   // recursively.

chrome/common/extensions/api/file_manager_private_internal.idl

@@ -85,7 +85,7 @@ namespace fileManagerPrivateInternal {
     static void getFileTasks(DOMString[] urls,
                              GetFileTasksCallback callback);
     static void getDownloadUrl(DOMString url, GetUrlCallback callback);
-    static void copyImageToClipboard(DOMString url, SimpleCallback callback);
+    static void copyImageToClipboard(DOMString url, BooleanCallback callback);
     static void startCopy(DOMString url,
                           DOMString parentUrl,
                           DOMString newName,

third_party/closure_compiler/externs/file_manager_private.js

@@ -794,7 +794,7 @@ chrome.fileManagerPrivate.startCopy = function(entry, parentEntry, newName,
  * Copies an image to the system clipboard. |entry| Entry of the image to copy
  * to the system clipboard.
  * @param {!Entry} entry
- * @param {function()} callback
+ * @param {function((boolean|undefined))} callback
  */
 chrome.fileManagerPrivate.copyImageToClipboard = function(entry, callback) {};
 

ui/base/clipboard/clipboard_non_backed.h

@@ -39,6 +39,9 @@ class COMPONENT_EXPORT(UI_BASE_CLIPBOARD) ClipboardNonBacked
   std::unique_ptr<ClipboardData> WriteClipboardData(
       std::unique_ptr<ClipboardData> data);
 
+  // Clipboard overrides:
+  uint64_t GetSequenceNumber(ClipboardBuffer buffer) const override;
+
  private:
   friend class Clipboard;
   friend class ClipboardNonBackedTest;
@@ -47,7 +50,6 @@ class COMPONENT_EXPORT(UI_BASE_CLIPBOARD) ClipboardNonBacked
 
   // Clipboard overrides:
   void OnPreShutdown() override;
-  uint64_t GetSequenceNumber(ClipboardBuffer buffer) const override;
   bool IsFormatAvailable(const ClipboardFormatType& format,
                          ClipboardBuffer buffer,
                          const ClipboardDataEndpoint* data_dst) const override;

ui/file_manager/file_manager/foreground/js/file_transfer_controller.js

@@ -286,6 +286,23 @@ class FileTransferController {
         !this.selectionHandler_.isAvailable());
     this.appendUriList_(
         clipboardData, this.selectionHandler_.selection.entries);
+    if (util.isCopyImageEnabled()) {
+      const entries = this.selectionHandler_.selection.entries;
+      if (entries.length == 1 && FileType.isImage(entries[0])) {
+        // We are using setTimeout to ensure that the previous copy commands
+        // execute successfully, so we can append our image to the system
+        // clipboard at the end of the event loop.
+        setTimeout(() => {
+          chrome.fileManagerPrivate.copyImageToClipboard(entries[0],
+            () => {
+              if (chrome.runtime.lastError) {
+                console.error(chrome.runtime.lastError.message);
+                return;
+              }
+          });
+        });
+      }
+    }
   }
 
   /**
@@ -314,15 +331,6 @@ class FileTransferController {
 
     clipboardData.setData(
         'fs/missingFileContents', missingFileContents.toString());
-
-    if(util.isCopyImageEnabled()) {
-      if ((entries.length == 1) && FileType.isImage(entries[0])) {
-        chrome.fileManagerPrivate.copyImageToClipboard(entries[0],
-          () => {
-            console.log("Image is being copied!");
-        });
-      }
-    }
   }
 
   /**