Make the Android GN debug component build actually build.

Make the Android GN debug component build actually build.

Changes required:
- fold //base/third_party/nspr/prtime.cc into base
- fixed a compile error causing issues in media_blink_unittests
- fixed various missing dependencies
- disabled chrome_sync_shell_test_apk which needs some more work

Note that this does not mean that things run. I expect they don't :).

R=cjhopman@chromium.org
BUG=359249
Committed: https://crrev.com/49d4d4d45b373b0d43514e6769f870c2a3b77099
Cr-Commit-Position: refs/heads/master@{#325568}

[Dirk Pranke, 2015-04-15 03:17:06]

This is a work-in-progress. With it, content_shell_apk, chrome_shell_apk, and
'gn_all' should all build, but there are still other targets that don't yet
work.

I think the only change that is correct (and not a hack) is the change to strip
out the one file in content/browser/BUILD.gn.

[Dirk Pranke, 2015-04-16 02:56:17]

dpranke@chromium.org changed reviewers:
+ brettw@chromium.org

[Dirk Pranke, 2015-04-16 02:57:44]

Please take a look. I don't think this is quite ready to land yet, as I hope to
be able to figure out the issue w/ components_unittests and
components_browsertests.

In particular, take a look at my comments in the description and the changes to
//base and //base/third_party/nspr. Brett, I'm not sure why broke it out into a
separate library, but that seems to cause problems and doing so for the one file
doesn't seem worth the hassle.

Thoughts?

[cjhopman, 2015-04-16 19:02:18]

https://codereview.chromium.org/1089143002/diff/60001/base/BUILD.gn
File base/BUILD.gn (left):

https://codereview.chromium.org/1089143002/diff/60001/base/BUILD.gn#oldcode1310
base/BUILD.gn:1310: "//base/third_party/nspr",
I think that this line is actually the problem with nspr. nothing but
//base:base should be depending on nspr (and everything else should get the
stuff there through a dependency on //base:base)

[cjhopman, 2015-04-16 19:13:21]

https://codereview.chromium.org/1089143002/diff/60001/BUILD.gn
File BUILD.gn (right):

https://codereview.chromium.org/1089143002/diff/60001/BUILD.gn#newcode234
BUILD.gn:234: # compiling //components/proximity_auth/cryptauth
for these, I think that there's probably just things being compiled in that
aren't compiled in in the gyp build (for example, we don't include
proximity_auth tests in the gyp build).

[Dirk Pranke, 2015-04-16 20:31:52]

https://codereview.chromium.org/1089143002/diff/60001/BUILD.gn
File BUILD.gn (right):

https://codereview.chromium.org/1089143002/diff/60001/BUILD.gn#newcode234
BUILD.gn:234: # compiling //components/proximity_auth/cryptauth
On 2015/04/16 19:13:21, cjhopman wrote:
> for these, I think that there's probably just things being compiled in that
> aren't compiled in in the gyp build (for example, we don't include
> proximity_auth tests in the gyp build).

Will keep investigating.

https://codereview.chromium.org/1089143002/diff/60001/base/BUILD.gn
File base/BUILD.gn (left):

https://codereview.chromium.org/1089143002/diff/60001/base/BUILD.gn#oldcode1310
base/BUILD.gn:1310: "//base/third_party/nspr",
On 2015/04/16 19:02:18, cjhopman wrote:
> I think that this line is actually the problem with nspr. nothing but
> //base:base should be depending on nspr (and everything else should get the
> stuff there through a dependency on //base:base)

//base:base can't depend on nspr, because nspr depends on //base:base :).

So, I think we just need to fold nspr into base like we have on GYP.

[cjhopman, 2015-04-16 20:50:33]

https://codereview.chromium.org/1089143002/diff/60001/base/BUILD.gn
File base/BUILD.gn (left):

https://codereview.chromium.org/1089143002/diff/60001/base/BUILD.gn#oldcode1310
base/BUILD.gn:1310: "//base/third_party/nspr",
On 2015/04/16 20:31:52, Dirk Pranke wrote:
> On 2015/04/16 19:02:18, cjhopman wrote:
> > I think that this line is actually the problem with nspr. nothing but
> > //base:base should be depending on nspr (and everything else should get the
> > stuff there through a dependency on //base:base)
> 
> //base:base can't depend on nspr, because nspr depends on //base:base :).
> 
> So, I think we just need to fold nspr into base like we have on GYP.

That's not true. Before your change nspr didn't have a dep on base (even though
it refers to things in base).

//base:base did have a dep on nspr and nspr is part of the base component (i.e.
that shared library exposes the appropriate nspr symbols).

my understanding is that current approach (w/ split out nspr) has the problem
that there is no way to pass a header check (because nspr uses headers from base
but doesn't depend on it and can't depend on it because then there'd be a
cycle). I guess the thing to do at that point would be to split the base target
into more smaller targets.

