- Implement VM parts of https://codereview.chromium.org/1240743003/

- Implement VM parts of https://codereview.chromium.org/1240743003/
- Pass additional state in the IsolateSpawnState to set
  error and exit handlers, as well as fatal errors on isolate start.

BUG=
R=floitsch@google.com

Committed: https://github.com/dart-lang/sdk/commit/fe03640e858662c53a6617df66e616896783524a

[Ivan Posva, 2015-07-20 12:40:06]

iposva@google.com changed reviewers:
+ floitsch@google.com, lrn@google.com

[floitsch, 2015-07-20 13:03:49]

LGTM.

https://codereview.chromium.org/1244733002/diff/1/runtime/lib/isolate.cc
File runtime/lib/isolate.cc (right):

https://codereview.chromium.org/1244733002/diff/1/runtime/lib/isolate.cc#newc...
runtime/lib/isolate.cc:241: bool fatal_errors = fatalErrors.IsNull() ? true :
fatalErrors.value();
I believe the default for the browser is 'false'. Not sure if the browser hits
this line, though.
Please verify that this doesn't change anything for Dartium.

'spawnFunction' doesn't work in the main-thread. So you would need to spawn a
spawnUri thread first.

https://codereview.chromium.org/1244733002/diff/1/runtime/lib/isolate.cc#newc...
runtime/lib/isolate.cc:293: bool fatal_errors = fatalErrors.IsNull() ? true :
fatalErrors.value();
ditto.

https://codereview.chromium.org/1244733002/diff/1/tests/isolate/error_exit_at...
File tests/isolate/error_exit_at_spawn_test.dart (right):

https://codereview.chromium.org/1244733002/diff/1/tests/isolate/error_exit_at...
tests/isolate/error_exit_at_spawn_test.dart:12: isomain(replyPort) {
Create two other tests:

- one that tests that the exitHandler is installed, even when the isolate exits
as fast as possible;

isomain(replyPort) {
  return;  // shut down the isolate as fast as possible.
}

- one that tests that the errorHandler is installed, when the new isolate throws
as fast as possible:

isomain(replyPort) {
  throw new ArgumentError("fast error");
}


I think I would prefer it in a separate test-file, but this file would be fine
too.

Same for the spawnUri test.

https://codereview.chromium.org/1244733002/diff/1/tests/isolate/error_exit_at...
tests/isolate/error_exit_at_spawn_test.dart:73: // onExit ports registered at
spawn cannot have a particular message associated.
long line.

[Ivan Posva, 2015-07-20 14:28:32]

Added 4 more tests as requested.

Thanks for the review,
-Ivan

https://codereview.chromium.org/1244733002/diff/1/runtime/lib/isolate.cc
File runtime/lib/isolate.cc (right):

https://codereview.chromium.org/1244733002/diff/1/runtime/lib/isolate.cc#newc...
runtime/lib/isolate.cc:241: bool fatal_errors = fatalErrors.IsNull() ? true :
fatalErrors.value();
On 2015/07/20 13:03:49, floitsch wrote:
> I believe the default for the browser is 'false'. Not sure if the browser hits
> this line, though.
> Please verify that this doesn't change anything for Dartium.

On the VM regardless of embedder the previous non-specifiable default for
isolates about fatality of errors  was always "true" so nothing has changed here
for Dartium.

> 'spawnFunction' doesn't work in the main-thread. So you would need to spawn a
> spawnUri thread first.

https://codereview.chromium.org/1244733002/diff/1/tests/isolate/error_exit_at...
File tests/isolate/error_exit_at_spawn_test.dart (right):

https://codereview.chromium.org/1244733002/diff/1/tests/isolate/error_exit_at...
tests/isolate/error_exit_at_spawn_test.dart:12: isomain(replyPort) {
On 2015/07/20 13:03:49, floitsch wrote:
> Create two other tests:
> 
> - one that tests that the exitHandler is installed, even when the isolate
exits
> as fast as possible;
> 
> isomain(replyPort) {
>   return;  // shut down the isolate as fast as possible.
> }
> 
> - one that tests that the errorHandler is installed, when the new isolate
throws
> as fast as possible:
> 
> isomain(replyPort) {
>   throw new ArgumentError("fast error");
> }
> 
> 
> I think I would prefer it in a separate test-file, but this file would be fine
> too.
> 
> Same for the spawnUri test.

Done.

https://codereview.chromium.org/1244733002/diff/1/tests/isolate/error_exit_at...
tests/isolate/error_exit_at_spawn_test.dart:73: // onExit ports registered at
spawn cannot have a particular message associated.
On 2015/07/20 13:03:49, floitsch wrote:
> long line.

Done.

[Ivan Posva, 2015-07-20 14:46:59]

Committed patchset #3 (id:40001) manually as
fe03640e858662c53a6617df66e616896783524a (presubmit successful).