Unify clipboard and dragdrop in data offer to use callbacks

Change drag and drop to also use callbacks for data fetching like how
clipboard does. Ensure callbacks are only invoked once with data
cached, and pending receive requests managed.

Files drag and drop will require this to allow file sharing to be done
inside FileHandler at the point where a client calls data_offer_receive,
rather than when FileHandler is currently invoked when data is offered.

This change provides performance improvements particularly for when an
image is requested multiple times from the clipboard since we now only
do the encoding once.

Bug: 1144138
Change-Id: I56c7d3b55a3365a591829313462af23e71e172b2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2509833Reviewed-by: Mitsuru Oshima <oshima@chromium.org>
Commit-Queue: Joel Hockey <joelhockey@chromium.org>
Auto-Submit: Joel Hockey <joelhockey@chromium.org>
Cr-Commit-Position: refs/heads/master@{#824129}

components/exo/data_offer.h

@@ -19,6 +19,7 @@
 #include "url/gurl.h"
 
 namespace base {
+class Pickle;
 class RefCountedMemory;
 }
 
@@ -37,6 +38,10 @@ enum class DndAction;
 // Object representing transferred data offered to a client.
 class DataOffer final : public ui::PropertyHandler {
  public:
+  using SendDataCallback =
+      base::OnceCallback<void(scoped_refptr<base::RefCountedMemory>)>;
+  using AsyncSendDataCallback = base::OnceCallback<void(SendDataCallback)>;
+
   enum Purpose {
     COPY_PASTE,
     DRAG_DROP,
@@ -84,28 +89,34 @@ class DataOffer final : public ui::PropertyHandler {
   bool finished() const { return finished_; }
 
  private:
-  void OnPickledUrlsResolved(const std::string& uri_list_mime_type,
+  void OnDataReady(const std::string& mime_type,
+                   base::ScopedFD fd,
+                   scoped_refptr<base::RefCountedMemory> data);
+  void GetUrlsFromPickle(FileHelper* file_helper,
+                         const base::Pickle& pickle,
+                         SendDataCallback callback);
+  void OnPickledUrlsResolved(SendDataCallback callback,
                              const std::vector<GURL>& urls);
 
   DataOfferDelegate* const delegate_;
 
-  // Map between mime type and drop data bytes.
-  // nullptr may be set as a temporary value until data bytes are populated.
-  base::flat_map<std::string, scoped_refptr<base::RefCountedMemory>> data_;
-  // Unprocessed receive requests (pairs of mime type and FD) that are waiting
-  // for unpopulated (nullptr) data bytes in |data_| to be populated.
+  // Data for a given mime type may not ever be requested, or may be requested
+  // more than once. Using callbacks and a cache allows us to delay any
+  // expensive operations until they are required, and then ensure that they are
+  // performed at most once. When we offer data for a given mime type we will
+  // populate |data_callbacks_| with mime type and a callback which will produce
+  // the required data. On the first request to |Receive()| we remove and invoke
+  // the callback and set |data_cache_| with null data. When the callback
+  // completes we populate |data_cache_| with data and fulfill any
+  // |pending_receive_requests|.
+  base::flat_map<std::string, AsyncSendDataCallback> data_callbacks_;
+  base::flat_map<std::string, scoped_refptr<base::RefCountedMemory>>
+      data_cache_;
   std::vector<std::pair<std::string, base::ScopedFD>> pending_receive_requests_;
 
-  using SendDataCallback = base::RepeatingCallback<void(base::ScopedFD)>;
-  // Map from mime type (or other offered data type) to a callback that sends
-  // data for that type. Using callbacks allows us to delay making copies or
-  // doing other expensive processing until actually necessary.
-  base::flat_map<std::string, SendDataCallback> data_callbacks_;
-
   base::flat_set<DndAction> source_actions_;
   DndAction dnd_action_;
   base::ObserverList<DataOfferObserver>::Unchecked observers_;
-  Purpose purpose_;
   bool finished_;
 
   base::WeakPtrFactory<DataOffer> weak_ptr_factory_{this};

components/exo/data_offer_unittest.cc

@@ -14,14 +14,19 @@
 #include "base/containers/flat_set.h"
 #include "base/files/file_util.h"
 #include "base/macros.h"
+#include "base/run_loop.h"
 #include "base/strings/string16.h"
 #include "base/strings/utf_string_conversions.h"
+#include "cc/test/pixel_comparator.h"
+#include "cc/test/pixel_test_utils.h"
 #include "components/exo/data_device.h"
 #include "components/exo/data_offer_delegate.h"
 #include "components/exo/file_helper.h"
 #include "components/exo/test/exo_test_base.h"
+#include "third_party/skia/include/core/SkBitmap.h"
 #include "ui/base/clipboard/scoped_clipboard_writer.h"
 #include "ui/base/dragdrop/os_exchange_data.h"
+#include "ui/gfx/codec/png_codec.h"
 #include "url/gurl.h"
 
 namespace exo {
@@ -308,42 +313,6 @@ TEST_F(DataOfferTest, ReceiveUriList) {
   EXPECT_EQ(base::ASCIIToUTF16("file:///test/downloads/file"), result);
 }
 
-TEST_F(DataOfferTest, ReceiveUriListFromPickle_ReceiveAfterUrlIsResolved) {
-  TestDataOfferDelegate delegate;
-  DataOffer data_offer(&delegate, DataOffer::Purpose::DRAG_DROP);
-
-  TestFileHelper file_helper;
-  ui::OSExchangeData data;
-
-  base::Pickle pickle;
-  pickle.WriteUInt32(1);  // num files
-  pickle.WriteString("filesystem:chrome-extension://path/to/file1");
-  pickle.WriteInt64(1000);   // file size
-  pickle.WriteString("id");  // filesystem id
-  data.SetPickledData(
-      ui::ClipboardFormatType::GetType("chromium/x-file-system-files"), pickle);
-  data_offer.SetDropData(&file_helper, data);
-
-  // Run callback with a resolved URL.
-  std::vector<GURL> urls;
-  urls.push_back(
-      GURL("content://org.chromium.arc.chromecontentprovider/path/to/file1"));
-  file_helper.RunUrlsCallback(urls);
-
-  base::ScopedFD read_pipe;
-  base::ScopedFD write_pipe;
-  ASSERT_TRUE(base::CreatePipe(&read_pipe, &write_pipe));
-
-  // Receive is called after UrlsCallback runs.
-  data_offer.Receive("text/uri-list", std::move(write_pipe));
-  base::string16 result;
-  ASSERT_TRUE(ReadString16(std::move(read_pipe), &result));
-  EXPECT_EQ(
-      base::ASCIIToUTF16(
-          "content://org.chromium.arc.chromecontentprovider/path/to/file1"),
-      result);
-}
-
 TEST_F(DataOfferTest, ReceiveUriListFromPickle_ReceiveBeforeUrlIsResolved) {
   TestDataOfferDelegate delegate;
   DataOffer data_offer(&delegate, DataOffer::Purpose::DRAG_DROP);
@@ -444,13 +413,21 @@ TEST_F(DataOfferTest, SetClipboardDataPlainText) {
 
   base::ScopedFD read_pipe;
   base::ScopedFD write_pipe;
-  ASSERT_TRUE(base::CreatePipe(&read_pipe, &write_pipe));
 
+  // Read as utf-8.
+  ASSERT_TRUE(base::CreatePipe(&read_pipe, &write_pipe));
   data_offer.Receive("text/plain;charset=utf-8", std::move(write_pipe));
   std::string result;
   ASSERT_TRUE(ReadString(std::move(read_pipe), &result));
   EXPECT_EQ("Test data", result);
 
+  // Read a second time.
+  ASSERT_TRUE(base::CreatePipe(&read_pipe, &write_pipe));
+  data_offer.Receive("text/plain;charset=utf-8", std::move(write_pipe));
+  ASSERT_TRUE(ReadString(std::move(read_pipe), &result));
+  EXPECT_EQ("Test data", result);
+
+  // Read as utf-16.
   ASSERT_TRUE(base::CreatePipe(&read_pipe, &write_pipe));
   data_offer.Receive("text/plain;charset=utf-16", std::move(write_pipe));
   base::string16 result16;
@@ -515,6 +492,56 @@ TEST_F(DataOfferTest, SetClipboardDataRTF) {
   EXPECT_EQ("Test data", result);
 }
 
+TEST_F(DataOfferTest, SetClipboardDataImage) {
+  TestDataOfferDelegate delegate;
+  DataOffer data_offer(&delegate, DataOffer::Purpose::COPY_PASTE);
+
+  SkBitmap image;
+  image.allocN32Pixels(10, 10);
+  image.eraseColor(SK_ColorMAGENTA);
+
+  TestFileHelper file_helper;
+  {
+    ui::ScopedClipboardWriter writer(ui::ClipboardBuffer::kCopyPaste);
+    writer.WriteImage(image);
+  }
+  data_offer.SetClipboardData(&file_helper,
+                              *ui::Clipboard::GetForCurrentThread());
+
+  EXPECT_EQ(1u, delegate.mime_types().size());
+  EXPECT_EQ(1u, delegate.mime_types().count("image/png"));
+
+  base::ScopedFD read_pipe;
+  base::ScopedFD write_pipe;
+  base::ScopedFD read_pipe2;
+  base::ScopedFD write_pipe2;
+  std::string result;
+
+  // Call Receive() twice in quick succession. Requires RunUntilIdle() since
+  // processing is done on worker thread.
+  ASSERT_TRUE(base::CreatePipe(&read_pipe, &write_pipe));
+  ASSERT_TRUE(base::CreatePipe(&read_pipe2, &write_pipe2));
+  data_offer.Receive("image/png", std::move(write_pipe));
+  data_offer.Receive("image/png", std::move(write_pipe2));
+  task_environment()->RunUntilIdle();
+  ASSERT_TRUE(ReadString(std::move(read_pipe), &result));
+  SkBitmap decoded;
+  ASSERT_TRUE(gfx::PNGCodec::Decode(
+      reinterpret_cast<const unsigned char*>(result.data()), result.size(),
+      &decoded));
+  EXPECT_TRUE(cc::MatchesBitmap(
+      image, decoded, cc::ExactPixelComparator(/*discard_alpha=*/false)));
+  std::string good = result;
+  ASSERT_TRUE(ReadString(std::move(read_pipe2), &result));
+  EXPECT_EQ(good, result);
+
+  // Receive() should now return immediately with result from cache.
+  ASSERT_TRUE(base::CreatePipe(&read_pipe, &write_pipe));
+  data_offer.Receive("image/png", std::move(write_pipe));
+  ASSERT_TRUE(ReadString(std::move(read_pipe), &result));
+  EXPECT_EQ(good, result);
+}
+
 TEST_F(DataOfferTest, AcceptWithNull) {
   TestDataOfferDelegate delegate;
   DataOffer data_offer(&delegate, DataOffer::Purpose::COPY_PASTE);