Removes grab input window and extra grab and ungrab in X11WholeScreenMoveLoop

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/extensions/XInput2.h>
 #include <X11/Xlib.h>
 // Get rid of a macro from Xlib.h that conflicts with Aura's RootWindow class.
 #undef RootWindow
 
 #include "base/bind.h"
 #include "base/message_loop/message_loop.h"
 #include "base/run_loop.h"
 #include "third_party/skia/include/core/SkBitmap.h"
 #include "ui/aura/env.h"
 #include "ui/aura/window.h"
@@ skipping 57 matching lines @@
   weak_factory_.InvalidateWeakPtrs();
   DCHECK_EQ(MotionNotify, last_xmotion_.type);
   delegate_->OnMouseMovement(&last_xmotion_);
   last_xmotion_.type = LASTEvent;
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 // 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;
 
   // Note: the escape key is handled in the tab drag controller, which has
   // keyboard focus even though we took pointer grab.
   switch (xev->type) {
     case MotionNotify: {
       if (drag_widget_.get()) {
         gfx::Screen* screen = gfx::Screen::GetNativeScreen();
@@ skipping 38 matching lines @@
         case ui::ET_MOUSE_DRAGGED:
         case ui::ET_MOUSE_RELEASED: {
           XEvent xevent = {0};
           if (type == ui::ET_MOUSE_RELEASED) {
             xevent.type = ButtonRelease;
             xevent.xbutton.button = ui::EventButtonFromNative(xev);
           } else {
             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.
           xevent.xmotion.root = DefaultRootWindow(xev->xgeneric.display);
           xevent.xmotion.time = ui::EventTimeFromNative(xev).InMilliseconds();
           gfx::Point point(ui::EventSystemLocationFromNative(xev));
           xevent.xmotion.x_root = point.x();
           xevent.xmotion.y_root = point.y();
           DispatchEvent(&xevent);
           return ui::POST_DISPATCH_NONE;

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

This should be: "return DispatchEvent(&xevent);"

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

Done.

         }
         default:
           break;
       }
     }
   }
 
-  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,
                                          gfx::NativeCursor cursor) {
   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();
   nested_dispatcher_ =
          ui::PlatformEventSource::GetInstance()->OverrideDispatcher(this);
   if (!drag_image_.isNull() && CheckIfIconValid())
     CreateDragImageWindow();
 
   // We are handling a mouse drag outside of the aura::RootWindow system. We
   // must manually make aura think that the mouse button is pressed so that we
   // don't draw extraneous tooltips.
   aura::Env* env = aura::Env::GetInstance();
   if (!env->IsMouseButtonDown()) {
     env->set_mouse_button_flags(ui::EF_LEFT_MOUSE_BUTTON);
     should_reset_mouse_flags_ = true;
   }
 
   in_move_loop_ = true;
   canceled_ = false;
   base::MessageLoopForUI* loop = base::MessageLoopForUI::current();
   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_) {

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

I wonder whether we could do a grab on the window returned by
aura::client::CaptureClient::GetGlobalCaptureWindow() and no-op if the result is
NULL

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

I don't think it will work. GetGlobalCaptureWindow() returns NULL while in the
drag since the drag widget loses focus.

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

I tried this quickly and GetGlobalCaptureWindow() seemed to do the trick. In
which situation ddoes it not work for you?

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

It did not work because I was still using ScopedCapturer. Works now. Done.

     // If we're still in the move loop, regrab 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.
     GrabPointerAndKeyboard(cursor);
   }
 }
 
 void X11WholeScreenMoveLoop::EndMoveLoop() {
   if (!in_move_loop_)
     return;
 
   // Prevent DispatchMouseMovement from dispatching any posted motion event.
   weak_factory_.InvalidateWeakPtrs();
   last_xmotion_.type = LASTEvent;
 
   // 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.
-  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;
   quit_closure_.Run();
 }
 
 void X11WholeScreenMoveLoop::SetDragImage(const gfx::ImageSkia& image,
                                           gfx::Vector2dF offset) {
   drag_image_ = image;
   drag_offset_ = offset;
@@ skipping 77 matching lines @@
   widget->Init(params);
   widget->SetOpacity(kDragWidgetOpacity);
   widget->GetNativeWindow()->SetName("DragWindow");
 
   ImageView* image = new ImageView();
   image->SetImage(drag_image_);
   image->SetBounds(0, 0, drag_image_.width(), drag_image_.height());
   widget->SetContentsView(image);
   widget->Show();
   widget->GetNativeWindow()->layer()->SetFillsBoundsOpaquely(false);
+  grab_input_window_ = widget->GetNativeWindow()->GetHost()->
+      GetAcceleratedWidget();
 
   drag_widget_.reset(widget);
 }
 
 bool X11WholeScreenMoveLoop::CheckIfIconValid() {
   // TODO(erg): I've tried at least five different strategies for trying to
   // build a mask based off the alpha channel. While all of them have worked,
   // none of them have been performant and introduced multiple second
   // delays. (I spent a day getting a rectangle segmentation algorithm polished
   // here...and then found that even through I had the rectangle extraction
@@ skipping 11 matching lines @@
     for (int x = 0; x < in_bitmap->width(); ++x) {
       if (SkColorGetA(in_row[x]) > kMinAlpha)
         return true;
     }
   }
 
   return false;
 }
 
 }  // namespace views