Factor out content shell and tests into a standalone gyp

Factor out content shell and tests into a standalone gyp

This simplifies using components in Content Shell.  As components can
depend on the content layer, trying to use a component from Content Shell
results in a cross-reference of gyp files which isn't allowed on Mac.

TBR=joi,robertshield (trivial one-line renames in gyp files)

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

[mnaganov (inactive), 2013-06-20 12:21:21]

Hello Jochen,

What do you think about such a refactoring? My understanding is that the main
reusable part of 'content/' is the library, while the shell is mostly like a
'sample' of using it, and a test environment. So the components obviously
shouldn't depend on the shell, while it's OK for them to depend on the content
library. But the current arrangement automatically includes content shell
targets into the whole content framework.

As a result of this, I've got a cyclic dependency problem (for some reason, only
on Mac) while trying to use a component in Content Shell:
https://codereview.chromium.org/17389005/

[jochen (gone - plz use gerrit), 2013-06-20 12:25:39]

lgtm

[mnaganov (inactive), 2013-06-20 12:30:54]

Thanks, Jochen!

Requesting OWNERS review:

jam@: content/
sky@: ash/, ui/

[jam, 2013-06-20 15:41:29]

lgtm

https://codereview.chromium.org/17366006/diff/1/content/content_shell.gyp
File content/content_shell.gyp (right):

https://codereview.chromium.org/17366006/diff/1/content/content_shell.gyp#new...
content/content_shell.gyp:1: # Copyright (c) 2012 The Chromium Authors. All
rights reserved.
please update the OWNERS file in this directory since it has a per-file entry
for jochen

[sky, 2013-06-20 18:04:30]

LGTM

[mnaganov (inactive), 2013-06-21 12:37:52]

jochen@: As it seems, according to Mac build file generation restrictions, no
two .gyp files can mutually reference each other. So I have to extract both the
shell and the tests into a separate gyp, and now it works on Mac. Please
reconsider the patch, if you are still OK with it.

https://codereview.chromium.org/17366006/diff/1/content/content_shell.gyp
File content/content_shell.gyp (right):

https://codereview.chromium.org/17366006/diff/1/content/content_shell.gyp#new...
content/content_shell.gyp:1: # Copyright (c) 2012 The Chromium Authors. All
rights reserved.
On 2013/06/20 15:41:29, jam wrote:
> please update the OWNERS file in this directory since it has a per-file entry
> for jochen

Thanks for pointing this out!

[jam, 2013-06-21 16:20:21]

https://codereview.chromium.org/17366006/diff/13002/content/content_shell_and...
File content/content_shell_and_tests.gyp (right):

https://codereview.chromium.org/17366006/diff/13002/content/content_shell_and...
content/content_shell_and_tests.gyp:1: # Copyright (c) 2013 The Chromium
Authors. All rights reserved.
what is the point of making a gyp that contains both of these?

[mnaganov (inactive), 2013-06-21 16:26:01]

On 2013/06/21 16:20:21, jam wrote:
>
https://codereview.chromium.org/17366006/diff/13002/content/content_shell_and...
> File content/content_shell_and_tests.gyp (right):
> 
>
https://codereview.chromium.org/17366006/diff/13002/content/content_shell_and...
> content/content_shell_and_tests.gyp:1: # Copyright (c) 2013 The Chromium
> Authors. All rights reserved.
> what is the point of making a gyp that contains both of these?

As I have explained in the initial mail--to avoid gyp files mutual dependency
when trying to use components in Content Shell. Although, my initial need for
doing this has ceased to exist, I'm still considering this to be a right move,
as Content Shell and unit tests should be considered as embedders of the content
layer, not as an integral part of it. Thus, it should be both OK from the
viewpoint of gyp file dependencies to use the content layer from components and
use components in Content Shell.

[jam, 2013-06-21 20:52:35]

