WebSocket client for VM service

WebSocket client for VM service

* Renames ServiceRequest to Message
* Renames ServiceRequestRouter to MessageRouter
* Service now tracks connected clients.
* Service responds to "clients" message with list of currently connected clients.
* Add WebSocketClient to dart:io service.
* WebSocketClient supports sequence 'seq' values.
* web socket client test.
* Update dartium service for ServiceRequest -> Message rename.

R=turnidge@google.com

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

[Cutch, 2013-12-11 00:26:41]

https://codereview.chromium.org/112223002/diff/1/runtime/bin/vmservice_impl.cc
File runtime/bin/vmservice_impl.cc (right):

https://codereview.chromium.org/112223002/diff/1/runtime/bin/vmservice_impl.c...
runtime/bin/vmservice_impl.cc:129: result = Dart_CompileAll();
Will remove before submitting.

[turnidge, 2013-12-11 17:56:06]

lgtm with a bit of pontificating below.

https://codereview.chromium.org/112223002/diff/20001/runtime/bin/vmservice/cl...
File runtime/bin/vmservice/client.dart (right):

https://codereview.chromium.org/112223002/diff/20001/runtime/bin/vmservice/cl...
runtime/bin/vmservice/client.dart:27: void onMessage(var seq, var message) {
Could we type message here?  I assume it is Message.

https://codereview.chromium.org/112223002/diff/20001/runtime/bin/vmservice/cl...
runtime/bin/vmservice/client.dart:41: };
Should we leave this abstract too?

https://codereview.chromium.org/112223002/diff/20001/runtime/bin/vmservice/me...
File runtime/bin/vmservice/message.dart (right):

https://codereview.chromium.org/112223002/diff/20001/runtime/bin/vmservice/me...
runtime/bin/vmservice/message.dart:10: Future get future => _completer.future;
I found myself looking for a less generic name here than 'future'.  I guess what
this is, is a a future for the response.  Perhaps responseFuture is too
long-winded.  Would it be misleading or wrong to call it 'response'?

Thinking out loud a bit.

