Remove dependency on --gc-sections for most Android debug targets.

Remove dependency on --gc-sections for most Android debug targets.

Android targets shouldn't rely (at least on Debug) on the linker GC.
This CL refactors gyp files to fix target dependencies
(some of them wrong for components builds due to the .so
split).
The only targets left, at this point, are chrome_shell
and content_browsertests (will take more work).

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

[Primiano Tucci (use gerrit), 2014-04-11 18:18:57]

John, could you PTAL.
This essentially should resolve all the nasty linking errors which bubble up on
Android if you try to build without --gc-sections.

Most of the issues being fixed here shared the same root:
Some unrechable pieces of code funcionally depends on functions defined in other
.cc files.
Those dependent .cc files are part of other targets of type=<(component), for
which there are no paths in the gyp graph for a component build (remember that
transitive dependencies are not propagated if a target is type=shared_library).

In some other cases, there were just file being compiled when not necessary (due
to some gyp vars false on Android, like speech) or files just in the wrong
place.

Thanks

[jam, 2014-04-11 18:25:40]

On 2014/04/11 18:18:57, Primiano Tucci wrote:
> John, could you PTAL.
> This essentially should resolve all the nasty linking errors which bubble up
on
> Android if you try to build without --gc-sections.
> 
> Most of the issues being fixed here shared the same root:
> Some unrechable pieces of code funcionally depends on functions defined in
other
> .cc files.
> Those dependent .cc files are part of other targets of type=<(component), for
> which there are no paths in the gyp graph for a component build (remember that
> transitive dependencies are not propagated if a target is
type=shared_library).
> 
> In some other cases, there were just file being compiled when not necessary
(due
> to some gyp vars false on Android, like speech) or files just in the wrong
> place.
> 
> Thanks

I'm not a good reviewer for this. suggest thakis

[Primiano Tucci (use gerrit), 2014-04-11 18:28:55]

Nico, would you mind taking a look to this? 
thanks

[Nico, 2014-04-11 20:36:58]

Exciting! lgtm with changing the CL description to call out that this is only
for Debug.

https://codereview.chromium.org/228003006/diff/110001/chrome/chrome_android.gypi
File chrome/chrome_android.gypi (right):

https://codereview.chromium.org/228003006/diff/110001/chrome/chrome_android.g...
chrome/chrome_android.gypi:51: '-Wl,--gc-sections',
The idea is to remove this too eventually, right (if I get the CL description
right)? Say "TODO: remove" in front of the bug link

https://codereview.chromium.org/228003006/diff/110001/chrome/chrome_tests_uni...
File chrome/chrome_tests_unit.gypi (right):

https://codereview.chromium.org/228003006/diff/110001/chrome/chrome_tests_uni...
chrome/chrome_tests_unit.gypi:2451: # Force --gc-sections also for Debug builds
(see crbug.com/159847).
todo here too

https://codereview.chromium.org/228003006/diff/110001/content/content_tests.gypi
File content/content_tests.gypi (right):

https://codereview.chromium.org/228003006/diff/110001/content/content_tests.g...
content/content_tests.gypi:208:
'../webkit/browser/quota/mock_special_storage_policy.h',
This looks really icky. It looks like two other targets do this already, but
these files should really be in some test support target that the 2 (now 3)
targets should depend on.

[Primiano Tucci (use gerrit), 2014-04-12 10:26:00]

https://codereview.chromium.org/228003006/diff/110001/chrome/chrome_android.gypi
File chrome/chrome_android.gypi (right):

https://codereview.chromium.org/228003006/diff/110001/chrome/chrome_android.g...
chrome/chrome_android.gypi:51: '-Wl,--gc-sections',
On 2014/04/11 20:36:59, Nico wrote:
> The idea is to remove this too eventually, right (if I get the CL description
> right)? 
Right. However the chrome layer on Android is damn more messy: there are several
hundreds linking errors (when disabling gc-sections) coming mostly from
extensions support code. That might take much more than a couple of days of
headaches, as it was in this case.
My priority at the moment is cleaning up at least the content layer at the other
targets, remove the global --gc-sections blanket and avoid future regressions,
at least on all-but-two targets.

FTR all this started as a consequence of the arm64 bringup. The linker in the
arm64 toolchain  still doesn't support --gc-sections.
In general the idea that we need the linker GC to build makes me extremely
grumpy. I find linker GC a great way to reduce binary size, but using it as a
precondition for building....brrrr)

https://codereview.chromium.org/228003006/diff/110001/content/content_tests.gypi
File content/content_tests.gypi (right):

https://codereview.chromium.org/228003006/diff/110001/content/content_tests.g...
content/content_tests.gypi:208:
'../webkit/browser/quota/mock_special_storage_policy.h',
On 2014/04/11 20:36:59, Nico wrote:
> This looks really icky. It looks like two other targets do this already, but
> these files should really be in some test support target that the 2 (now 3)
> targets should depend on.

