Mojo: Enable apps built against the chromium env to be built as shared libraries

Mojo: Enable apps built against the chromium env to be built as shared libraries

BUG=384434, 390573
R=viettrungluu@chromium.org

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

[qsr, 2014-07-08 07:48:13]

Hun, I already had this: https://chromiumcodereview.appspot.com/346613004/ with
the same aim.

I think I prefer your patch though, as it is simpler (but mean we do not inject
the implementation in component build, might not be that much of an issue for
now, but might be an issue when we use sandbox processes...)

But you will need to patch those 2 diffs for this to work, as loadable_module
are not created at the same location as dynamic libraries:

https://chromiumcodereview.appspot.com/346613004/diff/700001/mojo/shell/shell...
https://chromiumcodereview.appspot.com/346613004/diff/700001/mojo/shell/mojo_...

[darin (slow to review), 2014-07-08 07:57:17]

Sorry, I wish I knew you were already working on this! Sucks to duplicate
efforts :(

Thanks for the tip about loadable_module. I'll patch those changes in and
then re-upload... tomorrow. Time now to sleep ;-)

-Darin


On Tue, Jul 8, 2014 at 12:48 AM, <qsr@chromium.org> wrote:

> Hun, I already had this: https://chromiumcodereview.appspot.com/346613004/
> with
> the same aim.
>
> I think I prefer your patch though, as it is simpler (but mean we do not
> inject
> the implementation in component build, might not be that much of an issue
> for
> now, but might be an issue when we use sandbox processes...)
>
> But you will need to patch those 2 diffs for this to work, as
> loadable_module
> are not created at the same location as dynamic libraries:
>
> https://chromiumcodereview.appspot.com/346613004/diff/
> 700001/mojo/shell/shell_test_base.cc
> https://chromiumcodereview.appspot.com/346613004/diff/
> 700001/mojo/shell/mojo_url_resolver.cc
>
> https://codereview.chromium.org/370983002/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[viettrungluu, 2014-07-08 17:02:28]

Sorry, about the duplicated effort. I hadn't noticed that qsr's changes hadn't
landed yet.

LGTM w/nits & qsr's changes.

https://codereview.chromium.org/370983002/diff/140001/mojo/mojo.gyp
File mojo/mojo.gyp (right):

https://codereview.chromium.org/370983002/diff/140001/mojo/mojo.gyp#newcode20
mojo/mojo.gyp:20: ['component=="shared_library"', {
Maybe you should have a comment explaining this (and the mojo_none target)?

https://codereview.chromium.org/370983002/diff/140001/mojo/mojo.gyp#newcode438
mojo/mojo.gyp:438: '<(mojo_system_for_component)'
nit (here and elsewhere): I think we usually keep trailing commas in .gyp(i)
files.

[darin (slow to review), 2014-07-08 21:27:33]

The CQ bit was checked by darin@chromium.org

[commit-bot: I haz the power, 2014-07-08 21:30:02]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/darin@chromium.org/370983002/160001

[darin (slow to review), 2014-07-08 23:17:42]

The CQ bit was unchecked by darin@chromium.org

[darin (slow to review), 2014-07-08 23:25:18]

Committed patchset #9 manually as r281877 (presubmit successful).