Work around Chromium bug of iceConnectionState stuck as "disconnected"

Due to a bug in Chromium the "iceConnectionState" of a RTCPeerConnection
may get stuck as "disconnected" even if the connection has already
failed. However, in that case "connectionState" does change to "failed",
so now its listened too to changes in "connectionState" to handle that
case.

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>

5 files changed, 90 insertions, 4 deletions

diff --git a/src/utils/webrtc/analyzers/PeerConnectionAnalyzer.js b/src/utils/webrtc/analyzers/PeerConnectionAnalyzer.js
index d18774cd8..6d89c63d2 100644
--- a/src/utils/webrtc/analyzers/PeerConnectionAnalyzer.js
+++ b/src/utils/webrtc/analyzers/PeerConnectionAnalyzer.js
@@ -136,6 +136,7 @@ function PeerConnectionAnalyzer() {
 	this._getStatsInterval = null
 
 	this._handleIceConnectionStateChangedBound = this._handleIceConnectionStateChanged.bind(this)
+	this._handleConnectionStateChangedBound = this._handleConnectionStateChanged.bind(this)
 	this._processStatsBound = this._processStats.bind(this)
 
 	this._connectionQuality = {
@@ -209,6 +210,7 @@ PeerConnectionAnalyzer.prototype = {
 	setPeerConnection(peerConnection, peerDirection = null) {
 		if (this._peerConnection) {
 			this._peerConnection.removeEventListener('iceconnectionstatechange', this._handleIceConnectionStateChangedBound)
+			this._peerConnection.removeEventListener('connectionstatechange', this._handleConnectionStateChangedBound)
 			this._stopGetStatsInterval()
 		}
 
@@ -220,6 +222,7 @@ PeerConnectionAnalyzer.prototype = {
 
 		if (this._peerConnection) {
 			this._peerConnection.addEventListener('iceconnectionstatechange', this._handleIceConnectionStateChangedBound)
+			this._peerConnection.addEventListener('connectionstatechange', this._handleConnectionStateChangedBound)
 			this._handleIceConnectionStateChangedBound()
 		}
 	},
@@ -265,7 +268,10 @@ PeerConnectionAnalyzer.prototype = {
 		// Note that even if the ICE connection state is "disconnected" the
 		// connection is actually active, media is still transmitted, and the
 		// stats are properly updated.
-		if (!this._peerConnection || (this._peerConnection.iceConnectionState !== 'connected' && this._peerConnection.iceConnectionState !== 'completed' && this._peerConnection.iceConnectionState !== 'disconnected')) {
+		// "connectionState === failed" needs to be checked due to a Chromium
+		// bug in which "iceConnectionState" can get stuck as "disconnected"
+		// even if the connection has already failed.
+		if (!this._peerConnection || (this._peerConnection.iceConnectionState !== 'connected' && this._peerConnection.iceConnectionState !== 'completed' && this._peerConnection.iceConnectionState !== 'disconnected') || this._peerConnection.connectionState === 'failed') {
 			this._setConnectionQualityAudio(CONNECTION_QUALITY.UNKNOWN)
 			this._setConnectionQualityVideo(CONNECTION_QUALITY.UNKNOWN)
 
@@ -295,13 +301,37 @@ PeerConnectionAnalyzer.prototype = {
 		}, 1000)
 	},
 
+	_handleConnectionStateChanged() {
+		if (!this._peerConnection) {
+			return
+		}
+
+		if (this._peerConnection.connectionState !== 'failed') {
+			return
+		}
+
+		if (this._peerConnection.iceConnectionState === 'failed') {
+			return
+		}
+
+		// Work around Chromium bug where "iceConnectionState" never changes
+		// to "failed" (it stays as "disconnected"). When that happens
+		// "connectionState" actually does change to "failed", so the normal
+		// handling of "iceConnectionState === failed" is triggered here.
+
+		this._handleIceConnectionStateChanged()
+	},
+
 	_stopGetStatsInterval() {
 		window.clearInterval(this._getStatsInterval)
 		this._getStatsInterval = null
 	},
 
 	_processStats(stats) {
-		if (!this._peerConnection || (this._peerConnection.iceConnectionState !== 'connected' && this._peerConnection.iceConnectionState !== 'completed' && this._peerConnection.iceConnectionState !== 'disconnected')) {
+		// "connectionState === failed" needs to be checked due to a Chromium
+		// bug in which "iceConnectionState" can get stuck as "disconnected"
+		// even if the connection has already failed.
+		if (!this._peerConnection || (this._peerConnection.iceConnectionState !== 'connected' && this._peerConnection.iceConnectionState !== 'completed' && this._peerConnection.iceConnectionState !== 'disconnected') || this._peerConnection.connectionState === 'failed') {
 			return
 		}
 
diff --git a/src/utils/webrtc/models/CallParticipantModel.js b/src/utils/webrtc/models/CallParticipantModel.js
index 3237bdb9c..2e40ccbd8 100644
--- a/src/utils/webrtc/models/CallParticipantModel.js
+++ b/src/utils/webrtc/models/CallParticipantModel.js
@@ -282,7 +282,13 @@ CallParticipantModel.prototype = {
 		}
 
 		// Reset state that depends on the Peer object.
-		this._handleExtendedIceConnectionStateChange(this.get('peer').pc.iceConnectionState)
+		if (this.get('peer').pc.connectionState === 'failed' && this.get('peer').pc.iceConnectionState === 'disconnected') {
+			// Work around Chromium bug where "iceConnectionState" gets stuck as
+			// "disconnected" even if the connection already failed.
+			this._handleExtendedIceConnectionStateChange(this.get('peer').pc.connectionState)
+		} else {
+			this._handleExtendedIceConnectionStateChange(this.get('peer').pc.iceConnectionState)
+		}
 		this._handlePeerStreamAdded(this.get('peer'))
 
 		this.get('peer').on('extendedIceConnectionStateChange', this._handleExtendedIceConnectionStateChangeBound)
diff --git a/src/utils/webrtc/models/LocalCallParticipantModel.js b/src/utils/webrtc/models/LocalCallParticipantModel.js
index efabab337..f678da597 100644
--- a/src/utils/webrtc/models/LocalCallParticipantModel.js
+++ b/src/utils/webrtc/models/LocalCallParticipantModel.js
@@ -127,7 +127,13 @@ LocalCallParticipantModel.prototype = {
 		}
 
 		// Reset state that depends on the Peer object.
-		this._handleExtendedIceConnectionStateChange(this.get('peer').pc.iceConnectionState)
+		if (this.get('peer').pc.connectionState === 'failed' && this.get('peer').pc.iceConnectionState === 'disconnected') {
+			// Work around Chromium bug where "iceConnectionState" gets stuck as
+			// "disconnected" even if the connection already failed.
+			this._handleExtendedIceConnectionStateChange(this.get('peer').pc.connectionState)
+		} else {
+			this._handleExtendedIceConnectionStateChange(this.get('peer').pc.iceConnectionState)
+		}
 
 		this.get('peer').on('extendedIceConnectionStateChange', this._handleExtendedIceConnectionStateChangeBound)
 	},
diff --git a/src/utils/webrtc/simplewebrtc/peer.js b/src/utils/webrtc/simplewebrtc/peer.js
index 5e40e6d9b..efd9e89be 100644
--- a/src/utils/webrtc/simplewebrtc/peer.js
+++ b/src/utils/webrtc/simplewebrtc/peer.js
@@ -92,6 +92,25 @@ function Peer(options) {
 			break
 		}
 	})
+	this.pc.addEventListener('connectionstatechange', function() {
+		if (self.pc.connectionState !== 'failed') {
+			return
+		}
+
+		if (self.pc.iceConnectionState === 'failed') {
+			return
+		}
+
+		// Work around Chromium bug where "iceConnectionState" never changes to
+		// "failed" (it stays as "disconnected"). When that happens
+		// "connectionState" actually does change to "failed", so the normal
+		// handling of "iceConnectionState === failed" is triggered here.
+
+		if (self.pc.localDescription.type === 'offer') {
+			self.parent.emit('iceFailed', self)
+			self.send('connectivityError')
+		}
+	})
 	this.pc.addEventListener('signalingstatechange', this.emit.bind(this, 'signalingStateChange'))
 	this.logger = this.parent.logger
 
diff --git a/src/utils/webrtc/webrtc.js b/src/utils/webrtc/webrtc.js
index 88af1971b..01850d72d 100644
--- a/src/utils/webrtc/webrtc.js
+++ b/src/utils/webrtc/webrtc.js
@@ -789,6 +789,30 @@ export default function initWebRtc(signaling, _callParticipantCollection, _local
 	/**
 	 * @param peer
 	 */
+	function setHandlerForConnectionStateChange(peer) {
+		peer.pc.addEventListener('connectionstatechange', function() {
+			if (peer.pc.connectionState !== 'failed') {
+				return
+			}
+
+			if (peer.pc.iceConnectionState === 'failed') {
+				return
+			}
+
+			// Work around Chromium bug where "iceConnectionState" never changes
+			// to "failed" (it stays as "disconnected"). When that happens
+			// "connectionState" actually does change to "failed", so the normal
+			// handling of "iceConnectionState === failed" is triggered here.
+
+			peer.emit('extendedIceConnectionStateChange', peer.pc.connectionState)
+
+			handleIceConnectionStateFailed(peer)
+		})
+	}
+
+	/**
+	 * @param peer
+	 */
 	function setHandlerForOwnIceConnectionStateChange(peer) {
 		peer.pc.addEventListener('iceconnectionstatechange', function() {
 			peer.emit('extendedIceConnectionStateChange', peer.pc.iceConnectionState)
@@ -1000,6 +1024,7 @@ export default function initWebRtc(signaling, _callParticipantCollection, _local
 				setHandlerForOwnIceConnectionStateChange(peer)
 			} else {
 				setHandlerForIceConnectionStateChange(peer)
+				setHandlerForConnectionStateChange(peer)
 			}
 
 			setHandlerForNegotiationNeeded(peer)