On 2013/06/21 16:26:01, Mikhail Naganov (Chromium) wrote:
> On 2013/06/21 16:20:21, jam wrote:
> >
>
https://codereview.chromium.org/17366006/diff/13002/content/content_shell_and...
> > File content/content_shell_and_tests.gyp (right):
> > 
> >
>
https://codereview.chromium.org/17366006/diff/13002/content/content_shell_and...
> > content/content_shell_and_tests.gyp:1: # Copyright (c) 2013 The Chromium
> > Authors. All rights reserved.
> > what is the point of making a gyp that contains both of these?
> 
> As I have explained in the initial mail--to avoid gyp files mutual dependency
> when trying to use components in Content Shell. Although, my initial need for
> doing this has ceased to exist, I'm still considering this to be a right move,
> as Content Shell and unit tests should be considered as embedders of the
content
> layer, not as an integral part of it. Thus, it should be both OK from the
> viewpoint of gyp file dependencies to use the content layer from components
and
> use components in Content Shell.

your initial message was about creating a new gyp for content_shell. i'm asking
a different question, which is why is that new gyp now including tests?

if we don't have a need to use components from content_shell, it's not clear to
me why we need to make this change at all. content_shell is meant to be a simple
example of how to embed content, so I don't see why it should need components

[tfarina, 2013-06-23 00:51:11]

https://codereview.chromium.org/17366006/diff/13002/content/content_tests.gypi
File content/content_tests.gypi (right):

https://codereview.chromium.org/17366006/diff/13002/content/content_tests.gyp...
content/content_tests.gypi:1: # Copyright (c) 2013 The Chromium Authors. All
rights reserved.
better to make this a content_tests.gyp. I also faced the same circular problem
in ui/

[tfarina, 2013-06-23 00:51:37]

https://codereview.chromium.org/17366006/diff/13002/content/content_shell_and...
File content/content_shell_and_tests.gyp (right):

https://codereview.chromium.org/17366006/diff/13002/content/content_shell_and...
content/content_shell_and_tests.gyp:1: # Copyright (c) 2013 The Chromium
Authors. All rights reserved.
I also think you don't need this if you make content_tests.gyp

[mnaganov (inactive), 2013-06-24 15:47:15]

