Do not release capture when transferring capture between Chrome windows on Linux

Do not release capture when transferring capture between Chrome windows on
Linux.

This is part #1 for fixing the remaining cases where tab dragging leaves the
user in a weird state if the mouse is released at the "right" time.

BUG=392480
TEST=DesktopWindowTreeHostX11Test.CaptureEventForwarding
TBR=erg (For ui/views/views.gyp)

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=287381

[pkotwicz, 2014-07-10 03:08:22]

Sadrul, can you please let me know what you think?

This CL (and the ones which come after it) make the following problems
disappear:
- Tab drag not ending when mouse button is released
- Dragged tab not resetting itself back to the correct position when the mouse
button is released

This does not fix tab dragging ending unexpectedly when quickly detaching and
attaching a tab repeatedly. That bug is due to us releasing capture upon losing
activation and activation loss being asynchronous. This CL might make this bug
slightly worse

This CL depends on:
https://codereview.chromium.org/334933008/
https://codereview.chromium.org/375593002/
https://codereview.chromium.org/376513002/

[pkotwicz, 2014-07-17 14:10:59]

Sadrul, can you please take a look? All of the prerequesite CLs have either
landed or are in the commit queue.

[pkotwicz, 2014-07-18 20:29:03]

Elliot, can you please take a look? I think that sadrul@'s really busy

[Elliot Glaysher, 2014-07-18 22:19:10]

lgtm

[sadrul, 2014-07-21 06:57:59]

Re the '[Draft]' in the CL title/description: do you want to make additional
changes?

How about testing?

https://codereview.chromium.org/380943003/diff/70010/ui/events/event_utils.h
File ui/events/event_utils.h (right):

https://codereview.chromium.org/380943003/diff/70010/ui/events/event_utils.h#...
ui/events/event_utils.h:76: const base::NativeEvent& native_event);
Can this live inside desktop_window_tree_host_x11.cc instead? If not, move this
somewhere inside ui/events/x/ (x11_utils.h|cc perhaps).

[pkotwicz, 2014-07-28 01:04:44]

Sadrul, I have added tests. I have also fixed an issue where X11CaptureWindow
was not getting events due to the ordering of the PlatformEventDispatcher.

After some thought, I think that making X11CaptureWindow a singleton is
preferable. The CL as is creates an X window each time the user clicks the
mouse. (The delay of creating and mapping the X window is .5ms). What do you
think?

[pkotwicz, 2014-07-28 01:04:44]

Sadrul, I have added tests. I have also fixed an issue where X11CaptureWindow
was not getting events due to the ordering of the PlatformEventDispatcher.

After some thought, I think that making X11CaptureWindow a singleton is
preferable. The CL as is creates an X window each time the user clicks the
mouse. (The delay of creating and mapping the X window is .5ms). What do you
think?

[sadrul, 2014-07-29 18:47:50]

On 2014/07/28 01:04:44, pkotwicz wrote:
> Sadrul, I have added tests. I have also fixed an issue where X11CaptureWindow
> was not getting events due to the ordering of the PlatformEventDispatcher.
> 
> After some thought, I think that making X11CaptureWindow a singleton is
> preferable. The CL as is creates an X window each time the user clicks the
> mouse. (The delay of creating and mapping the X window is .5ms). What do you
> think?

Spending .5ms for each click does sound very expensive. So the proposal for
making the capture-window a singleton sounds like a good idea.

https://codereview.chromium.org/380943003/diff/110001/ui/views/widget/desktop...
File ui/views/widget/desktop_aura/desktop_window_tree_host_x11.cc (right):

https://codereview.chromium.org/380943003/diff/110001/ui/views/widget/desktop...
ui/views/widget/desktop_aura/desktop_window_tree_host_x11.cc:1410:
event->set_location(location);
ConvertLocationToTarget() should work correctly here. Does it not?

https://codereview.chromium.org/380943003/diff/110001/ui/views/widget/desktop...
ui/views/widget/desktop_aura/desktop_window_tree_host_x11.cc:1622:
ReleaseCapture();
Should NotifyGrab really trigger ReleaseCapture()?

[pkotwicz, 2014-08-01 03:35:00]

erg@ and sadrul@ can you please take another look?

The CL is now significantly simpler

It turns out that making X11CaptureWindow a singleton is a really bad idea. Some
window managers (e.g. Awesome window manager) treat override redirect windows as
modal and prevent most window manager interaction while an override redirect
window is mapped.
It turns out that it is unnecessary to release capture before grabbing capture
of a different window. Note: "XGrabPointer() overrides any active pointer grab
by this client." in http://tronche.com/gui/x/xlib/input/XGrabPointer.html

https://codereview.chromium.org/380943003/diff/110001/ui/views/widget/desktop...
File ui/views/widget/desktop_aura/desktop_window_tree_host_x11.cc (right):

https://codereview.chromium.org/380943003/diff/110001/ui/views/widget/desktop...
ui/views/widget/desktop_aura/desktop_window_tree_host_x11.cc:1410:
event->set_location(location);
I am splitting this change out to a different CL because it is no longer
necessary for the purpose of the CL.
ConvertLocationToTarget() does not work because it does not update the root
location. We do something similar in MenuMessageLoopAura::RepostEventToWindow()

https://codereview.chromium.org/380943003/diff/110001/ui/views/widget/desktop...
ui/views/widget/desktop_aura/desktop_window_tree_host_x11.cc:1622:
ReleaseCapture();
I am going to differ releasing capture as a result of LeaveNotify for now.
After doing some reading, receiving LeaveNotify with any mode can signify a loss
of capture. A LeaveNotify event with NotifyGrab/NotifyUngrab is not sent if the
window loses capture but the window is the topmost window underneath the mouse.

[pkotwicz, 2014-08-01 03:38:01]

For the sake of completeness, I have tested that releasing mouse grab is in fact
not necessary before initiating a new mouse grab on Unity, KDE, and Cinnamon

[pkotwicz, 2014-08-01 04:32:14]

https://codereview.chromium.org/380943003/diff/110001/ui/views/widget/desktop...
File ui/views/widget/desktop_aura/desktop_window_tree_host_x11.cc (right):

https://codereview.chromium.org/380943003/diff/110001/ui/views/widget/desktop...
ui/views/widget/desktop_aura/desktop_window_tree_host_x11.cc:1410:
event->set_location(location);
You are right ConvertLocationToTarget() does work properly. The event's root
location is set later in LocatedEvent::UpdateForRootTransform()

[Elliot Glaysher, 2014-08-01 16:55:07]

lgtm

[pkotwicz, 2014-08-04 15:36:11]

sadrul@, mentioned that he did not need to take a look at the CL again

[pkotwicz, 2014-08-04 15:36:15]

The CQ bit was checked by pkotwicz@chromium.org

[commit-bot: I haz the power, 2014-08-04 15:37:29]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/pkotwicz@chromium.org/380943003/190001

[pkotwicz, 2014-08-04 16:49:52]

The CQ bit was unchecked by pkotwicz@chromium.org

[pkotwicz, 2014-08-04 16:49:52]

The CQ bit was checked by pkotwicz@chromium.org

[commit-bot: I haz the power, 2014-08-04 16:50:07]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/pkotwicz@chromium.org/380943003/190001

[commit-bot: I haz the power, 2014-08-04 18:12:31]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  linux_chromium_rel_swarming on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[commit-bot: I haz the power, 2014-08-04 18:44:01]

Change committed as 287381