Send signaling messages directly from PeerConnectionWrapper

Note that the thread used to send the message does not change; the
EventBus subscriber mode was "BACKGROUND", but as the messages were
posted from a WebSocket handler (when requesting offers to the HPB) and
peer connection observers (when sending offers/answers and candidates,
both with and without HPB), which run in worker threads rather than in
the main thread, the subscriber was executed in the same thread as
the poster.

For legacy reasons, when the internal signaling server is used the
offers and answers are expected to also provide the nick of the local
participant. When the external signaling server is used the field can be
included, but it is just ignored and not sent to the other clients. As
the local participant nick is a value unrelated to the peer connection
and is only needed with one type of signaling server the messages are
adjusted as needed before being sent rather than handling this inside
the PeerConnectionWrapper.

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

app/src/main/java/com/nextcloud/talk/activities/CallActivity.java

@@ -66,7 +66,6 @@ import com.nextcloud.talk.events.ConfigurationChangeEvent;
 import com.nextcloud.talk.events.MediaStreamEvent;
 import com.nextcloud.talk.events.NetworkEvent;
 import com.nextcloud.talk.events.PeerConnectionEvent;
-import com.nextcloud.talk.events.SessionDescriptionSendEvent;
 import com.nextcloud.talk.events.WebSocketCommunicationEvent;
 import com.nextcloud.talk.models.ExternalSignalingServer;
 import com.nextcloud.talk.models.json.capabilities.CapabilitiesOverall;
@@ -1620,18 +1619,6 @@ public class CallActivity extends CallBaseActivity {
                     performCall();
                 }
                 break;
-            case "peerReadyForRequestingOffer":
-                Log.d(TAG, "onMessageEvent 'peerReadyForRequestingOffer'");
-
-                NCSignalingMessage ncSignalingMessage = new NCSignalingMessage();
-                // "to" property is not actually needed in the "requestoffer" signaling message, but it is used to
-                // set the recipient session ID in the assembled call message.
-                ncSignalingMessage.setTo(webSocketCommunicationEvent.getHashMap().get("sessionId"));
-                ncSignalingMessage.setRoomType("video");
-                ncSignalingMessage.setType("requestoffer");
-
-                signalingMessageSender.send(ncSignalingMessage);
-                break;
         }
     }
 
@@ -1967,7 +1954,8 @@ public class CallActivity extends CallBaseActivity {
                                                                   true,
                                                                   true,
                                                                   type,
-                                                                  signalingMessageReceiver);
+                                                                  signalingMessageReceiver,
+                                                                  signalingMessageSender);
 
             } else if (hasMCU) {
                 peerConnectionWrapper = new PeerConnectionWrapper(peerConnectionFactory,
@@ -1979,7 +1967,8 @@ public class CallActivity extends CallBaseActivity {
                                                                   false,
                                                                   true,
                                                                   type,
-                                                                  signalingMessageReceiver);
+                                                                  signalingMessageReceiver,
+                                                                  signalingMessageSender);
             } else {
                 if (!"screen".equals(type)) {
                     peerConnectionWrapper = new PeerConnectionWrapper(peerConnectionFactory,
@@ -1991,7 +1980,8 @@ public class CallActivity extends CallBaseActivity {
                                                                       false,
                                                                       false,
                                                                       type,
-                                                                      signalingMessageReceiver);
+                                                                      signalingMessageReceiver,
+                                                                      signalingMessageSender);
                 } else {
                     peerConnectionWrapper = new PeerConnectionWrapper(peerConnectionFactory,
                                                                       iceServers,
@@ -2002,7 +1992,8 @@ public class CallActivity extends CallBaseActivity {
                                                                       false,
                                                                       false,
                                                                       type,
-                                                                      signalingMessageReceiver);
+                                                                      signalingMessageReceiver,
+                                                                      signalingMessageSender);
                 }
             }
 
@@ -2246,27 +2237,6 @@ public class CallActivity extends CallBaseActivity {
         }
     }
 
