Fix panel dragging on Linux Aura

Temporary hack in order to make panel dragging work on Linux Aura for M-35.

Some background:
- We use |DesktopWindowTreeHostX11::bounds_| to convert from window coordinates to screen coordinates.
- We get mouse events (e.g. MotionNotify) with coordinates wrt to the real window bounds.
- DesktopWindowTreeHostX11::SetBounds() synchronously changes |DesktopWindowTreeHostX11::bounds_| but does not synchronously change the real window bounds.
As a result, there is a brief period of time where "real window bounds" != "requested window bounds" while a panel is being dragged. During that period of time the computed mouse event position in screen coordinates is wrong.

BUG=351730
TEST=Manual, see bug

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

[pkotwicz, 2014-03-13 03:40:50]

Sadrul, PTAL

[pkotwicz, 2014-03-13 03:44:58]

Sadrul, PTAL

[pkotwicz, 2014-03-13 03:49:45]

https://codereview.chromium.org/197283008/diff/10001/ui/views/widget/desktop_...
File ui/views/widget/desktop_aura/desktop_window_tree_host_x11.cc (right):

https://codereview.chromium.org/197283008/diff/10001/ui/views/widget/desktop_...
ui/views/widget/desktop_aura/desktop_window_tree_host_x11.cc:1344: 0, 0,
&translated_x, &translated_y, &unused);
The event coordinates matched exactly those that I got from the x_root and
y_root properties of the MotionNotify event if I skipped calling
XTranslateCoordinates().
Do you know why this would be the case?

[sadrul, 2014-03-13 15:41:30]

https://codereview.chromium.org/197283008/diff/10001/ui/views/widget/desktop_...
File ui/views/widget/desktop_aura/desktop_window_tree_host_x11.cc (right):

https://codereview.chromium.org/197283008/diff/10001/ui/views/widget/desktop_...
ui/views/widget/desktop_aura/desktop_window_tree_host_x11.cc:1131: 
This doesn't look right. Can you update
DesktopWindowTreeHostX11::GetClientAreaBoundsInScreen() instead to return
x_bounds_?

https://codereview.chromium.org/197283008/diff/10001/ui/views/widget/desktop_...
ui/views/widget/desktop_aura/desktop_window_tree_host_x11.cc:1344: 0, 0,
&translated_x, &translated_y, &unused);
On 2014/03/13 03:49:45, pkotwicz wrote:
> The event coordinates matched exactly those that I got from the x_root and
> y_root properties of the MotionNotify event if I skipped calling
> XTranslateCoordinates().
> Do you know why this would be the case?

I don't understand what you are asking. This is handling a ConfigureNotify
event, not MotionNotify. (see https://codereview.chromium.org/12319091 for
XTranslateCoordinates)

https://codereview.chromium.org/197283008/diff/10001/ui/views/widget/desktop_...
File ui/views/widget/desktop_aura/desktop_window_tree_host_x11.h (right):

https://codereview.chromium.org/197283008/diff/10001/ui/views/widget/desktop_...
ui/views/widget/desktop_aura/desktop_window_tree_host_x11.h:225: // The bounds
that were requested for |xwindow_|.
This can be confusing (e.g. |bounds_| can also be the real bounds of
|xwindow_|). It would be better to merge the docs for |bounds_| and |x_bounds_|

[pkotwicz, 2014-03-13 19:47:48]

https://codereview.chromium.org/197283008/diff/10001/ui/views/widget/desktop_...
File ui/views/widget/desktop_aura/desktop_window_tree_host_x11.cc (right):

https://codereview.chromium.org/197283008/diff/10001/ui/views/widget/desktop_...
ui/views/widget/desktop_aura/desktop_window_tree_host_x11.cc:1131: 
I think I understand what you are asking.
You are hoping that we will be able to return a correct position in the event
window and a correct position in screen coordinates.

Personally I prefer my solution over yours.

