Remove Event::system_location

Remove Event::system_location
 This is no longer used heavily and can be obtained
from native_event easily.

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

[Elliot Glaysher, 2013-04-09 23:55:47]

lgtm

[sadrul, 2013-04-10 00:12:41]

LGTM

https://codereview.chromium.org/13902003/diff/3001/ui/aura/root_window.cc
File ui/aura/root_window.cc (right):

https://codereview.chromium.org/13902003/diff/3001/ui/aura/root_window.cc#new...
ui/aura/root_window.cc:1181: ConvertPointToNativeScreen(&root_mouse_location);
Is this conversion necessary anymore?

https://codereview.chromium.org/13902003/diff/3001/ui/views/widget/desktop_au...
File ui/views/widget/desktop_aura/x11_window_event_filter.cc (right):

https://codereview.chromium.org/13902003/diff/3001/ui/views/widget/desktop_au...
ui/views/widget/desktop_aura/x11_window_event_filter.cc:116: const gfx::Point&
screen_location) {
This would ideally take the location in the window coord, and convert to screen
coord only when necessary. (this could be separate CL at some point in the
future)

[oshima, 2013-04-10 01:28:01]

https://codereview.chromium.org/13902003/diff/3001/ui/aura/root_window.cc
File ui/aura/root_window.cc (right):

https://codereview.chromium.org/13902003/diff/3001/ui/aura/root_window.cc#new...
ui/aura/root_window.cc:1181: ConvertPointToNativeScreen(&root_mouse_location);
doh, you're right. removed.

https://codereview.chromium.org/13902003/diff/3001/ui/views/widget/desktop_au...
File ui/views/widget/desktop_aura/x11_window_event_filter.cc (right):

https://codereview.chromium.org/13902003/diff/3001/ui/views/widget/desktop_au...
ui/views/widget/desktop_aura/x11_window_event_filter.cc:116: const gfx::Point&
screen_location) {
On 2013/04/10 00:12:41, sadrul wrote:
> This would ideally take the location in the window coord, and convert to
screen
> coord only when necessary. (this could be separate CL at some point in the
> future)

system_location is obtained directly from XEvent, and there
is no conversion applied, nor it is used as window coord.
I can get the location after switch though. Is it what you had
in your mind?

[oshima, 2013-04-10 01:28:54]

ben -> OWNERS review

[sadrul, 2013-04-10 05:31:40]

On 2013/04/10 01:28:01, oshima wrote:
> https://codereview.chromium.org/13902003/diff/3001/ui/aura/root_window.cc
> File ui/aura/root_window.cc (right):
> 
>
https://codereview.chromium.org/13902003/diff/3001/ui/aura/root_window.cc#new...
> ui/aura/root_window.cc:1181: ConvertPointToNativeScreen(&root_mouse_location);
> doh, you're right. removed.
> 
>
https://codereview.chromium.org/13902003/diff/3001/ui/views/widget/desktop_au...
> File ui/views/widget/desktop_aura/x11_window_event_filter.cc (right):
> 
>
https://codereview.chromium.org/13902003/diff/3001/ui/views/widget/desktop_au...
> ui/views/widget/desktop_aura/x11_window_event_filter.cc:116: const gfx::Point&
> screen_location) {
> On 2013/04/10 00:12:41, sadrul wrote:
> > This would ideally take the location in the window coord, and convert to
> screen
> > coord only when necessary. (this could be separate CL at some point in the
> > future)
> 
> system_location is obtained directly from XEvent, and there
> is no conversion applied, nor it is used as window coord.
> I can get the location after switch though. Is it what you had
> in your mind?

Nah. I thought there was some conversion happening. But since that isn't the
case, I don't think it's useful to change this now. Thanks!

SLGTM

[Ben Goodger (Google), 2013-04-10 16:38:15]

https://codereview.chromium.org/13902003/diff/11001/ui/aura/root_window.cc
File ui/aura/root_window.cc (right):

https://codereview.chromium.org/13902003/diff/11001/ui/aura/root_window.cc#ne...
ui/aura/root_window.cc:43: #if defined(OS_CHROMEOS)
can you achieve this without this ifdef. it's undesirable to have platform/wm
specific code in aura.

https://codereview.chromium.org/13902003/diff/11001/ui/aura/root_window.cc#ne...
ui/aura/root_window.cc:699: #if defined(OS_CHROMEOS)
this ifdef is also undesirable. can we move this somewhere else?

[oshima, 2013-04-10 17:03:04]

https://codereview.chromium.org/13902003/diff/11001/ui/aura/root_window.cc
File ui/aura/root_window.cc (right):

https://codereview.chromium.org/13902003/diff/11001/ui/aura/root_window.cc#ne...
ui/aura/root_window.cc:43: #if defined(OS_CHROMEOS)
On 2013/04/10 16:38:15, Ben Goodger (Google) wrote:
> can you achieve this without this ifdef. it's undesirable to have platform/wm
> specific code in aura.
see my comments below.

https://codereview.chromium.org/13902003/diff/11001/ui/aura/root_window.cc#ne...
ui/aura/root_window.cc:699: #if defined(OS_CHROMEOS)
On 2013/04/10 16:38:15, Ben Goodger (Google) wrote:
> this ifdef is also undesirable. can we move this somewhere else?

This will be removed pretty soon (https://codereview.chromium.org/13916004/).

I still need tests for this, but I'll land it that first so that remove this
file from this CL.

[Ben Goodger (Google), 2013-04-11 00:17:06]

ok, lgtm.


On Wed, Apr 10, 2013 at 10:03 AM, <oshima@chromium.org> wrote:

>
> https://codereview.chromium.**org/13902003/diff/11001/ui/**
>
aura/root_window.cc<https://codereview.chromium.org/13902003/diff/11001/ui/aura/root_window.cc>
> File ui/aura/root_window.cc (right):
>
> https://codereview.chromium.**org/13902003/diff/11001/ui/**
>
aura/root_window.cc#newcode43<https://codereview.chromium.org/13902003/diff/11001/ui/aura/root_window.cc#newcode43>
> ui/aura/root_window.cc:43: #if defined(OS_CHROMEOS)
> On 2013/04/10 16:38:15, Ben Goodger (Google) wrote:
>
>> can you achieve this without this ifdef. it's undesirable to have
>>
> platform/wm
>
>> specific code in aura.
>>
> see my comments below.
>
>
> https://codereview.chromium.**org/13902003/diff/11001/ui/**
>
aura/root_window.cc#newcode699<https://codereview.chromium.org/13902003/diff/11001/ui/aura/root_window.cc#newcode699>
> ui/aura/root_window.cc:699: #if defined(OS_CHROMEOS)
> On 2013/04/10 16:38:15, Ben Goodger (Google) wrote:
>
>> this ifdef is also undesirable. can we move this somewhere else?
>>
>
> This will be removed pretty soon
>
(https://codereview.chromium.**org/13916004/<https://codereview.chromium.org/1...
> ).
>
> I still need tests for this, but I'll land it that first so that remove
> this file from this CL.
>
>
https://codereview.chromium.**org/13902003/<https://codereview.chromium.org/1...
>

[oshima, 2013-04-11 13:07:52]

https://codereview.chromium.org/13916004/ landed and
ui/aura/root_window.cc has been removed from this CL.


On 2013/04/11 00:17:06, Ben Goodger (Google) wrote:
> ok, lgtm.
> 
> 
> On Wed, Apr 10, 2013 at 10:03 AM, <mailto:oshima@chromium.org> wrote:
> 
> >
> > https://codereview.chromium.**org/13902003/diff/11001/ui/**
> >
>
aura/root_window.cc<https://codereview.chromium.org/13902003/diff/11001/ui/aura/root_window.cc>
> > File ui/aura/root_window.cc (right):
> >
> > https://codereview.chromium.**org/13902003/diff/11001/ui/**
> >
>
aura/root_window.cc#newcode43<https://codereview.chromium.org/13902003/diff/11001/ui/aura/root_window.cc#newcode43>
> > ui/aura/root_window.cc:43: #if defined(OS_CHROMEOS)
> > On 2013/04/10 16:38:15, Ben Goodger (Google) wrote:
> >
> >> can you achieve this without this ifdef. it's undesirable to have
> >>
> > platform/wm
> >
> >> specific code in aura.
> >>
> > see my comments below.
> >
> >
> > https://codereview.chromium.**org/13902003/diff/11001/ui/**
> >
>
aura/root_window.cc#newcode699<https://codereview.chromium.org/13902003/diff/11001/ui/aura/root_window.cc#newcode699>
> > ui/aura/root_window.cc:699: #if defined(OS_CHROMEOS)
> > On 2013/04/10 16:38:15, Ben Goodger (Google) wrote:
> >
> >> this ifdef is also undesirable. can we move this somewhere else?
> >>
> >
> > This will be removed pretty soon
> >
>
(https://codereview.chromium.**org/13916004/%3Chttps://codereview.chromium.org...>
> > ).
> >
> > I still need tests for this, but I'll land it that first so that remove
> > this file from this CL.
> >
> >
>
https://codereview.chromium.**org/13902003/%3Chttps://codereview.chromium.org...>
> >

[commit-bot: I haz the power, 2013-04-11 13:08:44]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/oshima@chromium.org/13902003/22001

[commit-bot: I haz the power, 2013-04-11 13:25:09]

Sorry for I got bad news for ya.
Compile failed with a clobber build on win7_aura.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win7_aura&...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[oshima, 2013-04-11 14:03:10]

On 2013/04/11 13:07:52, oshima wrote:
> https://codereview.chromium.org/13916004/ landed and
> ui/aura/root_window.cc has been removed from this CL.

Oops, there is actually two lines deletion in ui/aura/root_window.cc.
#ifdef has been removed.

> 
> 
> On 2013/04/11 00:17:06, Ben Goodger (Google) wrote:
> > ok, lgtm.
> > 
> > 
> > On Wed, Apr 10, 2013 at 10:03 AM, <mailto:oshima@chromium.org> wrote:
> > 
> > >
> > > https://codereview.chromium.**org/13902003/diff/11001/ui/**
> > >
> >
>
aura/root_window.cc<https://codereview.chromium.org/13902003/diff/11001/ui/aura/root_window.cc>
> > > File ui/aura/root_window.cc (right):
> > >
> > > https://codereview.chromium.**org/13902003/diff/11001/ui/**
> > >
> >
>
aura/root_window.cc#newcode43<https://codereview.chromium.org/13902003/diff/11001/ui/aura/root_window.cc#newcode43>
> > > ui/aura/root_window.cc:43: #if defined(OS_CHROMEOS)
> > > On 2013/04/10 16:38:15, Ben Goodger (Google) wrote:
> > >
> > >> can you achieve this without this ifdef. it's undesirable to have
> > >>
> > > platform/wm
> > >
> > >> specific code in aura.
> > >>
> > > see my comments below.
> > >
> > >
> > > https://codereview.chromium.**org/13902003/diff/11001/ui/**
> > >
> >
>
aura/root_window.cc#newcode699<https://codereview.chromium.org/13902003/diff/11001/ui/aura/root_window.cc#newcode699>
> > > ui/aura/root_window.cc:699: #if defined(OS_CHROMEOS)
> > > On 2013/04/10 16:38:15, Ben Goodger (Google) wrote:
> > >
> > >> this ifdef is also undesirable. can we move this somewhere else?
> > >>
> > >
> > > This will be removed pretty soon
> > >
> >
>
(https://codereview.chromium.**org/13916004/%253Chttps://codereview.chromium.o...>
> > > ).
> > >
> > > I still need tests for this, but I'll land it that first so that remove
> > > this file from this CL.
> > >
> > >
> >
>
https://codereview.chromium.**org/13902003/%253Chttps://codereview.chromium.o...>
> > >

[commit-bot: I haz the power, 2013-04-11 14:03:31]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/oshima@chromium.org/13902003/26003

[commit-bot: I haz the power, 2013-04-11 16:17:48]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/oshima@chromium.org/13902003/26003

[commit-bot: I haz the power, 2013-04-11 16:48:13]

Change committed as 193668