Build targets from mojo_base on iOS

Build targets from mojo_base on iOS

This CL adds support for the mojo_base target on iOS that will be required to
implement the Mojo support that is feasible on iOS (namely, C++-C++
communication via Mojo and mojom interfaces).

Committed: https://crrev.com/475263c6949bad244ed906d92b349a32843c351e
Cr-Commit-Position: refs/heads/master@{#294580}

[blundell, 2014-09-04 13:02:01]

blundell@chromium.org changed reviewers:
+ jamesr@chromium.org, qsr@chromium.org, viettrungluu@chromium.org

[blundell, 2014-09-04 13:02:02]

After discussion with jamesr@ and qsr@, I believe that this CL brings up the
targets that will be needed on iOS as the Chromium codebase starts being
Mojo-ified.

[qsr, 2014-09-04 13:35:46]

https://chromiumcodereview.appspot.com/528763002/diff/1/mojo/mojo_base.gyp
File mojo/mojo_base.gyp (right):

https://chromiumcodereview.appspot.com/528763002/diff/1/mojo/mojo_base.gyp#ne...
mojo/mojo_base.gyp:232: 'embedder/embedder_unittest.cc',
You might want to cut this test in 2, one for in process, one for multi-process.
You really want to test the embedder on iOS, given that this is what contain the
mojo base implementation.

https://chromiumcodereview.appspot.com/528763002/diff/1/mojo/system/message_p...
File mojo/system/message_pipe_test_utils.h (right):

https://chromiumcodereview.appspot.com/528763002/diff/1/mojo/system/message_p...
mojo/system/message_pipe_test_utils.h:44: #if !defined(OS_IOS)
What is the reason for ifdefing this out? What cannot compile in iOS?

[blundell, 2014-09-04 15:02:51]

https://chromiumcodereview.appspot.com/528763002/diff/1/mojo/mojo_base.gyp
File mojo/mojo_base.gyp (right):

https://chromiumcodereview.appspot.com/528763002/diff/1/mojo/mojo_base.gyp#ne...
mojo/mojo_base.gyp:232: 'embedder/embedder_unittest.cc',
On 2014/09/04 13:35:45, qsr wrote:
> You might want to cut this test in 2, one for in process, one for
multi-process.
> You really want to test the embedder on iOS, given that this is what contain
the
> mojo base implementation.

I'll look at this but would prefer to save it for a followup in order to
separate concerns on that from issues in this Cl.

https://chromiumcodereview.appspot.com/528763002/diff/1/mojo/system/message_p...
File mojo/system/message_pipe_test_utils.h (right):

https://chromiumcodereview.appspot.com/528763002/diff/1/mojo/system/message_p...
mojo/system/message_pipe_test_utils.h:44: #if !defined(OS_IOS)
On 2014/09/04 13:35:45, qsr wrote:
> What is the reason for ifdefing this out? What cannot compile in iOS?

MultiprocessTestHelper. I could just slice and dice on |helper_|, but that seems
worse. I could also split this file into in-process and multiprocess utils
files.

[qsr, 2014-09-05 08:27:32]

https://chromiumcodereview.appspot.com/528763002/diff/1/DEPS
File DEPS (left):

https://chromiumcodereview.appspot.com/528763002/diff/1/DEPS#oldcode382
DEPS:382: 'src/third_party/mesa/src': None,
Do you want to have Stuart of Rohit on this review for this and the other one?

https://chromiumcodereview.appspot.com/528763002/diff/1/build/all.gyp
File build/all.gyp (right):

https://chromiumcodereview.appspot.com/528763002/diff/1/build/all.gyp#newcode42
build/all.gyp:42: '../mojo/mojo_base.gyp:mojo_common_lib',
why don't you put all of mojo_base here?

[blundell, 2014-09-05 08:33:11]

https://chromiumcodereview.appspot.com/528763002/diff/1/DEPS
File DEPS (left):

https://chromiumcodereview.appspot.com/528763002/diff/1/DEPS#oldcode382
DEPS:382: 'src/third_party/mesa/src': None,
I got their OK with these on an earlier iteration of this effort.

On 2014/09/05 08:27:32, qsr wrote:
> Do you want to have Stuart of Rohit on this review for this and the other one?

https://chromiumcodereview.appspot.com/528763002/diff/1/build/all.gyp
File build/all.gyp (right):

https://chromiumcodereview.appspot.com/528763002/diff/1/build/all.gyp#newcode42
build/all.gyp:42: '../mojo/mojo_base.gyp:mojo_common_lib',
On 2014/09/05 08:27:32, qsr wrote:
> why don't you put all of mojo_base here?

mojo_js_bindings_lib has a dependency on v8, which doesn't build on iOS. James
had previously been against the idea of putting dependencies of the mojo_base
target in a !iOS conditional.

[qsr, 2014-09-05 08:36:04]

LGTM

https://chromiumcodereview.appspot.com/528763002/diff/1/build/all.gyp
File build/all.gyp (right):

https://chromiumcodereview.appspot.com/528763002/diff/1/build/all.gyp#newcode42
build/all.gyp:42: '../mojo/mojo_base.gyp:mojo_common_lib',
:( Ok, I guess that means someone on the iOS team must make sure not target fell
into the cracks.

[blundell, 2014-09-05 11:05:20]

Thanks. James/Trung, any comments?

[viettrungluu, 2014-09-05 16:59:06]

It mostly looks okay to me (with the question below) ... but jamesr probably has
stronger opinions than me on this topic.

https://codereview.chromium.org/528763002/diff/1/build/all.gyp
File build/all.gyp (right):

https://codereview.chromium.org/528763002/diff/1/build/all.gyp#newcode42
build/all.gyp:42: '../mojo/mojo_base.gyp:mojo_common_lib',
On 2014/09/05 08:33:11, blundell wrote:
> On 2014/09/05 08:27:32, qsr wrote:
> > why don't you put all of mojo_base here?
> 
> mojo_js_bindings_lib has a dependency on v8, which doesn't build on iOS. James
> had previously been against the idea of putting dependencies of the mojo_base
> target in a !iOS conditional.

I agree that this is a little unfortunate. What bots is this going to build on,
and who's going to be responsible for keeping them green?

(Alternatively, I wonder if mojo_js_bindings_lib belongs in mojo_base at all....
It does seem a little suspect to me.)

[qsr, 2014-09-08 08:52:14]

On 2014/09/05 16:59:06, viettrungluu wrote:
> It mostly looks okay to me (with the question below) ... but jamesr probably
has
> stronger opinions than me on this topic.

 James -> Can you please weight in if you have a problem with this?

[blundell, 2014-09-08 08:59:52]

Thanks for the comments.

https://codereview.chromium.org/528763002/diff/1/build/all.gyp
File build/all.gyp (right):

https://codereview.chromium.org/528763002/diff/1/build/all.gyp#newcode42
build/all.gyp:42: '../mojo/mojo_base.gyp:mojo_common_lib',
On 2014/09/05 16:59:06, viettrungluu wrote:
> On 2014/09/05 08:33:11, blundell wrote:
> > On 2014/09/05 08:27:32, qsr wrote:
> > > why don't you put all of mojo_base here?
> > 
> > mojo_js_bindings_lib has a dependency on v8, which doesn't build on iOS.
James
> > had previously been against the idea of putting dependencies of the
mojo_base
> > target in a !iOS conditional.
> 
> I agree that this is a little unfortunate. What bots is this going to build
on,
> and who's going to be responsible for keeping them green?
> 

It will build on the iOS bots on the main Chromium waterfall, so the
responsibility will be the same as with other main waterfall bots. The main
pitfall that I see with having this hardcoded list of targets rather than
mojo_base is that as new targets get added to mojo_base.gyp that should be built
on iOS no one will think to look here and update this list. The responsibility
of finding such targets and having them be built on iOS would then fall to the
team maintaining the iOS port. I agree that it's far from ideal, but I'd far
rather have this situation than have no Mojo targets at all building on iOS in
Chromium. Alternatively, if there's agreement, I'm more than happy to fiddle
with the definition of mojo_base on iOS such that iOS can depend on that target.


> (Alternatively, I wonder if mojo_js_bindings_lib belongs in mojo_base at
all....
> It does seem a little suspect to me.)

It's depended on by content_renderer, so unless we want to change the definition
of mojo_base.gyp, it does belong there.

[blundell, 2014-09-09 11:32:39]

Unless there's more feedback, I'll go ahead and land this tomorrow. Thanks!

[viettrungluu, 2014-09-09 21:55:47]

On 2014/09/08 08:59:52, blundell wrote:
> Thanks for the comments.
> 
> https://codereview.chromium.org/528763002/diff/1/build/all.gyp
> File build/all.gyp (right):
> 
> https://codereview.chromium.org/528763002/diff/1/build/all.gyp#newcode42
> build/all.gyp:42: '../mojo/mojo_base.gyp:mojo_common_lib',
> On 2014/09/05 16:59:06, viettrungluu wrote:
> > On 2014/09/05 08:33:11, blundell wrote:
> > > On 2014/09/05 08:27:32, qsr wrote:
> > > > why don't you put all of mojo_base here?
> > > 
> > > mojo_js_bindings_lib has a dependency on v8, which doesn't build on iOS.
> James
> > > had previously been against the idea of putting dependencies of the
> mojo_base
> > > target in a !iOS conditional.
> > 
> > I agree that this is a little unfortunate. What bots is this going to build
> on,
> > and who's going to be responsible for keeping them green?
> > 
> 
> It will build on the iOS bots on the main Chromium waterfall, so the
> responsibility will be the same as with other main waterfall bots. The main
> pitfall that I see with having this hardcoded list of targets rather than
> mojo_base is that as new targets get added to mojo_base.gyp that should be
built
> on iOS no one will think to look here and update this list. The responsibility
> of finding such targets and having them be built on iOS would then fall to the
> team maintaining the iOS port. I agree that it's far from ideal, but I'd far
> rather have this situation than have no Mojo targets at all building on iOS in
> Chromium. Alternatively, if there's agreement, I'm more than happy to fiddle
> with the definition of mojo_base on iOS such that iOS can depend on that
target.
> 
> 
> > (Alternatively, I wonder if mojo_js_bindings_lib belongs in mojo_base at
> all....
> > It does seem a little suspect to me.)
> 
> It's depended on by content_renderer, so unless we want to change the
definition
> of mojo_base.gyp, it does belong there.

It's tempting to re-define mojo_base.gyp, but I suspect that that would just
lead to yet another (even more mysterious) gyp file.

LGTM, I guess, since I don't know of a good way to avoid this explicit list. :(

[blundell, 2014-09-10 14:02:32]

The CQ bit was checked by blundell@chromium.org

[commit-bot: I haz the power, 2014-09-10 14:03:15]

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

[commit-bot: I haz the power, 2014-09-10 14:07:13]

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

[commit-bot: I haz the power, 2014-09-10 14:07:14]

Try jobs failed on following builders:
  ios_rel_device_ninja on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...)

[blundell, 2014-09-12 12:15:12]

The CQ bit was checked by blundell@chromium.org

[commit-bot: I haz the power, 2014-09-12 12:15:43]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patchset/528763002/20001

[commit-bot: I haz the power, 2014-09-12 13:15:36]

Committed patchset #2 (id:20001) as fbf83c199c77a01c259795b376fea2caad3e9942

[commit-bot: I haz the power, 2014-09-12 13:18:01]

Patchset 2 (id:??) landed as
https://crrev.com/475263c6949bad244ed906d92b349a32843c351e
Cr-Commit-Position: refs/heads/master@{#294580}