Shared library for embedding VM in Android apps.

Shared library for embedding VM in Android apps.
Committed: https://code.google.com/p/dart/source/detail?r=15556

[gram, 2012-11-29 22:09:08]

PTAL

[vsm, 2012-11-29 23:13:30]

Nice!  LGTM with comments.

https://codereview.chromium.org/11419237/diff/7/dart.gyp
File dart.gyp (right):

https://codereview.chromium.org/11419237/diff/7/dart.gyp#newcode140
dart.gyp:140: 'samples/sample_extension/sample_extension.gyp:sample_extension',
Does this really need to be conditional?

https://codereview.chromium.org/11419237/diff/7/samples/android_embedder/andr...
File samples/android_embedder/android_embedder.gyp (right):

https://codereview.chromium.org/11419237/diff/7/samples/android_embedder/andr...
samples/android_embedder/android_embedder.gyp:15: # designed to be called from
Java (e.g. on Android)
How about:
# Dart shared library for Android.

https://codereview.chromium.org/11419237/diff/7/tools/gyp/configurations_andr...
File tools/gyp/configurations_android.gypi (right):

https://codereview.chromium.org/11419237/diff/7/tools/gyp/configurations_andr...
tools/gyp/configurations_android.gypi:48: 'Dart_Release': {
Why do you need Release and Dart_Release?

https://codereview.chromium.org/11419237/diff/7/tools/gyp/configurations_andr...
tools/gyp/configurations_android.gypi:62: },  # Release
I think "#Release" meant of end-of-"Release" in the original.  It's Dart_Release
here.  Maybe just delete the comment.

[gram, 2012-11-29 23:23:22]

PTAL

https://codereview.chromium.org/11419237/diff/7/dart.gyp
File dart.gyp (right):

https://codereview.chromium.org/11419237/diff/7/dart.gyp#newcode140
dart.gyp:140: 'samples/sample_extension/sample_extension.gyp:sample_extension',
On 2012/11/29 23:13:30, vsm wrote:
> Does this really need to be conditional?

Yes, this doesn't build under Android any more due to the Mozilla TLS stuff in
IO. SO it's somewhat orthogonal but caused a build break if you used the
'samples' target.

https://codereview.chromium.org/11419237/diff/7/samples/android_embedder/andr...
File samples/android_embedder/android_embedder.gyp (right):

https://codereview.chromium.org/11419237/diff/7/samples/android_embedder/andr...
samples/android_embedder/android_embedder.gyp:15: # designed to be called from
Java (e.g. on Android)
On 2012/11/29 23:13:30, vsm wrote:
> How about:
> # Dart shared library for Android.

Done.

https://codereview.chromium.org/11419237/diff/7/tools/gyp/configurations_andr...
File tools/gyp/configurations_android.gypi (right):

https://codereview.chromium.org/11419237/diff/7/tools/gyp/configurations_andr...
tools/gyp/configurations_android.gypi:48: 'Dart_Release': {
On 2012/11/29 23:13:30, vsm wrote:
> Why do you need Release and Dart_Release?

We probably don't - but Release was there before, and didn't work.
Dart_Release/Dart_Debug did work so this was an addition, but if you think I
should remove the old Release stuff I will.

https://codereview.chromium.org/11419237/diff/7/tools/gyp/configurations_andr...
tools/gyp/configurations_android.gypi:62: },  # Release
On 2012/11/29 23:13:30, vsm wrote:
> I think "#Release" meant of end-of-"Release" in the original.  It's
Dart_Release
> here.  Maybe just delete the comment.

Done.

[vsm, 2012-11-29 23:41:32]

Still LGTM

https://codereview.chromium.org/11419237/diff/7/tools/gyp/configurations_andr...
File tools/gyp/configurations_android.gypi (right):

https://codereview.chromium.org/11419237/diff/7/tools/gyp/configurations_andr...
tools/gyp/configurations_android.gypi:48: 'Dart_Release': {
Sure, I'd get rid of the old code if it doesn't seem to do anything.  Less
confusion on which one to edit.  We can always add it back in if it really meant
something.  Doesn't look like it though.

On 2012/11/29 23:23:23, gram wrote:
> On 2012/11/29 23:13:30, vsm wrote:
> > Why do you need Release and Dart_Release?
> 
> We probably don't - but Release was there before, and didn't work.
> Dart_Release/Dart_Debug did work so this was an addition, but if you think I
> should remove the old Release stuff I will.