Use XGrabPointer instead of XChangeActivatePointerGrab() to change the cursor during a pointer grab

ui/views/widget/desktop_aura/x11_whole_screen_move_loop.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/x11_whole_screen_move_loop.h"
 
 #include <X11/keysym.h>
 #include <X11/Xlib.h>
 
 #include "base/bind.h"
@@ skipping 21 matching lines @@
   Mod5Mask,                         // Scroll lock
   Mod2Mask | LockMask,
   Mod2Mask | Mod5Mask,
   LockMask | Mod5Mask,
   Mod2Mask | LockMask | Mod5Mask
 };
 
 X11WholeScreenMoveLoop::X11WholeScreenMoveLoop(X11MoveLoopDelegate* delegate)
     : delegate_(delegate),
       in_move_loop_(false),
-      initial_cursor_(ui::kCursorNull),
       should_reset_mouse_flags_(false),
       grab_input_window_(None),
       grabbed_pointer_(false),
       canceled_(false),
       weak_factory_(this) {
 }
 
 X11WholeScreenMoveLoop::~X11WholeScreenMoveLoop() {}
 
 void X11WholeScreenMoveLoop::DispatchMouseMovement() {
@@ skipping 62 matching lines @@
         EndMoveLoop();
         return ui::POST_DISPATCH_NONE;
       }
       break;
     default:
       break;
   }
   return ui::POST_DISPATCH_PERFORM_DEFAULT;
 }
 