Hmm, I am not completely sure I got the sense of "do this already". It looks to
me that mock_special_storage_policy.cc is included only here and nowhere else
(after my removal from test_support_common).
You are referring to the general fact of including ../webkit/..cc files in the
various targets here, right?
Would you feel better grouping them in a new static_library target (something
like 'webkit_content_test_support') and have the targets depending on them?
(provided that they don't create *more magic cycles*)

[Nico, 2014-04-14 15:23:07]

https://codereview.chromium.org/228003006/diff/110001/content/content_tests.gypi
File content/content_tests.gypi (right):

https://codereview.chromium.org/228003006/diff/110001/content/content_tests.g...
content/content_tests.gypi:208:
'../webkit/browser/quota/mock_special_storage_policy.h',
On 2014/04/12 10:26:00, Primiano Tucci wrote:
> On 2014/04/11 20:36:59, Nico wrote:
> > This looks really icky. It looks like two other targets do this already, but
> > these files should really be in some test support target that the 2 (now 3)
> > targets should depend on.
> 
> Hmm, I am not completely sure I got the sense of "do this already". It looks
to
> me that mock_special_storage_policy.cc is included only here and nowhere else
> (after my removal from test_support_common).
> You are referring to the general fact of including ../webkit/..cc files in the
> various targets here, right?

Yes.

> Would you feel better grouping them in a new static_library target (something
> like 'webkit_content_test_support') and have the targets depending on them?
> (provided that they don't create *more magic cycles*)

I think that'd be better (I feel that in general, each source file should be in
exactly one target), but it doesn't have to be in this CL.

[jam, 2014-04-14 16:18:06]

https://codereview.chromium.org/228003006/diff/110001/content/content_tests.gypi
File content/content_tests.gypi (right):

https://codereview.chromium.org/228003006/diff/110001/content/content_tests.g...
content/content_tests.gypi:208:
'../webkit/browser/quota/mock_special_storage_policy.h',
On 2014/04/14 15:23:07, Nico wrote:
> On 2014/04/12 10:26:00, Primiano Tucci wrote:
> > On 2014/04/11 20:36:59, Nico wrote:
> > > This looks really icky. It looks like two other targets do this already,
but
> > > these files should really be in some test support target that the 2 (now
3)
> > > targets should depend on.
> > 
> > Hmm, I am not completely sure I got the sense of "do this already". It looks
> to
> > me that mock_special_storage_policy.cc is included only here and nowhere
else
> > (after my removal from test_support_common).
> > You are referring to the general fact of including ../webkit/..cc files in
the
> > various targets here, right?
> 
> Yes.
> 
> > Would you feel better grouping them in a new static_library target
(something
> > like 'webkit_content_test_support') and have the targets depending on them?
> > (provided that they don't create *more magic cycles*)
> 
> I think that'd be better (I feel that in general, each source file should be
in
> exactly one target), but it doesn't have to be in this CL.

note: i would ignore this for now, these files are moving to cntent soon, and
that'll solve this problem

[Primiano Tucci (use gerrit), 2014-04-14 17:22:27]

Nico, John, would you mind taking another look?
I refactored all the ../webkit/mock_something.cc|h into an intermediate target
(webkit_test_support_content).
It should be cleaner now. (John, I'm afraid I got your comment too late in the
stage :/, thanks btw)

https://codereview.chromium.org/228003006/diff/110001/chrome/chrome_tests_uni...
File chrome/chrome_tests_unit.gypi (right):

https://codereview.chromium.org/228003006/diff/110001/chrome/chrome_tests_uni...
chrome/chrome_tests_unit.gypi:2451: # Force --gc-sections also for Debug builds
(see crbug.com/159847).
On 2014/04/11 20:36:59, Nico wrote:
> todo here too

Done.

[Nico, 2014-04-14 18:01:10]

Patch sets 2 and 3 both lgtm

[Primiano Tucci (use gerrit), 2014-04-14 18:46:17]

On 2014/04/14 18:01:10, Nico wrote:
> Patch sets 2 and 3 both lgtm

FTR, waiting to land this until Wednesday:  this depends on WebRTC r5898 which
should roll in two days (that is the reason why the android bots are unhappy on
the video decoder lib)

[jam, 2014-04-14 19:40:21]

lgtm.

i would prefer we don't create yet another target (webkit_test_support_content),
but that'll go away soon anyways so i don't feel too strongly about it

[Primiano Tucci (use gerrit), 2014-04-22 09:15:07]

The CQ bit was checked by primiano@chromium.org

[commit-bot: I haz the power, 2014-04-22 09:15:17]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/primiano@chromium.org/228003006/210001

[Primiano Tucci (use gerrit), 2014-04-22 09:16:43]

It looks like WebRTC has not rolled in the last ~10 days because of some other
unrelated issues.
Therefore I'm landing this CL (which is becoming hard to maintain offline)
modulo keeping the global --gc-section Android flag, which I'll remove in a
separate CL as soon as WebRTC rolls.

[commit-bot: I haz the power, 2014-04-22 09:29:51]

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

[commit-bot: I haz the power, 2014-04-22 09:29:51]

Try jobs failed on following builders:
  tryserver.chromium on chromium_presubmit

[Primiano Tucci (use gerrit), 2014-04-22 11:01:07]

Adding missing OWNERS:
+isherman for components/autofill.gypi
+piman for gpu/gpu.gyp

Can you PTAL to these two gyp one-liners?

[dhollowa, 2014-04-22 17:17:16]

On 2014/04/22 11:01:07, Primiano Tucci wrote:
> Adding missing OWNERS:
> +isherman for components/autofill.gypi
> +piman for gpu/gpu.gyp
> 
> Can you PTAL to these two gyp one-liners?

LGTM.

[piman, 2014-04-22 17:30:49]

lgtm

[Ilya Sherman, 2014-04-22 20:46:46]

lgtm

[Ilya Sherman, 2014-04-22 20:46:57]

The CQ bit was checked by isherman@chromium.org

[commit-bot: I haz the power, 2014-04-22 20:47:22]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/primiano@chromium.org/228003006/230001

[commit-bot: I haz the power, 2014-04-22 22:08:23]

Change committed as 265378