do not automacially hang up failed webrtc sessions

src/main/java/eu/siacs/conversations/ui/RtpSessionActivity.java

@@ -96,7 +96,12 @@ public class RtpSessionActivity extends XmppActivity implements XmppConnectionSe
     );
     private static final List<RtpEndUserState> STATES_SHOWING_SWITCH_TO_CHAT = Arrays.asList(
             RtpEndUserState.CONNECTING,
-            RtpEndUserState.CONNECTED
+            RtpEndUserState.CONNECTED,
+            RtpEndUserState.RECONNECTING
+    );
+    private static final List<RtpEndUserState> STATES_CONSIDERED_CONNECTED = Arrays.asList(
+            RtpEndUserState.CONNECTED,
+            RtpEndUserState.RECONNECTING
     );
     private static final String PROXIMITY_WAKE_LOCK_TAG = "conversations:in-rtp-session";
     private static final int REQUEST_ACCEPT_CALL = 0x1111;
@@ -502,7 +507,7 @@ public class RtpSessionActivity extends XmppActivity implements XmppConnectionSe
 
     private boolean isConnected() {
         final JingleRtpConnection connection = this.rtpConnectionReference != null ? this.rtpConnectionReference.get() : null;
-        return connection != null && connection.getEndUserState() == RtpEndUserState.CONNECTED;
+        return connection != null && STATES_CONSIDERED_CONNECTED.contains(connection.getEndUserState());
     }
 
     private boolean switchToPictureInPicture() {
@@ -661,6 +666,9 @@ public class RtpSessionActivity extends XmppActivity implements XmppConnectionSe
             case CONNECTED:
                 setTitle(R.string.rtp_state_connected);
                 break;
+            case RECONNECTING:
+                setTitle(R.string.rtp_state_reconnecting);
+                break;
             case ACCEPTING_CALL:
                 setTitle(R.string.rtp_state_accepting_call);
                 break;
@@ -803,7 +811,7 @@ public class RtpSessionActivity extends XmppActivity implements XmppConnectionSe
 
     @SuppressLint("RestrictedApi")
     private void updateInCallButtonConfiguration(final RtpEndUserState state, final Set<Media> media) {
-        if (state == RtpEndUserState.CONNECTED && !isPictureInPicture()) {
+        if (STATES_CONSIDERED_CONNECTED.contains(state) && !isPictureInPicture()) {
             Preconditions.checkArgument(media.size() > 0, "Media must not be empty");
             if (media.contains(Media.VIDEO)) {
                 final JingleRtpConnection rtpConnection = requireRtpConnection();
@@ -998,7 +1006,7 @@ public class RtpSessionActivity extends XmppActivity implements XmppConnectionSe
                     RendererCommon.ScalingType.SCALE_ASPECT_FILL,
                     RendererCommon.ScalingType.SCALE_ASPECT_FIT
             );
-            if (state == RtpEndUserState.CONNECTED) {
+            if (STATES_CONSIDERED_CONNECTED.contains(state)) {
                 binding.appBarLayout.setVisibility(View.GONE);
                 getWindow().addFlags(WindowManager.LayoutParams.FLAG_FULLSCREEN);
                 binding.remoteVideoWrapper.setVisibility(View.VISIBLE);

src/main/java/eu/siacs/conversations/xmpp/jingle/JingleRtpConnection.java

@@ -1035,24 +1035,7 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web
                     return RtpEndUserState.CONNECTING;
                 }
             case SESSION_ACCEPTED:
-                //TODO refactor this out into separate method (that uses switch for better readability)
+                return getPeerConnectionStateAsEndUserState();
-                final PeerConnection.PeerConnectionState state;
-                try {
-                    state = webRTCWrapper.getState();
-                } catch (final WebRTCWrapper.PeerConnectionNotInitialized e) {
-                    //We usually close the WebRTCWrapper *before* transitioning so we might still
-                    //be in SESSION_ACCEPTED even though the peerConnection has been torn down
-                    return RtpEndUserState.ENDING_CALL;
-                }
-                if (state == PeerConnection.PeerConnectionState.CONNECTED) {
-                    return RtpEndUserState.CONNECTED;
-                } else if (state == PeerConnection.PeerConnectionState.NEW || state == PeerConnection.PeerConnectionState.CONNECTING) {
-                    return RtpEndUserState.CONNECTING;
-                } else if (state == PeerConnection.PeerConnectionState.CLOSED) {
-                    return RtpEndUserState.ENDING_CALL;
-                } else {
-                    return rtpConnectionStarted == 0 ? RtpEndUserState.CONNECTIVITY_ERROR : RtpEndUserState.CONNECTIVITY_LOST_ERROR;
-                }
             case REJECTED:
             case REJECTED_RACED:
             case TERMINATED_DECLINED_OR_BUSY:
@@ -1082,6 +1065,29 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web
         throw new IllegalStateException(String.format("%s has no equivalent EndUserState", this.state));
     }
 
+
+    private RtpEndUserState getPeerConnectionStateAsEndUserState() {
+        final PeerConnection.PeerConnectionState state;
+        try {
+            state = webRTCWrapper.getState();
+        } catch (final WebRTCWrapper.PeerConnectionNotInitialized e) {
+            //We usually close the WebRTCWrapper *before* transitioning so we might still
+            //be in SESSION_ACCEPTED even though the peerConnection has been torn down
+            return RtpEndUserState.ENDING_CALL;
+        }
+        switch (state) {
+            case CONNECTED:
+                return RtpEndUserState.CONNECTED;
+            case NEW:
+            case CONNECTING:
+                return RtpEndUserState.CONNECTING;
+            case CLOSED:
+                return RtpEndUserState.ENDING_CALL;
+            default:
+                return rtpConnectionStarted == 0 ? RtpEndUserState.CONNECTIVITY_ERROR : RtpEndUserState.RECONNECTING;
+        }
+    }
+
     public Set<Media> getMedia() {
         final State current = getState();
         if (current == State.NULL) {
@@ -1331,29 +1337,31 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web
 
     @Override
     public void onConnectionChange(final PeerConnection.PeerConnectionState oldState, final PeerConnection.PeerConnectionState newState) {
-        Log.d(Config.LOGTAG, id.account.getJid().asBareJid() + ": PeerConnectionState changed: "+oldState+"->" + newState);
+        Log.d(Config.LOGTAG, id.account.getJid().asBareJid() + ": PeerConnectionState changed: " + oldState + "->" + newState);
+        final boolean neverConnected = this.rtpConnectionStarted == 0;
         if (newState == PeerConnection.PeerConnectionState.CONNECTED && this.rtpConnectionStarted == 0) {
             this.rtpConnectionStarted = SystemClock.elapsedRealtime();
         }
         if (newState == PeerConnection.PeerConnectionState.CLOSED && this.rtpConnectionEnded == 0) {
             this.rtpConnectionEnded = SystemClock.elapsedRealtime();
         }
-        //TODO 'failed' means we need to restart ICE
+
-        //
+        if (neverConnected && Arrays.asList(PeerConnection.PeerConnectionState.FAILED, PeerConnection.PeerConnectionState.DISCONNECTED).contains(newState)) {
-        //TODO 'disconnected' can probably be ignored as "This is a less stringent test than failed
-        // and may trigger intermittently and resolve just as spontaneously on less reliable networks,
-        // or during temporary disconnections. When the problem resolves, the connection may return
-        // to the connected state."
-        // Obviously the UI needs to reflect this new state with a 'reconnecting' display or something
-        if (Arrays.asList(PeerConnection.PeerConnectionState.FAILED, PeerConnection.PeerConnectionState.DISCONNECTED).contains(newState)) {
             if (isTerminated()) {
                 Log.d(Config.LOGTAG, id.account.getJid().asBareJid() + ": not sending session-terminate after connectivity error because session is already in state " + this.state);
                 return;
             }
             new Thread(this::closeWebRTCSessionAfterFailedConnection).start();
-        } else {
+        } else if (newState == PeerConnection.PeerConnectionState.FAILED) {
-            updateEndUserState();
+            Log.d(Config.LOGTAG, "attempting to restart ICE");
+            webRTCWrapper.restartIce();
         }
+        updateEndUserState();
+    }
+
+    @Override
+    public void onRenegotiationNeeded() {
+        Log.d(Config.LOGTAG, "onRenegotiationNeeded()");
     }
 
     private void closeWebRTCSessionAfterFailedConnection() {

src/main/java/eu/siacs/conversations/xmpp/jingle/RtpEndUserState.java

@@ -4,6 +4,7 @@ public enum RtpEndUserState {
     INCOMING_CALL, //received a 'propose' message
     CONNECTING, //session-initiate or session-accepted but no webrtc peer connection yet
     CONNECTED, //session-accepted and webrtc peer connection is connected
+    RECONNECTING, //session-accepted and webrtc peer connection was connected once but is currently disconnected or failed
     FINDING_DEVICE, //'propose' has been sent out; no 184 ack yet
     RINGING, //'propose' has been sent out and it has been 184 acked
     ACCEPTING_CALL, //'proceed' message has been sent; but no session-initiate has been received

src/main/java/eu/siacs/conversations/xmpp/jingle/WebRTCWrapper.java

@@ -100,13 +100,14 @@ public class WebRTCWrapper {
 
         @Override
         public void onConnectionChange(final PeerConnection.PeerConnectionState newState) {
-            eventCallback.onConnectionChange(currentState, newState);
+            final PeerConnection.PeerConnectionState oldState = currentState;
             currentState = newState;
+            eventCallback.onConnectionChange(oldState, newState);
         }
 
         @Override
         public void onIceConnectionChange(PeerConnection.IceConnectionState iceConnectionState) {
-
+            Log.d(EXTENDED_LOGGING_TAG, "onIceConnectionChange(" + iceConnectionState + ")");
         }
 
         @Override
@@ -152,7 +153,10 @@ public class WebRTCWrapper {
 
         @Override
         public void onRenegotiationNeeded() {
-            Log.d(EXTENDED_LOGGING_TAG,"onRenegotiationNeeded - current state: "+currentState);
+            Log.d(EXTENDED_LOGGING_TAG, "onRenegotiationNeeded()");
+            if (currentState != null && currentState != PeerConnection.PeerConnectionState.NEW) {
+                eventCallback.onRenegotiationNeeded();
+            }
         }
 
         @Override
@@ -293,6 +297,10 @@ public class WebRTCWrapper {
         this.peerConnection = peerConnection;
     }
 
+    void restartIce() {
+        requirePeerConnection().restartIce();
+    }
+
     synchronized void close() {
         final PeerConnection peerConnection = this.peerConnection;
         final CapturerChoice capturerChoice = this.capturerChoice;
@@ -538,6 +546,8 @@ public class WebRTCWrapper {
         void onConnectionChange(PeerConnection.PeerConnectionState oldState, PeerConnection.PeerConnectionState newState);
 
         void onAudioDeviceChanged(AppRTCAudioManager.AudioDevice selectedAudioDevice, Set<AppRTCAudioManager.AudioDevice> availableAudioDevices);
+
+        void onRenegotiationNeeded();
     }
 
     private static abstract class SetSdpObserver implements SdpObserver {

src/main/res/values/strings.xml

@@ -904,6 +904,7 @@
     <string name="rtp_state_incoming_video_call">Incoming video call</string>
     <string name="rtp_state_connecting">Connecting</string>
     <string name="rtp_state_connected">Connected</string>
+    <string name="rtp_state_reconnecting">Reconnecting</string>
     <string name="rtp_state_accepting_call">Accepting call</string>
     <string name="rtp_state_ending_call">Ending call</string>
     <string name="answer_call">Answer</string>