linux/x11: Fix event dispatch in menus in High DPI.

linux/x11: Fix event dispatch in menus in High DPI.

In 'normal' setting (i.e. non-HighDPI), the coordinate space of a
DesktopWindowTreeHostX11 and the aura::Window it hosts is the same.
So when converting an event's location from one host's coordinate
space to another host's coordinate space, it is sufficient to use
the aura::Window API to convert the location between the hosts'
aura::Window. However, in HighDPI, the coordinate space of the host
is different from that of its aura::Window, and using aura::Window
API to do the coordinate conversion results in incorrect location.
Use a simpler method to do the conversion to avoid this.

BUG=400837
R=pkotwicz@chromium.org

Committed: https://chromium.googlesource.com/chromium/src/+/c4a76234ea701f22a870231d1b900a367693acc6

[sadrul, 2015-02-13 17:18:50]

sadrul@chromium.org changed reviewers:
+ pkotwicz@chromium.org

[pkotwicz, 2015-02-13 20:15:35]

LGTM with nits

https://codereview.chromium.org/922293002/diff/1/ui/views/widget/desktop_aura...
File ui/views/widget/desktop_aura/desktop_window_tree_host_x11.cc (right):

https://codereview.chromium.org/922293002/diff/1/ui/views/widget/desktop_aura...
ui/views/widget/desktop_aura/desktop_window_tree_host_x11.cc:1450: void
DesktopWindowTreeHostX11::UpdateEventLocationForDifferentHost(
Nit: I think that ConvertEventToDifferentHost() is clearer.

https://codereview.chromium.org/922293002/diff/1/ui/views/widget/desktop_aura...
ui/views/widget/desktop_aura/desktop_window_tree_host_x11.cc:1461:
g_current_capture->GetLocationOnNativeScreen();
g_current_capture -> host

https://codereview.chromium.org/922293002/diff/1/ui/views/widget/desktop_aura...
ui/views/widget/desktop_aura/desktop_window_tree_host_x11.cc:1462: gfx::Point
location_in_pixel = located_event->location() + offset;
Nit: This variable name is confusing. Is "pixel" the name of yet another
coordinate system? Maybe rename |host| to |other_host| and rename
|location_in_pixel| to |location_in_other_host|

https://codereview.chromium.org/922293002/diff/1/ui/views/widget/desktop_aura...
File ui/views/widget/desktop_aura/desktop_window_tree_host_x11_unittest.cc
(right):

https://codereview.chromium.org/922293002/diff/1/ui/views/widget/desktop_aura...
ui/views/widget/desktop_aura/desktop_window_tree_host_x11_unittest.cc:544:
TEST_F(DesktopWindowTreeHostX11HighDPITest, LocatedEventDispatchWithCapture) {
This test must be an interactive ui test. (Because tests which use native
capture cannot be run in parallel)

[sadrul, 2015-02-13 20:27:03]

New patchsets have been uploaded after l-g-t-m from pkotwicz@chromium.org

[sadrul, 2015-02-13 20:27:36]

https://codereview.chromium.org/922293002/diff/1/ui/views/widget/desktop_aura...
File ui/views/widget/desktop_aura/desktop_window_tree_host_x11.cc (right):

https://codereview.chromium.org/922293002/diff/1/ui/views/widget/desktop_aura...
ui/views/widget/desktop_aura/desktop_window_tree_host_x11.cc:1450: void
DesktopWindowTreeHostX11::UpdateEventLocationForDifferentHost(
On 2015/02/13 20:15:35, pkotwicz wrote:
> Nit: I think that ConvertEventToDifferentHost() is clearer.

Done.

https://codereview.chromium.org/922293002/diff/1/ui/views/widget/desktop_aura...
ui/views/widget/desktop_aura/desktop_window_tree_host_x11.cc:1461:
g_current_capture->GetLocationOnNativeScreen();
On 2015/02/13 20:15:35, pkotwicz wrote:
> g_current_capture -> host

whoops. Done.

https://codereview.chromium.org/922293002/diff/1/ui/views/widget/desktop_aura...
ui/views/widget/desktop_aura/desktop_window_tree_host_x11.cc:1462: gfx::Point
location_in_pixel = located_event->location() + offset;
On 2015/02/13 20:15:35, pkotwicz wrote:
> Nit: This variable name is confusing. Is "pixel" the name of yet another
> coordinate system? Maybe rename |host| to |other_host| and rename
> |location_in_pixel| to |location_in_other_host|

'pixel' here means 'physical pixels' (for device-independent pixels). We use
_in_pixel (and _in_dip) suffixes in a lot of ui code to mean these coord space.
I want to be explicit here too that the location here is in physical pixel
space.

Renamed to location_in_pixel_in_host (since |host| is the variable name).

https://codereview.chromium.org/922293002/diff/1/ui/views/widget/desktop_aura...
File ui/views/widget/desktop_aura/desktop_window_tree_host_x11_unittest.cc
(right):

https://codereview.chromium.org/922293002/diff/1/ui/views/widget/desktop_aura...
ui/views/widget/desktop_aura/desktop_window_tree_host_x11_unittest.cc:544:
TEST_F(DesktopWindowTreeHostX11HighDPITest, LocatedEventDispatchWithCapture) {
On 2015/02/13 20:15:35, pkotwicz wrote:
> This test must be an interactive ui test. (Because tests which use native
> capture cannot be run in parallel)

That's why this test doesn't actually do native capture. (see PretendCapture()
above).

[sadrul, 2015-02-13 20:31:09]

https://codereview.chromium.org/922293002/diff/1/ui/views/widget/desktop_aura...
File ui/views/widget/desktop_aura/desktop_window_tree_host_x11.cc (right):

https://codereview.chromium.org/922293002/diff/1/ui/views/widget/desktop_aura...
ui/views/widget/desktop_aura/desktop_window_tree_host_x11.cc:1462: gfx::Point
location_in_pixel = located_event->location() + offset;
On 2015/02/13 20:27:36, sadrul wrote:
> On 2015/02/13 20:15:35, pkotwicz wrote:
> > Nit: This variable name is confusing. Is "pixel" the name of yet another
> > coordinate system? Maybe rename |host| to |other_host| and rename
> > |location_in_pixel| to |location_in_other_host|
> 
> 'pixel' here means 'physical pixels' (for device-independent pixels). We use

Should read: "'pixel' here means 'physical pixels' (*as opposed to*
device-independent pixels). ..."


> _in_pixel (and _in_dip) suffixes in a lot of ui code to mean these coord
space.
> I want to be explicit here too that the location here is in physical pixel
> space.
> 
> Renamed to location_in_pixel_in_host (since |host| is the variable name).

[commit-bot: I haz the power, 2015-02-14 04:36:14]

Patchset 2 (id:??) landed as
https://crrev.com/c4a76234ea701f22a870231d1b900a367693acc6
Cr-Commit-Position: refs/heads/master@{#316373}

[sadrul, 2015-02-14 04:36:25]

Committed patchset #2 (id:20001) manually as
c4a76234ea701f22a870231d1b900a367693acc6 (presubmit successful).