Remove old isolate API and update all code in the repository to use

Remove old isolate API and update all code in the repository to use
the new API.

R=kasperl@google.com,sigmund@google.com,turnidge@google.com
BUG=

Committed: https://code.google.com/p/dart/source/detail?r=10210

[Mads Ager (google), 2012-08-02 12:51:54]

Kasper, could you check the dart2js part.

Todd, could you check the vm part.

Siggi, could you do a general review and check that I haven't dropped something
on the floor here. It is hard not to loose track when the changelist grows this
way. :-)

[Mads Ager (google), 2012-08-02 15:04:20]

https://chromiumcodereview.appspot.com/10837070/diff/3001/runtime/vm/isolate_...
File runtime/vm/isolate_test.cc (left):

https://chromiumcodereview.appspot.com/10837070/diff/3001/runtime/vm/isolate_...
runtime/vm/isolate_test.cc:25: TEST_CASE(IsolateSpawn) {
Looking at this test more with the help of Ivan it turns out that this test
should be updated to use the new API. It crashes if you do that with:

Running test: IsolateSpawn
runtime/lib/isolate.cc:503: error: expected: callback != NULL

So, in the new API we are missing a check that the isolate creation callback is
set. Tomorrow morning, I'll add this test back and fix the missing callback
check.

[Siggi Cherem (dart-lang), 2012-08-02 16:43:35]

so nice to see this :) 

Thanks Mads!

generally lgtm, just a couple comments.

https://chromiumcodereview.appspot.com/10837070/diff/3001/runtime/vm/snapshot...
File runtime/vm/snapshot_test.dart (right):

https://chromiumcodereview.appspot.com/10837070/diff/3001/runtime/vm/snapshot...
runtime/vm/snapshot_test.dart:1182: // THE REST OF THIS FILE COULD BE
AUTOGENERATED
I used to edit this file by hand, for future reference, how do I auto regenerate
it?

https://chromiumcodereview.appspot.com/10837070/diff/3001/samples/dartcombat/...
File samples/dartcombat/player.dart (left):