https://codereview.chromium.org/112223002/diff/20001/runtime/bin/vmservice/me...
runtime/bin/vmservice/message.dart:24: if (pathSegment == '') {
Does uri parsing often yield these empty path segments?  Do we need to edit out
empty segments in the fromMap constructor?

https://codereview.chromium.org/112223002/diff/20001/runtime/bin/vmservice/me...
runtime/bin/vmservice/message.dart:48: Future send(SendPort sendPort) {
What do you think of making this return Future<String>?  If you agree, this
might apply to the caller of send, declaration of "future", etc.

https://codereview.chromium.org/112223002/diff/20001/runtime/bin/vmservice/ru...
File runtime/bin/vmservice/running_isolates.dart (right):

https://codereview.chromium.org/112223002/diff/20001/runtime/bin/vmservice/ru...
runtime/bin/vmservice/running_isolates.dart:71: message.setErrorResponse('Empty
path for isolate: /isolates/$isolateId');
btw, I think we should have an <isolate-view> page eventually that has info
about the isolate.  For example it could have ways to access all libraries and
all classes instead of just pointing to the main library.

https://codereview.chromium.org/112223002/diff/20001/runtime/bin/vmservice/vm...
File runtime/bin/vmservice/vmservice_dartium.dart (right):

https://codereview.chromium.org/112223002/diff/20001/runtime/bin/vmservice/vm...
runtime/bin/vmservice/vmservice_dartium.dart:19: var message = new
Message.fromUri(uri);
So dartium will not support the /ws clients yet, right?

https://codereview.chromium.org/112223002/diff/20001/tests/standalone/vmservi...
File tests/standalone/vmservice/websocket_client_test.dart (right):

https://codereview.chromium.org/112223002/diff/20001/tests/standalone/vmservi...
tests/standalone/vmservice/websocket_client_test.dart:28: runTest() {
Could use a high-level comment somewhere in the test file... "We send two blahs
and check that blah doesn't blah".

[Cutch, 2013-12-12 22:37:42]

https://codereview.chromium.org/112223002/diff/20001/runtime/bin/vmservice/cl...
File runtime/bin/vmservice/client.dart (right):

https://codereview.chromium.org/112223002/diff/20001/runtime/bin/vmservice/cl...
runtime/bin/vmservice/client.dart:27: void onMessage(var seq, var message) {
On 2013/12/11 17:56:07, turnidge wrote:
> Could we type message here?  I assume it is Message.

Done.

https://codereview.chromium.org/112223002/diff/20001/runtime/bin/vmservice/cl...
runtime/bin/vmservice/client.dart:41: };
On 2013/12/11 17:56:07, turnidge wrote:
> Should we leave this abstract too?

Done.

https://codereview.chromium.org/112223002/diff/20001/runtime/bin/vmservice/me...
File runtime/bin/vmservice/message.dart (right):

https://codereview.chromium.org/112223002/diff/20001/runtime/bin/vmservice/me...
runtime/bin/vmservice/message.dart:10: Future get future => _completer.future;
On 2013/12/11 17:56:07, turnidge wrote:
> I found myself looking for a less generic name here than 'future'.  I guess
what
> this is, is a a future for the response.  Perhaps responseFuture is too
> long-winded.  Would it be misleading or wrong to call it 'response'?
> 
> Thinking out loud a bit.

I think we can do this:

Future<String> response;

https://codereview.chromium.org/112223002/diff/20001/runtime/bin/vmservice/me...
runtime/bin/vmservice/message.dart:24: if (pathSegment == '') {
On 2013/12/11 17:56:07, turnidge wrote:
> Does uri parsing often yield these empty path segments?  Do we need to edit
out
> empty segments in the fromMap constructor?

Splitting the path on '/' yields many empty strings. If the path is "/foo/" the
result of the split is ['', 'foo', '']. I want the messages delivered to C++
code to be as easy to deal with as possible. 

Good point about the fromMap constructor, I will be sure to remove any empties
there.

https://codereview.chromium.org/112223002/diff/20001/runtime/bin/vmservice/me...
runtime/bin/vmservice/message.dart:48: Future send(SendPort sendPort) {
On 2013/12/11 17:56:07, turnidge wrote:
> What do you think of making this return Future<String>?  If you agree, this
> might apply to the caller of send, declaration of "future", etc.

Done.

https://codereview.chromium.org/112223002/diff/20001/runtime/bin/vmservice/ru...
File runtime/bin/vmservice/running_isolates.dart (right):

https://codereview.chromium.org/112223002/diff/20001/runtime/bin/vmservice/ru...
runtime/bin/vmservice/running_isolates.dart:71: message.setErrorResponse('Empty
path for isolate: /isolates/$isolateId');
On 2013/12/11 17:56:07, turnidge wrote:
> btw, I think we should have an <isolate-view> page eventually that has info
> about the isolate.  For example it could have ways to access all libraries and
> all classes instead of just pointing to the main library.

There is an <isolate-summary> which sounds similar to what you've described.
Let's discuss offline.

https://codereview.chromium.org/112223002/diff/20001/runtime/bin/vmservice/vm...
File runtime/bin/vmservice/vmservice_dartium.dart (right):

https://codereview.chromium.org/112223002/diff/20001/runtime/bin/vmservice/vm...
runtime/bin/vmservice/vmservice_dartium.dart:19: var message = new
Message.fromUri(uri);
On 2013/12/11 17:56:07, turnidge wrote:
> So dartium will not support the /ws clients yet, right?

Correct but the Observatory in Dartium communicates via postMessage. I've gone
ahead and made a DartiumClient which will serve as a persistent client on
Dartium.

https://codereview.chromium.org/112223002/diff/20001/tests/standalone/vmservi...
File tests/standalone/vmservice/websocket_client_test.dart (right):

https://codereview.chromium.org/112223002/diff/20001/tests/standalone/vmservi...
tests/standalone/vmservice/websocket_client_test.dart:28: runTest() {
On 2013/12/11 17:56:07, turnidge wrote:
> Could use a high-level comment somewhere in the test file... "We send two
blahs
> and check that blah doesn't blah".

Done.

[Cutch, 2013-12-12 22:52:02]

Committed patchset #4 manually as r31116 (presubmit successful).