Add ScreenPositionClient::ConvertNativePointToScreen.

Add ScreenPositionClient::ConvertNativePointToScreen.

This will be used to convert mouse location properly in RootWindow when X's native input is captured by a drag operation.

BUG=148686
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=157702

[oshima, 2012-09-18 10:02:33]

Do you have CL that uses this? Did you split because it's too big?

http://codereview.chromium.org/10911342/diff/1/ash/display/screen_position_co...
File ash/display/screen_position_controller_unittest.cc (right):

http://codereview.chromium.org/10911342/diff/1/ash/display/screen_position_co...
ash/display/screen_position_controller_unittest.cc:13: #include
"ash/display/display_controller.h"
sort includes

http://codereview.chromium.org/10911342/diff/1/ash/display/screen_position_co...
ash/display/screen_position_controller_unittest.cc:29: point);
use gfx::Screen::GetDisplayNearestPoint()

http://codereview.chromium.org/10911342/diff/1/ash/display/screen_position_co...
ash/display/screen_position_controller_unittest.cc:87: EXPECT_EQ("150,150",
point.ToString());
since you have window_ as instance and controller is accessible globally, you
can simplify like

EXPECT_EQ("150,150", ConvertNativeToScreen(gfx::Point(50, 50));

http://codereview.chromium.org/10911342/diff/1/ash/display/screen_position_co...
ash/display/screen_position_controller_unittest.cc:264: } // namespace ash
two spaces before //

http://codereview.chromium.org/10911342/diff/1/ui/aura/client/screen_position...
File ui/aura/client/screen_position_client.h (right):

http://codereview.chromium.org/10911342/diff/1/ui/aura/client/screen_position...
ui/aura/client/screen_position_client.h:38: // See the comments in
ash::wm::GetRootWindowRelativeToWindow for details.
This description isn't exactly correct, i think. 
This is to convert an event whose location is outside of
the mouse event target's root (mouse_moved_handler_ i believe). Passive grab is
one scenario and aura capture is another scenario. (see my comment in
coordinate_conversion.cc)

[mazda, 2012-09-18 16:35:01]

> Do you have CL that uses this?
I'm working on converting location in RootWindow, but it's not complete. I'll
send it for review shrotly.

> Did you split because it's too big?
Yes. As I need to modify all the places that use
wm::GetRootWindowRelatieToWindow, the change is moderately big.

http://codereview.chromium.org/10911342/diff/1/ash/display/screen_position_co...
File ash/display/screen_position_controller_unittest.cc (right):

http://codereview.chromium.org/10911342/diff/1/ash/display/screen_position_co...
ash/display/screen_position_controller_unittest.cc:13: #include
"ash/display/display_controller.h"
On 2012/09/18 10:02:34, oshima wrote:
> sort includes

Done.

http://codereview.chromium.org/10911342/diff/1/ash/display/screen_position_co...
ash/display/screen_position_controller_unittest.cc:29: point);
On 2012/09/18 10:02:34, oshima wrote:
> use gfx::Screen::GetDisplayNearestPoint()

Done.

http://codereview.chromium.org/10911342/diff/1/ash/display/screen_position_co...
ash/display/screen_position_controller_unittest.cc:87: EXPECT_EQ("150,150",
point.ToString());
On 2012/09/18 10:02:34, oshima wrote:
> since you have window_ as instance and controller is accessible globally, you
> can simplify like
> 
> EXPECT_EQ("150,150", ConvertNativeToScreen(gfx::Point(50, 50));
Done.
That's much simpler. Thanks.

http://codereview.chromium.org/10911342/diff/1/ash/display/screen_position_co...
ash/display/screen_position_controller_unittest.cc:264: } // namespace ash
On 2012/09/18 10:02:34, oshima wrote:
> two spaces before //

Done.

http://codereview.chromium.org/10911342/diff/1/ui/aura/client/screen_position...
File ui/aura/client/screen_position_client.h (right):

http://codereview.chromium.org/10911342/diff/1/ui/aura/client/screen_position...
ui/aura/client/screen_position_client.h:38: // See the comments in
ash::wm::GetRootWindowRelativeToWindow for details.
On 2012/09/18 10:02:34, oshima wrote:
> This description isn't exactly correct, i think. 
> This is to convert an event whose location is outside of
> the mouse event target's root (mouse_moved_handler_ i believe). Passive grab
is
> one scenario and aura capture is another scenario. (see my comment in
> coordinate_conversion.cc)

I'm thinking about letting this function handle only passive grab case because
trying to handle another scenario causes problems when bounds in native and
scree overlaps as we talked before.

[mazda, 2012-09-18 17:58:06]

File upload was not done properly, sorry.
Please take another look.

[oshima, 2012-09-19 01:46:27]

lgtm with nits

http://codereview.chromium.org/10911342/diff/4004/ash/display/screen_position...
File ash/display/screen_position_controller.cc (right):

http://codereview.chromium.org/10911342/diff/4004/ash/display/screen_position...
ash/display/screen_position_controller.cc:84:
wm::GetRootWindowRelativeToWindow(window, *point);
Please move GetRootWindowRelativeToWindow to this file in next CL as we
discussed offline.

http://codereview.chromium.org/10911342/diff/4004/ash/display/screen_position...
File ash/display/screen_position_controller_unittest.cc (right):

http://codereview.chromium.org/10911342/diff/4004/ash/display/screen_position...
ash/display/screen_position_controller_unittest.cc:54: std::string
ConvertNativePointToScreen(int x, int y) {
can this be const method?

http://codereview.chromium.org/10911342/diff/4004/ash/shell.cc
File ash/shell.cc (right):

http://codereview.chromium.org/10911342/diff/4004/ash/shell.cc#newcode176
ash/shell.cc:176: Shell::TestApi::screen_position_controller() {
indent (like other accessors)

[mazda, 2012-09-19 02:06:56]

http://codereview.chromium.org/10911342/diff/4004/ash/display/screen_position...
File ash/display/screen_position_controller.cc (right):

http://codereview.chromium.org/10911342/diff/4004/ash/display/screen_position...
ash/display/screen_position_controller.cc:84:
wm::GetRootWindowRelativeToWindow(window, *point);
On 2012/09/19 01:46:27, oshima wrote:
> Please move GetRootWindowRelativeToWindow to this file in next CL as we
> discussed offline.
Sure, I will.

http://codereview.chromium.org/10911342/diff/4004/ash/display/screen_position...
File ash/display/screen_position_controller_unittest.cc (right):

http://codereview.chromium.org/10911342/diff/4004/ash/display/screen_position...
ash/display/screen_position_controller_unittest.cc:54: std::string
ConvertNativePointToScreen(int x, int y) {
On 2012/09/19 01:46:27, oshima wrote:
> can this be const method?

Done.

http://codereview.chromium.org/10911342/diff/4004/ash/shell.cc
File ash/shell.cc (right):

http://codereview.chromium.org/10911342/diff/4004/ash/shell.cc#newcode176
ash/shell.cc:176: Shell::TestApi::screen_position_controller() {
On 2012/09/19 01:46:27, oshima wrote:
> indent (like other accessors)

Done.

[sky, 2012-09-19 13:59:38]

LGTM

[commit-bot: I haz the power, 2012-09-20 01:18:01]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mazda@chromium.org/10911342/4009

[commit-bot: I haz the power, 2012-09-20 03:38:40]

Change committed as 157702