Check DataDevice still alive after nested RunLoop in OnPerformDrop()

DataDevice::OnPerformDrop() has a nested RunLoop() and the current
object can be destroyed inside that loop.

Bug: 1160553, 1160925
Change-Id: I09872e5f88d4d5c3e582ce7b3e6e4085afed0518
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2598519
Commit-Queue: Joel Hockey <joelhockey@chromium.org>
Reviewed-by: Mitsuru Oshima (slow: gardener) <oshima@chromium.org>
Auto-Submit: Joel Hockey <joelhockey@chromium.org>
Cr-Commit-Position: refs/heads/master@{#838700}

components/exo/data_device.cc

@@ -124,14 +124,18 @@ int DataDevice::OnPerformDrop(const ui::DropTargetEvent& event) {
 
   delegate_->OnDrop();
 
-  // TODO(tetsui): Avoid using nested loop by adding asynchronous callback to
-  // aura::client::DragDropDelegate.
+  // TODO(crbug.com/1160925): Avoid using nested loop by adding asynchronous
+  // callback to aura::client::DragDropDelegate.
+  base::WeakPtr<DataDevice> alive(weak_factory_.GetWeakPtr());
   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 (!alive)
+    return ui::DragDropTypes::DRAG_NONE;
+
   if (quit_closure_) {
     // DataOffer not destroyed by the client until the timeout.
     quit_closure_.Reset();

components/exo/data_device.h

@@ -8,6 +8,7 @@
 #include <cstdint>
 
 #include "base/macros.h"
+#include "base/memory/weak_ptr.h"
 #include "components/exo/data_offer_observer.h"
 #include "components/exo/seat_observer.h"
 #include "components/exo/surface.h"
@@ -87,6 +88,7 @@ class DataDevice : public WMHelper::DragDropObserver,
 
   base::OnceClosure quit_closure_;
   bool drop_succeeded_;
+  base::WeakPtrFactory<DataDevice> weak_factory_{this};
 
   DISALLOW_COPY_AND_ASSIGN(DataDevice);
 };

components/exo/data_device_unittest.cc

@@ -11,6 +11,7 @@
 #include "ash/shell.h"
 #include "base/strings/string16.h"
 #include "base/strings/utf_string_conversions.h"
+#include "base/test/bind.h"
 #include "components/exo/data_device_delegate.h"
 #include "components/exo/data_exchange_delegate.h"
 #include "components/exo/data_offer.h"
@@ -199,6 +200,17 @@ TEST_F(DataDeviceTest, DataEventsExit) {
   EXPECT_EQ(DataEvent::kLeave, events[0]);
 }
 
+TEST_F(DataDeviceTest, DeleteDataDeviceDuringDrop) {
+  ui::DropTargetEvent event(data_, gfx::PointF(), gfx::PointF(),
+                            ui::DragDropTypes::DRAG_MOVE);
+  ui::Event::DispatcherApi(&event).set_target(surface_->window());
+  device_->OnDragEntered(event);
+  base::ThreadTaskRunnerHandle::Get()->PostTask(
+      FROM_HERE, base::BindLambdaForTesting([&]() { device_.reset(); }));
+  int result = device_->OnPerformDrop(event);
+  EXPECT_EQ(ui::DragDropTypes::DRAG_NONE, result);
+}
+
 TEST_F(DataDeviceTest, DeleteDataOfferDuringDrag) {
   ui::DropTargetEvent event(data_, gfx::PointF(), gfx::PointF(),
                             ui::DragDropTypes::DRAG_MOVE);