Make drag and drop more robust on Linux Aura

This CL:
- Avoids sending XdndPosition after the XdndDrop message has been sent. From experimentation this fixes the flakiness of dragging to the desktop
- Ends the move loop if the target takes too long to respond with XdndStatus or XdndFinished after the mouse is released. This is not perfect but prevents us from keeping the pointer lock indefinitely.

BUG=355061, 349131
TEST=Attempt dragging out a link to the desktop 20 times. Ensure that the operation is successful each time

Review URL: https://codereview.chromium.org/208623006

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@259128 0039d316-1c4b-4281-b951-d872f2087c98

2 files changed, 71 insertions, 10 deletions

diff --git a/ui/views/widget/desktop_aura/desktop_drag_drop_client_aurax11.cc b/ui/views/widget/desktop_aura/desktop_drag_drop_client_aurax11.cc
index 3be4d0f..f18f087 100644
--- a/ui/views/widget/desktop_aura/desktop_drag_drop_client_aurax11.cc
+++ b/ui/views/widget/desktop_aura/desktop_drag_drop_client_aurax11.cc
@@ -61,6 +61,10 @@ const char* kAtomsToCache[] = {
   NULL
 };
 
+// The time to wait for the target to respond after the user has released the
+// mouse button before ending the move loop.
+const int kEndMoveLoopTimeoutMs = 30000;
+
 static base::LazyInstance<
     std::map< ::Window, views::DesktopDragDropClientAuraX11*> >::Leaky
         g_live_client_map = LAZY_INSTANCE_INITIALIZER;
