Move mojo/services/public/cpp TypeConverters to mojo/converters.

Move mojo/services/public/cpp TypeConverters to mojo/converters.

Move geometry, input_event and surfaces TypeConverters.
Move conversion-related helpers and tests as well.

Flatten lib sub-directories into their parents.
Update includes, guards, DEPS, GYP, and GN.
Add mojo_converters.gypi to avoid a false-positive cycle.

BUG=421234
TEST=No GYP, GN, compile, or link problems.
R=jamesr@chromium.org
TBR=reed@google.com,sadrul@chromium.org,piman@chromium.org

Committed: https://crrev.com/500dcd12ad141f66af11b7a1b371642bcb5fee98
Cr-Commit-Position: refs/heads/master@{#299055}

[msw, 2014-10-09 19:23:09]

Hey James, can you take a first look and answer some Qs?

https://codereview.chromium.org/640893003/diff/20001/mojo/converters/BUILD.gn
File mojo/converters/BUILD.gn (right):

https://codereview.chromium.org/640893003/diff/20001/mojo/converters/BUILD.gn...
mojo/converters/BUILD.gn:6: component("converters") {
Should we have consistent naming between GYP's "mojo_converters[_unittests]" and
GN's "converters[_unittests]" targets? It seems like some other places follow
the inconsistent pattern I use here, like:
https://code.google.com/p/chromium/codesearch#chromium/src/mojo/common/BUILD....

Also, what's the "_lib" post-fix convention? Should that be used here?

https://codereview.chromium.org/640893003/diff/20001/mojo/converters/BUILD.gn...
mojo/converters/BUILD.gn:47: test("converters_unittests") {
Should I split the test target into it's own sub-directory and BUILD.gn?

https://codereview.chromium.org/640893003/diff/20001/mojo/converters/surfaces...
File mojo/converters/surfaces_utils.h (right):

https://codereview.chromium.org/640893003/diff/20001/mojo/converters/surfaces...
mojo/converters/surfaces_utils.h:5: #ifndef MOJO_CONVERTERS_SURFACES_UTILS_H_
Should I rename support files or merge them into their related *_type_converters
files?

https://codereview.chromium.org/640893003/diff/20001/mojo/services/window_man...
File mojo/services/window_manager/DEPS (right):

https://codereview.chromium.org/640893003/diff/20001/mojo/services/window_man...
mojo/services/window_manager/DEPS:5:
"+mojo/converters/input_events_type_converters.h",
Should I prefer to add narrow DEPS include rules like this, or should I just do
"+mojo/converters" everywhere that only one of the files is needed?

[jamesr, 2014-10-09 19:30:54]

You should leave the converters in separate components and subdirs, i.e.

//mojo/converters/geometry/
//mojo/converters/input_events/

etc

https://codereview.chromium.org/640893003/diff/20001/mojo/converters/BUILD.gn
File mojo/converters/BUILD.gn (right):

https://codereview.chromium.org/640893003/diff/20001/mojo/converters/BUILD.gn...
mojo/converters/BUILD.gn:6: component("converters") {
On 2014/10/09 19:23:09, msw wrote:
> Should we have consistent naming between GYP's "mojo_converters[_unittests]"
and
> GN's "converters[_unittests]" targets? It seems like some other places follow
> the inconsistent pattern I use here, like:
>
https://code.google.com/p/chromium/codesearch#chromium/src/mojo/common/BUILD....

The unit test executable names all have mojo_ prefixes and the mojo test()
target's name should match the executable name.  Targets that are not actual
binary outputs (i.e. intermediate components and source_sets) should have names
that are natural which typically means omitting the mojo_ prefix.

In GYP, all target names have to be namespaced by convention because the targets
exist in a global namespace (there are generators that blow up if you repeat
things).  In GN, the path is part of the target label so you don't have to
repeat yourself.  I.e. in GYP you have to say 'mojo_common' to mean the things
in src/mojo/common, but in GN you can just say 'common' in
src/mojo/common/BUILD.gn since the full label is //mojo/common:common (or you
can just say //mojo/common since the default label is the directory name).

> 
> Also, what's the "_lib" post-fix convention? Should that be used here?

I'm not sure where it's used. It typically doesn't provide value.

https://codereview.chromium.org/640893003/diff/20001/mojo/converters/BUILD.gn...
mojo/converters/BUILD.gn:27: "MOJO_CONVERTERS_IMPLEMENTATION",
why put all converters in one component? seems like it'd be better to leave them
as independent components so folks can link in just what they want.

[jamesr, 2014-10-09 19:34:57]

https://codereview.chromium.org/640893003/diff/20001/mojo/converters/surfaces...
File mojo/converters/surfaces_utils.h (right):

https://codereview.chromium.org/640893003/diff/20001/mojo/converters/surfaces...
mojo/converters/surfaces_utils.h:5: #ifndef MOJO_CONVERTERS_SURFACES_UTILS_H_
On 2014/10/09 19:23:09, msw wrote:
> Should I rename support files or merge them into their related
*_type_converters
> files?

Keep them separate

https://codereview.chromium.org/640893003/diff/20001/mojo/services/window_man...
File mojo/services/window_manager/DEPS (right):

https://codereview.chromium.org/640893003/diff/20001/mojo/services/window_man...
mojo/services/window_manager/DEPS:5:
"+mojo/converters/input_events_type_converters.h",
On 2014/10/09 19:23:09, msw wrote:
> Should I prefer to add narrow DEPS include rules like this, or should I just
do
> "+mojo/converters" everywhere that only one of the files is needed?

I think it should be "+mojo/converters/input_events/"

[msw, 2014-10-10 00:12:01]

Patchset #4 (id:270001) has been deleted

[msw, 2014-10-10 00:12:11]

Patchset #4 (id:500001) has been deleted

[msw, 2014-10-10 00:28:59]

Hey James, please take another look; thanks!

https://codereview.chromium.org/640893003/diff/20001/mojo/converters/BUILD.gn
File mojo/converters/BUILD.gn (right):

https://codereview.chromium.org/640893003/diff/20001/mojo/converters/BUILD.gn...
mojo/converters/BUILD.gn:6: component("converters") {
On 2014/10/09 19:30:54, jamesr wrote:
> On 2014/10/09 19:23:09, msw wrote:
> > Should we have consistent naming between GYP's "mojo_converters[_unittests]"
> and
> > GN's "converters[_unittests]" targets? It seems like some other places
follow
> > the inconsistent pattern I use here, like:
> >
>
https://code.google.com/p/chromium/codesearch#chromium/src/mojo/common/BUILD....
> 
> The unit test executable names all have mojo_ prefixes and the mojo test()
> target's name should match the executable name.  Targets that are not actual
> binary outputs (i.e. intermediate components and source_sets) should have
names
> that are natural which typically means omitting the mojo_ prefix.
> 
> In GYP, all target names have to be namespaced by convention because the
targets
> exist in a global namespace (there are generators that blow up if you repeat
> things).  In GN, the path is part of the target label so you don't have to
> repeat yourself.  I.e. in GYP you have to say 'mojo_common' to mean the things
> in src/mojo/common, but in GN you can just say 'common' in
> src/mojo/common/BUILD.gn since the full label is //mojo/common:common (or you
> can just say //mojo/common since the default label is the directory name).
> 
> > 
> > Also, what's the "_lib" post-fix convention? Should that be used here?
> 
> I'm not sure where it's used. It typically doesn't provide value.

Thanks for the naming explanations, that makes sense! I reverted the gyp targets
to retain their _lib postfixes for now, but I can remove them [in a separate CL]
if you'd like.

https://codereview.chromium.org/640893003/diff/20001/mojo/converters/BUILD.gn...
mojo/converters/BUILD.gn:27: "MOJO_CONVERTERS_IMPLEMENTATION",
On 2014/10/09 19:30:54, jamesr wrote:
> why put all converters in one component? seems like it'd be better to leave
them
> as independent components so folks can link in just what they want.

Done, I misunderstood this as your intention, but should have asked.

https://codereview.chromium.org/640893003/diff/20001/mojo/converters/surfaces...
File mojo/converters/surfaces_utils.h (right):

https://codereview.chromium.org/640893003/diff/20001/mojo/converters/surfaces...
mojo/converters/surfaces_utils.h:5: #ifndef MOJO_CONVERTERS_SURFACES_UTILS_H_
On 2014/10/09 19:34:57, jamesr wrote:
> On 2014/10/09 19:23:09, msw wrote:
> > Should I rename support files or merge them into their related
> *_type_converters
> > files?
> 
> Keep them separate

Acknowledged.

https://codereview.chromium.org/640893003/diff/20001/mojo/services/window_man...
File mojo/services/window_manager/DEPS (right):

https://codereview.chromium.org/640893003/diff/20001/mojo/services/window_man...
mojo/services/window_manager/DEPS:5:
"+mojo/converters/input_events_type_converters.h",
On 2014/10/09 19:34:57, jamesr wrote:
> On 2014/10/09 19:23:09, msw wrote:
> > Should I prefer to add narrow DEPS include rules like this, or should I just
> do
> > "+mojo/converters" everywhere that only one of the files is needed?
> 
> I think it should be "+mojo/converters/input_events/"

Done.

[jamesr, 2014-10-10 01:06:13]

lgtm

excellent!  my worry about having all the converters in one component is that if
the implementation of one of these services wants to start depending on mojo and
using the converters for a lower-level library, having all the converters in one
component will create cycles.

[msw, 2014-10-10 02:11:15]

The CQ bit was checked by msw@chromium.org

[commit-bot: I haz the power, 2014-10-10 02:12:15]

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

[commit-bot: I haz the power, 2014-10-10 02:25:12]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-10-10 02:25:13]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[msw, 2014-10-10 02:46:36]

msw@chromium.org changed reviewers:
+ darin@chromium.org

[msw, 2014-10-10 03:51:34]

msw@chromium.org changed reviewers:
+ piman@chromium.org, reed@google.com, sadrul@chromium.org
- darin@chromium.org

[msw, 2014-10-10 04:04:20]

TBR'ing owners of DEPS referenced in this code move.

[msw, 2014-10-10 04:04:29]

The CQ bit was checked by msw@chromium.org

[commit-bot: I haz the power, 2014-10-10 04:04:40]

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

[commit-bot: I haz the power, 2014-10-10 04:06:08]

Committed patchset #4 (id:520001)

[commit-bot: I haz the power, 2014-10-10 04:06:51]

Patchset 4 (id:??) landed as
https://crrev.com/500dcd12ad141f66af11b7a1b371642bcb5fee98
Cr-Commit-Position: refs/heads/master@{#299055}