Rework standalone to use a synchronous loader that does not invoke Dart code

Rework standalone to use a synchronous loader that does not invoke Dart code

- [x] The first caller of the tag handler blocks, recursive callers queue work and exit.
- [x] Use a NativeMessageHandler to receive I/O results from the service isolate.
- [x] Preserve load error message format.
- [x] Move packages map into service isolate.
- [x] Wire up Todd's native URI code.

R=turnidge@google.com

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

[Cutch, 2016-05-20 17:34:01]

Description was changed from

==========
Rework standalone to use a synchronous loader that does not invoke Dart code

BUG=
==========

to

==========
Rework standalone to use a synchronous loader that does not invoke Dart code

- [x] The first caller of the tag handler blocks, recursive callers queue work
and exit.
- [x] Use a NativeMessageHandler to receive I/O results from the service
isolate.
- [x] Preserve load error message format.
- [ ] Move packages map into service isolate.
- [ ] Wire up Todd's native URI code.
==========

[Cutch, 2016-05-25 13:27:25]

Description was changed from

==========
Rework standalone to use a synchronous loader that does not invoke Dart code

- [x] The first caller of the tag handler blocks, recursive callers queue work
and exit.
- [x] Use a NativeMessageHandler to receive I/O results from the service
isolate.
- [x] Preserve load error message format.
- [ ] Move packages map into service isolate.
- [ ] Wire up Todd's native URI code.
==========

to

==========
Rework standalone to use a synchronous loader that does not invoke Dart code

- [x] The first caller of the tag handler blocks, recursive callers queue work
and exit.
- [x] Use a NativeMessageHandler to receive I/O results from the service
isolate.
- [x] Preserve load error message format.
- [x] Move packages map into service isolate.
- [ ] Wire up Todd's native URI code.
==========

[Cutch, 2016-05-25 20:55:00]

johnmccutchan@google.com changed reviewers:
+ turnidge@google.com

[Cutch, 2016-05-25 22:09:52]

Todd,

Please take over this CL. The final version uploaded passes all tests:

$ ./tools/test.py --exclude-suite=pkg
Test configuration: none_vm_debug_x64
[04:51 | 100% | +13520 | -    0]

Thanks,
John

[Cutch, 2016-06-02 15:52:06]

I've merged and updated this CL.

PTAL

[turnidge, 2016-06-03 17:50:25]

lgtm

https://codereview.chromium.org/1998963003/diff/120001/runtime/bin/builtin.dart
File runtime/bin/builtin.dart (right):

https://codereview.chromium.org/1998963003/diff/120001/runtime/bin/builtin.da...
runtime/bin/builtin.dart:208: return VMLibraryHooks.packageRootString;
Is this return value used?

https://codereview.chromium.org/1998963003/diff/120001/runtime/bin/builtin_co...
File runtime/bin/builtin_common.cc (right):

https://codereview.chromium.org/1998963003/diff/120001/runtime/bin/builtin_co...
runtime/bin/builtin_common.cc:47: RETURN_IF_ERROR(result);
Would be nice if the function failed on a second call when this field fails to
set.  Maybe move this to the top of the function.

https://codereview.chromium.org/1998963003/diff/120001/runtime/bin/gen_snapsh...
File runtime/bin/gen_snapshot.cc (right):

https://codereview.chromium.org/1998963003/diff/120001/runtime/bin/gen_snapsh...
runtime/bin/gen_snapshot.cc:602: fprintf(stderr, "%s", Dart_GetError(lib));
Debug stuff....

https://codereview.chromium.org/1998963003/diff/120001/runtime/bin/loader.cc
File runtime/bin/loader.cc (right):

