Removes grab input window and extra grab and ungrab in X11WholeScreenMoveLoop

ui/views/widget/desktop_aura/desktop_drag_drop_client_aurax11.cc

Index: ui/views/widget/desktop_aura/desktop_drag_drop_client_aurax11.cc
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 a569eaeee06c60f66f3e25a138164e01c3155337..fbf6fca2b881de83d128c0fcf6f00630f0163106 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
@@ -9,6 +9,7 @@
 #include "base/event_types.h"
 #include "base/lazy_instance.h"
 #include "base/message_loop/message_loop.h"
+#include "ui/aura/client/capture_client.h"
 #include "ui/aura/window.h"
 #include "ui/aura/window_tree_host.h"
 #include "ui/base/clipboard/clipboard.h"
@@ -546,17 +547,7 @@ void DesktopDragDropClientAuraX11::OnXdndStatus(
     return;
   }
 
-  switch (negotiated_operation_) {
-    case ui::DragDropTypes::DRAG_COPY:
-      move_loop_.UpdateCursor(copy_grab_cursor_);
-      break;
-    case ui::DragDropTypes::DRAG_MOVE:
-      move_loop_.UpdateCursor(move_grab_cursor_);
-      break;
-    default:
-      move_loop_.UpdateCursor(grab_cursor_);
-      break;
-  }
+  UpdateCursor();
 
   // Note: event.data.[2,3] specify a rectangle. It is a request by the other
   // window to not send further XdndPosition messages while the cursor is
@@ -899,6 +890,50 @@ ui::SelectionFormatMap DesktopDragDropClientAuraX11::GetFormatMap() const {
       ui::SelectionFormatMap();
 }
 
+void DesktopDragDropClientAuraX11::UpdateCursor() {
+  if (!move_loop_.in_move_loop())
+    return;
+
+  // If we're still in the move loop, re-grab the pointer with the updated
+  // cursor. Note: we can be called from handling an XdndStatus message after
+  // EndMoveLoop() was called, but before we return from the nested RunLoop.

[pkotwicz, 2014/05/05 18:14:37]

I do not completely understand this comment.
move_loop_.in_move_loop() is set to false before the quit closure is run
asynchronously.

OnMoveLoopEnded() sets |source_current_window_| to None so UpdateCursor() should
hopefully not be invoked after OnMoveLoopEnded() was called. Did you find a case
where UpdateCursor() can be called after OnMoveLoopEnded() is invoked?

[varkha, 2014/05/05 19:06:04]

Yes, with the source_current_window_ set to None in OnMoveLoopEnded() I don't
expect this to be an issue. I've removed the check.

+  aura::client::CaptureClient* capture_client =
+      aura::client::GetCaptureClient(root_window_);
+  if (!capture_client)
+    return;
+  aura::Window* capture_window = capture_client->GetGlobalCaptureWindow();
+  if (!capture_window)
+    return;
+
+  gfx::NativeCursor cursor = grab_cursor_;
+  switch (negotiated_operation_) {
+    case ui::DragDropTypes::DRAG_COPY:
+      cursor = copy_grab_cursor_;
+      break;
+    case ui::DragDropTypes::DRAG_MOVE:
+      cursor = move_grab_cursor_;
+      break;
+    default:
+      break;
+  }
+

[pkotwicz, 2014/05/05 18:14:37]

Would XChangeActivePointerGrab() be more appropriate here?
http://tronche.com/gui/x/xlib/input/XChangeActivePointerGrab.html

[varkha, 2014/05/05 19:06:04]

Done. This also eliminates the need to get the current capture since this call
is a non-nop only when "pointer is actively grabbed by the client".

+  XUngrabPointer(xdisplay_, CurrentTime);
+  int ret = XGrabPointer(
+      xdisplay_,
+      capture_window->GetHost()->GetAcceleratedWidget(),
+      False,
+      ButtonPressMask | ButtonReleaseMask | PointerMotionMask,
+      GrabModeAsync,
+      GrabModeAsync,
+      None,
+      cursor.platform(),
+      CurrentTime);
+  if (ret != GrabSuccess) {
+    DLOG(ERROR) << "Grabbing pointer for dragging failed: "
+        << ui::GetX11ErrorString(xdisplay_, ret);
+  }
+}
+
 void DesktopDragDropClientAuraX11::CompleteXdndPosition(
     ::Window source_window,
     const gfx::Point& screen_point) {