Move all event related devices from ui/events/ to ui/events/devices/

Move all event related devices from ui/events/ to ui/events/devices/

BUG=none

Committed: https://crrev.com/78a7347037f09a028e53a0c68ddf01d62a43703f
Cr-Commit-Position: refs/heads/master@{#302647}

[dnicoara, 2014-10-30 14:20:33]

dnicoara@chromium.org changed reviewers:
+ derat@chromium.org, isherman@chromium.org, sadrul@chromium.org

[dnicoara, 2014-10-30 14:20:34]

PTAL

sadrul@ for ui/*
derat@ for ash/* and chrome/*
isherman@ for chrome/browser/metrics/*

[Daniel Erat, 2014-10-30 14:27:47]

lgtm for ash/ and chrome/

[sadrul, 2014-10-30 16:20:53]

https://codereview.chromium.org/685793002/diff/40001/ui/events/events.gyp
File ui/events/events.gyp (right):

https://codereview.chromium.org/685793002/diff/40001/ui/events/events.gyp#new...
ui/events/events.gyp:55: 'devices/touchscreen_device.h',
Should 'devices' be a separate component?

[Ilya Sherman, 2014-10-30 21:39:42]

//chrome/browser/metrics/* lgtm

[dnicoara, 2014-10-31 15:17:39]

https://codereview.chromium.org/685793002/diff/40001/ui/events/events.gyp
File ui/events/events.gyp (right):

https://codereview.chromium.org/685793002/diff/40001/ui/events/events.gyp#new...
ui/events/events.gyp:55: 'devices/touchscreen_device.h',
On 2014/10/30 16:20:52, sadrul wrote:
> Should 'devices' be a separate component?

Done.

[sadrul, 2014-11-03 17:09:59]

Some comments/nits. Otherwise, LGTM

https://codereview.chromium.org/685793002/diff/160001/ui/events/devices/BUILD.gn
File ui/events/devices/BUILD.gn (right):

https://codereview.chromium.org/685793002/diff/160001/ui/events/devices/BUILD...
ui/events/devices/BUILD.gn:31: "//skia",
I suppose you need to pull in skia because of gfx (and gfx::Transform)?

https://codereview.chromium.org/685793002/diff/160001/ui/events/devices/BUILD...
ui/events/devices/BUILD.gn:45: "x11/touch_factory_x11.h",
It may be necessary to pull this out into a separate devices_x11 target (like we
have gfx_x11), but this is good for now.

https://codereview.chromium.org/685793002/diff/160001/ui/events/events.gyp
File ui/events/events.gyp (right):

https://codereview.chromium.org/685793002/diff/160001/ui/events/events.gyp#ne...
ui/events/events.gyp:141: 'devices/events_devices.gyp:events_devices',
Why just for use_x11?

https://codereview.chromium.org/685793002/diff/160001/ui/events/events.gyp#ne...
ui/events/events.gyp:299: 'devices/events_devices.gyp:events_devices',
sort

https://codereview.chromium.org/685793002/diff/160001/ui/events/platform/x11/...
File ui/events/platform/x11/x11_hotplug_event_handler.cc (right):

https://codereview.chromium.org/685793002/diff/160001/ui/events/platform/x11/...
ui/events/platform/x11/x11_hotplug_event_handler.cc:5: #include
"ui/events/platform/x11/x11_hotplug_event_handler.h"
I assume you renamed from _x11 to x11_ to match the rest of the files in
platform/x11/? (it's unfortunate that we have some files x11_ and some _x11 at
the moment ... sorry about that!)

[dnicoara, 2014-11-03 18:13:17]

https://codereview.chromium.org/685793002/diff/160001/ui/events/devices/BUILD.gn
File ui/events/devices/BUILD.gn (right):

https://codereview.chromium.org/685793002/diff/160001/ui/events/devices/BUILD...
ui/events/devices/BUILD.gn:31: "//skia",
On 2014/11/03 17:09:59, sadrul wrote:
> I suppose you need to pull in skia because of gfx (and gfx::Transform)?

Unfortunately yes.

https://codereview.chromium.org/685793002/diff/160001/ui/events/devices/BUILD...
ui/events/devices/BUILD.gn:45: "x11/touch_factory_x11.h",
On 2014/11/03 17:09:59, sadrul wrote:
> It may be necessary to pull this out into a separate devices_x11 target (like
we
> have gfx_x11), but this is good for now.

The dependency chain is OK without the extra target, so I didn't want to add
more targets. Are you thinking that this is going to get more complicated and
we'll be forced to add another target? If so, it may be best to just make the
target now since most of the GYP changes are necessary due to dependencies on
the X11 files. What do you think?

https://codereview.chromium.org/685793002/diff/160001/ui/events/events.gyp
File ui/events/events.gyp (right):

https://codereview.chromium.org/685793002/diff/160001/ui/events/events.gyp#ne...
ui/events/events.gyp:141: 'devices/events_devices.gyp:events_devices',
On 2014/11/03 17:09:59, sadrul wrote:
> Why just for use_x11?

x/events_x.cc is the only file pulling in the events_devices dependency.

https://codereview.chromium.org/685793002/diff/160001/ui/events/events.gyp#ne...
ui/events/events.gyp:299: 'devices/events_devices.gyp:events_devices',
On 2014/11/03 17:09:59, sadrul wrote:
> sort

Done.

https://codereview.chromium.org/685793002/diff/160001/ui/events/platform/x11/...
File ui/events/platform/x11/x11_hotplug_event_handler.cc (right):

https://codereview.chromium.org/685793002/diff/160001/ui/events/platform/x11/...
ui/events/platform/x11/x11_hotplug_event_handler.cc:5: #include
"ui/events/platform/x11/x11_hotplug_event_handler.h"
On 2014/11/03 17:09:59, sadrul wrote:
> I assume you renamed from _x11 to x11_ to match the rest of the files in
> platform/x11/? (it's unfortunate that we have some files x11_ and some _x11 at
> the moment ... sorry about that!)

Yes, just wanted to follow the same naming convention for all the files. If you
prefer the other form let me know and I'll just update it.

[sadrul, 2014-11-03 18:19:20]

still lgtm

https://codereview.chromium.org/685793002/diff/160001/ui/events/devices/BUILD.gn
File ui/events/devices/BUILD.gn (right):

https://codereview.chromium.org/685793002/diff/160001/ui/events/devices/BUILD...
ui/events/devices/BUILD.gn:45: "x11/touch_factory_x11.h",
On 2014/11/03 18:13:17, dnicoara wrote:
> On 2014/11/03 17:09:59, sadrul wrote:
> > It may be necessary to pull this out into a separate devices_x11 target
(like
> we
> > have gfx_x11), but this is good for now.
> 
> The dependency chain is OK without the extra target, so I didn't want to add
> more targets. Are you thinking that this is going to get more complicated and
> we'll be forced to add another target? If so, it may be best to just make the
> target now since most of the GYP changes are necessary due to dependencies on
> the X11 files. What do you think?

I am unsure if we will definitely need the x11-specific target or not. Let's go
with what you have now, and we can have the new target if/when we need to. (or
talk to achaulk@ on what he thinks, since is working on that part)

https://codereview.chromium.org/685793002/diff/160001/ui/events/platform/x11/...
File ui/events/platform/x11/x11_hotplug_event_handler.cc (right):

https://codereview.chromium.org/685793002/diff/160001/ui/events/platform/x11/...
ui/events/platform/x11/x11_hotplug_event_handler.cc:5: #include
"ui/events/platform/x11/x11_hotplug_event_handler.h"
On 2014/11/03 18:13:17, dnicoara wrote:
> On 2014/11/03 17:09:59, sadrul wrote:
> > I assume you renamed from _x11 to x11_ to match the rest of the files in
> > platform/x11/? (it's unfortunate that we have some files x11_ and some _x11
at
> > the moment ... sorry about that!)
> 
> Yes, just wanted to follow the same naming convention for all the files. If
you
> prefer the other form let me know and I'll just update it.

What you have in this CL is fine. If you want to rename all the files in
devices/ and platform/, then I think we should go with <thing>_x11. But if you
do that, let's do that in a separate CL.

[dnicoara, 2014-11-03 20:23:32]

dnicoara@chromium.org changed reviewers:
+ msw@chromium.org, mukai@chromium.org, pfeldman@chromium.org

[dnicoara, 2014-11-03 20:23:33]

+ pfeldman@ for content/shell/*
+ mukai@ for athena/*
+ msw@ for chrome/browser/ui/*

[msw, 2014-11-03 21:05:10]

lgtm with nit and q.

https://codereview.chromium.org/685793002/diff/200001/ui/events/ozone/evdev/e...
File ui/events/ozone/evdev/event_factory_evdev.cc (right):

https://codereview.chromium.org/685793002/diff/200001/ui/events/ozone/evdev/e...
ui/events/ozone/evdev/event_factory_evdev.cc:16: #include
"ui/events/devices/device_util_linux.h"
Is this actually needed? I don't see a call to IsTouchscreenInternal.

https://codereview.chromium.org/685793002/diff/200001/ui/events/platform/x11/...
File ui/events/platform/x11/BUILD.gn (right):

https://codereview.chromium.org/685793002/diff/200001/ui/events/platform/x11/...
ui/events/platform/x11/BUILD.gn:23: "//ui/events/devices",
nit: reorder after "//ui/events:events_base" (like ash/BUILD.gn)

[dnicoara, 2014-11-03 21:13:01]

https://codereview.chromium.org/685793002/diff/200001/ui/events/ozone/evdev/e...
File ui/events/ozone/evdev/event_factory_evdev.cc (right):

https://codereview.chromium.org/685793002/diff/200001/ui/events/ozone/evdev/e...
ui/events/ozone/evdev/event_factory_evdev.cc:16: #include
"ui/events/devices/device_util_linux.h"
On 2014/11/03 21:05:10, msw wrote:
> Is this actually needed? I don't see a call to IsTouchscreenInternal.

Removed.

Thanks, missed it during the rebase.

https://codereview.chromium.org/685793002/diff/200001/ui/events/platform/x11/...
File ui/events/platform/x11/BUILD.gn (right):

https://codereview.chromium.org/685793002/diff/200001/ui/events/platform/x11/...
ui/events/platform/x11/BUILD.gn:23: "//ui/events/devices",
On 2014/11/03 21:05:10, msw wrote:
> nit: reorder after "//ui/events:events_base" (like ash/BUILD.gn)

Done.

[Elliot Glaysher, 2014-11-03 22:44:14]

erg@chromium.org changed reviewers:
+ erg@chromium.org

[Elliot Glaysher, 2014-11-03 22:44:15]

lgtm

[Jun Mukai, 2014-11-04 06:35:46]

athena lgtm

[dnicoara, 2014-11-04 14:51:48]

dnicoara@chromium.org changed reviewers:
+ davidben@chromium.org
- pfeldman@chromium.org

[dnicoara, 2014-11-04 14:51:49]

-pfeldman@
+davidben@

for content/*

[davidben, 2014-11-04 18:57:17]

content lgtm

[dnicoara, 2014-11-04 18:57:42]

The CQ bit was checked by dnicoara@chromium.org

[commit-bot: I haz the power, 2014-11-04 18:59:58]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/685793002/280001

[commit-bot: I haz the power, 2014-11-04 19:55:10]

Committed patchset #15 (id:280001)

[commit-bot: I haz the power, 2014-11-04 19:55:48]

Patchset 15 (id:??) landed as
https://crrev.com/78a7347037f09a028e53a0c68ddf01d62a43703f
Cr-Commit-Position: refs/heads/master@{#302647}