Allow Observatory to run as a hosted web service

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

 // Copyright (c) 2014, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 library service_html;
 
 import 'dart:async';
 import 'dart:convert';
 import 'dart:html';
 
 import 'package:logging/logging.dart';
 import 'package:observatory/service.dart';
 
 // Export the service library.
 export 'package:observatory/service.dart';
 
-class HttpVM extends VM {
-  String host;
-
-  bool runningInJavaScript() => identical(1.0, 1);
-
-  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}/';
+/// 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.

+
+  // Chrome VM or standalone;
+  bool chrome = false;
+  bool get standalone => !chrome;
+
+  // User defined name.
+  String name;
+  // Network address of VM.
+  String networkAddress;
+
+  NetworkVMTarget(this.networkAddress) {
+    name = networkAddress;
+  }
+
+  NetworkVMTarget.fromMap(Map json) {
+    lastConnectionTime = json['lastConnectionTime'];
+    chrome = json['chrome'];
+    name = json['name'];
+    networkAddress = json['networkAddress'];
+    if (name == null) {
+      name = networkAddress;
+    }
+  }
+
+  Map toJson() {
+    return {
+      'lastConnectionTime': lastConnectionTime,
+      'chrome': chrome,
+      'name': name,
+      'networkAddress': networkAddress,
+    };
+  }
+}
+
+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;
+
+  NetworkVM(this.target) {
+    assert(target != null);
+  }
+
+  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;
+
+  void disconnect() {
+    if (_webSocket != null) {
+      _webSocket.close();
+    }
+    _cancelAllPendingRequests();
+    _notifyDisconnect();
+  }
+
+  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.

+  }
+
+  // WebSocket message event handler.
+  void _onMessage(MessageEvent event) {
+    var map = JSON.decode(event.data);
+    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 {
-      // Otherwise, assume we are running from the Dart Editor and
-      // want to connect on the default port.
-      host = 'http://127.0.0.1:8181/';
-    }
-  }
-
-  Future<String> getString(String id) {
-    // Ensure we don't request host//id.
-    if (host.endsWith('/') && id.startsWith('/')) {
-      id = id.substring(1);
-    }
-    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'
-        });
+      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);
+  }
+
+  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;
+    }
+    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
+        }
+      });
+    } 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 WebSocketVM extends VM {
-  final Map<int, Completer> _pendingRequests =
-      new Map<int, Completer>();
-  int _requestSerial = 0;
-
-  String _host;
-  Future<WebSocket> _socketFuture;
-
-  bool runningInJavaScript() => identical(1.0, 1);
-
-  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';
-    }
-
-    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;
-      });
-  }
-
-  void _handleMessage(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}');
-    } else {
-      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);
-    }
-    return _socketFuture.then((socket) {
-        int seq = _requestSerial++;
-        if (!id.endsWith('/profile/tag')) {
-          Logger.root.info('Fetching $id from $_host');
-        }
-        var completer = new Completer<String>();
-        _pendingRequests[seq] = completer;
-        var message = JSON.encode({'seq': seq, 'request': id});
-        socket.send(message);
-        return 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) {
     var id = msg.data['id'];
     var name = msg.data['name'];
     var data = msg.data['data'];
     if (name != 'observatoryData') {
       return;
     }
     var completer = _pendingRequests[id];
     assert(completer != null);
     _pendingRequests.remove(id);
     completer.complete(data);
   }
 
   Future<String> getString(String path) {
     var idString = '$_requestSerial';
     Map message = {};
     message['id'] = idString;
     message['method'] = 'observatoryQuery';
     message['query'] = '$path';
     _requestSerial++;
     var completer = new Completer();
     _pendingRequests[idString] = completer;
     window.parent.postMessage(JSON.encode(message), '*');
     return completer.future;
   }
 }