fix(JingleSession) Remove ice restart option and functionality. (#2586)

JitsiConference.js

@@ -106,8 +106,6 @@ function _getCodecMimeType(codec) {
  * @param {number} [options.config.avgRtpStatsN=15] how many samples are to be
  * collected by {@link AvgRTPStatsReporter}, before arithmetic mean is
  * calculated and submitted to the analytics module.
- * @param {boolean} [options.config.enableIceRestart=false] - enables the ICE
- * restart logic.
  * @param {boolean} [options.config.p2p.enabled] when set to <tt>true</tt>
  * the peer to peer mode will be enabled. It means that when there are only 2
  * participants in the conference an attempt to make direct connection will be

JitsiConferenceEventManager.js

@@ -42,19 +42,6 @@ JitsiConferenceEventManager.prototype.setupChatRoomListeners = function() {
     this.chatRoomForwarder = new EventEmitterForwarder(chatRoom,
         this.conference.eventEmitter);
 
-    chatRoom.addListener(XMPPEvents.ICE_RESTARTING, jingleSession => {
-        if (!jingleSession.isP2P) {
-            // If using DataChannel as bridge channel, it must be closed
-            // before ICE restart, otherwise Chrome will not trigger "opened"
-            // event for the channel established with the new bridge.
-            // TODO: This may be bypassed when using a WebSocket as bridge
-            // channel.
-            conference.rtc.closeBridgeChannel();
-        }
-
-        // else: there are no DataChannels in P2P session (at least for now)
-    });
-
     chatRoom.addListener(XMPPEvents.PARTICIPANT_FEATURES_CHANGED, (from, features) => {
         const participant = conference.getParticipantById(Strophe.getResourceFromJid(from));
 
@@ -64,17 +51,6 @@ JitsiConferenceEventManager.prototype.setupChatRoomListeners = function() {
         }
     });
 
-    chatRoom.addListener(
-        XMPPEvents.ICE_RESTART_SUCCESS,
-        (jingleSession, offerIq) => {
-            // The JVB data chanel needs to be reopened in case the conference
-            // has been moved to a new bridge.
-            !jingleSession.isP2P
-                && conference._setBridgeChannel(
-                    offerIq, jingleSession.peerconnection);
-        });
-
-
     chatRoom.addListener(XMPPEvents.AUDIO_MUTED_BY_FOCUS,
         actor => {
             // TODO: Add a way to differentiate between commands which caused

modules/connectivity/IceFailedHandling.js

@@ -8,13 +8,10 @@ const logger = getLogger(__filename);
 /**
  * This class deals with shenanigans around JVB media session's ICE failed status handling.
  *
- * If ICE restarts are NOT explicitly enabled by the {@code enableIceRestart} config option, then the conference will
- * delay emitting the {@JitsiConferenceErrors.ICE_FAILED} event by 15 seconds. If the network info module reports
- * the internet offline status then the time will start counting after the internet comes back online.
- *
- * If ICE restart are enabled, then a delayed ICE failed notification to Jicofo will be sent, only if the ICE connection
- * does not recover soon after or before the XMPP connection is restored (if it was ever broken). If ICE fails while
- * the XMPP connection is not broken then the notifications will be sent after 2 seconds delay.
+ * If ICE connection is not re-established within 2 secs after the internet comes back online, the client will initiate
+ * a session restart via 'session-terminate'. This results in Jicofo re-inviting the participant into the conference by
+ * recreating the jvb media session so that there is minimla disruption to the user by default. However, if the
+ * 'enableForcedReload' option is set in config.js, the conference will be forcefully reloaded.
  */
 export default class IceFailedHandling {
     /**
@@ -36,23 +33,15 @@ export default class IceFailedHandling {
             return;
         }
 
-        const { enableForcedReload, enableIceRestart } = this._conference.options.config;
-        const explicitlyDisabled = typeof enableIceRestart !== 'undefined' && !enableIceRestart;
-        const supportsRestartByTerminate = this._conference.room.supportsRestartByTerminate();
-        const useTerminateForRestart = supportsRestartByTerminate && !enableIceRestart;
-
-        logger.info('ICE failed,'
-            + ` enableForcedReload: ${enableForcedReload},`
-            + ` enableIceRestart: ${enableIceRestart},`
-            + ` supports restart by terminate: ${supportsRestartByTerminate}`);
+        const { enableForcedReload } = this._conference.options.config;
 
-        if (explicitlyDisabled || (!enableIceRestart && !supportsRestartByTerminate) || enableForcedReload) {
-            logger.info('ICE failed, but ICE restarts are disabled');
-            const reason = enableForcedReload
-                ? JitsiConferenceErrors.CONFERENCE_RESTARTED
-                : JitsiConferenceErrors.ICE_FAILED;
+        logger.info(`ICE failed, enableForcedReload: ${enableForcedReload}`);
 
-            this._conference.eventEmitter.emit(JitsiConferenceEvents.CONFERENCE_FAILED, reason);
+        if (enableForcedReload) {
+            logger.info('ICE failed, force reloading the conference');
+            this._conference.eventEmitter.emit(
+                JitsiConferenceEvents.CONFERENCE_FAILED,
+                JitsiConferenceErrors.CONFERENCE_RESTARTED);
 
             return;
         }
@@ -65,19 +54,13 @@ export default class IceFailedHandling {
         } else if (jvbConnIceState === 'connected') {
             logger.info('ICE connection restored - not sending ICE failed');
         } else {
-            logger.info('Sending ICE failed - the connection did not recover, '
-                + `ICE state: ${jvbConnIceState}, `
-                + `use 'session-terminate': ${useTerminateForRestart}`);
-            if (useTerminateForRestart) {
-                this._conference._stopJvbSession({
-                    reason: 'connectivity-error',
-                    reasonDescription: 'ICE FAILED',
-                    requestRestart: true,
-                    sendSessionTerminate: true
-                });
-            } else {
-                this._conference.jvbJingleSession.sendIceFailedNotification();
-            }
+            logger.info(`Sending ICE failed - the connection did not recover, ICE state: ${jvbConnIceState}`);
+            this._conference._stopJvbSession({
+                reason: 'connectivity-error',
+                reasonDescription: 'ICE FAILED',
+                requestRestart: true,
+                sendSessionTerminate: true
+            });
         }
     }
 

modules/connectivity/IceFailedHandling.spec.js

@@ -36,9 +36,7 @@ describe('IceFailedHandling', () => {
             // eslint-disable-next-line no-empty-function
             emit: () => { }
         };
-        mockConference.room = {
-            supportsRestartByTerminate: () => false
-        };
+        mockConference.room = {};
         mockConference.xmpp = {
             ping: () => Promise.resolve()
         };
@@ -49,83 +47,11 @@ describe('IceFailedHandling', () => {
         jasmine.clock().uninstall();
     });
 
-    describe('when ICE restarts are disabled', () => {
-        beforeEach(() => {
-            mockConference.options.config.enableIceRestart = false;
-        });
-        it('emits ICE failed with 2 seconds delay after XMPP ping comes through', () => {
-            iceFailedHandling.start();
-
-            return nextTick() // tick for ping
-                .then(() => {
-                    expect(emitEventSpy).not.toHaveBeenCalled();
-
-                    return nextTick(2500); // tick for the 2 sec ice timeout
-                })
-                .then(() => {
-                    expect(emitEventSpy).toHaveBeenCalled();
-                });
-        });
-        it('cancel method cancels the ICE failed event', () => {
-            iceFailedHandling.start();
-
-            return nextTick(1000) // tick for ping
-                .then(() => {
-                    expect(emitEventSpy).not.toHaveBeenCalled();
-                    iceFailedHandling.cancel();
-
-                    return nextTick(2500); // tick for ice timeout
-                })
-                .then(() => {
-                    expect(emitEventSpy).not.toHaveBeenCalled();
-                });
-        });
-    });
-    describe('when ICE restart are enabled', () => {
-        let sendIceFailedSpy;
-
-        beforeEach(() => {
-            mockConference.options.config.enableIceRestart = true;
-            mockConference.jvbJingleSession = {
-                getIceConnectionState: () => 'failed',
-                // eslint-disable-next-line no-empty-function
-                sendIceFailedNotification: () => { }
-            };
-            sendIceFailedSpy = spyOn(mockConference.jvbJingleSession, 'sendIceFailedNotification');
-        });
-        it('send ICE failed notification to Jicofo', () => {
-            iceFailedHandling.start();
-
-            return nextTick() // tick for ping
-                .then(() => nextTick(2500)) // tick for ice timeout
-                .then(() => {
-                    expect(sendIceFailedSpy).toHaveBeenCalled();
-                });
-        });
-        it('not send ICE failed notification to Jicofo if canceled', () => {
-            iceFailedHandling.start();
-
-            // first it send ping which is async - need next tick
-            return nextTick(1000)
-                .then(() => {
-                    expect(sendIceFailedSpy).not.toHaveBeenCalled();
-                    iceFailedHandling.cancel();
-
-                    return nextTick(3000); // tick for ice timeout
-                })
-                .then(() => {
-                    expect(sendIceFailedSpy).not.toHaveBeenCalled();
-                });
-        });
-    });
     describe('if Jingle session restarts are supported', () => {
         let sendSessionTerminateSpy;
 
         beforeEach(() => {
-            mockConference.options.config.enableIceRestart = undefined;
-            mockConference.room = {
-                supportsRestartByTerminate: () => true
-            };
+            mockConference.room = {};
             mockConference.jvbJingleSession = {
                 getIceConnectionState: () => 'failed',
                 // eslint-disable-next-line no-empty-function
@@ -148,15 +74,26 @@ describe('IceFailedHandling', () => {
                         });
                 });
         });
+        it('cancel method cancels the call to terminate session', () => {
+            iceFailedHandling.start();
+
+            return nextTick(1000) // tick for ping
+                .then(() => {
+                    expect(sendSessionTerminateSpy).not.toHaveBeenCalled();
+                    iceFailedHandling.cancel();
+
+                    return nextTick(2500); // tick for ice timeout
+                })
+                .then(() => {
+                    expect(sendSessionTerminateSpy).not.toHaveBeenCalled();
+                });
+        });
     });
     describe('when forced reloads are enabled', () => {
         beforeEach(() => {
-            mockConference.options.config.enableIceRestart = undefined;
             mockConference.options.config.enableForcedReload = true;
 
-            mockConference.room = {
-                supportsRestartByTerminate: () => true
-            };
+            mockConference.room = {};
         });
 
         it('emits conference restarted when force reloads are enabled', () => {

modules/proxyconnection/ProxyConnectionPC.js

@@ -225,8 +225,7 @@ export default class ProxyConnectionPC {
             connectionTimes: [],
             eventEmitter: { emit: emitter },
             removeEventListener: () => { /* no op */ },
-            removePresenceListener: () => { /* no-op */ },
-            supportsRestartByTerminate: () => false
+            removePresenceListener: () => { /* no-op */ }
         };
 
         /**

modules/xmpp/ChatRoom.js

@@ -816,13 +816,6 @@ export default class ChatRoom extends Listenable {
                     }
 
                     this.eventEmitter.emit(XMPPEvents.CONFERENCE_PROPERTIES_CHANGED, properties);
-
-                    // Log if Jicofo supports restart by terminate only once. This conference property does not change
-                    // during the call.
-                    if (typeof this.restartByTerminateSupported === 'undefined') {
-                        this.restartByTerminateSupported = properties['support-terminate-restart'] === 'true';
-                        logger.info(`Jicofo supports restart by terminate: ${this.supportsRestartByTerminate()}`);
-                    }
                 }
                 break;
             case 'transcription-status': {
@@ -913,14 +906,6 @@ export default class ChatRoom extends Listenable {
         this.participantPropertyListener = listener;
     }
 
-    /**
-     * Checks if Jicofo supports restarting Jingle session after 'session-terminate'.
-     * @returns {boolean}
-     */
-    supportsRestartByTerminate() {
-        return this.restartByTerminateSupported;
-    }
-
     /**
      *
      * @param node