A couple comments about your solution:
- We really should not be highjacking
DesktopWindowTreeHostX11::GetClientAreaBoundsInScreen() to return the real
window bounds. Retrieving |x_bounds_| is not the same as calling GetClientRect()
http://msdn.microsoft.com/en-us/library/aa932115.aspx
- |x_bounds_| should only have the real origin of the window (not its real
size). I foresee a plethora of theoretical problems with the local size being
different from the size in screen coordinates.

I think that it is ok for the position of the mouse event in the coordinates of
the event window to be off while the window is being moved. Do you foresee any
problems with this being the case?

https://codereview.chromium.org/197283008/diff/10001/ui/views/widget/desktop_...
ui/views/widget/desktop_aura/desktop_window_tree_host_x11.cc:1344: 0, 0,
&translated_x, &translated_y, &unused);
Sorry for not being clear. I understand why XTranslateCoordinates() is
desireable.

I implemented a different technique to correct the mouse event location in
DesktopWindowTreeHostX11::DispatchMouseEvent() by using x_root and y_root in
MotionNotify. (root_x and root_y in XI_Motion)
By removing the call to XTranslateCoordinates() here, the mouse position
computed in DispatchMouseEvent() using that technique and with this CL (minus
XTranslateCoordinates) returns the same value.

[pkotwicz, 2014-03-18 02:48:26]

Sadrul, can you please take a look.

I rewrote the CL so that it is much less risky

https://codereview.chromium.org/197283008/diff/70001/chrome/browser/ui/views/...
File chrome/browser/ui/views/panels/panel_frame_view.cc (right):

https://codereview.chromium.org/197283008/diff/70001/chrome/browser/ui/views/...
chrome/browser/ui/views/panels/panel_frame_view.cc:67: #if defined(USE_X11) &&
!defined(OS_CHROMEOS)
I did not put this method in events_x.cc because I am unsure if we are going to
use this method elsewhere. There was no clear improvement when I added similar
to code to X11WholeScreenMoveLoop::Dispatch()

[sadrul, 2014-03-18 04:25:22]

This change uses much more of X event than I thought would be necessary. Let's
avoid that, and use Screen::GetCursorScreenPoint() instead.

[pkotwicz, 2014-03-18 17:23:31]

I made the changes that you requested. Sadrul, can you please take another look?

[sadrul, 2014-03-18 19:21:39]

lgtm

[sadrul, 2014-03-18 19:21:52]

On 2014/03/18 19:21:39, sadrul wrote:
> lgtm

Please make sure to update the CL description.

[pkotwicz, 2014-03-18 19:42:30]

I actually think that the CL description is appropriate. sadrul@ can you please
suggest improvements to the CL description.

jianli@ for OWNERS

[pkotwicz, 2014-03-18 19:42:54]

jianli@ for OWNERS

[sadrul, 2014-03-18 20:49:36]

On 2014/03/18 19:42:30, pkotwicz wrote:
> I actually think that the CL description is appropriate. sadrul@ can you
please
> suggest improvements to the CL description.

Yep, the description looks good.

> 
> jianli@ for OWNERS

[pkotwicz, 2014-03-20 18:41:22]

jianli@, Ping?

[jianli, 2014-03-21 19:02:47]

lgtm

[pkotwicz, 2014-03-21 19:19:17]

The CQ bit was checked by pkotwicz@chromium.org

[commit-bot: I haz the power, 2014-03-21 19:19:42]

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

[commit-bot: I haz the power, 2014-03-21 19:51:26]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-21 19:51:27]

Try jobs failed on following builders:
  tryserver.chromium on mac_chromium_compile_dbg

[pkotwicz, 2014-03-24 19:09:41]

The CQ bit was checked by pkotwicz@chromium.org

[commit-bot: I haz the power, 2014-03-24 19:10:04]

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

[commit-bot: I haz the power, 2014-03-24 22:48:19]

Change committed as 259047