@@ -404,6 +408,7 @@ DesktopDragDropClientAuraX11::DesktopDragDropClientAuraX11(
       target_window_(NULL),
       source_provider_(NULL),
       source_current_window_(None),
+      source_state_(SOURCE_STATE_OTHER),
       drag_operation_(0),
       resulting_operation_(0),
       grab_cursor_(cursor_manager->GetInitializedCursor(ui::kCursorGrabbing)),
@@ -489,6 +494,11 @@ void DesktopDragDropClientAuraX11::OnXdndStatus(
   DVLOG(1) << "XdndStatus";
 
   unsigned long source_window = event.data.l[0];
+
+  waiting_on_status_.erase(source_window);
+  if (source_window != source_current_window_)
+    return;
+
   int drag_operation = ui::DragDropTypes::DRAG_NONE;
   if (event.data.l[1] & 1) {
     ::Atom atom_operation = event.data.l[4];
@@ -514,17 +524,16 @@ void DesktopDragDropClientAuraX11::OnXdndStatus(
   // the spec) the other side must handle further position messages within
   // it. GTK+ doesn't bother with this, so neither should we.
 
-  waiting_on_status_.erase(source_window);
-
-  if (ContainsKey(pending_drop_, source_window)) {
+  if (source_state_ == SOURCE_STATE_PENDING_DROP) {
     // We were waiting on the status message so we could send the XdndDrop.
+    source_state_ = SOURCE_STATE_DROPPED;
     SendXdndDrop(source_window);
-    pending_drop_.erase(source_window);
     return;
   }
 
   NextPositionMap::iterator it = next_position_message_.find(source_window);
-  if (it != next_position_message_.end()) {
+  if (source_state_ == SOURCE_STATE_OTHER &&
+      it != next_position_message_.end()) {
     // We were waiting on the status message so we could send off the next
     // position message we queued up.
     gfx::Point p = it->second.first;
@@ -600,6 +609,8 @@ int DesktopDragDropClientAuraX11::StartDragAndDrop(
   source_current_window_ = None;
   DCHECK(!g_current_drag_drop_client);
   g_current_drag_drop_client = this;
+  waiting_on_status_.clear();
+  source_state_ = SOURCE_STATE_OTHER;
   drag_operation_ = operation;
   resulting_operation_ = ui::DragDropTypes::DRAG_NONE;
 
@@ -664,6 +675,9 @@ void DesktopDragDropClientAuraX11::OnWindowDestroyed(aura::Window* window) {
 void DesktopDragDropClientAuraX11::OnMouseMovement(XMotionEvent* event) {
   gfx::Point screen_point(event->x_root, event->y_root);
 
+  if (source_state_ != SOURCE_STATE_OTHER)
+    return;
+
   // Find the current window the cursor is over.
   ::Window mouse_window = None;
   ::Window dest_window = None;
@@ -696,7 +710,11 @@ void DesktopDragDropClientAuraX11::OnMouseReleased() {
     if (ContainsKey(waiting_on_status_, source_current_window_)) {
       // If we are waiting for an XdndStatus message, we need to wait for it to
       // complete.
-      pending_drop_.insert(source_current_window_);
+      source_state_ = SOURCE_STATE_PENDING_DROP;
+
+      // Start timer to end the move loop if the target takes too long to send
+      // the XdndStatus and XdndFinished messages.
+      StartEndMoveLoopTimer();
       return;
     }
 
@@ -704,7 +722,12 @@ void DesktopDragDropClientAuraX11::OnMouseReleased() {
         negotiated_operation_.find(source_current_window_);
     if (it != negotiated_operation_.end() && it->second != None) {
       // We have negotiated an action with the other end.
+      source_state_ = SOURCE_STATE_DROPPED;
       SendXdndDrop(source_current_window_);
+
+      // Start timer to end the move loop if the target takes too long to send
+      // an XdndFinished message.
+      StartEndMoveLoopTimer();
       return;
     }
 
@@ -719,6 +742,20 @@ void DesktopDragDropClientAuraX11::OnMoveLoopEnded() {
   if (source_current_window_ != None)
     SendXdndLeave(source_current_window_);
   target_current_context_.reset();
+  source_state_ = SOURCE_STATE_OTHER;
+  end_move_loop_timer_.Stop();
+}
+
+void DesktopDragDropClientAuraX11::StartEndMoveLoopTimer() {
+  end_move_loop_timer_.Start(FROM_HERE,
+                             base::TimeDelta::FromMilliseconds(
+                                 kEndMoveLoopTimeoutMs),
+                             this,
+                             &DesktopDragDropClientAuraX11::EndMoveLoop);
+}
+
+void DesktopDragDropClientAuraX11::EndMoveLoop() {
+  move_loop_.EndMoveLoop();
 }
 
 void DesktopDragDropClientAuraX11::DragTranslate(
diff --git a/ui/views/widget/desktop_aura/desktop_drag_drop_client_aurax11.h b/ui/views/widget/desktop_aura/desktop_drag_drop_client_aurax11.h
index 4f492e4..360e31c 100644
--- a/ui/views/widget/desktop_aura/desktop_drag_drop_client_aurax11.h
+++ b/ui/views/widget/desktop_aura/desktop_drag_drop_client_aurax11.h
@@ -11,6 +11,7 @@
 #include "base/compiler_specific.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/weak_ptr.h"
+#include "base/timer/timer.h"
 #include "ui/aura/window_observer.h"
 #include "ui/base/cursor/cursor.h"
 #include "ui/gfx/point.h"
@@ -96,6 +97,28 @@ class VIEWS_EXPORT DesktopDragDropClientAuraX11
   virtual void OnMoveLoopEnded() OVERRIDE;
 
  private:
+  enum SourceState {
+    // |source_current_window_| will receive a drop once we receive an
+    // XdndStatus from it.
+    SOURCE_STATE_PENDING_DROP,
+
+    // The move looped will be ended once we receive XdndFinished from
+    // |source_current_window_|. We should not send XdndPosition to
+    // |source_current_window_| while in this state.
+    SOURCE_STATE_DROPPED,
+
+    // There is no drag in progress or there is a drag in progress and the
+    // user has not yet released the mouse.
+    SOURCE_STATE_OTHER,
+  };
+
+  // Start timer to end the move loop if the target is too slow to respond after
+  // the mouse is released.
+  void StartEndMoveLoopTimer();
+
+  // Ends the move loop.
+  void EndMoveLoop();
+
   typedef std::map< ::Window, std::pair<gfx::Point, unsigned long> >
       NextPositionMap;
 
@@ -184,6 +207,7 @@ class VIEWS_EXPORT DesktopDragDropClientAuraX11
   // Source side information.
   ui::OSExchangeDataProviderAuraX11 const* source_provider_;
   ::Window source_current_window_;
+  SourceState source_state_;
 
   // The current drag-drop client that has an active operation. Since we have
   // multiple root windows and multiple DesktopDragDropClientAuraX11 instances
@@ -198,10 +222,6 @@ class VIEWS_EXPORT DesktopDragDropClientAuraX11
   // message.
   int resulting_operation_;
 
-  // This window will be receiving a drop as soon as we receive an XdndStatus
-  // from it.
-  std::set< ::Window> pending_drop_;
-
   // We offer the other window a list of possible operations,
   // XdndActionsList. This is the requested action from the other window. This
   // is None if we haven't sent out an XdndPosition message yet, haven't yet
@@ -213,6 +233,10 @@ class VIEWS_EXPORT DesktopDragDropClientAuraX11
   // window responds to an XdndStatus.
   std::map< ::Window, ::Atom> negotiated_operation_;
 
+  // Ends the move loop if the target is too slow to respond after the mouse is
+  // released.
+  base::OneShotTimer<DesktopDragDropClientAuraX11> end_move_loop_timer_;
+
   // We use these cursors while dragging.
   gfx::NativeCursor grab_cursor_;
   gfx::NativeCursor copy_grab_cursor_;