https://codereview.chromium.org/1998963003/diff/120001/runtime/bin/loader.cc#...
runtime/bin/loader.cc:189: if (!ProcessQueueLocked()) {
Maybe add some small comments here in each case...

https://codereview.chromium.org/1998963003/diff/120001/runtime/bin/loader.cc#...
runtime/bin/loader.cc:254: // Re-acquire the lock before decrementing pending
operations.
Confusing comment...

https://codereview.chromium.org/1998963003/diff/120001/runtime/bin/loader.cc#...
runtime/bin/loader.cc:291: Dart_Handle url) {
Merge this by hand with my new version that I just committed.

https://codereview.chromium.org/1998963003/diff/120001/runtime/bin/loader.cc#...
runtime/bin/loader.cc:356: // Mark that we are a blocking call.
This comment could be bigger.  This is subtle

https://codereview.chromium.org/1998963003/diff/120001/runtime/bin/loader.cc#...
runtime/bin/loader.cc:440: void Loader::AddLoader(Dart_Port port, IsolateData*
isolate_data) {
Maybe a comment about what this does.

https://codereview.chromium.org/1998963003/diff/120001/runtime/bin/vmservice/...
File runtime/bin/vmservice/loader.dart (right):

https://codereview.chromium.org/1998963003/diff/120001/runtime/bin/vmservice/...
runtime/bin/vmservice/loader.dart:30: 
extra line

https://codereview.chromium.org/1998963003/diff/120001/runtime/bin/vmservice/...
runtime/bin/vmservice/loader.dart:48: 
extra line - make the file consistent...

https://codereview.chromium.org/1998963003/diff/120001/runtime/bin/vmservice/...
runtime/bin/vmservice/loader.dart:823: sp.send("Unknown packages request tag:
$tag for '$resource'.");
Might be worth a comment that this sendport is now local... not sure....

https://codereview.chromium.org/1998963003/diff/120001/runtime/lib/lib_prefix...
File runtime/lib/lib_prefix.dart (right):

https://codereview.chromium.org/1998963003/diff/120001/runtime/lib/lib_prefix...
runtime/lib/lib_prefix.dart:52: var stillOutstandingLoadRequests = new
List<List>();
Comment?

[Cutch, 2016-06-03 22:07:32]

https://codereview.chromium.org/1998963003/diff/120001/runtime/bin/builtin.dart
File runtime/bin/builtin.dart (right):

https://codereview.chromium.org/1998963003/diff/120001/runtime/bin/builtin.da...
runtime/bin/builtin.dart:208: return VMLibraryHooks.packageRootString;
On 2016/06/03 17:50:24, turnidge wrote:
> Is this return value used?

Removed this.

https://codereview.chromium.org/1998963003/diff/120001/runtime/bin/builtin_co...
File runtime/bin/builtin_common.cc (right):

https://codereview.chromium.org/1998963003/diff/120001/runtime/bin/builtin_co...
runtime/bin/builtin_common.cc:47: RETURN_IF_ERROR(result);
On 2016/06/03 17:50:24, turnidge wrote:
> Would be nice if the function failed on a second call when this field fails to
> set.  Maybe move this to the top of the function.

Done.

https://codereview.chromium.org/1998963003/diff/120001/runtime/bin/gen_snapsh...
File runtime/bin/gen_snapshot.cc (right):

https://codereview.chromium.org/1998963003/diff/120001/runtime/bin/gen_snapsh...
runtime/bin/gen_snapshot.cc:602: fprintf(stderr, "%s", Dart_GetError(lib));
On 2016/06/03 17:50:25, turnidge wrote:
> Debug stuff....

Done.

https://codereview.chromium.org/1998963003/diff/120001/runtime/bin/loader.cc
File runtime/bin/loader.cc (right):

https://codereview.chromium.org/1998963003/diff/120001/runtime/bin/loader.cc#...
runtime/bin/loader.cc:189: if (!ProcessQueueLocked()) {
On 2016/06/03 17:50:25, turnidge wrote:
> Maybe add some small comments here in each case...

Done.

https://codereview.chromium.org/1998963003/diff/120001/runtime/bin/loader.cc#...
runtime/bin/loader.cc:254: // Re-acquire the lock before decrementing pending
operations.
On 2016/06/03 17:50:25, turnidge wrote:
> Confusing comment...

Done.

https://codereview.chromium.org/1998963003/diff/120001/runtime/bin/loader.cc#...
runtime/bin/loader.cc:291: Dart_Handle url) {
On 2016/06/03 17:50:25, turnidge wrote:
> Merge this by hand with my new version that I just committed.

Done.

https://codereview.chromium.org/1998963003/diff/120001/runtime/bin/loader.cc#...
runtime/bin/loader.cc:356: // Mark that we are a blocking call.
On 2016/06/03 17:50:25, turnidge wrote:
> This comment could be bigger.  This is subtle

Done.

https://codereview.chromium.org/1998963003/diff/120001/runtime/bin/loader.cc#...
runtime/bin/loader.cc:440: void Loader::AddLoader(Dart_Port port, IsolateData*
isolate_data) {
On 2016/06/03 17:50:25, turnidge wrote:
> Maybe a comment about what this does.

Done.

https://codereview.chromium.org/1998963003/diff/120001/runtime/bin/vmservice/...
File runtime/bin/vmservice/loader.dart (right):

https://codereview.chromium.org/1998963003/diff/120001/runtime/bin/vmservice/...
runtime/bin/vmservice/loader.dart:30: 
On 2016/06/03 17:50:25, turnidge wrote:
> extra line

Acknowledged.

https://codereview.chromium.org/1998963003/diff/120001/runtime/bin/vmservice/...
runtime/bin/vmservice/loader.dart:48: 
On 2016/06/03 17:50:25, turnidge wrote:
> extra line - make the file consistent...

Done here and elsewhere.

https://codereview.chromium.org/1998963003/diff/120001/runtime/bin/vmservice/...
runtime/bin/vmservice/loader.dart:823: sp.send("Unknown packages request tag:
$tag for '$resource'.");
On 2016/06/03 17:50:25, turnidge wrote:
> Might be worth a comment that this sendport is now local... not sure....

Done.

[Cutch, 2016-06-03 22:07:43]

Description was changed from

==========
Rework standalone to use a synchronous loader that does not invoke Dart code

- [x] The first caller of the tag handler blocks, recursive callers queue work
and exit.
- [x] Use a NativeMessageHandler to receive I/O results from the service
isolate.
- [x] Preserve load error message format.
- [x] Move packages map into service isolate.
- [ ] Wire up Todd's native URI code.
==========

to

==========
Rework standalone to use a synchronous loader that does not invoke Dart code

- [x] The first caller of the tag handler blocks, recursive callers queue work
and exit.
- [x] Use a NativeMessageHandler to receive I/O results from the service
isolate.
- [x] Preserve load error message format.
- [x] Move packages map into service isolate.
- [x] Wire up Todd's native URI code.
==========

[turnidge, 2016-06-03 22:42:11]

lgtm

https://codereview.chromium.org/1998963003/diff/160001/runtime/bin/builtin.dart
File runtime/bin/builtin.dart (right):

https://codereview.chromium.org/1998963003/diff/160001/runtime/bin/builtin.da...
runtime/bin/builtin.dart:370: if (response[3] is String) {
I wonder if we should have named constants for the response fields.  "3" is a
little funky.

[Cutch, 2016-06-06 21:15:07]

Description was changed from

==========
Rework standalone to use a synchronous loader that does not invoke Dart code

- [x] The first caller of the tag handler blocks, recursive callers queue work
and exit.
- [x] Use a NativeMessageHandler to receive I/O results from the service
isolate.
- [x] Preserve load error message format.
- [x] Move packages map into service isolate.
- [x] Wire up Todd's native URI code.
==========

to

==========
Rework standalone to use a synchronous loader that does not invoke Dart code

- [x] The first caller of the tag handler blocks, recursive callers queue work
and exit.
- [x] Use a NativeMessageHandler to receive I/O results from the service
isolate.
- [x] Preserve load error message format.
- [x] Move packages map into service isolate.
- [x] Wire up Todd's native URI code.

R=turnidge@google.com

Committed:
https://github.com/dart-lang/sdk/commit/6fd8fd79875ec2869b7861b51bafeda592311d58
==========

[Cutch, 2016-06-06 21:15:09]

Committed patchset #10 (id:180001) manually as
6fd8fd79875ec2869b7861b51bafeda592311d58 (presubmit successful).