Removes grab input window and extra grab and ungrab in X11WholeScreenMoveLoop

ui/views/widget/desktop_aura/x11_whole_screen_move_loop.cc

Index: ui/views/widget/desktop_aura/x11_whole_screen_move_loop.cc
diff --git a/ui/views/widget/desktop_aura/x11_whole_screen_move_loop.cc b/ui/views/widget/desktop_aura/x11_whole_screen_move_loop.cc
index fa9b75f4efeb59889512169274bc7b9fbe6fb76d..c993d5467b3ccf75a8e992c6decc0e29a600c536 100644
--- a/ui/views/widget/desktop_aura/x11_whole_screen_move_loop.cc
+++ b/ui/views/widget/desktop_aura/x11_whole_screen_move_loop.cc
@@ -4,6 +4,7 @@
 
 #include "ui/views/widget/desktop_aura/x11_whole_screen_move_loop.h"
 
+#include <X11/extensions/XInput2.h>
 #include <X11/Xlib.h>
 // Get rid of a macro from Xlib.h that conflicts with Aura's RootWindow class.
 #undef RootWindow
@@ -81,17 +82,13 @@ void X11WholeScreenMoveLoop::DispatchMouseMovement() {
 // DesktopWindowTreeHostLinux, ui::PlatformEventDispatcher implementation:
 
 bool X11WholeScreenMoveLoop::CanDispatchEvent(const ui::PlatformEvent& event) {
+  NOTREACHED();
   return in_move_loop_;
 }
 
 uint32_t X11WholeScreenMoveLoop::DispatchEvent(const ui::PlatformEvent& event) {
-  // This method processes all events for the grab_input_window_ as well as
-  // mouse events for all windows while the move loop is active - even before
-  // the grab is granted by X. This allows mouse notification events that were
-  // sent after the capture was requested but before the capture was granted
-  // to be dispatched. It is especially important to process the mouse release
-  // event that should have stopped the drag even if that mouse release happened
-  // before the grab was granted.
+  // This method processes all mouse events for all windows while the move loop
+  // is active.
   if (!in_move_loop_)
     return ui::POST_DISPATCH_PERFORM_DEFAULT;
   XEvent* xev = event;
@@ -150,7 +147,9 @@ uint32_t X11WholeScreenMoveLoop::DispatchEvent(const ui::PlatformEvent& event) {
             xevent.type = MotionNotify;
           }
           xevent.xany.display = xev->xgeneric.display;
-          xevent.xany.window = grab_input_window_;
+          XIDeviceEvent* xievent =
+              static_cast<XIDeviceEvent*>(xev->xcookie.data);
+          xevent.xany.window = xievent->event;
           // The fields used below are in the same place for all of events
           // above. Using xmotion from XEvent's unions to avoid repeating
           // the code.
@@ -168,8 +167,7 @@ uint32_t X11WholeScreenMoveLoop::DispatchEvent(const ui::PlatformEvent& event) {
     }
   }
 
-  return (event->xany.window == grab_input_window_) ?
-      ui::POST_DISPATCH_NONE : ui::POST_DISPATCH_PERFORM_DEFAULT;
+  return ui::POST_DISPATCH_PERFORM_DEFAULT;
 }
 
 bool X11WholeScreenMoveLoop::RunMoveLoop(aura::Window* source,
@@ -177,25 +175,8 @@ bool X11WholeScreenMoveLoop::RunMoveLoop(aura::Window* source,
   DCHECK(!in_move_loop_);  // Can only handle one nested loop at a time.
 
   // Start a capture on the host, so that it continues to receive events during
-  // the drag. This may be second time we are capturing the mouse events - the
-  // first being when a mouse is first pressed. That first capture needs to be
-  // released before the call to GrabPointerAndKeyboard below, otherwise it may
-  // get released while we still need the pointer grab, which is why we restrict
-  // the scope here.
-  {
-    ScopedCapturer capturer(source->GetHost());
-
-    grab_input_window_ = CreateDragInputWindow(gfx::GetXDisplay());
-    // Releasing ScopedCapturer ensures that any other instance of
-    // X11ScopedCapture will not prematurely release grab that will be acquired
-    // below.
-  }
-  // TODO(varkha): Consider integrating GrabPointerAndKeyboard with
-  // ScopedCapturer to avoid possibility of logically keeping multiple grabs.
-  if (!GrabPointerAndKeyboard(cursor)) {
-    XDestroyWindow(gfx::GetXDisplay(), grab_input_window_);
-    return false;
-  }
+  // the drag.
+  ScopedCapturer capturer(source->GetHost());

[pkotwicz, 2014/05/02 19:50:39]

This is not really right. X11WholeScreenMoveLoop::RunMoveLoop() is called when
|in_move_loop_| is true if you call set source->SetCapture() instead.

It looks like for drag and drop specifically there is an expectation that
capture must be lost at the beginning of the drag and drop operation. There
seems to be also no expectation for capture to be restored at the end of the
drag.
I wonder whether it would be possible to:
- Set capture to the drag widget for drag and drop only.
  This could be done in DesktopDragDropClientAuraX11::StartDragAndDrop()
  You could change things around such that |source| is the drag window when
RunMoveLoop() is called as a result of drag and drop
- Set capture on |source| otherwise.

[varkha, 2014/05/02 23:58:11]

I have changed it so that the capture is only kept for the tab dragging and not
for drag an drop. The capture needs to be restored at the end of the tab
dragging (or we need an extra click when a tab is dragged out).

[pkotwicz, 2014/05/05 00:09:05]

On CrOS the capture is released as a result of views::Widget::OnMouseReleased().
Can we do the same here?
This "just works" if ui::POST_DISPATCH_NONE is replaced with
ui::POST_DISPATCH_PERFORM_DEFAULT when handling ButtonRelease.

Would it be possible to move as much of the logic as possible which is specific
to drag and drop into DesktopDragDropClientAuraX11? e.g. UpdateCursor() could
potentially be moved out of this class.

[varkha, 2014/05/05 16:37:14]

Yes we can! Done.
Done.
Done.

 
   scoped_ptr<ui::ScopedEventDispatcher> old_dispatcher =
       nested_dispatcher_.Pass();
@@ -251,16 +232,10 @@ void X11WholeScreenMoveLoop::EndMoveLoop() {
   // 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.
-  XDisplay* display = gfx::GetXDisplay();
-  XUngrabPointer(display, CurrentTime);
-  XUngrabKeyboard(display, CurrentTime);
-
   // Restore the previous dispatcher.
   nested_dispatcher_.reset();
   drag_widget_.reset();
   delegate_->OnMoveLoopEnded();
-  XDestroyWindow(display, grab_input_window_);
   grab_input_window_ = None;
 
   in_move_loop_ = false;
@@ -358,6 +333,8 @@ void X11WholeScreenMoveLoop::CreateDragImageWindow() {
   widget->SetContentsView(image);
   widget->Show();
   widget->GetNativeWindow()->layer()->SetFillsBoundsOpaquely(false);
+  grab_input_window_ = widget->GetNativeWindow()->GetHost()->
+      GetAcceleratedWidget();
 
   drag_widget_.reset(widget);
 }