@twilio/webrtc - npm Package Compare versions

Comparing version 2.1.2 to 3.0.0

CHANGELOG.md

@@ -0,1 +1,14 @@
+3.0.0 (August 10, 2018)
+=======================
+
+Breaking Changes
+----------------
+
+- In 2.0.0, calling `removeTrack` in Firefox or Safari didn't actually remove
+  the RTCRtpSender. We did this because we found bugs in the browsers'
+  `removeTrack` behavior; however, shielding applications from that behavior
+  made it difficult to work around those bugs. For example, `removeTrack` works
+  fine in Safari assuming you don't add back the same MediaStreamTrack. On this
+  principle, we updated `removeTrack` to actually call `removeTrack`. (JSDK-1980)
+
 2.1.2 (August 7, 2018)
@@ -2,0 +15,0 @@ ======================

lib/rtcpeerconnection/chrome.js

@@ -133,74 +133,71 @@ /* globals RTCDataChannel, RTCSessionDescription, webkitRTCPeerConnection */
 
-// NOTE(mmalavalli): This shim supports our limited case of adding
-// all MediaStreamTracks to one MediaStream. It has been implemented this
-// keeping in mind that this is to be maintained only until "addTrack" is
-// supported natively in Chrome.
-ChromeRTCPeerConnection.prototype.addTrack = function addTrack() {
-  var args = [].slice.call(arguments);
-  if (this._peerConnection.addTrack) {
-    return this._peerConnection.addTrack.apply(this._peerConnection, args);
-  }
+if (typeof PeerConnection.prototype.addTrack !== 'function') {
+  // NOTE(mmalavalli): This shim supports our limited case of adding
+  // all MediaStreamTracks to one MediaStream. It has been implemented this
+  // keeping in mind that this is to be maintained only until "addTrack" is
+  // supported natively in Chrome.
+  ChromeRTCPeerConnection.prototype.addTrack = function addTrack() {
+    var args = [].slice.call(arguments);
+    var track = args[0];
+    if (this._peerConnection.signalingState === 'closed') {
+      throw new Error('Cannot add MediaStreamTrack [' + track.id + ', '
+        + track.kind + ']: RTCPeerConnection is closed');
+    }
 
-  var track = args[0];
-  if (this._peerConnection.signalingState === 'closed') {
-    throw new Error('Cannot add MediaStreamTrack [' + track.id + ', '
-      + track.kind + ']: RTCPeerConnection is closed');
-  }
+    var sender = this._senders.get(track);
+    if (sender && sender.track) {
+      throw new Error('Cannot add MediaStreamTrack [' + track.id + ', '
+        + track.kind + ']: RTCPeerConnection already has it');
+    }
+    this._peerConnection.removeStream(this._localStream);
+    this._localStream.addTrack(track);
+    this._peerConnection.addStream(this._localStream);
 
-  var sender = this._senders.get(track);
-  if (sender && sender.track) {
-    throw new Error('Cannot add MediaStreamTrack [' + track.id + ', '
-      + track.kind + ']: RTCPeerConnection already has it');
-  }
-  this._peerConnection.removeStream(this._localStream);
-  this._localStream.addTrack(track);
-  this._peerConnection.addStream(this._localStream);
+    sender = new RTCRtpSenderShim(track);
+    this._senders.set(track, sender);
+    return sender;
+  };
 
-  sender = new RTCRtpSenderShim(track);
-  this._senders.set(track, sender);
-  return sender;
-};
+  // NOTE(mmalavalli): This shim supports our limited case of removing
+  // MediaStreamTracks from one MediaStream. It has been implemented this
+  // keeping in mind that this is to be maintained only until "removeTrack" is
+  // supported natively in Chrome.
+  ChromeRTCPeerConnection.prototype.removeTrack = function removeTrack(sender) {
+    if (this._peerConnection.signalingState === 'closed') {
+      throw new Error('Cannot remove MediaStreamTrack: RTCPeerConnection is closed');
+    }
 
-// NOTE(mmalavalli): This shim supports our limited case of removing
-// MediaStreamTracks from one MediaStream. It has been implemented this
-// keeping in mind that this is to be maintained only until "removeTrack" is
-// supported natively in Chrome.
-ChromeRTCPeerConnection.prototype.removeTrack = function removeTrack(sender) {
-  if (this._peerConnection.signalingState === 'closed') {
-    throw new Error('Cannot remove MediaStreamTrack: RTCPeerConnection is closed');
-  }
+    var track = sender.track;
+    if (!track) {
+      return;
+    }
+    sender = this._senders.get(track);
+    if (sender && sender.track) {
+      sender.track = null;
+      this._peerConnection.removeStream(this._localStream);
+      this._localStream.removeTrack(track);
+      this._peerConnection.addStream(this._localStream);
+    }
+  };
 
-  if (this._peerConnection.removeTrack) {
+  ChromeRTCPeerConnection.prototype.getSenders = function getSenders() {
+    return Array.from(this._senders.values());
+  };
+} else {
+  ChromeRTCPeerConnection.prototype.removeTrack = function removeTrack(sender) {
+    if (this._peerConnection.signalingState === 'closed') {
+      throw new Error('Cannot remove MediaStreamTrack: RTCPeerConnection is closed');
+    }
     try {
       this._peerConnection.removeTrack(sender);
+    } catch (e) {
+      // NOTE(mhuynh): Do nothing. In Chrome, will throw if a 'sender was not
+      // created by this peer connection'. This behavior does not seem to be
+      // spec compliant, so a temporary shim is introduced. A bug has been filed,
+      // and is tracked here:
+      // https://bugs.chromium.org/p/chromium/issues/detail?id=860853
     }
-    catch (error) {
-      // NOTE(mhuynh): Do nothing.
-      // In Chrome, will throw a 'sender was not created by this peer connection'
-      // This behavior does not seem to be spec compliant, so a temporary shim
-      // is introduced.
-    }
-    return;
-  }
+  };
+}
 
-  var track = sender.track;
-  if (!track) {
-    return;
-  }
-  sender = this._senders.get(track);
-  if (sender && sender.track) {
-    sender.track = null;
-    this._peerConnection.removeStream(this._localStream);
-    this._localStream.removeTrack(track);
-    this._peerConnection.addStream(this._localStream);
-  }
-};
-
-ChromeRTCPeerConnection.prototype.getSenders = function getSenders() {
-  if (this._peerConnection.getSenders) {
-    return this._peerConnection.getSenders();
-  }
-  return Array.from(this._senders.values());
-};
-
 ChromeRTCPeerConnection.prototype.addIceCandidate = function addIceCandidate(candidate) {
@@ -207,0 +204,0 @@ var args = [].slice.call(arguments);

lib/rtcpeerconnection/firefox.js

@@ -6,3 +6,2 @@ /* globals mozRTCPeerConnection, RTCPeerConnection */
 var FirefoxRTCSessionDescription = require('../rtcsessiondescription/firefox');
-var RTCRtpSenderShim = require('../rtcrtpsender');
 var inherits = require('util').inherits;
@@ -71,5 +70,2 @@ var updateTracksToSSRCs = require('../util/sdp').updateUnifiedPlanTrackIdsToSSRCs;
     },
