exo: Report DnD failure to ash::DragDropController

According to https://wayland.freedesktop.org/docs/html/apa.html,
the drag source should consider the drag is successful when
wl_data_offer.finish is called, and unsuccessful when
wl_data_offer.destroy is called without finish.

TEST=DataDeviceTest.DataOfferNotFinished
BUG=b:64963392

Change-Id: I3338e1c1dff7f0860daa8f36bc2d5dcb0b30b873
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2206300Reviewed-by: Yuichiro Hanada <yhanada@chromium.org>
Reviewed-by: Mitsuru Oshima <oshima@chromium.org>
Commit-Queue: Tetsui Ohkubo <tetsui@chromium.org>
Cr-Commit-Position: refs/heads/master@{#771453}

components/exo/data_device.cc

@@ -4,6 +4,7 @@
 
 #include "components/exo/data_device.h"
 
+#include "base/run_loop.h"
 #include "components/exo/data_device_delegate.h"
 #include "components/exo/data_offer.h"
 #include "components/exo/data_source.h"
@@ -16,10 +17,34 @@
 
 namespace exo {
 
+namespace {
+
+constexpr base::TimeDelta kDataOfferDestructionTimeout =
+    base::TimeDelta::FromMilliseconds(1000);
+
+ui::DragDropTypes::DragOperation DndActionToDragOperation(
+    DndAction dnd_action) {
+  switch (dnd_action) {
+    case DndAction::kMove:
+      return ui::DragDropTypes::DRAG_MOVE;
+    case DndAction::kCopy:
+      return ui::DragDropTypes::DRAG_COPY;
+    case DndAction::kAsk:
+      return ui::DragDropTypes::DRAG_LINK;
+    case DndAction::kNone:
+      return ui::DragDropTypes::DRAG_NONE;
+  }
+}
+
+}  // namespace
+
 DataDevice::DataDevice(DataDeviceDelegate* delegate,
                        Seat* seat,
                        FileHelper* file_helper)
-    : delegate_(delegate), seat_(seat), file_helper_(file_helper) {
+    : delegate_(delegate),
+      seat_(seat),
+      file_helper_(file_helper),
+      drop_succeeded_(false) {
   WMHelper::GetInstance()->AddDragDropObserver(this);
   ui::ClipboardMonitor::GetInstance()->AddObserver(this);
 
@@ -83,16 +108,7 @@ int DataDevice::OnDragUpdated(const ui::DropTargetEvent& event) {
 
   // TODO(hirono): dnd_action() here may not be updated. Chrome needs to provide
   // a way to update DND action asynchronously.
-  switch (data_offer_->get()->dnd_action()) {
-    case DndAction::kMove:
-      return ui::DragDropTypes::DRAG_MOVE;
-    case DndAction::kCopy:
-      return ui::DragDropTypes::DRAG_COPY;
-    case DndAction::kAsk:
-      return ui::DragDropTypes::DRAG_LINK;
-    case DndAction::kNone:
-      return ui::DragDropTypes::DRAG_NONE;
-  }
+  return DndActionToDragOperation(data_offer_->get()->dnd_action());
 }
 
 void DataDevice::OnDragExited() {
@@ -107,9 +123,29 @@ int DataDevice::OnPerformDrop(const ui::DropTargetEvent& event) {
   if (!data_offer_)
     return ui::DragDropTypes::DRAG_NONE;
 
+  DndAction dnd_action = data_offer_->get()->dnd_action();
+
   delegate_->OnDrop();
-  data_offer_.reset();
-  return ui::DragDropTypes::DRAG_NONE;
+
+  // TODO(tetsui): Avoid using nested loop by adding asynchronous callback to
+  // aura::client::DragDropDelegate.
+  base::RunLoop run_loop(base::RunLoop::Type::kNestableTasksAllowed);
+  base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
+      FROM_HERE, run_loop.QuitClosure(), kDataOfferDestructionTimeout);
+  quit_closure_ = run_loop.QuitClosure();
+  run_loop.Run();
+
+  if (quit_closure_) {
+    // DataOffer not destroyed by the client until the timeout.
+    quit_closure_.Reset();
+    data_offer_.reset();
+    drop_succeeded_ = false;
+  }
+
+  if (!drop_succeeded_)
+    return ui::DragDropTypes::DRAG_NONE;
+
+  return DndActionToDragOperation(dnd_action);
 }
 
 void DataDevice::OnClipboardDataChanged() {
@@ -140,8 +176,12 @@ void DataDevice::OnSurfaceFocusing(Surface* surface) {
 void DataDevice::OnSurfaceFocused(Surface* surface) {}
 
 void DataDevice::OnDataOfferDestroying(DataOffer* data_offer) {
-  if (data_offer_ && data_offer_->get() == data_offer)
+  if (data_offer_ && data_offer_->get() == data_offer) {
+    drop_succeeded_ = data_offer_->get()->finished();
+    if (quit_closure_)
+      std::move(quit_closure_).Run();
     data_offer_.reset();
+  }
 }
 
 void DataDevice::OnSurfaceDestroying(Surface* surface) {

components/exo/data_device.h

@@ -91,6 +91,9 @@ class DataDevice : public WMHelper::DragDropObserver,
   std::unique_ptr<ScopedDataOffer> data_offer_;
   std::unique_ptr<ScopedSurface> focused_surface_;
 
+  base::OnceClosure quit_closure_;
+  bool drop_succeeded_;
+
   DISALLOW_COPY_AND_ASSIGN(DataDevice);
 };
 

components/exo/data_device_unittest.cc

@@ -61,7 +61,11 @@ class TestDataDeviceDelegate : public DataDeviceDelegate {
     return out->size();
   }
   Surface* entered_surface() const { return entered_surface_; }
-  void DeleteDataOffer() { data_offer_.reset(); }
+  void DeleteDataOffer(bool finished) {
+    if (finished)
+      data_offer_->Finish();
+    data_offer_.reset();
+  }
   void set_can_accept_data_events_for_surface(bool value) {
     can_accept_data_events_for_surface_ = value;
   }
@@ -186,7 +190,12 @@ TEST_F(DataDeviceTest, DataEventsDrop) {
   ASSERT_EQ(1u, delegate_.PopEvents(&events));
   EXPECT_EQ(DataEvent::kMotion, events[0]);
 
-  device_->OnPerformDrop(event);
+  base::ThreadTaskRunnerHandle::Get()->PostTask(
+      FROM_HERE, base::BindOnce(&TestDataDeviceDelegate::DeleteDataOffer,
+                                base::Unretained(&delegate_), true));
+
+  int result = device_->OnPerformDrop(event);
+  EXPECT_EQ(ui::DragDropTypes::DRAG_LINK, result);
   ASSERT_EQ(1u, delegate_.PopEvents(&events));
   EXPECT_EQ(DataEvent::kDrop, events[0]);
 }
@@ -222,7 +231,7 @@ TEST_F(DataDeviceTest, DeleteDataOfferDuringDrag) {
   EXPECT_EQ(DataEvent::kOffer, events[0]);
   EXPECT_EQ(DataEvent::kEnter, events[1]);
 
-  delegate_.DeleteDataOffer();
+  delegate_.DeleteDataOffer(false);
 
   EXPECT_EQ(ui::DragDropTypes::DRAG_NONE, device_->OnDragUpdated(event));
   EXPECT_EQ(0u, delegate_.PopEvents(&events));
@@ -231,6 +240,31 @@ TEST_F(DataDeviceTest, DeleteDataOfferDuringDrag) {
   EXPECT_EQ(0u, delegate_.PopEvents(&events));
 }
 
+TEST_F(DataDeviceTest, DataOfferNotFinished) {
+  ui::DropTargetEvent event(data_, gfx::PointF(), gfx::PointF(),
+                            ui::DragDropTypes::DRAG_MOVE);
+  ui::Event::DispatcherApi(&event).set_target(surface_->window());
+
+  std::vector<DataEvent> events;
+  device_->OnDragEntered(event);
+  ASSERT_EQ(2u, delegate_.PopEvents(&events));
+  EXPECT_EQ(DataEvent::kOffer, events[0]);
+  EXPECT_EQ(DataEvent::kEnter, events[1]);
+
+  EXPECT_EQ(ui::DragDropTypes::DRAG_LINK, device_->OnDragUpdated(event));
+  ASSERT_EQ(1u, delegate_.PopEvents(&events));
+  EXPECT_EQ(DataEvent::kMotion, events[0]);
+
+  base::ThreadTaskRunnerHandle::Get()->PostTask(
+      FROM_HERE, base::BindOnce(&TestDataDeviceDelegate::DeleteDataOffer,
+                                base::Unretained(&delegate_), false));
+
+  int result = device_->OnPerformDrop(event);
+  EXPECT_EQ(ui::DragDropTypes::DRAG_NONE, result);
+  ASSERT_EQ(1u, delegate_.PopEvents(&events));
+  EXPECT_EQ(DataEvent::kDrop, events[0]);
+}
+
 TEST_F(DataDeviceTest, NotAcceptDataEventsForSurface) {
   ui::DropTargetEvent event(data_, gfx::PointF(), gfx::PointF(),
                             ui::DragDropTypes::DRAG_MOVE);

components/exo/data_offer.cc

@@ -14,6 +14,7 @@
 #include "base/strings/utf_string_conversions.h"
 #include "base/task/post_task.h"
 #include "base/task/thread_pool.h"
+#include "components/exo/data_device.h"
 #include "components/exo/data_offer_delegate.h"
 #include "components/exo/data_offer_observer.h"
 #include "components/exo/file_helper.h"
@@ -181,7 +182,10 @@ ScopedDataOffer::~ScopedDataOffer() {
 }
 
 DataOffer::DataOffer(DataOfferDelegate* delegate, Purpose purpose)
-    : delegate_(delegate), purpose_(purpose) {}
+    : delegate_(delegate),
+      dnd_action_(DndAction::kNone),
+      purpose_(purpose),
+      finished_(false) {}
 
 DataOffer::~DataOffer() {
   delegate_->OnDataOfferDestroying(this);
@@ -226,7 +230,9 @@ void DataOffer::Receive(const std::string& mime_type, base::ScopedFD fd) {
   }
 }
 
-void DataOffer::Finish() {}
+void DataOffer::Finish() {
+  finished_ = true;
+}
 
 void DataOffer::SetActions(const base::flat_set<DndAction>& dnd_actions,
                            DndAction preferred_action) {

components/exo/data_offer.h

@@ -79,7 +79,8 @@ class DataOffer final : public ui::PropertyHandler {
   // DataOffer object.
   void SetSourceActions(const base::flat_set<DndAction>& source_actions);
 
-  DndAction dnd_action() { return dnd_action_; }
+  DndAction dnd_action() const { return dnd_action_; }
+  bool finished() const { return finished_; }
 
  private:
   void OnPickledUrlsResolved(const std::string& uri_list_mime_type,
@@ -104,6 +105,7 @@ class DataOffer final : public ui::PropertyHandler {
   DndAction dnd_action_;
   base::ObserverList<DataOfferObserver>::Unchecked observers_;
   Purpose purpose_;
+  bool finished_;
 
   base::WeakPtrFactory<DataOffer> weak_ptr_factory_{this};
 

components/exo/wm_helper_chromeos.cc

@@ -120,11 +120,10 @@ void WMHelperChromeOS::OnDragExited() {
 
 int WMHelperChromeOS::OnPerformDrop(const ui::DropTargetEvent& event,
                                     std::unique_ptr<ui::OSExchangeData> data) {
+  int valid_operation = ui::DragDropTypes::DRAG_NONE;
   for (DragDropObserver& observer : drag_drop_observers_)
-    observer.OnPerformDrop(event);
-  // TODO(hirono): Return the correct result instead of always returning
-  // DRAG_MOVE.
-  return ui::DragDropTypes::DRAG_MOVE;
+    valid_operation = valid_operation | observer.OnPerformDrop(event);
+  return valid_operation;
 }
 
 void WMHelperChromeOS::AddVSyncParameterObserver(