Removes grab input window and extra grab and ungrab in X11WholeScreenMoveLoop

ui/views/widget/desktop_aura/desktop_drag_drop_client_aurax11.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "ui/views/widget/desktop_aura/desktop_drag_drop_client_aurax11.h"
 
 #include <X11/Xatom.h>
 
 #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"
 #include "ui/base/dragdrop/drop_target_event.h"
 #include "ui/base/dragdrop/os_exchange_data.h"
 #include "ui/base/dragdrop/os_exchange_data_provider_aurax11.h"
 #include "ui/base/x/selection_utils.h"
 #include "ui/base/x/x11_util.h"
 #include "ui/events/event.h"
 #include "ui/events/platform/platform_event_source.h"
@@ skipping 517 matching lines @@
     // We were waiting on the status message so we could send the XdndDrop.
     if (negotiated_operation_ == ui::DragDropTypes::DRAG_NONE) {
       move_loop_.EndMoveLoop();
       return;
     }
     source_state_ = SOURCE_STATE_DROPPED;
     SendXdndDrop(source_window);
     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
   // within it. However, it is considered advisory and (at least according to
   // the spec) the other side must handle further position messages within
   // it. GTK+ doesn't bother with this, so neither should we.
 
   if (next_position_message_.get()) {
     // We were waiting on the status message so we could send off the next
     // position message we queued up.
@@ skipping 322 matching lines @@
   if (drag_operation_ & ui::DragDropTypes::DRAG_LINK)
     operations.push_back(atom_cache_.GetAtom(kXdndActionLink));
   return operations;
 }
 
 ui::SelectionFormatMap DesktopDragDropClientAuraX11::GetFormatMap() const {
   return source_provider_ ? source_provider_->GetFormatMap() :
       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) {
   int drag_operation = ui::DragDropTypes::DRAG_NONE;
   scoped_ptr<ui::OSExchangeData> data;
   scoped_ptr<ui::DropTargetEvent> drop_target_event;
   DragDropDelegate* delegate = NULL;
   DragTranslate(screen_point, &data, &drop_target_event, &delegate);
   if (delegate)
     drag_operation = delegate->OnDragUpdated(*drop_target_event);
@@ skipping 128 matching lines @@
   // GdkEvent about the failed drag. (And sending this message doesn't appear
   // to go through normal xlib machinery, but instead passes through the low
   // level xProto (the x11 wire format) that I don't understand.
   //
   // I'm unsure if I have to jump through those hoops, or if XSendEvent is
   // sufficient.
   XSendEvent(xdisplay_, xid, False, 0, xev);
 }
 
 }  // namespace views