https://chromiumcodereview.appspot.com/10837070/diff/3001/samples/dartcombat/...
samples/dartcombat/player.dart:36: final Future<SendPort> portToPlayer;
another note - we tried to make spawnFunction return a port directly, but that
needs to be fixed, I added a bug for it (http://dartbug.com/4329)

https://chromiumcodereview.appspot.com/10837070/diff/3001/samples/dartcombat/...
File samples/dartcombat/player.dart (right):

https://chromiumcodereview.appspot.com/10837070/diff/3001/samples/dartcombat/...
samples/dartcombat/player.dart:38: PlayerImpl() : portToPlayer =
spawnFunction(spawnPlayer);
FYI - this will break once dart2js bug is fixed, this should be
spawnFunctionInDOM (which doesn't exist yet, http://dartbug.com/3050)

https://chromiumcodereview.appspot.com/10837070/diff/3001/tests/isolate/mixed...
File tests/isolate/mixed2_test.dart (left):

https://chromiumcodereview.appspot.com/10837070/diff/3001/tests/isolate/mixed...
tests/isolate/mixed2_test.dart:7: #library('Mixed2Test');
eventually this might come back to life once we have DOM vs non-DOM isolates in
place. maybe file a bug for it?

https://chromiumcodereview.appspot.com/10837070/diff/3001/tests/isolate/spawn...
File tests/isolate/spawn_test.dart (right):

https://chromiumcodereview.appspot.com/10837070/diff/3001/tests/isolate/spawn...
tests/isolate/spawn_test.dart:5: #library("SpawnTest");
this test might be redundant with v2_spawn_function_test.dart. If we want to
keep it, we should rename this file to spawn_function_test.dart, since 'spawn'
alone is not descriptive enough anymore.


Related to this. I think we should remove the 'v2_' prefix from all the other
isolate test files.

[turnidge, 2012-08-02 17:54:47]

lgtm, one comment.

VM changes seem pretty straightforward.  Once you resolve the one remaining
issue of the test you are discussing with Ivan, then I'm okay with the CL.

Thanks.

[kasperl, 2012-08-03 05:20:27]

LGTM. This is huge leap forward!

https://chromiumcodereview.appspot.com/10837070/diff/3001/samples/chat/chat_s...
File samples/chat/chat_server_lib.dart (right):

https://chromiumcodereview.appspot.com/10837070/diff/3001/samples/chat/chat_s...
samples/chat/chat_server_lib.dart:13: port.receive((message, replyTo) {
port.receive(server.dispatch)?

https://chromiumcodereview.appspot.com/10837070/diff/3001/samples/dartcombat/...
File samples/dartcombat/state.dart (right):

https://chromiumcodereview.appspot.com/10837070/diff/3001/samples/dartcombat/...
samples/dartcombat/state.dart:7: port.receive((message, SendPort replyTo) {
port.receive(state.dispatch)?

https://chromiumcodereview.appspot.com/10837070/diff/3001/samples/tests/sampl...
File samples/tests/samples/chat/chat_server_test.dart (right):

https://chromiumcodereview.appspot.com/10837070/diff/3001/samples/tests/sampl...
samples/tests/samples/chat/chat_server_test.dart:31: port.receive((message,
replyTo) {
port.receive(client.dispatch)?

https://chromiumcodereview.appspot.com/10837070/diff/3001/tests/standalone/io...
File tests/standalone/io/http_advanced_test.dart (right):

https://chromiumcodereview.appspot.com/10837070/diff/3001/tests/standalone/io...
tests/standalone/io/http_advanced_test.dart:96: server.dispatch(message,
replyTo);
port.receive(server.dispatch)?

https://chromiumcodereview.appspot.com/10837070/diff/3001/tests/standalone/io...
File tests/standalone/io/http_basic_test.dart (right):

https://chromiumcodereview.appspot.com/10837070/diff/3001/tests/standalone/io...
tests/standalone/io/http_basic_test.dart:95: port.receive((message, replyTo) {
port.receive(server.dispatch)?

https://chromiumcodereview.appspot.com/10837070/diff/3001/tests/standalone/io...
File tests/standalone/io/http_read_test.dart (right):

https://chromiumcodereview.appspot.com/10837070/diff/3001/tests/standalone/io...
tests/standalone/io/http_read_test.dart:95: port.receive((message, replyTo) {
port.receive(server.dispatch)? Maybe this is a pattern to look for? :-)

https://chromiumcodereview.appspot.com/10837070/diff/3001/tests/standalone/io...
File tests/standalone/io/socket_close_test.dart (right):

https://chromiumcodereview.appspot.com/10837070/diff/3001/tests/standalone/io...
tests/standalone/io/socket_close_test.dart:210: server.dispatch(message,
replyTo);
!

https://chromiumcodereview.appspot.com/10837070/diff/3001/tests/standalone/io...
File tests/standalone/io/socket_many_connections_test.dart (right):

https://chromiumcodereview.appspot.com/10837070/diff/3001/tests/standalone/io...
tests/standalone/io/socket_many_connections_test.dart:70:
server.dispatch(message, replyTo);
!

https://chromiumcodereview.appspot.com/10837070/diff/3001/tests/standalone/io...
File tests/standalone/io/socket_stream_close_test.dart (right):

https://chromiumcodereview.appspot.com/10837070/diff/3001/tests/standalone/io...
tests/standalone/io/socket_stream_close_test.dart:219: server.dispatch(message,
replyTo);
!

https://chromiumcodereview.appspot.com/10837070/diff/3001/tests/standalone/io...
File tests/standalone/io/stream_pipe_test.dart (right):

https://chromiumcodereview.appspot.com/10837070/diff/3001/tests/standalone/io...
tests/standalone/io/stream_pipe_test.dart:151: server.dispatch(message,
replyTo);
!

[Mads Ager (google), 2012-08-03 05:51:35]

Thanks for the comments!

https://chromiumcodereview.appspot.com/10837070/diff/3001/runtime/vm/isolate_...
File runtime/vm/isolate_test.cc (left):

https://chromiumcodereview.appspot.com/10837070/diff/3001/runtime/vm/isolate_...
runtime/vm/isolate_test.cc:25: TEST_CASE(IsolateSpawn) {
On 2012/08/02 15:04:20, Mads Ager wrote:
> Looking at this test more with the help of Ivan it turns out that this test
> should be updated to use the new API. It crashes if you do that with:
> 
> Running test: IsolateSpawn
> runtime/lib/isolate.cc:503: error: expected: callback != NULL
> 
> So, in the new API we are missing a check that the isolate creation callback
is
> set. Tomorrow morning, I'll add this test back and fix the missing callback
> check.

Done. Reintroduced. Added the test for a NULL callback and used EXPECT_ERROR to
test that it is the right exception being thrown. Updated the comment for the
test as well.

https://chromiumcodereview.appspot.com/10837070/diff/3001/runtime/vm/snapshot...
File runtime/vm/snapshot_test.dart (right):

https://chromiumcodereview.appspot.com/10837070/diff/3001/runtime/vm/snapshot...
runtime/vm/snapshot_test.dart:1182: // THE REST OF THIS FILE COULD BE
AUTOGENERATED
On 2012/08/02 16:43:35, sigmund wrote:
> I used to edit this file by hand, for future reference, how do I auto
regenerate
> it?

I edited it my hand as well. :( As far as I can tell, the comment just says that
in principle the rest of the file *could* be auto generated. But I can't see
anything in the repo to do that so this was manual work.

https://chromiumcodereview.appspot.com/10837070/diff/3001/samples/chat/chat_s...
File samples/chat/chat_server_lib.dart (right):

https://chromiumcodereview.appspot.com/10837070/diff/3001/samples/chat/chat_s...
samples/chat/chat_server_lib.dart:13: port.receive((message, replyTo) {
On 2012/08/03 05:20:27, kasperl wrote:
> port.receive(server.dispatch)?

D'oh! Done for all of them.

https://chromiumcodereview.appspot.com/10837070/diff/3001/samples/dartcombat/...
File samples/dartcombat/player.dart (left):

https://chromiumcodereview.appspot.com/10837070/diff/3001/samples/dartcombat/...
samples/dartcombat/player.dart:36: final Future<SendPort> portToPlayer;
On 2012/08/02 16:43:35, sigmund wrote:
> another note - we tried to make spawnFunction return a port directly, but that
> needs to be fixed, I added a bug for it (http://dartbug.com/4329)

Thanks for filing that bug Siggi. Based on your comments in the bug report I
agree that we should go back to Future<SendPort> here.

https://chromiumcodereview.appspot.com/10837070/diff/3001/samples/dartcombat/...
File samples/dartcombat/player.dart (right):

https://chromiumcodereview.appspot.com/10837070/diff/3001/samples/dartcombat/...
samples/dartcombat/player.dart:38: PlayerImpl() : portToPlayer =
spawnFunction(spawnPlayer);
On 2012/08/02 16:43:35, sigmund wrote:
> FYI - this will break once dart2js bug is fixed, this should be
> spawnFunctionInDOM (which doesn't exist yet, http://dartbug.com/3050)

Thanks Siggi! I'll add a comment that the players need access to the DOM and
that something like spawnFunctionInDOM is needed.

I think I got this working as well as it did before these changes. However, the
game is not really working. It doesn't work at all in Dartium (because the
isolates don't have DOM access). With dart2js some of it is working, but the
'shooting' part is not really working.

https://chromiumcodereview.appspot.com/10837070/diff/3001/tests/isolate/mixed...
File tests/isolate/mixed2_test.dart (left):

https://chromiumcodereview.appspot.com/10837070/diff/3001/tests/isolate/mixed...
tests/isolate/mixed2_test.dart:7: #library('Mixed2Test');
On 2012/08/02 16:43:35, sigmund wrote:
> eventually this might come back to life once we have DOM vs non-DOM isolates
in
> place. maybe file a bug for it?

I'll file a bug when I can. dartbug.com is down right now.

https://chromiumcodereview.appspot.com/10837070/diff/3001/tests/isolate/spawn...
File tests/isolate/spawn_test.dart (right):

https://chromiumcodereview.appspot.com/10837070/diff/3001/tests/isolate/spawn...
tests/isolate/spawn_test.dart:5: #library("SpawnTest");
On 2012/08/02 16:43:35, sigmund wrote:
> this test might be redundant with v2_spawn_function_test.dart. If we want to
> keep it, we should rename this file to spawn_function_test.dart, since 'spawn'
> alone is not descriptive enough anymore.
> 
> 
> Related to this. I think we should remove the 'v2_' prefix from all the other
> isolate test files.

I have removed this test. It is redundant with the other spawn_function test. I
have removed the v2 prefixes.