-    _senders: {
-      value: new Map()
-    },
     _tracksToSSRCs: {
@@ -135,53 +131,11 @@ value: new Map()
 
-// NOTE(mmalavalli): Because we are not delegating to the native
-// RTCPeerConnection#removeTrack(), we have to manually maintain a list of added
-// tracks. So we disable the delegation to the native RTCPeerConnection#addTrack()
-// for now.
-FirefoxRTCPeerConnection.prototype.addTrack = function addTrack() {
-  var args = [].slice.call(arguments);
-  var track = args[0];
-  var sender = this._senders.get(track);
-  if (sender && sender.track) {
-    throw new Error('Cannot add MediaStreamTrack [' + track.id + ', '
-      + track.kind + ']: RTCPeerConnection already has it');
-  }
-  sender = getActiveSenders(this._peerConnection).get(track)
-    || this._peerConnection.addTrack.apply(this._peerConnection, args);
-
-  if (needsWorkaroundForBug1480277) {
+if (needsWorkaroundForBug1480277) {
+  FirefoxRTCPeerConnection.prototype.addTrack = function addTrack() {
+    var track = arguments[0];
+    var sender = this._peerConnection.addTrack.apply(this._peerConnection, arguments);
     sender.replaceTrack(track);
-  }
+    return sender;
+  };
+}
 
-  this._senders.set(track, sender);
-  return sender;
-};
-
-// NOTE(mmalavalli): RTCPeerConnection#removeTrack() has a bug in the
-// Firefox <--> Chrome interop case, which is mentioned below. So we disable
-// its delegation for now. Also, we maintain only one RTCRtpSender per
-// MediaStreamTrack for our use case, and not worry about multiple RTCRtpSenders
-// due to replaceTrack().
-// Bugzilla: https://bugzilla.mozilla.org/show_bug.cgi?id=1133874
-FirefoxRTCPeerConnection.prototype.removeTrack = function removeTrack(sender) {
-  if (this._isClosed) {
-    throw new Error('Cannot remove MediaStreamTrack: RTCPeerConnection is closed');
-  }
-  var track = sender.track;
-  if (!track) {
-    return;
-  }
-  sender = this._senders.get(track);
-  if (sender && sender.track) {
-    this._senders.set(track, new RTCRtpSenderShim(null));
-  }
-};
-
-// NOTE(mmalavalli): Because we are not delegating to the native
-// RTCPeerConnection#removeTrack(), we have to manually maintain a list of added
-// tracks. So we disable the delegation to the native RTCPeerConnection#getSenders()
-// for now.
-FirefoxRTCPeerConnection.prototype.getSenders = function getSenders() {
-  return Array.from(this._senders.values());
-};
-
 FirefoxRTCPeerConnection.prototype.createAnswer = function createAnswer() {
@@ -384,15 +338,2 @@ var args = [].slice.call(arguments);
 
-/**
- * Gets the active RTCRtpSenders of the RTCPeerConnection.
- * @param peerConnection
- * @returns {Map<MediaStreamTrack, RTCRtpSender>}
- */
-function getActiveSenders(peerConnection) {
-  return new Map(peerConnection.getSenders().filter(function(sender) {
-    return sender.track;
-  }).map(function(sender) {
-    return [sender.track, sender];
-  }));
-}
-
 module.exports = FirefoxRTCPeerConnection;

lib/rtcpeerconnection/safari.js

@@ -7,3 +7,2 @@ /* globals RTCPeerConnection, RTCSessionDescription */
 var Latch = require('../util/latch');
-var RTCRtpSenderShim = require('../rtcrtpsender');
 var updateTracksToSSRCs = require('../util/sdp').updatePlanBTrackIdsToSSRCs;
@@ -45,5 +44,2 @@ var util = require('../util');
     },
-    _senders: {
-      value: new Map()
-    },
     _signalingStateLatch: {
@@ -197,2 +193,7 @@ value: new Latch()
 
+SafariRTCPeerConnection.prototype.removeTrack = function removeTrack(sender) {
+  sender.replaceTrack(null);
+  this._peerConnection.removeTrack(sender);
+};
+
 SafariRTCPeerConnection.prototype.setLocalDescription = function setLocalDescription(description) {
@@ -219,55 +220,2 @@ return setDescription(this, true, description);
 
-// NOTE(mmalavalli): Because we are not delegating to the native
-// RTCPeerConnection#removeTrack(), we have to manually maintain a list of added
-// tracks. So we disable the delegation to the native RTCPeerConnection#addTrack()
-// for now. Also, we maintain only one RTCRtpSender per MediaStreamTrack for our
-// use case, and not worry about multiple RTCRtpSenders due to replaceTrack().
-SafariRTCPeerConnection.prototype.addTrack = function addTrack() {
-  var args = [].slice.call(arguments);
-  var track = args[0];
-  var sender = this._senders.get(track);
-  if (sender && sender.track) {
-    throw new Error('Cannot add MediaStreamTrack [' + track.id + ', '
-      + track.kind + ']: RTCPeerConnection already has it');
-  }
-  sender = getActiveSenders(this._peerConnection).get(track)
-    || this._peerConnection.addTrack.apply(this._peerConnection, args);
-
-  // NOTE(mmalavalli): webrtc-adapter has a bug where the "addTrack" shim
-  // does not return an RTCRtpSender and returns undefined instead. An issue
-  // [https://github.com/webrtc/adapter/issues/714] has been filed. For now,
-  // we manually get the RTCRtpSender associated with the added track and
-  // return it.
-  sender = sender || getActiveSenders(this._peerConnection).get(track);
-  this._senders.set(track, sender);
-  return sender;
-};
-
-// NOTE(mroberts): We can't really remove tracks right now, at least if we
-// ever want to add them back...
-//
-//     https://bugs.webkit.org/show_bug.cgi?id=174327
-//
-SafariRTCPeerConnection.prototype.removeTrack = function removeTrack(sender) {
-  if (this._isClosed) {
-    throw new Error('Cannot remove MediaStreamTrack: RTCPeerConnection is closed');
-  }
-  var track = sender.track;
-  if (!track) {
-    return;
-  }
-  sender = this._senders.get(track);
-  if (sender && sender.track) {
-    this._senders.set(track, new RTCRtpSenderShim(null));
-  }
-};
-
-// NOTE(mmalavalli): Because we are not delegating to the native
-// RTCPeerConnection#removeTrack(), we have to manually maintain a list of added
-// tracks. So we disable the delegation to the native RTCPeerConnection#getSenders()
-// for now.
-SafariRTCPeerConnection.prototype.getSenders = function getSenders() {
-  return Array.from(this._senders.values());
-};
-
 util.delegateMethods(
@@ -366,15 +314,2 @@ RTCPeerConnection.prototype,
 
-/**
- * Gets the active RTCRtpSenders of the RTCPeerConnection.
- * @param peerConnection
- * @returns {Map<MediaStreamTrack, RTCRtpSender>}
- */
-function getActiveSenders(peerConnection) {
-  return new Map(peerConnection.getSenders().filter(function(sender) {
-    return sender.track;
-  }).map(function(sender) {
-    return [sender.track, sender];
-  }));
-}
-
 module.exports = SafariRTCPeerConnection;

package.json

 {
   "name": "@twilio/webrtc",
-  "version": "2.1.2",
+  "version": "3.0.0",
   "description": "WebRTC-related APIs and shims used by twilio-video.js",
@@ -46,3 +46,3 @@ "scripts": {
     "karma-mocha": "^1.3.0",
-    "karma-safaritechpreview-launcher": "0.0.6",
+    "karma-safari-launcher": "~0.1",
     "karma-spec-reporter": "0.0.31",
@@ -55,4 +55,4 @@ "mocha": "^3.5.0",
     "watchify": "^3.9.0",
-    "webrtc-adapter": "^6.0.1"
+    "webrtc-adapter": "^6.1.5"
   }
 }

README.md

@@ -83,2 +83,4 @@ twilio-webrtc.js
   We have filed [this bug](https://bugs.chromium.org/p/chromium/issues/detail?id=783433) specifically for this.
+* Provides a workaround for [this bug](https://bugs.chromium.org/p/chromium/issues/detail?id=860853), where calling `removeTrack`
+  with an `RTCRtpSender` that is not created by the `RTCPeerConnection` in question throws an exception.
 
@@ -94,7 +96,2 @@ #### Firefox
   where the browser throws when `RTCPeerConnection.prototype.peerIdentity` is accessed.
-* Provides a shim for the `removeTrack` method in order to work around [this bug](https://bugzilla.mozilla.org/show_bug.cgi?id=1133874).
-* Provides a shim for the `addTrack` method. Since `removeTrack` is shimmed, there is a necessity to
-  maintain an explicit list of added `RTCRtpSender`s.
-* Provides a shim for the `getSenders` method. Since `removeTrack` is shimmed, there is a necessity to
-  maintain an explicit list of added `RTCRtpSender`s.
 * Works around Firefox [Bug 1480277](https://bugzilla.mozilla.org/show_bug.cgi?id=1480277).
@@ -109,9 +106,4 @@
   trying to access the `localDescription` or `remoteDescription` throws an exception.
-* Provides a shim for the `removeTrack` method in order to work around [this bug](https://bugs.webkit.org/show_bug.cgi?id=174327).
-* Provides a shim for the `addTrack` method. Since `removeTrack` is shimmed, there is a necessity to
-  maintain an explicit list of added `RTCRtpSender`s.
 * Provides a workaround for [this bug](https://github.com/webrtc/adapter/issues/714), where webrtc-adapter's shimmed
   `addTrack` method does not return the `RTCRtpSender` associated with the added track.
-* Provides a shim for the `getSenders` method. Since `removeTrack` is shimmed, there is a necessity to
-  maintain an explicit list of added `RTCRtpSender`s.
 
@@ -118,0 +110,0 @@ ### RTCSessionDescription

No alert changes

Worsened metrics

Total package byte prevSize

103616

decreased by-4.21%

Lines of code

2404

decreased by-4.6%

Number of lines in readme file

118

decreased by-6.35%

No dependency changes