ref: make XMPP ping feature mandatory

There's a plan for using ping to detect XMPP WebSocket
disconnected instead of waiting for timeouts on the network layer.

This also simplifies ICE failed handling logic.

modules/connectivity/IceFailedHandling.js

@@ -91,18 +91,18 @@ export default class IceFailedHandling {
             }, 15000);
             this._delayedIceFailedEvent.start();
 
-            return;
-        } else if (!this._conference.xmpp.isPingSupported()) {
-            // Let Jicofo know that the JVB's ICE connection has failed
-            logger.info('PING not supported - sending ICE failed notification immediately');
-            this._conference.jvbJingleSession.sendIceFailedNotification();
-
             return;
         }
 
-        // The 65 seconds are greater than the default Prosody's BOSH
-        // timeout of 60. This gives some time for the XMPP connection
-        // to recover.
+        //  Using xmpp.ping allows to handle both XMPP being disconnected and internet offline cases. The ping function
+        // uses sendIQ2 method which is resilient to XMPP connection disconnected state and will patiently wait until it
+        // gets reconnected.
+        //  This also handles the case about waiting for the internet to come back online, because ping
+        // will only succeed when the internet is online and then there's a chance for the ICE to recover from FAILED to
+        // CONNECTED which is the extra 2 second timeout after ping.
+        //  The 65 second timeout is given on purpose as there's no chance for XMPP to recover after 65 seconds of no
+        // communication with the server. Such resume attempt will result in unrecoverable conference failed event due
+        // to 'item-not-found' error returned by the server.
         this._conference.xmpp.ping(65000).then(
             () => {
                 if (this._canceled) {

modules/connectivity/IceFailedHandling.spec.js

@@ -82,7 +82,6 @@ describe('IceFailedHandling', () => {
         beforeEach(() => {
             mockConference.options.config.enableIceRestart = true;
             mockConference.xmpp = {
-                isPingSupported: () => true,
                 ping: () => Promise.resolve()
             };
             mockConference.jvbJingleSession = {

modules/xmpp/xmpp.js

@@ -178,15 +178,6 @@ export default class XMPP extends Listenable {
         }
     }
 
-    /**
-     * Returns {@code true} if the PING functionality is supported by the server
-     * or {@code false} otherwise.
-     * @returns {boolean}
-     */
-    isPingSupported() {
-        return this._pingSupported !== false;
-    }
-
     /**
      *
      */
@@ -233,13 +224,15 @@ export default class XMPP extends Listenable {
             // FIXME no need to do it again on stream resume
             this.caps.getFeaturesAndIdentities(pingJid)
                 .then(({ features, identities }) => {
-                    if (features.has(Strophe.NS.PING)) {
-                        this._pingSupported = true;
-                        this.connection.ping.startInterval(pingJid);
-                    } else {
-                        logger.warn(`Ping NOT supported by ${pingJid}`);
+                    if (!features.has(Strophe.NS.PING)) {
+                        logger.error(
+                            `Ping NOT supported by ${pingJid} - please enable ping in your XMPP server config`);
                     }
 
+                    // It counterintuitive to start ping task when it's not supported, but since PING is now mandatory
+                    // it's done on purpose in order to print error logs and bring more attention.
+                    this.connection.ping.startInterval(pingJid);
+
                     // check for speakerstats
                     identities.forEach(identity => {
                         if (identity.type === 'speakerstats') {
@@ -533,20 +526,15 @@ export default class XMPP extends Listenable {
     }
 
     /**
-     * Pings the server. Remember to check {@link isPingSupported} before using
-     * this method.
+     * Pings the server.
      * @param timeout how many ms before a timeout should occur.
      * @returns {Promise} resolved on ping success and reject on an error or
      * a timeout.
      */
     ping(timeout) {
         return new Promise((resolve, reject) => {
-            if (this.isPingSupported()) {
-                this.connection.ping
+            this.connection.ping
                     .ping(this.connection.domain, resolve, reject, timeout);
-            } else {
-                reject('PING operation is not supported by the server');
-            }
         });
     }