aura: Remove WindowTreeHost::QueryMouseLocation().

aura: Remove WindowTreeHost::QueryMouseLocation().

The primary use-case for QueryMouseLocation() is for X11 interactive
tests. The only other use was in WindowEventDispatcher, but that code
is no longer necessary.

BUG=none
R=sky@chromium.org

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

[sadrul, 2014-04-25 21:20:23]

Hi! This is still a WIP CL. I was looking at the use of QueryMouseLocation(),
and it looks like this is the only non-test place where this is used. From what
I can see from history, it looks like we used to store the 'last mouse location'
per RootWindow when this code was originally added (waaaaay back in
https://chromiumcodereview.appspot.com/9701098/diff/40001/ui/aura/root_window...).
At that time, it made sense to update this position when the host-bounds
changed, since this location was in the host's coordinate system.

But now we store this 'last mouse location' in the Env, and it is in screen
coordinate space. So it should no longer be necessary to update this when the
host-bounds is changed. I made this change and ran aura/ash tests locally, and
there are no failures. I have sent it out to trybots to see if something breaks
there. In the meantime, can you think of anything that may regress from this
change? If not (and if no tests break), then I will complete the rest of the CL
and remove QueryMouseLocation() from WindowTreeHost interface.

Thanks!

[oshima, 2014-04-25 21:32:59]

On 2014/04/25 21:20:23, sadrul wrote:
> Hi! This is still a WIP CL. I was looking at the use of QueryMouseLocation(),
> and it looks like this is the only non-test place where this is used. From
what
> I can see from history, it looks like we used to store the 'last mouse
location'
> per RootWindow when this code was originally added (waaaaay back in
>
https://chromiumcodereview.appspot.com/9701098/diff/40001/ui/aura/root_window...).
> At that time, it made sense to update this position when the host-bounds
> changed, since this location was in the host's coordinate system.
> 
> But now we store this 'last mouse location' in the Env, and it is in screen
> coordinate space. So it should no longer be necessary to update this when the
> host-bounds is changed. I made this change and ran aura/ash tests locally, and
> there are no failures. I have sent it out to trybots to see if something
breaks
> there. In the meantime, can you think of anything that may regress from this
> change? If not (and if no tests break), then I will complete the rest of the
CL
> and remove QueryMouseLocation() from WindowTreeHost interface.
> 
> Thanks!

I think this is ok, although the reason is slightly different. When the host
window bounds
changes, the screen coordinates may change, and we still have to update the
location in that case,
but this must have been done elsewhere now so this should be fine.

[sadrul, 2014-04-25 21:37:21]

On 2014/04/25 21:32:59, oshima wrote:
> On 2014/04/25 21:20:23, sadrul wrote:
> > Hi! This is still a WIP CL. I was looking at the use of
QueryMouseLocation(),
> > and it looks like this is the only non-test place where this is used. From
> what
> > I can see from history, it looks like we used to store the 'last mouse
> location'
> > per RootWindow when this code was originally added (waaaaay back in
> >
>
https://chromiumcodereview.appspot.com/9701098/diff/40001/ui/aura/root_window...).
> > At that time, it made sense to update this position when the host-bounds
> > changed, since this location was in the host's coordinate system.
> > 
> > But now we store this 'last mouse location' in the Env, and it is in screen
> > coordinate space. So it should no longer be necessary to update this when
the
> > host-bounds is changed. I made this change and ran aura/ash tests locally,
and
> > there are no failures. I have sent it out to trybots to see if something
> breaks
> > there. In the meantime, can you think of anything that may regress from this
> > change? If not (and if no tests break), then I will complete the rest of the
> CL
> > and remove QueryMouseLocation() from WindowTreeHost interface.
> > 
> > Thanks!
> 
> I think this is ok, although the reason is slightly different. When the host
> window bounds
> changes, the screen coordinates may change, and we still have to update the
> location in that case,
> but this must have been done elsewhere now so this should be fine.

Cool. Thanks for the quick reply!

[sadrul, 2014-04-25 22:25:21]

+sky@

The CL is now complete and ready for formal review.

[sky, 2014-04-25 22:46:19]

https://codereview.chromium.org/251743004/diff/20001/ui/aura/test/ui_controls...
File ui/aura/test/ui_controls_factory_aurax11.cc (right):

https://codereview.chromium.org/251743004/diff/20001/ui/aura/test/ui_controls...
ui/aura/test/ui_controls_factory_aurax11.cc:114: WindowTreeHostTestApi
host_test_api(host_);
How about a static function that does all this? It can be on
WindowTreeHostTestApi if you want, or a standalone function. I'm fine with
either.

[sadrul, 2014-04-26 01:34:58]

https://codereview.chromium.org/251743004/diff/20001/ui/aura/test/ui_controls...
File ui/aura/test/ui_controls_factory_aurax11.cc (right):

https://codereview.chromium.org/251743004/diff/20001/ui/aura/test/ui_controls...
ui/aura/test/ui_controls_factory_aurax11.cc:114: WindowTreeHostTestApi
host_test_api(host_);
On 2014/04/25 22:46:19, sky wrote:
> How about a static function that does all this? It can be on
> WindowTreeHostTestApi if you want, or a standalone function. I'm fine with
> either.

Done. (moved WindowTreeHostTestApi into the cc file (and into aura::test
namespace), and added a static
aura::test::QueryLatestMousePositionRequestInHost()

[sky, 2014-04-28 13:42:06]

LGTM

[sadrul, 2014-04-28 14:43:04]

Committed patchset #3 manually as r266565 (presubmit successful).