Merge pull request #6169 from nextcloud/work-around-chromium-bug-of-iceconnectionstate-stuck-as-disconnected

Work around Chromium bug of iceConnectionState stuck as "disconnected"

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)