Allow Observatory to run as a hosted web service

runtime/bin/vmservice/client/lib/service_html.dart

Index: runtime/bin/vmservice/client/lib/service_html.dart
diff --git a/runtime/bin/vmservice/client/lib/service_html.dart b/runtime/bin/vmservice/client/lib/service_html.dart
index d611bfec03a881321f2525d1eeb910bcea00e137..e931cbe2f8a70e2230d7350d221820f60f1d885c 100644
--- a/runtime/bin/vmservice/client/lib/service_html.dart
+++ b/runtime/bin/vmservice/client/lib/service_html.dart
@@ -14,134 +14,233 @@ import 'package:observatory/service.dart';
 // Export the service library.
 export 'package:observatory/service.dart';
 
-class HttpVM extends VM {
-  String host;
+/// Description of a VM target.
+class NetworkVMTarget {
+  // Last time this VM has been connected to.
+  int lastConnectionTime = 0;
+  bool get everConnected => lastConnectionTime > 0;

[turnidge, 2014/07/01 00:27:47]

Maybe "hasConnected" or "hasEverConnected"?

[Cutch, 2014/07/01 17:14:51]

Done.

 
-  bool runningInJavaScript() => identical(1.0, 1);
+  // Chrome VM or standalone;
+  bool chrome = false;
+  bool get standalone => !chrome;
 
-  HttpVM() : super() {
-    if (runningInJavaScript()) {
-      // When we are running as JavaScript use the same hostname:port
-      // that the Observatory is loaded from.
-      host = 'http://${window.location.host}/';
-    } else {
-      // Otherwise, assume we are running from the Dart Editor and
-      // want to connect on the default port.
-      host = 'http://127.0.0.1:8181/';
-    }
+  // User defined name.
+  String name;
+  // Network address of VM.
+  String networkAddress;
+
+  NetworkVMTarget(this.networkAddress) {
+    name = networkAddress;
   }
 
-  Future<String> getString(String id) {
-    // Ensure we don't request host//id.
-    if (host.endsWith('/') && id.startsWith('/')) {
-      id = id.substring(1);
+  NetworkVMTarget.fromMap(Map json) {
+    lastConnectionTime = json['lastConnectionTime'];
+    chrome = json['chrome'];
+    name = json['name'];
+    networkAddress = json['networkAddress'];
+    if (name == null) {
+      name = networkAddress;
     }
-    Logger.root.info('Fetching $id from $host');
-    return HttpRequest.request(host + id,
-                               requestHeaders: {
-                                  'Observatory-Version': '1.0'
-                               }).then((HttpRequest request) {
-        return request.responseText;
-      }).catchError((error) {
-      // If we get an error here, the network request has failed.
-      Logger.root.severe('HttpRequest.request failed.');
-      var request = error.target;
-      return JSON.encode({
-          'type': 'ServiceException',
-          'id': '',
-          'response': request.responseText,
-          'kind': 'NetworkException',
-          'message': 'Could not connect to service (${request.statusText}). '
-                     'Check that you started the VM with the following flags: '
-                     '--observe'
-        });
-    });
+  }
+
+  Map toJson() {
+    return {
+      'lastConnectionTime': lastConnectionTime,
+      'chrome': chrome,
+      'name': name,
+      'networkAddress': networkAddress,
+    };
   }
 }
 
-class WebSocketVM extends VM {
-  final Map<int, Completer> _pendingRequests =
-      new Map<int, Completer>();
+class _WebSocketRequest {
+  final String id;
+  final Completer<String> completer;
+  _WebSocketRequest(this.id)
+      : completer = new Completer<String>();
+}
+

[turnidge, 2014/07/01 00:27:47]

Maybe a class-level comment about NetworkVM.

Also, why the name NetworkVM and not WebSocketVM?  Just curious.

[Cutch, 2014/07/01 17:14:51]

Done.
Renamed back to WebSocketVM. Mid-way through the work I was thinking of making
this support HTTP requests as well but abandoned that.

+class NetworkVM extends VM {
+  final Completer _connected = new Completer();
+  final Completer _disconnected = new Completer();
+  final NetworkVMTarget target;
+  final Map<String, _WebSocketRequest> _pendingRequests =
+      new Map<String, _WebSocketRequest>();
   int _requestSerial = 0;
+  WebSocket _webSocket;
 
-  String _host;
-  Future<WebSocket> _socketFuture;
+  NetworkVM(this.target) {
+    assert(target != null);
+  }
 
-  bool runningInJavaScript() => identical(1.0, 1);
+  void _notifyConnect() {
+    if (!_connected.isCompleted) {
+      Logger.root.info('NetworkVM connection opened: ${target.networkAddress}');
+      _connected.complete(this);
+    }
+  }
+  Future get onConnect => _connected.future;
+  void _notifyDisconnect() {
+    if (!_disconnected.isCompleted) {
+      Logger.root.info('NetworkVM connection error: ${target.networkAddress}');
+      _disconnected.complete(this);
+    }
+  }
+  Future get onDisconnect => _disconnected.future;
 
-  WebSocketVM() : super() {
-    if (runningInJavaScript()) {
-      // When we are running as JavaScript use the same hostname:port
-      // that the Observatory is loaded from.
-      _host = 'ws://${window.location.host}/ws';
-    } else {
-      // Otherwise, assume we are running from the Dart Editor and
-      // want to connect on the default port.
-      _host = 'ws://127.0.0.1:8181/ws';
+  void disconnect() {
+    if (_webSocket != null) {
+      _webSocket.close();
     }
+    _cancelAllPendingRequests();
+    _notifyDisconnect();
+  }
 
-    var completer = new Completer<WebSocket>();
-    _socketFuture = completer.future;
-    var socket = new WebSocket(_host);
-    socket.onOpen.first.then((_) {
-        socket.onMessage.listen(_handleMessage);
-        socket.onClose.first.then((_) {
-            _socketFuture = null;
-          });
-        completer.complete(socket);
-      });
-    socket.onError.first.then((_) {
-        _socketFuture = null;
-      });
+  Future<String> getString(String id) {
+    if (_webSocket == null) {
+      // Create a WebSocket.
+      _webSocket = new WebSocket(target.networkAddress);
+      _webSocket.onClose.listen(_onClose);
+      _webSocket.onError.listen(_onError);
+      _webSocket.onOpen.listen(_onOpen);
+      _webSocket.onMessage.listen(_onMessage);
+    }
+    return _makeRequest(id);
+  }
+
+  void _onClose(CloseEvent event) {
+    _cancelAllPendingRequests();
+    _notifyDisconnect();
+  }
+
+  // WebSocket error event handler.
+  void _onError(Event) {
+    _cancelAllPendingRequests();
+    _notifyDisconnect();
+  }
+
+  // WebSocket open event handler.
+  void _onOpen(Event) {
+    target.lastConnectionTime = new DateTime.now().millisecondsSinceEpoch;
+    _notifyConnect();
+    _sendAllPendingRequests();

[turnidge, 2014/07/01 00:27:47]

I'm wondering if we should move the call to _sendAllPendingRequests before the
call to _notifyConnect?

The scenario I'm worried about is that:

1. somebody uses vm.onConnect to wait for the connection.

2. _notifyConnect is called, _connected.future is completed and they then
immediately call getString(ID1).

3. The request for ID1 is then added to the pending requests AND immediately
sent,  since the websocket is in the OPEN state.

Another option would be to not give _pendingRequests double duty.  Instead you
could have two data structures:

1. _delayedRequests - a list of (id, completer) pairs for messages which have
not yet been sent
2. _pendingRequests - a map from seq -> completer for all outstanding requests
(no id needed anymore).

Your call.

4. We *then* call _sendAllPendingRequests and send the request for ID1 a second
time.

In short, I'm worried about the fact that sometime the pending requests have
been sent already and sometimes they haven't.  Reversing the calls above seems
to make this better.

[turnidge, 2014/07/01 00:30:02]

This comment got garbled during editing -- it should read like this:

I'm wondering if we should move the call to _sendAllPendingRequests before the
call to _notifyConnect?

The scenario I'm worried about is that:

1. somebody uses vm.onConnect to wait for the connection.

2. _notifyConnect is called, _connected.future is completed and they then
immediately call getString(ID1).

3. The request for ID1 is then added to the pending requests AND immediately
sent,  since the websocket is in the OPEN state.

4. We *then* call _sendAllPendingRequests and send the request for ID1 a second
time.

In short, I'm worried about the fact that sometime the pending requests have
been sent already and sometimes they haven't.  Reversing the calls above seems
to make this better.

Another option would be to not give _pendingRequests double duty.  Instead you
could have two data structures:

1. _delayedRequests - a list of (id, completer) pairs for messages which have
not yet been sent
2. _pendingRequests - a map from seq -> completer for all outstanding requests
(no id needed anymore).

Your call.

On 2014/07/01 00:27:47, turnidge wrote:

[Cutch, 2014/07/01 17:14:51]

Good catch. I've gone ahead and split pendingRequests and delayedRequests into
two sets. Now _sendRequest adds the request to pendingRequests and when the
connection goes up, all delayedRequests are sent.

   }
 
-  void _handleMessage(MessageEvent event) {
+  // WebSocket message event handler.
+  void _onMessage(MessageEvent event) {
     var map = JSON.decode(event.data);
-    int seq = map['seq'];
-    var response = map['response'];
-    var completer = _pendingRequests.remove(seq);
-    if (completer == null) {
-      Logger.root.severe('Received unexpected message: ${map}');
+    if (map == null) {
+      Logger.root.severe('NetworkVM got empty message');
+      return;
+    }
+    // Extract serial and response.
+    var serial;
+    var response;
+    if (target.chrome) {
+      if (map['method'] != 'Dart.observatoryData') {
+        // ignore devtools protocol spam.
+        return;
+      }
+      serial = map['params']['id'].toString();
+      response = map['params']['data'];
     } else {
-      completer.complete(response);
+      serial = map['seq'];
+      response = map['response'];

[turnidge, 2014/07/01 00:27:47]

Should we consider changing our naming to be consistent with crdp?  (id, data) 
Probably doesn't matter, but curious what you think.

[Cutch, 2014/07/01 17:14:51]

The ['params']['id'] and ['params']['data'] are dictated by crdp. I don't think
we should change our response to match, it doesn't buy us much.

I am thinking of changing the 'Dart.observatoryData' command to be
'Dart.serviceResponse' in a future round of Dartium cleanups.

+    }
+    // Complete request.
+    var request = _pendingRequests.remove(serial);
+    if (request == null) {
+      Logger.root.severe('Received unexpected message: ${map}');
+      return;
     }
+    request.completer.complete(response);
   }
 
-  Future<String> getString(String id) {
-    if (_socketFuture == null) {
-      var errorResponse = JSON.encode({
-              'type': 'ServiceException',
-              'id': '',
-              'response': '',
-              'kind': 'NetworkException',
-              'message': 'Could not connect to service. Check that you started the'
-              ' VM with the following flags:\n --enable-vm-service'
-              ' --pause-isolates-on-exit'
-          });
-      return new Future.value(errorResponse);
+  String _generateNetworkError(String userMessage) {
+    return JSON.encode({
+      'type': 'ServiceException',
+      'id': '',
+      'kind': 'NetworkException',
+      'message': userMessage
+    });
+  }
+
+  /// Cancel all pending requests by completing them with an error.
+  void _cancelAllPendingRequests() {
+    if (_pendingRequests.length == 0) {
+      return;
     }
-    return _socketFuture.then((socket) {
-        int seq = _requestSerial++;
-        if (!id.endsWith('/profile/tag')) {
-          Logger.root.info('Fetching $id from $_host');
+    Logger.root.info('Cancelling all pending requests.');
+    _pendingRequests.forEach((String serial, _WebSocketRequest request) {
+      request.completer.complete(
+          _generateNetworkError('WebSocket disconnected'));
+    });
+    _pendingRequests.clear();
+  }
+
+  /// Send all pending requests.
+  void _sendAllPendingRequests() {
+    assert(_webSocket != null);
+    if (_pendingRequests.length == 0) {
+      return;
+    }
+    Logger.root.info('Sending all pending requests.');
+    _pendingRequests.forEach(_sendRequest);
+  }
+
+  /// Send the request over WebSocket.
+  void _sendRequest(String serial, _WebSocketRequest request) {
+    assert (_webSocket.readyState == WebSocket.OPEN);
+    if (!request.id.endsWith('/profile/tag')) {
+      Logger.root.info('GET ${request.id} from ${target.networkAddress}');
+    }
+    var message;
+    // Encode message.
+    if (target.chrome) {
+      message = JSON.encode({
+        'id': int.parse(serial),
+        'method': 'Dart.observatoryQuery',
+        'params': {
+          'id': serial,
+          'query': request.id
         }
-        var completer = new Completer<String>();
-        _pendingRequests[seq] = completer;
-        var message = JSON.encode({'seq': seq, 'request': id});
-        socket.send(message);
-        return completer.future;
       });
+    } else {
+      message = JSON.encode({'seq': serial, 'request': request.id});
+    }
+    // Send message.
+    _webSocket.send(message);
+  }
+
+  /// Add a request for [id] to pending requests.
+  Future<String> _makeRequest(String id) {

[turnidge, 2014/07/01 00:27:47]

I would move this function closer to getString -- it is basically the send half
of getString.

[Cutch, 2014/07/01 17:14:51]

Done.

+    assert(_webSocket != null);
+    // Create request.
+    String serial = (_requestSerial++).toString();
+    var request = new _WebSocketRequest(id);
+    _pendingRequests[serial] = request;
+    if (_webSocket.readyState == WebSocket.OPEN) {
+      // Already connected, send request immediately.
+      _sendRequest(serial, request);
+    }
+    return request.completer.future;
   }
 }
 
-class DartiumVM extends VM {
+// A VM that communicates with the service via posting messages from DevTools.
+class PostMessageVM extends VM {
+  final Completer _connected = new Completer();
+  final Completer _disconnected = new Completer();
+  void disconnect() { /* nope */ }
+  Future get onConnect => _connected.future;
+  Future get onDisconnect => _disconnected.future;
   final Map<String, Completer> _pendingRequests =
       new Map<String, Completer>();
   int _requestSerial = 0;
 
-  DartiumVM() : super() {
+  PostMessageVM() : super() {
     window.onMessage.listen(_messageHandler);
-    Logger.root.info('Connected to DartiumVM');
+    _connected.complete(this);
   }
 
   void _messageHandler(msg) {