More TODOs, refactoring

lib/dataconnection.js

@@ -19,6 +19,15 @@ function DataConnection(peer, options) {
   this.peer = peer;
   this.reliable = options.reliable;
 
+  this.id = options.id; // TODO: or randomly generated ID.
+  if (options.sdp) {
+    // TODO: startConnection ideally calls handleSDP with the offer.
+    // On this side though, do we also need to pass in this peer's ID? Probably.
+    // Maybe instead of having a global negotiator, have a negotiator instance
+    // per Peer?
+    Negotiator.startConnection(this.peer, this.id, options.sdp, options.config);
+  }
+
   /*this._dc = dc;
   if (this._dc) {
     this._configureDataChannel();
@@ -95,12 +104,12 @@ DataConnection.prototype._handleDataMessage = function(e) {
     data = JSON.parse(data);
   }
   this.emit('data', data);
-};
+}
 
 DataConnection.prototype.addDC = function(dc) {
   this._dc = dc;
   this._configureDataChannel();
-};
+}
 
 
 /**
@@ -121,7 +130,7 @@ DataConnection.prototype.send = function(data) {
     this.emit('error', new Error('Connection no longer open.'));
   }
   if (this._reliable) {
-    // Note: reliable sending will make it so that you cannot customize
+    // Note: reliable shim sending will make it so that you cannot customize
     // serialization.
     this._reliable.send(data);
     return;
@@ -149,29 +158,14 @@ DataConnection.prototype.handleMessage = function(message) {
   var payload = message.payload;
 
   switch (message.type) {
-    case 'EXPIRE':
-      peer.emit('error', new Error('Could not connect to peer ' + manager.peer));
-      break;
     case 'ANSWER':
-      // Forward to specific manager
-      if (manager) {
-        manager.handleSDP(payload.sdp, message.type);
-      }
+      // TODO: assert sdp exists.
+      // Should we pass `this`?
+      // Forward to negotiator
+      Negotiator.handleSDP(this.peer, this.id, payload.sdp, message.type);
       break;
     case 'CANDIDATE':
-      // Forward to specific manager
-      if (manager) {
-        manager.handleCandidate(payload);
-      }
-      break;
-    case 'LEAVE':
-      // Leave on all managers for a user
-      if (this._managers[peer]) {
-        var ids = Object.keys(this._managers[peer].managers);
-        for (var i = 0; i < ids.length; i++) {
-          this._managers[peer].managers[ids[i]].handleLeave();
-        }
-      }
+      Negotiator.handleCandidate(this.peer, this.id, payload.candidate);
       break;
     default:
       util.warn('Unrecognized message type:', message.type, 'from peer:', this.peer);

lib/peer.js

@@ -139,44 +139,54 @@ Peer.prototype._initialize = function(id) {
 /** Handles messages from the server. */
 Peer.prototype._handleMessage = function(message) {
   var type = message.type;
-  var payload = message.payload
+  var payload = message.payload;
+  var peer = message.src;
+
   switch (type) {
-    case 'OPEN':
-      this._processQueue();
+    case 'OPEN': // The connection to the server is open.
       this.emit('open', this.id);
       break;
-    case 'ERROR':
+    case 'ERROR': // Server error.
       this._abort('server-error', payload.msg);
       break;
-    case 'ID-TAKEN':
+    case 'ID-TAKEN': // The selected ID is taken.
       this._abort('unavailable-id', 'ID `' + this.id + '` is taken');
       break;
-    case 'INVALID-KEY':
+    case 'INVALID-KEY': // The given API key cannot be found.
       this._abort('invalid-key', 'API KEY "' + this._key + '" is invalid');
       break;
+
+    case 'LEAVE': // Another peer has closed its connection to this peer.
+      this._cleanupPeer(peer);
+      break;
+
+    case 'EXPIRE': // The offer sent to a peer has expired without response.
+      // TODO: should this be on the DataConnection? It's currently here but I'm not so sure it belongs.
+      this.emit('error', new Error('Could not connect to peer ' + peer));
+      break;
     case 'OFFER': // we should consider switching this to CALL/CONNECT, but this is the least breaking option.
-      var peer = message.src;
       var id = message.id;
       var connection = this._getConnection(peer, id);
 
       if (connection) {
         // Pass it on
+        // TODO: is this just a no-op for DataConnection then? I am under the
+        // impression that only MediaConnection need to renegotiate. I'm not
+        // even sure how a DataConnection would handle renegotiation.
         connection.handleMessage(message);
       } else {
         // Create a new connection.
         if (payload.type === 'call') {
           var call = new MediaConnection(peer, {
             id: id,
-            offer: offer,
-            sdp: payload.sdp
+            offer: payload.sdp
           });
           this._addConnection(peer, call);
           this.emit('call', call);
         } else if (payload.type === 'connect') {
           var connection = new DataConnection(peer, {
             id: id,
-            offer: offer,
-            sdp: payload.sdp,
+            offer: payload.sdp,
             config: this.options.config
           });
           this._addConnection(peer, connection);
@@ -187,7 +197,6 @@ Peer.prototype._handleMessage = function(message) {
       }
       break;
     default:
-      var peer = message.src;
       var id = message.id;
       var connection = this._getConnection(peer, id);
 
@@ -200,12 +209,20 @@ Peer.prototype._handleMessage = function(message) {
   }
 }
 
+/**
+ * Returns a DataConnection to the specified peer. See documentation for a
+ * complete list of options.
+ */
 Peer.prototype.connect = function(peer, options) {
   var connection = new DataConnection(peer, options);
   this._addConnection(peer, connection);
   return connection;
 }
 
+/**
+ * Returns a MediaConnection to the specified peer. See documentation for a
+ * complete list of options.
+ */
 Peer.prototype.call = function(peer, stream, options) {
   if (!stream) {
     util.error('To call a peer, you must provide a stream from your browser\'s `getUserMedia`.');
@@ -217,6 +234,7 @@ Peer.prototype.call = function(peer, stream, options) {
   return call;
 }
 
+/** Add a data/media connection to this peer. */
 Peer.prototype._addConnection = function(peer, connection) {
   if (!this.connections[peer]) {
     this.connections[peer] = [];
@@ -224,6 +242,7 @@ Peer.prototype._addConnection = function(peer, connection) {
   this.connections[peer].push(connection);
 }
 
+/** Retrieve a data/media connection for this peer. */
 Peer.prototype._getConnection = function(peer, id) {
   var connections = this.connections[peer];
   if (!connections) {
@@ -268,18 +287,23 @@ Peer.prototype.destroy = function() {
 }
 
 
-/* Disconnects every connection on this peer. */
+/** Disconnects every connection on this peer. */
 Peer.prototype._cleanup = function() {
   var peers = Object.keys(this.connections);
   for (var i = 0, ii = peers.length; i < ii; i++) {
-    var connections = this.connections[peers[i]];
-    for (var j = 0, jj = connections; j < jj; j++) {
-      connections[j].close();
-    }
+    this._cleanupPeer(peers[i]);
   }
   this.emit('close');
 }
 
+/** Closes all connections to this peer. */
+Peer.prototype._cleanupPeer = function(peer) {
+  var connections = this.connections[peer];
+  for (var j = 0, jj = connections; j < jj; j += 1) {
+    connections[j].close();
+  }
+}
+
 /**
  * Disconnects the Peer's connection to the PeerServer. Does not close any
  *  active connections.