-bool X11WholeScreenMoveLoop::RunMoveLoop(aura::Window* source,
-                                         gfx::NativeCursor cursor) {
+bool X11WholeScreenMoveLoop::RunMoveLoop(aura::Window* source) {
   DCHECK(!in_move_loop_);  // Can only handle one nested loop at a time.
 
-  // Query the mouse cursor prior to the move loop starting so that it can be
-  // restored when the move loop finishes.
-  initial_cursor_ = source->GetHost()->last_cursor();
-
   grab_input_window_ = CreateDragInputWindow(gfx::GetXDisplay());
 
   // Only grab mouse capture of |grab_input_window_| if |source| does not have
   // capture.
   // - The caller may intend to transfer capture to a different aura::Window
   //   when the move loop ends and not release capture.
   // - Releasing capture and X window destruction are both asynchronous. We drop
   //   events targeted at |grab_input_window_| in the time between the move
   //   loop ends and |grab_input_window_| loses capture.
   grabbed_pointer_ = false;
   if (!source->HasCapture()) {
     aura::client::CaptureClient* capture_client =
         aura::client::GetCaptureClient(source->GetRootWindow());
     CHECK(capture_client->GetGlobalCaptureWindow() == NULL);
-    grabbed_pointer_ = GrabPointer(cursor);
+    grabbed_pointer_ = GrabPointer();
     if (!grabbed_pointer_) {
       XDestroyWindow(gfx::GetXDisplay(), grab_input_window_);
       return false;
     }
   }
 
   GrabEscKey();
 
   scoped_ptr<ui::ScopedEventDispatcher> old_dispatcher =
       nested_dispatcher_.Pass();
@@ skipping 15 matching lines @@
   base::MessageLoop::ScopedNestableTaskAllower allow_nested(loop);
   base::RunLoop run_loop;
   quit_closure_ = run_loop.QuitClosure();
   run_loop.Run();
   nested_dispatcher_ = old_dispatcher.Pass();
   return !canceled_;
 }
 
 void X11WholeScreenMoveLoop::UpdateCursor(gfx::NativeCursor cursor) {
   if (in_move_loop_) {
-    // We cannot call GrabPointer() because we do not want to change the
-    // "owner_events" property of the active pointer grab.
-    XChangeActivePointerGrab(
-        gfx::GetXDisplay(),
-        ButtonPressMask | ButtonReleaseMask | PointerMotionMask,
-        cursor.platform(),
-        CurrentTime);
+    if (grabbed_pointer_) {
+      // Do not grab the server. We already have pointer grab.
+      ui::GrabPointer(grab_input_window_, false, cursor.platform());
+    } else {
+      NOTREACHED();
+    }
   }
 }
 
 void X11WholeScreenMoveLoop::EndMoveLoop() {
   if (!in_move_loop_)
     return;
 
   // Prevent DispatchMouseMovement from dispatching any posted motion event.
   weak_factory_.InvalidateWeakPtrs();
   last_motion_in_screen_.reset();
 
   // We undo our emulated mouse click from RunMoveLoop();
   if (should_reset_mouse_flags_) {
     aura::Env::GetInstance()->set_mouse_button_flags(0);
     should_reset_mouse_flags_ = false;
   }
 
   // TODO(erg): Is this ungrab the cause of having to click to give input focus
   // on drawn out windows? Not ungrabbing here screws the X server until I kill
   // the chrome process.
 
   // Ungrab before we let go of the window.
+  if (grabbed_pointer_)
+    ui::UngrabPointer();
+
   XDisplay* display = gfx::GetXDisplay();
-  if (grabbed_pointer_)
-    XUngrabPointer(display, CurrentTime);
-  else
-    UpdateCursor(initial_cursor_);
-
   unsigned int esc_keycode = XKeysymToKeycode(display, XK_Escape);
   for (size_t i = 0; i < arraysize(kModifiersMasks); ++i) {
     XUngrabKey(display, esc_keycode, kModifiersMasks[i], grab_input_window_);
   }
 
   // Restore the previous dispatcher.
   nested_dispatcher_.reset();
   delegate_->OnMoveLoopEnded();
   XDestroyWindow(display, grab_input_window_);
   grab_input_window_ = None;
 
   in_move_loop_ = false;
   quit_closure_.Run();
 }
 
-bool X11WholeScreenMoveLoop::GrabPointer(gfx::NativeCursor cursor) {
+bool X11WholeScreenMoveLoop::GrabPointer() {
   XDisplay* display = gfx::GetXDisplay();
   XGrabServer(display);
 
   // Pass "owner_events" as false so that X sends all mouse events to
   // |grab_input_window_|.
-  int ret = XGrabPointer(
-      display,
-      grab_input_window_,
-      False,  // owner_events
-      ButtonPressMask | ButtonReleaseMask | PointerMotionMask,
-      GrabModeAsync,
-      GrabModeAsync,
-      None,
-      cursor.platform(),
-      CurrentTime);
+  int ret = ui::GrabPointer(grab_input_window_, false, None);

[pkotwicz, 2014/12/22 23:44:16]

Passing None for the Cursor argument seems like the right thing to do.

According to http://tronche.com/gui/x/xlib/input/XGrabPointer.html
"If None is specified, the normal cursor for that window is displayed when the
pointer is in grab_window or one of its subwindows; otherwise, the cursor for
grab_window is displayed."

There is a bug where the mouse cursor unexpectedly changes to a text caret while
dragging a tab from one browser window to another. This CL does not make that
bug better or worse.

[sadrul, 2014/12/23 16:53:50]

This is different from what we do now though, right? So let's keep this change
(i.e. using None for Cursor) as a separate CL, so revert is easier if we need
to.

[pkotwicz, 2014/12/23 18:22:22]

I do not understand. If X11WholeScreenMoveLoop::RunMoveLoop() does not take a
cursor argument, which cursor should be passed in to GrabPointer()?
WindowTreeHost::last_cursor() is an option, but it will change the behavior of
Drag and Drop.
Previously, we were passing in the cursor passed in to RunMoveLoop() to
GrabPointer()

[sadrul, 2015/01/14 20:34:01]

Why not continue to do the same (i.e. why should we require two calls to
ui::GrabPointer() through a call to RunMoveLoop() and another call to
UpdateCursor())?

[pkotwicz, 2015/01/14 20:50:07]

The problem is that if RunMoveLoop() takes a cursor argument, then we need to
somehow revert to the old cursor when the move loop ends regardless of the value
of |grabbed_pointer_|.

In order to update the cursor of an XI2 grab, I would need to do a new grab and
know the value of owner_events in the original grab.

   if (ret != GrabSuccess) {
     DLOG(ERROR) << "Grabbing pointer for dragging failed: "
                 << ui::GetX11ErrorString(display, ret);
   }
   XUngrabServer(display);
   XFlush(display);
   return ret == GrabSuccess;
 }
 
 void X11WholeScreenMoveLoop::GrabEscKey() {
@@ skipping 16 matching lines @@
                                 DefaultRootWindow(display),
                                 -100, -100, 10, 10,
                                 0, CopyFromParent, InputOnly, CopyFromParent,
                                 attribute_mask, &swa);
   XMapRaised(display, window);
   ui::X11EventSource::GetInstance()->BlockUntilWindowMapped(window);
   return window;
 }
 
 }  // namespace views