Avoids releasing pointer grab prematurely

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 966a791862c327e371e8971162ffeef169bf65fd..2633401e267c0a68af20f009fb5b576df13298ef 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
@@ -93,21 +93,25 @@ bool X11WholeScreenMoveLoop::Dispatch(const base::NativeEvent& event) {
 
 bool X11WholeScreenMoveLoop::RunMoveLoop(aura::Window* source,
                                          gfx::NativeCursor cursor) {
-  // Start a capture on the host, so that it continues to receive events during
-  // the drag.
-  ScopedCapturer capturer(source->GetDispatcher()->host());
-
-  DCHECK(!in_move_loop_);  // Can only handle one nested loop at a time.
-  in_move_loop_ = true;
-
-  XDisplay* display = gfx::GetXDisplay();
-
-  grab_input_window_ = CreateDragInputWindow(display);
-  if (!drag_image_.isNull())
-    CreateDragImageWindow();
-  base::MessagePumpX11::Current()->AddDispatcherForWindow(
-      this, grab_input_window_);
-
+  {
+    // A capture is started on the host, so that it continues to receive events
+    // during the drag.
+    ScopedCapturer capturer(source->GetDispatcher()->host());
+
+    DCHECK(!in_move_loop_);  // Can only handle one nested loop at a time.
+    in_move_loop_ = true;
+
+    XDisplay* display = gfx::GetXDisplay();
+
+    grab_input_window_ = CreateDragInputWindow(display);
+    if (!drag_image_.isNull())
+      CreateDragImageWindow();
+    base::MessagePumpX11::Current()->AddDispatcherForWindow(
+        this, grab_input_window_);
+    // Releasing ScopedCapturer ensures that any other instance of
+    // X11ScopedCapture will not prematurely release grab that will be acquired
+    // below.

[sadrul, 2014/02/06 19:14:46]

I do not understand what this means. Mind explaining with more detail?

The capture is meant to persist during the entire drag. Releasing the capture
early here seems wrong?

[varkha, 2014/02/06 19:22:55]

There are three points where we set capture, all three are involved. The first
point is when DesktopWindowTreeHostX11::SetCapture is called. The second is
ScopedCapturer which releases that capture and sets another and the third is
GrabPointerWithCursor which releases that and grabs another one. This one is
supposed to stay through the drag. It gets broken when one tab looses focus (and
another one gains focus). When that happens the 
DesktopWindowTreeHostX11::OnNativeWidgetBlur releases capture and that is the
same capture as the one obtained last (in GrabPointerWithCursor). The result is
that mouse events are no longer reaching the drag input window.

Limiting the scope causes the capture in DesktopWindowTreeHostX11 (pointed by
x11_capture_ member there) to get released which then allows OnNativeWidgetBlur
to avoid releasing the capture.

[sadrul, 2014/02/07 00:08:37]

In the context of a drag, who calls this first SetCapture()? I suppose it's done
by Widget in response to the mouse-press event?
A DesktopWindowTreeHostX11 is supposed to receive events only directed to its
|xwindow_|. When the grab happens on |grab_input_window_|, do we get a FocusOut
X-event directed to |xwindow_|? I guess if this happens, then X11ScopedCapture
should not reset the grab in such situations.

If we reset the capture (i.e. the one from ScopedCapture) here and things work,
does it make any difference if we just don't hold the capture (i.e. don't use
ScopedCapture) in the first place?

+  }
   if (!GrabPointerWithCursor(cursor))
     return false;