[Dirk Pranke, 2015-04-16 20:59:32]

https://codereview.chromium.org/1089143002/diff/60001/base/BUILD.gn
File base/BUILD.gn (left):

https://codereview.chromium.org/1089143002/diff/60001/base/BUILD.gn#oldcode1310
base/BUILD.gn:1310: "//base/third_party/nspr",
On 2015/04/16 20:50:33, cjhopman wrote:
> On 2015/04/16 20:31:52, Dirk Pranke wrote:
> > On 2015/04/16 19:02:18, cjhopman wrote:
> > > I think that this line is actually the problem with nspr. nothing but
> > > //base:base should be depending on nspr (and everything else should get
the
> > > stuff there through a dependency on //base:base)
> > 
> > //base:base can't depend on nspr, because nspr depends on //base:base :).
> > 
> > So, I think we just need to fold nspr into base like we have on GYP.
> 
> That's not true. Before your change nspr didn't have a dep on base (even
though
> it refers to things in base).

Well, yeah, it didn't have an explicit dependency on base, but it should've (due
to the header include you mentioned).

Do you not want to just fold nspr back into base? I'm not seeing the downside to
that, and Brett didn't have any objections. Creating a separate component for
one file that causes cyclical header dependencies just seems like a lot of pain
for no gain. It seems like splitting out os_compat_android (and whatever else we
might need) into a third target just to solve this is adding more complexity to
solve an already unnecessarily complex thing.

[cjhopman, 2015-04-16 21:03:48]

https://codereview.chromium.org/1089143002/diff/60001/base/BUILD.gn
File base/BUILD.gn (left):

https://codereview.chromium.org/1089143002/diff/60001/base/BUILD.gn#oldcode1310
base/BUILD.gn:1310: "//base/third_party/nspr",
On 2015/04/16 20:59:32, Dirk Pranke wrote:
> On 2015/04/16 20:50:33, cjhopman wrote:
> > On 2015/04/16 20:31:52, Dirk Pranke wrote:
> > > On 2015/04/16 19:02:18, cjhopman wrote:
> > > > I think that this line is actually the problem with nspr. nothing but
> > > > //base:base should be depending on nspr (and everything else should get
> the
> > > > stuff there through a dependency on //base:base)
> > > 
> > > //base:base can't depend on nspr, because nspr depends on //base:base :).
> > > 
> > > So, I think we just need to fold nspr into base like we have on GYP.
> > 
> > That's not true. Before your change nspr didn't have a dep on base (even
> though
> > it refers to things in base).
> 
> Well, yeah, it didn't have an explicit dependency on base, but it should've
(due
> to the header include you mentioned).
> 
> Do you not want to just fold nspr back into base? I'm not seeing the downside
to
> that, and Brett didn't have any objections. Creating a separate component for
> one file that causes cyclical header dependencies just seems like a lot of
pain
> for no gain. It seems like splitting out os_compat_android (and whatever else
we
> might need) into a third target just to solve this is adding more complexity
to
> solve an already unnecessarily complex thing.

Folding it in is definitely fine with me.

It seemed that you were unsure why it was failing to link and I was letting you
know why in case you wanted to do something different once you had that info.

[Dirk Pranke, 2015-04-16 21:05:45]

On 2015/04/16 21:03:48, cjhopman wrote:
> It seemed that you were unsure why it was failing to link and I was letting
you
> know why in case you wanted to do something different once you had that info.

Oh, no, I understood (eventually) why it was failing to link :)

[Dirk Pranke, 2015-04-16 23:28:25]

Okay, I think this is ready for review / landing.

[Dirk Pranke, 2015-04-16 23:30:49]

The CQ bit was checked by dpranke@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-04-16 23:33:34]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1089143002/80001

[Dirk Pranke, 2015-04-16 23:34:44]

The CQ bit was checked by dpranke@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-04-16 23:35:34]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1089143002/100001

[cjhopman, 2015-04-16 23:51:08]

lgtm

[commit-bot: I haz the power, 2015-04-17 00:27:00]

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

[commit-bot: I haz the power, 2015-04-17 00:27:01]

Dry run: This issue passed the CQ dry run.

[Dirk Pranke, 2015-04-17 00:30:07]

The CQ bit was checked by dpranke@chromium.org

[commit-bot: I haz the power, 2015-04-17 00:31:25]

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

[commit-bot: I haz the power, 2015-04-17 00:35:22]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2015-04-17 00:36:25]

Patchset 6 (id:??) landed as
https://crrev.com/49d4d4d45b373b0d43514e6769f870c2a3b77099
Cr-Commit-Position: refs/heads/master@{#325568}