Build components.gyp:* and components_test.gyp:* on iOS.

Build components.gyp:* and components_test.gyp:* on iOS.

These targets are now built as part of the All build on iOS.

BUG=324837, 324839
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=238737

[blundell, 2013-12-03 14:00:22]

https://codereview.chromium.org/101503002/diff/60001/components/components.gyp
File components/components.gyp (right):

https://codereview.chromium.org/101503002/diff/60001/components/components.gy...
components/components.gyp:29: ['OS != "ios"', {
This seemed more straightforward to me than having each gypi file that is not
used by iOS individually guard itself in a !iOS condition. If you agree, I can
eliminate the redundancy with some existing gypi files that internally do the
check.

https://codereview.chromium.org/101503002/diff/60001/components/components_te...
File components/components_tests.gyp (right):

https://codereview.chromium.org/101503002/diff/60001/components/components_te...
components/components_tests.gyp:64:
'../content/content_shell_and_tests.gyp:test_support_content',
This dependency is needed because run_all_unittests.cc sets up a test content
client. I think the right approach is probably to have (at least) two sets of
components unittests: (1) core unittests with a run_core_unittests.cc file that
doesn't mention //content, and (2) content unittests with a
run_content_unittests.cc file that sets up the test content client. I'd prefer
to not do that separation in this CL, though. If you agree, I'll file a bug and
reference it here.

https://codereview.chromium.org/101503002/diff/60001/components/components_te...
components/components_tests.gyp:97: ['OS != "ios"', {
I changed to this style because it's in line with how we handle this in other
files. However, if you strongly prefer the style that was there previously I can
revert back to it.

[Jói, 2013-12-03 15:49:58]

LGTM with an indentation nit.

https://codereview.chromium.org/101503002/diff/60001/components/components.gyp
File components/components.gyp (right):

https://codereview.chromium.org/101503002/diff/60001/components/components.gy...
components/components.gyp:29: ['OS != "ios"', {
On 2013/12/03 14:00:23, blundell wrote:
> This seemed more straightforward to me than having each gypi file that is not
> used by iOS individually guard itself in a !iOS condition. If you agree, I can
> eliminate the redundancy with some existing gypi files that internally do the
> check.

Sounds good.

https://codereview.chromium.org/101503002/diff/60001/components/components_te...
File components/components_tests.gyp (right):

https://codereview.chromium.org/101503002/diff/60001/components/components_te...
components/components_tests.gyp:64:
'../content/content_shell_and_tests.gyp:test_support_content',
On 2013/12/03 14:00:23, blundell wrote:
> This dependency is needed because run_all_unittests.cc sets up a test content
> client. I think the right approach is probably to have (at least) two sets of
> components unittests: (1) core unittests with a run_core_unittests.cc file
that
> doesn't mention //content, and (2) content unittests with a
> run_content_unittests.cc file that sets up the test content client. I'd prefer
> to not do that separation in this CL, though. If you agree, I'll file a bug
and
> reference it here.

That seems right to me. Fine to do in a separate CL.

https://codereview.chromium.org/101503002/diff/60001/components/components_te...
components/components_tests.gyp:97: ['OS != "ios"', {
On 2013/12/03 14:00:23, blundell wrote:
> I changed to this style because it's in line with how we handle this in other
> files. However, if you strongly prefer the style that was there previously I
can
> revert back to it.

This looks fine.

https://codereview.chromium.org/101503002/diff/60001/components/dom_distiller...
File components/dom_distiller.gypi (right):

https://codereview.chromium.org/101503002/diff/60001/components/dom_distiller...
components/dom_distiller.gypi:114:
'dom_distiller/content/distiller_page_web_contents.cc',
indentation, this and next two lines

[blundell, 2013-12-03 15:54:47]

Stuart, can you take a quick look over where I've placed the components in terms
of being built or not on iOS? You can just look at components.gyp.

Note that we'll have to reverse some of these dependencies temporarily
downstream.

[blundell, 2013-12-03 16:42:00]

Thanks!

https://codereview.chromium.org/101503002/diff/60001/components/components.gyp
File components/components.gyp (right):

https://codereview.chromium.org/101503002/diff/60001/components/components.gy...
components/components.gyp:29: ['OS != "ios"', {
Filed crbug.com/325241 against myself to do the work on the individual gypi
files; it would introduce a lot of noise into this CL, so I'd rather keep it
separate.

On 2013/12/03 15:49:59, Jói wrote:
> On 2013/12/03 14:00:23, blundell wrote:
> > This seemed more straightforward to me than having each gypi file that is
not
> > used by iOS individually guard itself in a !iOS condition. If you agree, I
can
> > eliminate the redundancy with some existing gypi files that internally do
the
> > check.
> 
> Sounds good.

https://codereview.chromium.org/101503002/diff/60001/components/components_te...
File components/components_tests.gyp (right):

https://codereview.chromium.org/101503002/diff/60001/components/components_te...
components/components_tests.gyp:64:
'../content/content_shell_and_tests.gyp:test_support_content',
Filed crbug.com/325243 against myself.

On 2013/12/03 15:49:59, Jói wrote:
> On 2013/12/03 14:00:23, blundell wrote:
> > This dependency is needed because run_all_unittests.cc sets up a test
content
> > client. I think the right approach is probably to have (at least) two sets
of
> > components unittests: (1) core unittests with a run_core_unittests.cc file
> that
> > doesn't mention //content, and (2) content unittests with a
> > run_content_unittests.cc file that sets up the test content client. I'd
prefer
> > to not do that separation in this CL, though. If you agree, I'll file a bug
> and
> > reference it here.
> 
> That seems right to me. Fine to do in a separate CL.

https://codereview.chromium.org/101503002/diff/60001/components/dom_distiller...
File components/dom_distiller.gypi (right):

https://codereview.chromium.org/101503002/diff/60001/components/dom_distiller...
components/dom_distiller.gypi:114:
'dom_distiller/content/distiller_page_web_contents.cc',
On 2013/12/03 15:49:59, Jói wrote:
> indentation, this and next two lines

Done.

[stuartmorgan, 2013-12-04 15:07:30]

lgtm

https://codereview.chromium.org/101503002/diff/30007/build/all.gyp
File build/all.gyp (right):

https://codereview.chromium.org/101503002/diff/30007/build/all.gyp#newcode248
build/all.gyp:248: '../components/components_tests.gyp:components_unittests',
This should move too; I don't think we currently use this target, but it ought
to be right. Anything else obvious we've missed could go up too (sync, I believe
jingle, maybe others)

https://codereview.chromium.org/101503002/diff/30007/components/components_te...
File components/components_tests.gyp (right):

https://codereview.chromium.org/101503002/diff/30007/components/components_te...
components/components_tests.gyp:136: ['include', '^translate'],
Since these are directories, they should really end in / (to avoid accidental
prefix matches)

https://codereview.chromium.org/101503002/diff/30007/components/dom_distiller...
File components/dom_distiller.gypi (right):

https://codereview.chromium.org/101503002/diff/30007/components/dom_distiller...
components/dom_distiller.gypi:116:
'dom_distiller/content/dom_distiller_service_factory.cc',
While you are moving these, can you swap cc and h for both pairs? These lists
should be alhpa-sorted.

[blundell, 2013-12-04 15:58:19]

Thanks!

https://codereview.chromium.org/101503002/diff/30007/build/all.gyp
File build/all.gyp (right):

https://codereview.chromium.org/101503002/diff/30007/build/all.gyp#newcode248
build/all.gyp:248: '../components/components_tests.gyp:components_unittests',
Done. Just sync it looks like.

On 2013/12/04 15:07:31, stuartmorgan wrote:
> This should move too; I don't think we currently use this target, but it ought
> to be right. Anything else obvious we've missed could go up too (sync, I
believe
> jingle, maybe others)

https://codereview.chromium.org/101503002/diff/30007/components/components_te...
File components/components_tests.gyp (right):

https://codereview.chromium.org/101503002/diff/30007/components/components_te...
components/components_tests.gyp:136: ['include', '^translate'],
On 2013/12/04 15:07:31, stuartmorgan wrote:
> Since these are directories, they should really end in / (to avoid accidental
> prefix matches)

Done.

https://codereview.chromium.org/101503002/diff/30007/components/dom_distiller...
File components/dom_distiller.gypi (right):

https://codereview.chromium.org/101503002/diff/30007/components/dom_distiller...
components/dom_distiller.gypi:116:
'dom_distiller/content/dom_distiller_service_factory.cc',
On 2013/12/04 15:07:31, stuartmorgan wrote:
> While you are moving these, can you swap cc and h for both pairs? These lists
> should be alhpa-sorted.

Done.

[commit-bot: I haz the power, 2013-12-04 15:59:49]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/blundell@chromium.org/101503002/90001

[commit-bot: I haz the power, 2013-12-04 19:50:39]

Change committed as 238737