From 5e39d45d9a57eca60efde4b39dd506ab041da95c Mon Sep 17 00:00:00 2001 From: Peter Cai Date: Sat, 12 Mar 2022 21:30:03 -0500 Subject: [PATCH 1/4] ConnectionService: handle connection events on the main thread Not handling these events on the main thread could leave the connection in an inconsistent state. --- .../cheogram/android/ConnectionService.java | 41 ++++++++++++------- 1 file changed, 27 insertions(+), 14 deletions(-) diff --git a/src/cheogram/java/com/cheogram/android/ConnectionService.java b/src/cheogram/java/com/cheogram/android/ConnectionService.java index be497a8b8..c66889a47 100644 --- a/src/cheogram/java/com/cheogram/android/ConnectionService.java +++ b/src/cheogram/java/com/cheogram/android/ConnectionService.java @@ -11,6 +11,8 @@ import com.google.common.base.Joiner; import com.google.common.collect.ImmutableSet; import android.os.Build; +import android.os.Handler; +import android.os.Looper; import android.telecom.CallAudioState; import android.telecom.Connection; import android.telecom.ConnectionRequest; @@ -54,6 +56,8 @@ import eu.siacs.conversations.xmpp.jingle.Media; import eu.siacs.conversations.xmpp.jingle.RtpEndUserState; public class ConnectionService extends android.telecom.ConnectionService { + private final Handler mHandler = new Handler(Looper.getMainLooper()); + public XmppConnectionService xmppConnectionService = null; protected ServiceConnection mConnection = new ServiceConnection() { @Override @@ -271,28 +275,35 @@ public class ConnectionService extends android.telecom.ConnectionService { // so we have to acquire the rtp connection object here this.rtpConnection = xmppConnectionService.getJingleConnectionManager().findJingleRtpConnection(account, with, sessionId); - rtpConnection.get().acceptCall(); + mHandler.post(() -> { + rtpConnection.get().acceptCall(); + }); } @Override public void onReject() { this.rtpConnection = xmppConnectionService.getJingleConnectionManager().findJingleRtpConnection(account, with, sessionId); - rtpConnection.get().rejectCall(); - setDisconnected(new DisconnectCause(DisconnectCause.LOCAL)); + + mHandler.post(() -> { + rtpConnection.get().rejectCall(); + setDisconnected(new DisconnectCause(DisconnectCause.LOCAL)); + }); } @Override public void onDisconnect() { - if (rtpConnection == null || rtpConnection.get() == null) { - xmppConnectionService.getJingleConnectionManager().retractSessionProposal(account, with.asBareJid()); - } else { - rtpConnection.get().endCall(); - } - destroy(); - xmppConnectionService.setDiallerIntegrationActive(false); - xmppConnectionService.removeRtpConnectionUpdateListener( - (XmppConnectionService.OnJingleRtpConnectionUpdate) this - ); + mHandler.post(() -> { + if (rtpConnection == null || rtpConnection.get() == null) { + xmppConnectionService.getJingleConnectionManager().retractSessionProposal(account, with.asBareJid()); + } else { + rtpConnection.get().endCall(); + } + destroy(); + xmppConnectionService.setDiallerIntegrationActive(false); + xmppConnectionService.removeRtpConnectionUpdateListener( + (XmppConnectionService.OnJingleRtpConnectionUpdate) this + ); + }); } @Override @@ -302,7 +313,9 @@ public class ConnectionService extends android.telecom.ConnectionService { @Override public void onPlayDtmfTone(char c) { - rtpConnection.get().applyDtmfTone("" + c); + mHandler.post(() -> { + rtpConnection.get().applyDtmfTone("" + c); + }); } @Override From 259b0e69ad1175b7e099d29e2e9eaae486703b6a Mon Sep 17 00:00:00 2001 From: Peter Cai Date: Sat, 12 Mar 2022 21:38:12 -0500 Subject: [PATCH 2/4] ConnectionService: handle disconnection correctly onDisconnect() is only called when the user *requests* to hang up. If the state is changed by setDisconnected(), then only onStateChanged() will be called. Handle this correctly, otherwise the connection state could be inconsistent (e.g. call is ended by the other side but the Connection object is still around) --- .../cheogram/android/ConnectionService.java | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/src/cheogram/java/com/cheogram/android/ConnectionService.java b/src/cheogram/java/com/cheogram/android/ConnectionService.java index c66889a47..c46560caa 100644 --- a/src/cheogram/java/com/cheogram/android/ConnectionService.java +++ b/src/cheogram/java/com/cheogram/android/ConnectionService.java @@ -290,6 +290,17 @@ public class ConnectionService extends android.telecom.ConnectionService { }); } + @Override + public void onStateChanged(int state) { + if (state == STATE_DISCONNECTED) { + mHandler.post(() -> { + destroy(); + xmppConnectionService.setDiallerIntegrationActive(false); + xmppConnectionService.removeRtpConnectionUpdateListener(this); + }); + } + } + @Override public void onDisconnect() { mHandler.post(() -> { @@ -298,11 +309,8 @@ public class ConnectionService extends android.telecom.ConnectionService { } else { rtpConnection.get().endCall(); } - destroy(); - xmppConnectionService.setDiallerIntegrationActive(false); - xmppConnectionService.removeRtpConnectionUpdateListener( - (XmppConnectionService.OnJingleRtpConnectionUpdate) this - ); + + setDisconnected(new DisconnectCause(DisconnectCause.LOCAL)); }); } From 1f2420b38b017cac0b6273ee31af053f20b32d74 Mon Sep 17 00:00:00 2001 From: Peter Cai Date: Sat, 12 Mar 2022 21:43:36 -0500 Subject: [PATCH 3/4] ConnectionService: remove unneeded setDisconnected() It will be called by the rtp event handler anyway --- src/cheogram/java/com/cheogram/android/ConnectionService.java | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/cheogram/java/com/cheogram/android/ConnectionService.java b/src/cheogram/java/com/cheogram/android/ConnectionService.java index c46560caa..e50a13084 100644 --- a/src/cheogram/java/com/cheogram/android/ConnectionService.java +++ b/src/cheogram/java/com/cheogram/android/ConnectionService.java @@ -286,7 +286,6 @@ public class ConnectionService extends android.telecom.ConnectionService { mHandler.post(() -> { rtpConnection.get().rejectCall(); - setDisconnected(new DisconnectCause(DisconnectCause.LOCAL)); }); } @@ -306,11 +305,10 @@ public class ConnectionService extends android.telecom.ConnectionService { mHandler.post(() -> { if (rtpConnection == null || rtpConnection.get() == null) { xmppConnectionService.getJingleConnectionManager().retractSessionProposal(account, with.asBareJid()); + setDisconnected(new DisconnectCause(DisconnectCause.LOCAL)); } else { rtpConnection.get().endCall(); } - - setDisconnected(new DisconnectCause(DisconnectCause.LOCAL)); }); } From a50b50938ee8b183e7e6fa52197637f6aa46ff86 Mon Sep 17 00:00:00 2001 From: Peter Cai Date: Sat, 12 Mar 2022 21:50:23 -0500 Subject: [PATCH 4/4] ConnectionService: initialize rtpConnection for incoming calls Removes the need for repetitve findJingleRtpConnection() calls for onAnswer() and onReject() --- .../cheogram/android/ConnectionService.java | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/src/cheogram/java/com/cheogram/android/ConnectionService.java b/src/cheogram/java/com/cheogram/android/ConnectionService.java index e50a13084..c67e38b01 100644 --- a/src/cheogram/java/com/cheogram/android/ConnectionService.java +++ b/src/cheogram/java/com/cheogram/android/ConnectionService.java @@ -117,7 +117,7 @@ public class ConnectionService extends android.telecom.ConnectionService { Account account = xmppConnectionService.findAccountByJid(Jid.of(gateway[0])); Jid with = Jid.ofLocalAndDomain(tel, gateway[1]); - CheogramConnection connection = new CheogramConnection(account, with, postDial); + CheogramConnection connection = new CheogramConnection(account, with, postDial, null); PermissionManager permissionManager = PermissionManager.getInstance(this); permissionManager.setNotificationSettings( @@ -167,7 +167,10 @@ public class ConnectionService extends android.telecom.ConnectionService { Account account = xmppConnectionService.findAccountByJid(Jid.of(accountJid)); Jid with = Jid.of(withJid); - CheogramConnection connection = new CheogramConnection(account, with, null); + CheogramConnection connection = new CheogramConnection( + account, with, null, + xmppConnectionService.getJingleConnectionManager().findJingleRtpConnection(account, with, sessionId) + ); connection.setSessionId(sessionId); connection.setAddress( Uri.fromParts("tel", with.getLocal(), null), @@ -186,12 +189,13 @@ public class ConnectionService extends android.telecom.ConnectionService { protected String sessionId = null; protected Stack postDial = new Stack<>(); protected Icon gatewayIcon; - protected WeakReference rtpConnection = null; + protected WeakReference rtpConnection; - CheogramConnection(Account account, Jid with, String postDialString) { + CheogramConnection(Account account, Jid with, String postDialString, WeakReference rtpConnection) { super(); this.account = account; this.with = with; + this.rtpConnection = rtpConnection; gatewayIcon = Icon.createWithBitmap(xmppConnectionService.getAvatarService().get( account.getRoster().getContact(Jid.of(with.getDomain())), @@ -271,10 +275,6 @@ public class ConnectionService extends android.telecom.ConnectionService { @Override public void onAnswer() { - // For incoming calls, a connection update may not have been triggered before answering - // so we have to acquire the rtp connection object here - this.rtpConnection = xmppConnectionService.getJingleConnectionManager().findJingleRtpConnection(account, with, sessionId); - mHandler.post(() -> { rtpConnection.get().acceptCall(); }); @@ -282,8 +282,6 @@ public class ConnectionService extends android.telecom.ConnectionService { @Override public void onReject() { - this.rtpConnection = xmppConnectionService.getJingleConnectionManager().findJingleRtpConnection(account, with, sessionId); - mHandler.post(() -> { rtpConnection.get().rejectCall(); });