On 2013/06/21 20:52:35, jam wrote:
> On 2013/06/21 16:26:01, Mikhail Naganov (Chromium) wrote:
> > On 2013/06/21 16:20:21, jam wrote:
> > >
> >
>
https://codereview.chromium.org/17366006/diff/13002/content/content_shell_and...
> > > File content/content_shell_and_tests.gyp (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/17366006/diff/13002/content/content_shell_and...
> > > content/content_shell_and_tests.gyp:1: # Copyright (c) 2013 The Chromium
> > > Authors. All rights reserved.
> > > what is the point of making a gyp that contains both of these?
> > 
> > As I have explained in the initial mail--to avoid gyp files mutual
dependency
> > when trying to use components in Content Shell. Although, my initial need
for
> > doing this has ceased to exist, I'm still considering this to be a right
move,
> > as Content Shell and unit tests should be considered as embedders of the
> content
> > layer, not as an integral part of it. Thus, it should be both OK from the
> > viewpoint of gyp file dependencies to use the content layer from components
> and
> > use components in Content Shell.
> 
> your initial message was about creating a new gyp for content_shell. i'm
asking
> a different question, which is why is that new gyp now including tests?
>

That's because targets from the proposed content_shell.gyp are referencing
targets from content_tests.gypi (thus, from content.gyp) and vice versa. What I
have figured out is that build files generation on Mac doesn't allow two gyp
files to mutually reference each other (I suppose, this is due some limitations
in Xcode). So we can only extract contents shell and tests targets in a separate
gyp file together. 
 
> if we don't have a need to use components from content_shell, it's not clear
to
> me why we need to make this change at all. content_shell is meant to be a
simple
> example of how to embed content, so I don't see why it should need components

This restriction on Macs is pretty much discouraging. I've spent some time
figuring out what's wrong. As I think a lot of things can depend on content,
it's pretty easy to run into this Mac restriction while trying to add any
reference into Content shell.

Although, I don't anymore feeling a strong need for this change, so I'd let it
up to jochen@ to decide, if it's stull useful.

[mnaganov (inactive), 2013-07-01 08:56:34]

jochen@ sorry for annoyance -- do you think that having the shell and the tests
in a separate gyp to avoid bumping into the gyp cross-reference issue on Mac
will help anyone?

[jochen (gone - plz use gerrit), 2013-07-01 09:02:54]

On 2013/07/01 08:56:34, Mikhail Naganov (Chromium) wrote:
> jochen@ sorry for annoyance -- do you think that having the shell and the
tests
> in a separate gyp to avoid bumping into the gyp cross-reference issue on Mac
> will help anyone?

I'm currently working on adding a breakpad component which I want to use from
content_shell, so I guess this use case should somehow work.

While content shell is a simple (haha) example of how to use the component api,
it's also part of the test infrastructure, so having a gyp file that includes
tests and content shell would make sense IMO.

[mnaganov (inactive), 2013-07-01 09:17:50]

https://codereview.chromium.org/17366006/diff/13002/content/content_shell_and...
File content/content_shell_and_tests.gyp (right):

https://codereview.chromium.org/17366006/diff/13002/content/content_shell_and...
content/content_shell_and_tests.gyp:1: # Copyright (c) 2013 The Chromium
Authors. All rights reserved.
On 2013/06/23 00:51:38, tfarina wrote:
> I also think you don't need this if you make content_tests.gyp

If content_shell and content_tests will be separate gyps (not gypis), they will
end up cross-referencing each other, which isn't allowed on Mac.

https://codereview.chromium.org/17366006/diff/13002/content/content_tests.gypi
File content/content_tests.gypi (right):

https://codereview.chromium.org/17366006/diff/13002/content/content_tests.gyp...
content/content_tests.gypi:1: # Copyright (c) 2013 The Chromium Authors. All
rights reserved.
On 2013/06/23 00:51:11, tfarina wrote:
> better to make this a content_tests.gyp. I also faced the same circular
problem
> in ui/

See my previous reply.

[commit-bot: I haz the power, 2013-07-01 09:23:35]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mnaganov@chromium.org/17366006/28001

[commit-bot: I haz the power, 2013-07-01 09:51:07]

Retried try job too often on chromium_presubmit for step(s) presubmit
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=chromium_p...

[commit-bot: I haz the power, 2013-07-01 09:56:33]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mnaganov@chromium.org/17366006/28001

[commit-bot: I haz the power, 2013-07-01 13:38:48]

Change committed as 209420

[jam, 2013-07-02 20:44:20]

this has no l-g-t-m from an owner. I don't know why you landed this without
approval. i'm reverting until we get to agreement.

[jam, 2013-07-02 20:45:53]

On 2013/07/02 20:44:20, jam wrote:
> this has no l-g-t-m from an owner. I don't know why you landed this without
> approval. i'm reverting until we get to agreement.

to be clear: my previous lgtm was for patchset 1, which is quite different from
what was landed

[mnaganov (inactive), 2013-07-03 09:22:31]

On 2013/07/02 20:45:53, jam wrote:
> On 2013/07/02 20:44:20, jam wrote:
> > this has no l-g-t-m from an owner. I don't know why you landed this without
> > approval. i'm reverting until we get to agreement.
> 
> to be clear: my previous lgtm was for patchset 1, which is quite different
from
> what was landed

OK. Sorry for the misunderstanding.

I have answered your questions in the comment #12. Then we have discussed the
final patch with Jochen, and he has supported the change. I apologize for not
contacting you afterwards for getting another approval. Do you have any concerns
about the final patch?

[jam, 2013-07-03 16:54:26]

On 2013/07/03 09:22:31, Mikhail Naganov (Chromium) wrote:
> On 2013/07/02 20:45:53, jam wrote:
> > On 2013/07/02 20:44:20, jam wrote:
> > > this has no l-g-t-m from an owner. I don't know why you landed this
without
> > > approval. i'm reverting until we get to agreement.
> > 
> > to be clear: my previous lgtm was for patchset 1, which is quite different
> from
> > what was landed
> 
> OK. Sorry for the misunderstanding.
> 
> I have answered your questions in the comment #12. Then we have discussed the
> final patch with Jochen, and he has supported the change. I apologize for not
> contacting you afterwards for getting another approval. Do you have any
concerns
> about the final patch?

I don't see the need to do this at this point. The gyp files are already
complicated enough, and so let's only split them up and add some more
indirection only when we know that we need it. My other concern was that
grouping tests and content shell alone in one file together didn't seem to make
sense.