dart2js.

dart2js.

[floitsch, 2013-10-25 11:45:58]

I will merge this CL into the other isolate CL once you think it is ok.

Just keeping it separate to make reviewing easier.

[Lasse Reichstein Nielsen, 2013-10-25 12:45:58]

lgtm

https://codereview.chromium.org/43663003/diff/1/sdk/lib/_internal/lib/isolate...
File sdk/lib/_internal/lib/isolate_helper.dart (right):

https://codereview.chromium.org/43663003/diff/1/sdk/lib/_internal/lib/isolate...
sdk/lib/_internal/lib/isolate_helper.dart:475: var args = msg['args'];
Are we sure we don't need to deserializer args too?
I know it's supposed to just be a list of strings, but do we check that it is?

https://codereview.chromium.org/43663003/diff/1/sdk/lib/_internal/lib/isolate...
sdk/lib/_internal/lib/isolate_helper.dart:631: if (uri != null) throw new
UnsupportedError(
Multi-line if needs braces.

https://codereview.chromium.org/43663003/diff/1/sdk/lib/_internal/lib/isolate...
sdk/lib/_internal/lib/isolate_helper.dart:649: } else if (topLevel is
_MainFunctionArgsMessage) {
If message was null, should we have preferred to call with one argument if
possible (i.e., for main(args, [message = someDefault]) )
?

https://codereview.chromium.org/43663003/diff/1/sdk/lib/_internal/lib/isolate...
sdk/lib/_internal/lib/isolate_helper.dart:666: print("message: $message");
Debug print.
Was that mine?

https://codereview.chromium.org/43663003/diff/1/sdk/lib/_internal/lib/isolate...
File sdk/lib/_internal/lib/isolate_patch.dart (right):

https://codereview.chromium.org/43663003/diff/1/sdk/lib/_internal/lib/isolate...
sdk/lib/_internal/lib/isolate_patch.dart:21: SendPort controlPort =
IsolateNatives.spawn(uri, args, message);
spawn->spawnUri

https://codereview.chromium.org/43663003/diff/1/sdk/lib/isolate/isolate.dart
File sdk/lib/isolate/isolate.dart (right):

https://codereview.chromium.org/43663003/diff/1/sdk/lib/isolate/isolate.dart#...
sdk/lib/isolate/isolate.dart:102: * The second argument [replyTo] is deprecated
and its value is ignored.
Consider removing it now.

[floitsch, 2013-10-25 13:52:56]

I will merge this into the big isolate CL now.

https://codereview.chromium.org/43663003/diff/1/sdk/lib/_internal/lib/isolate...
File sdk/lib/_internal/lib/isolate_helper.dart (right):

https://codereview.chromium.org/43663003/diff/1/sdk/lib/_internal/lib/isolate...
sdk/lib/_internal/lib/isolate_helper.dart:475: var args = msg['args'];
On 2013/10/25 12:45:59, Lasse Reichstein Nielsen wrote:
> Are we sure we don't need to deserializer args too?
> I know it's supposed to just be a list of strings, but do we check that it is?

We need to check, because dart2js will (or should) treeshake everything else
away.
I added a check.

https://codereview.chromium.org/43663003/diff/1/sdk/lib/_internal/lib/isolate...
sdk/lib/_internal/lib/isolate_helper.dart:631: if (uri != null) throw new
UnsupportedError(
On 2013/10/25 12:45:59, Lasse Reichstein Nielsen wrote:
> Multi-line if needs braces.

Done.

https://codereview.chromium.org/43663003/diff/1/sdk/lib/_internal/lib/isolate...
sdk/lib/_internal/lib/isolate_helper.dart:649: } else if (topLevel is
_MainFunctionArgsMessage) {
On 2013/10/25 12:45:59, Lasse Reichstein Nielsen wrote:
> If message was null, should we have preferred to call with one argument if
> possible (i.e., for main(args, [message = someDefault]) )
> ?

I prefer providing the argument. This is the explicit case where you are called
from spawnUri. The default is only given if you are spawned from the embedder.

https://codereview.chromium.org/43663003/diff/1/sdk/lib/_internal/lib/isolate...
sdk/lib/_internal/lib/isolate_helper.dart:666: print("message: $message");
On 2013/10/25 12:45:59, Lasse Reichstein Nielsen wrote:
> Debug print.
> Was that mine?

probably mine.

https://codereview.chromium.org/43663003/diff/1/sdk/lib/_internal/lib/isolate...
File sdk/lib/_internal/lib/isolate_patch.dart (right):

https://codereview.chromium.org/43663003/diff/1/sdk/lib/_internal/lib/isolate...
sdk/lib/_internal/lib/isolate_patch.dart:21: SendPort controlPort =
IsolateNatives.spawn(uri, args, message);
On 2013/10/25 12:45:59, Lasse Reichstein Nielsen wrote:
> spawn->spawnUri

argh. That was already fixed locally in a different branch :(

https://codereview.chromium.org/43663003/diff/1/sdk/lib/isolate/isolate.dart
File sdk/lib/isolate/isolate.dart (right):

https://codereview.chromium.org/43663003/diff/1/sdk/lib/isolate/isolate.dart#...
sdk/lib/isolate/isolate.dart:102: * The second argument [replyTo] is deprecated
and its value is ignored.
On 2013/10/25 12:45:59, Lasse Reichstein Nielsen wrote:
> Consider removing it now.

Can't.

[Lasse Reichstein Nielsen, 2013-10-25 14:28:24]

dbc

https://codereview.chromium.org/43663003/diff/80001/runtime/lib/isolate_patch...
File runtime/lib/isolate_patch.dart (right):

https://codereview.chromium.org/43663003/diff/80001/runtime/lib/isolate_patch...
runtime/lib/isolate_patch.dart:222: throw new ArgumentError("Args must be a list
of Strings $args");
If we allow null, will it be null at the other end too, or an empty list?

https://codereview.chromium.org/43663003/diff/80001/sdk/lib/_internal/lib/iso...
File sdk/lib/_internal/lib/isolate_helper.dart (right):

https://codereview.chromium.org/43663003/diff/80001/sdk/lib/_internal/lib/iso...
sdk/lib/_internal/lib/isolate_helper.dart:66: rootContext.eval(entry);
Ack, it works. If it can accept only one argument, that's what we do. Otherwise
it must require either zero or two arguments, and it can't be both, so the order
isn't important.

https://codereview.chromium.org/43663003/diff/80001/sdk/lib/isolate/isolate.dart
File sdk/lib/isolate/isolate.dart (right):

https://codereview.chromium.org/43663003/diff/80001/sdk/lib/isolate/isolate.d...
sdk/lib/isolate/isolate.dart:35: : this._controlPort = controlPort;
This could be abbreviated again, now that it's not public (but that's just
source size).

https://codereview.chromium.org/43663003/diff/80001/sdk/lib/isolate/isolate.d...
sdk/lib/isolate/isolate.dart:102: * The second argument [replyTo] is deprecated
and its value is ignored.
How long will it be before we remove this deprecated parameter?
It can't be more than a week anyway, so we might as well remove it now, just to
be sure.

https://codereview.chromium.org/43663003/diff/80001/tests/isolate/count_test....
File tests/isolate/count_test.dart (right):

https://codereview.chromium.org/43663003/diff/80001/tests/isolate/count_test....
tests/isolate/count_test.dart:14: print("iso: $message");
debug print.

https://codereview.chromium.org/43663003/diff/80001/tests/isolate/count_test....
tests/isolate/count_test.dart:19: print("iso-closed");
ditto

https://codereview.chromium.org/43663003/diff/80001/tests/isolate/count_test....
tests/isolate/count_test.dart:36: print("Main: ${msg[0]}");
And again. Did I forget to upload a version with these removed?

[floitsch, 2013-10-25 14:33:13]

https://codereview.chromium.org/43663003/diff/80001/runtime/lib/isolate_patch...
File runtime/lib/isolate_patch.dart (right):

https://codereview.chromium.org/43663003/diff/80001/runtime/lib/isolate_patch...
runtime/lib/isolate_patch.dart:222: throw new ArgumentError("Args must be a list
of Strings $args");
On 2013/10/25 14:28:24, Lasse Reichstein Nielsen wrote:
> If we allow null, will it be null at the other end too, or an empty list?

I would say `null`.

https://codereview.chromium.org/43663003/diff/80001/sdk/lib/isolate/isolate.dart
File sdk/lib/isolate/isolate.dart (right):

https://codereview.chromium.org/43663003/diff/80001/sdk/lib/isolate/isolate.d...
sdk/lib/isolate/isolate.dart:35: : this._controlPort = controlPort;
On 2013/10/25 14:28:24, Lasse Reichstein Nielsen wrote:
> This could be abbreviated again, now that it's not public (but that's just
> source size).

Not worth a separate CL.

https://codereview.chromium.org/43663003/diff/80001/sdk/lib/isolate/isolate.d...
sdk/lib/isolate/isolate.dart:102: * The second argument [replyTo] is deprecated
and its value is ignored.
On 2013/10/25 14:28:24, Lasse Reichstein Nielsen wrote:
> How long will it be before we remove this deprecated parameter?
> It can't be more than a week anyway, so we might as well remove it now, just
to
> be sure.

Soeren just uploaded a CL. So hopefully Monday.

Until then we can't remove it, because we need to adapt the C API samples, ...

https://codereview.chromium.org/43663003/diff/80001/tests/isolate/count_test....
File tests/isolate/count_test.dart (right):

https://codereview.chromium.org/43663003/diff/80001/tests/isolate/count_test....
tests/isolate/count_test.dart:36: print("Main: ${msg[0]}");
On 2013/10/25 14:28:24, Lasse Reichstein Nielsen wrote:
> And again. Did I forget to upload a version with these removed?

I hadn't merged with the latest version yet.
The full CL doesn't contain the prints.