-    @Subscribe(threadMode = ThreadMode.BACKGROUND)
-    public void onMessageEvent(SessionDescriptionSendEvent sessionDescriptionSend) {
-        NCSignalingMessage ncSignalingMessage = new NCSignalingMessage();
-        ncSignalingMessage.setTo(sessionDescriptionSend.getPeerId());
-        ncSignalingMessage.setRoomType(sessionDescriptionSend.getVideoStreamType());
-        ncSignalingMessage.setType(sessionDescriptionSend.getType());
-        NCMessagePayload ncMessagePayload = new NCMessagePayload();
-        ncMessagePayload.setType(sessionDescriptionSend.getType());
-
-        if (!"candidate".equals(sessionDescriptionSend.getType())) {
-            ncMessagePayload.setSdp(sessionDescriptionSend.getSessionDescription().description);
-            ncMessagePayload.setNick(conversationUser.getDisplayName());
-        } else {
-            ncMessagePayload.setIceCandidate(sessionDescriptionSend.getNcIceCandidate());
-        }
-
-        ncSignalingMessage.setPayload(ncMessagePayload);
-
-        signalingMessageSender.send(ncSignalingMessage);
-    }
-
     @Override
     public void onRequestPermissionsResult(int requestCode, @NonNull String[] permissions,
                                            @NonNull int[] grantResults) {
@@ -2607,6 +2577,8 @@ public class CallActivity extends CallBaseActivity {
 
         @Override
         public void send(NCSignalingMessage ncSignalingMessage) {
+            addLocalParticipantNickIfNeeded(ncSignalingMessage);
+
             String serializedNcSignalingMessage;
             try {
                 serializedNcSignalingMessage = LoganSquare.serialize(ncSignalingMessage);
@@ -2663,6 +2635,29 @@ public class CallActivity extends CallBaseActivity {
                     }
                 });
         }
+
+        /**
+         * Adds the local participant nick to offers and answers.
+         *
+         * For legacy reasons the offers and answers sent when the internal signaling server is used are expected to
+         * provide the nick of the local participant.
+         *
+         * @param ncSignalingMessage the message to add the nick to
+         */
+        private void addLocalParticipantNickIfNeeded(NCSignalingMessage ncSignalingMessage) {
+            String type = ncSignalingMessage.getType();
+            if (!"offer".equals(type) && !"answer".equals(type)) {
+                return;
+            }
+
+            NCMessagePayload payload = ncSignalingMessage.getPayload();
+            if (payload == null) {
+                // Broken message, this should not happen
+                return;
+            }
+
+            payload.setNick(conversationUser.getDisplayName());
+        }
     }
 
     private class MicrophoneButtonTouchListener implements View.OnTouchListener {

app/src/main/java/com/nextcloud/talk/events/SessionDescriptionSendEvent.java

@@ -1,129 +0,0 @@
-/*
- * Nextcloud Talk application
- *
- * @author Mario Danic
- * Copyright (C) 2017 Mario Danic <mario@lovelyhq.com>
- *
- * This program is free software: you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation, either version 3 of the License, or
- * at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program.  If not, see <http://www.gnu.org/licenses/>.
- */
-
-package com.nextcloud.talk.events;
-
-import com.nextcloud.talk.models.json.signaling.NCIceCandidate;
-
-import org.webrtc.SessionDescription;
-
-import androidx.annotation.Nullable;
-
-public class SessionDescriptionSendEvent {
-    @Nullable
-    private final SessionDescription sessionDescription;
-    private final String peerId;
-    private final String type;
-    @Nullable
-    private final NCIceCandidate ncIceCandidate;
-    private final String videoStreamType;
-
-    public SessionDescriptionSendEvent(@Nullable SessionDescription sessionDescription, String peerId, String type,
-                                       @Nullable NCIceCandidate ncIceCandidate, @Nullable String videoStreamType) {
-        this.sessionDescription = sessionDescription;
-        this.peerId = peerId;
-        this.type = type;
-        this.ncIceCandidate = ncIceCandidate;
-        this.videoStreamType = videoStreamType;
-    }
-
-    @Nullable
-    public SessionDescription getSessionDescription() {
-        return this.sessionDescription;
-    }
-
-    public String getPeerId() {
-        return this.peerId;
-    }
-
-    public String getType() {
-        return this.type;
-    }
-
-    @Nullable
-    public NCIceCandidate getNcIceCandidate() {
-        return this.ncIceCandidate;
-    }
-
-    public String getVideoStreamType() {
-        return this.videoStreamType;
-    }
-
-    public boolean equals(final Object o) {
-        if (o == this) {
-            return true;
-        }
-        if (!(o instanceof SessionDescriptionSendEvent)) {
-            return false;
-        }
-        final SessionDescriptionSendEvent other = (SessionDescriptionSendEvent) o;
-        if (!other.canEqual((Object) this)) {
-            return false;
-        }
-        final Object this$sessionDescription = this.getSessionDescription();
-        final Object other$sessionDescription = other.getSessionDescription();
-        if (this$sessionDescription == null ? other$sessionDescription != null : !this$sessionDescription.equals(other$sessionDescription)) {
-            return false;
-        }
-        final Object this$peerId = this.getPeerId();
-        final Object other$peerId = other.getPeerId();
-        if (this$peerId == null ? other$peerId != null : !this$peerId.equals(other$peerId)) {
-            return false;
-        }
-        final Object this$type = this.getType();
-        final Object other$type = other.getType();
-        if (this$type == null ? other$type != null : !this$type.equals(other$type)) {
-            return false;
-        }
-        final Object this$ncIceCandidate = this.getNcIceCandidate();
-        final Object other$ncIceCandidate = other.getNcIceCandidate();
-        if (this$ncIceCandidate == null ? other$ncIceCandidate != null : !this$ncIceCandidate.equals(other$ncIceCandidate)) {
-            return false;
-        }
-        final Object this$videoStreamType = this.getVideoStreamType();
-        final Object other$videoStreamType = other.getVideoStreamType();
-
-        return this$videoStreamType == null ? other$videoStreamType == null : this$videoStreamType.equals(other$videoStreamType);
-    }
-
-    protected boolean canEqual(final Object other) {
-        return other instanceof SessionDescriptionSendEvent;
-    }
-
-    public int hashCode() {
-        final int PRIME = 59;
-        int result = 1;
-        final Object $sessionDescription = this.getSessionDescription();
-        result = result * PRIME + ($sessionDescription == null ? 43 : $sessionDescription.hashCode());
-        final Object $peerId = this.getPeerId();
-        result = result * PRIME + ($peerId == null ? 43 : $peerId.hashCode());
-        final Object $type = this.getType();
-        result = result * PRIME + ($type == null ? 43 : $type.hashCode());
-        final Object $ncIceCandidate = this.getNcIceCandidate();
-        result = result * PRIME + ($ncIceCandidate == null ? 43 : $ncIceCandidate.hashCode());
-        final Object $videoStreamType = this.getVideoStreamType();
-        result = result * PRIME + ($videoStreamType == null ? 43 : $videoStreamType.hashCode());
-        return result;
-    }
-
-    public String toString() {
-        return "SessionDescriptionSendEvent(sessionDescription=" + this.getSessionDescription() + ", peerId=" + this.getPeerId() + ", type=" + this.getType() + ", ncIceCandidate=" + this.getNcIceCandidate() + ", videoStreamType=" + this.getVideoStreamType() + ")";
-    }
-}

app/src/main/java/com/nextcloud/talk/webrtc/PeerConnectionWrapper.java

@@ -32,12 +32,13 @@ import com.nextcloud.talk.R;
 import com.nextcloud.talk.application.NextcloudTalkApplication;
 import com.nextcloud.talk.events.MediaStreamEvent;
 import com.nextcloud.talk.events.PeerConnectionEvent;
-import com.nextcloud.talk.events.SessionDescriptionSendEvent;
-import com.nextcloud.talk.events.WebSocketCommunicationEvent;
 import com.nextcloud.talk.models.json.signaling.DataChannelMessage;
 import com.nextcloud.talk.models.json.signaling.DataChannelMessageNick;
 import com.nextcloud.talk.models.json.signaling.NCIceCandidate;
+import com.nextcloud.talk.models.json.signaling.NCMessagePayload;
+import com.nextcloud.talk.models.json.signaling.NCSignalingMessage;
 import com.nextcloud.talk.signaling.SignalingMessageReceiver;
+import com.nextcloud.talk.signaling.SignalingMessageSender;
 
 import org.greenrobot.eventbus.EventBus;
 import org.webrtc.AudioTrack;
@@ -78,6 +79,8 @@ public class PeerConnectionWrapper {
     private final SignalingMessageReceiver signalingMessageReceiver;
     private final WebRtcMessageListener webRtcMessageListener = new WebRtcMessageListener();
 
+    private final SignalingMessageSender signalingMessageSender;
+
     private List<IceCandidate> iceCandidates = new ArrayList<>();
     private PeerConnection peerConnection;
     private String sessionId;
@@ -101,7 +104,8 @@ public class PeerConnectionWrapper {
                                  MediaConstraints mediaConstraints,
                                  String sessionId, String localSession, @Nullable MediaStream localStream,
                                  boolean isMCUPublisher, boolean hasMCU, String videoStreamType,
-                                 SignalingMessageReceiver signalingMessageReceiver) {
+                                 SignalingMessageReceiver signalingMessageReceiver,
+                                 SignalingMessageSender signalingMessageSender) {
 
         Objects.requireNonNull(NextcloudTalkApplication.Companion.getSharedApplication()).getComponentApplication().inject(this);
 
@@ -122,6 +126,8 @@ public class PeerConnectionWrapper {
         this.signalingMessageReceiver = signalingMessageReceiver;
         this.signalingMessageReceiver.addListener(webRtcMessageListener, sessionId, videoStreamType);
 
+        this.signalingMessageSender = signalingMessageSender;
+
         if (peerConnection != null) {
             if (this.localStream != null) {
                 List<String> localStreamIds = Collections.singletonList(this.localStream.getId());
@@ -143,9 +149,10 @@ public class PeerConnectionWrapper {
                 } else if (hasMCU && this.videoStreamType.equals("video")) {
                     // If the connection type is "screen" the client sharing the screen will send an
                     // offer; offers should be requested only for videos.
-                    HashMap<String, String> hashMap = new HashMap<>();
-                    hashMap.put("sessionId", sessionId);
-                    EventBus.getDefault().post(new WebSocketCommunicationEvent("peerReadyForRequestingOffer", hashMap));
+                    // "to" property is not actually needed in the "requestoffer" signaling message, but it is used to
+                    // set the recipient session ID in the assembled call message.
+                    NCSignalingMessage ncSignalingMessage = createBaseSignalingMessage("requestoffer");
+                    signalingMessageSender.send(ncSignalingMessage);
                 } else if (!hasMCU && hasInitiated) {
                     peerConnection.createOffer(magicSdpObserver, mediaConstraints);
                 }
@@ -269,6 +276,15 @@ public class PeerConnectionWrapper {
         return false;
     }
 
+    private NCSignalingMessage createBaseSignalingMessage(String type) {
+        NCSignalingMessage ncSignalingMessage = new NCSignalingMessage();
+        ncSignalingMessage.setTo(sessionId);
+        ncSignalingMessage.setRoomType(videoStreamType);
+        ncSignalingMessage.setType(type);
+
+        return ncSignalingMessage;
+    }
+
     private class WebRtcMessageListener implements SignalingMessageReceiver.WebRtcMessageListener {
 
         public void onOffer(String sdp, String nick) {
@@ -425,12 +441,19 @@ public class PeerConnectionWrapper {
 
         @Override
         public void onIceCandidate(IceCandidate iceCandidate) {
+            NCSignalingMessage ncSignalingMessage = createBaseSignalingMessage("candidate");
+            NCMessagePayload ncMessagePayload = new NCMessagePayload();
+            ncMessagePayload.setType("candidate");
+
             NCIceCandidate ncIceCandidate = new NCIceCandidate();
             ncIceCandidate.setSdpMid(iceCandidate.sdpMid);
             ncIceCandidate.setSdpMLineIndex(iceCandidate.sdpMLineIndex);
             ncIceCandidate.setCandidate(iceCandidate.sdp);
-            EventBus.getDefault().post(new SessionDescriptionSendEvent(null, sessionId,
-                    "candidate", ncIceCandidate, videoStreamType));
+            ncMessagePayload.setIceCandidate(ncIceCandidate);
+
+            ncSignalingMessage.setPayload(ncMessagePayload);
+
+            signalingMessageSender.send(ncSignalingMessage);
         }
 
         @Override
@@ -484,6 +507,12 @@ public class PeerConnectionWrapper {
 
         @Override
         public void onCreateSuccess(SessionDescription sessionDescription) {
+            String type = sessionDescription.type.canonicalForm();
+
+            NCSignalingMessage ncSignalingMessage = createBaseSignalingMessage(type);
+            NCMessagePayload ncMessagePayload = new NCMessagePayload();
+            ncMessagePayload.setType(type);
+
             SessionDescription sessionDescriptionWithPreferredCodec;
             String sessionDescriptionStringWithPreferredCodec = MagicWebRTCUtils.preferCodec
                     (sessionDescription.description,
@@ -492,9 +521,11 @@ public class PeerConnectionWrapper {
                     sessionDescription.type,
                     sessionDescriptionStringWithPreferredCodec);
 
+            ncMessagePayload.setSdp(sessionDescriptionWithPreferredCodec.description);
+
+            ncSignalingMessage.setPayload(ncMessagePayload);
 
-            EventBus.getDefault().post(new SessionDescriptionSendEvent(sessionDescriptionWithPreferredCodec, sessionId,
-                    sessionDescription.type.canonicalForm(), null, videoStreamType));
+            signalingMessageSender.send(ncSignalingMessage);
 
             if (peerConnection != null) {
                 peerConnection.setLocalDescription(magicSdpObserver, sessionDescriptionWithPreferredCodec);