athena: Add a ScreenPositionClient on the root-window.

athena: Add a ScreenPositionClient on the root-window.

Some of the code (e.g. virtual keyboard related code in RenderWidgetHostViewAura)
expects a non-null ScreenPositionClient on the root window. So install a simple
implementation of the client that provides implementation for a single display.

BUG=380215
R=oshima@chromium.org

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

[oshima, 2014-06-19 17:37:27]

https://codereview.chromium.org/330973003/diff/20001/athena/screen/screen_man...
File athena/screen/screen_manager_impl.cc (right):

https://codereview.chromium.org/330973003/diff/20001/athena/screen/screen_man...
athena/screen/screen_manager_impl.cc:95: point->Offset(origin.x(), origin.y());
Looks like this convert to the native coords, instead of screen.
You can just convert to the root.

[sadrul, 2014-06-19 17:59:00]

https://codereview.chromium.org/330973003/diff/20001/athena/screen/screen_man...
File athena/screen/screen_manager_impl.cc (right):

https://codereview.chromium.org/330973003/diff/20001/athena/screen/screen_man...
athena/screen/screen_manager_impl.cc:95: point->Offset(origin.x(), origin.y());
On 2014/06/19 17:37:27, oshima wrote:
> Looks like this convert to the native coords, instead of screen.
> You can just convert to the root. 

The doc for this is a bit lacking; but the current ash and desktop
implementations seem to do the conversion in native-screen coords, instead of
gfx::Screen coords.

[oshima, 2014-06-19 18:03:13]

The comment says

"Converts the |screen_point| from a given |window|'s coordinate space into
screen coordinate space."

ash code does convert to the screen coordinates, but not native.
(Note that gfx::Display::bounds() are in screen coordinates.)


On Thu, Jun 19, 2014 at 10:59 AM, <sadrul@chromium.org> wrote:

>
> https://codereview.chromium.org/330973003/diff/20001/
> athena/screen/screen_manager_impl.cc
> File athena/screen/screen_manager_impl.cc (right):
>
> https://codereview.chromium.org/330973003/diff/20001/
> athena/screen/screen_manager_impl.cc#newcode95
> athena/screen/screen_manager_impl.cc:95: point->Offset(origin.x(),
> origin.y());
> On 2014/06/19 17:37:27, oshima wrote:
>
>> Looks like this convert to the native coords, instead of screen.
>> You can just convert to the root.
>>
>
> The doc for this is a bit lacking; but the current ash and desktop
> implementations seem to do the conversion in native-screen coords,
> instead of gfx::Screen coords.
>
> https://codereview.chromium.org/330973003/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[sadrul, 2014-06-19 18:17:44]

https://codereview.chromium.org/330973003/diff/20001/athena/screen/screen_man...
File athena/screen/screen_manager_impl.cc (right):

https://codereview.chromium.org/330973003/diff/20001/athena/screen/screen_man...
athena/screen/screen_manager_impl.cc:95: point->Offset(origin.x(), origin.y());
On 2014/06/19 17:59:00, sadrul wrote:
> On 2014/06/19 17:37:27, oshima wrote:
> > Looks like this convert to the native coords, instead of screen.
> > You can just convert to the root. 
> 
> The doc for this is a bit lacking; but the current ash and desktop
> implementations seem to do the conversion in native-screen coords, instead of
> gfx::Screen coords.

Done.

[oshima, 2014-06-19 18:22:46]

lgtm with a nit

https://codereview.chromium.org/330973003/diff/40001/athena/screen/screen_man...
File athena/screen/screen_manager_impl.cc (right):

https://codereview.chromium.org/330973003/diff/40001/athena/screen/screen_man...
athena/screen/screen_manager_impl.cc:105: ConvertPointToScreen(root, point);
can you leave this to NOTREACHED() and add

// TODO(oshima): implement this when adding multiple display support.

[sadrul, 2014-06-19 22:24:52]

https://codereview.chromium.org/330973003/diff/40001/athena/screen/screen_man...
File athena/screen/screen_manager_impl.cc (right):

https://codereview.chromium.org/330973003/diff/40001/athena/screen/screen_man...
athena/screen/screen_manager_impl.cc:105: ConvertPointToScreen(root, point);
On 2014/06/19 18:22:46, oshima wrote:
> can you leave this to NOTREACHED() and add
> 
> // TODO(oshima): implement this when adding multiple display support.

Done.

[sadrul, 2014-06-19 22:26:17]

Committed patchset #4 manually as r278508 (presubmit successful).