Cleanup Connections in conference controllers on CDMA<-->GSM change.
When a call switches technology between CDMA and GSM, the
TelephonyConnectionService will end up re-adding it to the appropriate
conference controller due to the original connection change. This is
problematic, as it means that the connection will still be tracked by the
conference controller of its previous technology. Worse yet, if a call
makes a GSM-->CDMA-->GSM transition, it will be added again to the
GSM call tracker.
There is still an underlying issue in that the TelephonyConnection subclass
type doesn't change when the radio technology changes -- that is a larger
issue to solve and should be addressed at some point (ie not now).
To fix this, added code to ensure that connections are removed from other
conference controllers when added, and also to prevent adding duplicates.
Bug: 31288401
Change-Id: I5e95d4f72be6af473eeec1b92f47ad77a783c9b2
(cherry picked from commit be89962c279f43a49fd66d8af8345767a9f23039)
diff --git a/src/com/android/services/telephony/CdmaConferenceController.java b/src/com/android/services/telephony/CdmaConferenceController.java
index 9aee300..0a7c18b 100644
--- a/src/com/android/services/telephony/CdmaConferenceController.java
+++ b/src/com/android/services/telephony/CdmaConferenceController.java
@@ -90,6 +90,12 @@
private CdmaConference mConference;
void add(final CdmaConnection connection) {
+ if (mCdmaConnections.contains(connection)) {
+ // Adding a duplicate realistically shouldn't happen.
+ Log.w(this, "add - connection already tracked; connection=%s", connection);
+ return;
+ }
+
if (!mCdmaConnections.isEmpty() && connection.isOutgoing()) {
// There already exists a connection, so this will probably result in a conference once
// it is added. For outgoing connections which are added while another connection
@@ -127,7 +133,14 @@
recalculateConference();
}
- private void remove(CdmaConnection connection) {
+ void remove(CdmaConnection connection) {
+ if (!mCdmaConnections.contains(connection)) {
+ // Debug only since TelephonyConnectionService tries to clean up the connections tracked
+ // when the original connection changes. It does this proactively.
+ Log.d(this, "remove - connection not tracked; connection=%s", connection);
+ return;
+ }
+
connection.removeConnectionListener(mConnectionListener);
mCdmaConnections.remove(connection);
recalculateConference();
diff --git a/src/com/android/services/telephony/ImsConferenceController.java b/src/com/android/services/telephony/ImsConferenceController.java
index d017a9e..6669482 100644
--- a/src/com/android/services/telephony/ImsConferenceController.java
+++ b/src/com/android/services/telephony/ImsConferenceController.java
@@ -126,6 +126,12 @@
return;
}
+ if (mTelephonyConnections.contains(connection)) {
+ // Adding a duplicate realistically shouldn't happen.
+ Log.w(this, "add - connection already tracked; connection=%s", connection);
+ return;
+ }
+
// Note: Wrap in Log.VERBOSE to avoid calling connection.toString if we are not going to be
// outputting the value.
if (Log.VERBOSE) {
@@ -149,10 +155,18 @@
return;
}
+ if (!mTelephonyConnections.contains(connection)) {
+ // Debug only since TelephonyConnectionService tries to clean up the connections tracked
+ // when the original connection changes. It does this proactively.
+ Log.d(this, "remove - connection not tracked; connection=%s", connection);
+ return;
+ }
+
if (Log.VERBOSE) {
Log.v(this, "remove connection: %s", connection);
}
+ connection.removeConnectionListener(mConnectionListener);
mTelephonyConnections.remove(connection);
recalculateConferenceable();
}
diff --git a/src/com/android/services/telephony/TelephonyConferenceController.java b/src/com/android/services/telephony/TelephonyConferenceController.java
index fbf5ad0..5d59e17 100644
--- a/src/com/android/services/telephony/TelephonyConferenceController.java
+++ b/src/com/android/services/telephony/TelephonyConferenceController.java
@@ -84,12 +84,24 @@
}
void add(TelephonyConnection connection) {
+ if (mTelephonyConnections.contains(connection)) {
+ // Adding a duplicate realistically shouldn't happen.
+ Log.w(this, "add - connection already tracked; connection=%s", connection);
+ return;
+ }
+
mTelephonyConnections.add(connection);
connection.addConnectionListener(mConnectionListener);
recalculate();
}
void remove(Connection connection) {
+ if (!mTelephonyConnections.contains(connection)) {
+ // Debug only since TelephonyConnectionService tries to clean up the connections tracked
+ // when the original connection changes. It does this proactively.
+ Log.d(this, "remove - connection not tracked; connection=%s", connection);
+ return;
+ }
connection.removeConnectionListener(mConnectionListener);
mTelephonyConnections.remove(connection);
recalculate();
diff --git a/src/com/android/services/telephony/TelephonyConnectionService.java b/src/com/android/services/telephony/TelephonyConnectionService.java
index a8f6bca..54cbda7 100644
--- a/src/com/android/services/telephony/TelephonyConnectionService.java
+++ b/src/com/android/services/telephony/TelephonyConnectionService.java
@@ -785,22 +785,32 @@
* @param connection The connection to be added to the controller
*/
public void addConnectionToConferenceController(TelephonyConnection connection) {
- // TODO: Do we need to handle the case of the original connection changing
- // and triggering this callback multiple times for the same connection?
- // If that is the case, we might want to remove this connection from all
- // conference controllers first before re-adding it.
+ // TODO: Need to revisit what happens when the original connection for the
+ // TelephonyConnection changes. If going from CDMA --> GSM (for example), the
+ // instance of TelephonyConnection will still be a CdmaConnection, not a GsmConnection.
+ // The CDMA conference controller makes the assumption that it will only have CDMA
+ // connections in it, while the other conference controllers aren't as restrictive. Really,
+ // when we go between CDMA and GSM we should replace the TelephonyConnection.
if (connection.isImsConnection()) {
Log.d(this, "Adding IMS connection to conference controller: " + connection);
mImsConferenceController.add(connection);
+ mTelephonyConferenceController.remove(connection);
+ if (connection instanceof CdmaConnection) {
+ mCdmaConferenceController.remove((CdmaConnection) connection);
+ }
} else {
int phoneType = connection.getCall().getPhone().getPhoneType();
if (phoneType == TelephonyManager.PHONE_TYPE_GSM) {
Log.d(this, "Adding GSM connection to conference controller: " + connection);
mTelephonyConferenceController.add(connection);
+ if (connection instanceof CdmaConnection) {
+ mCdmaConferenceController.remove((CdmaConnection) connection);
+ }
} else if (phoneType == TelephonyManager.PHONE_TYPE_CDMA &&
connection instanceof CdmaConnection) {
Log.d(this, "Adding CDMA connection to conference controller: " + connection);
- mCdmaConferenceController.add((CdmaConnection)connection);
+ mCdmaConferenceController.add((CdmaConnection) connection);
+ mTelephonyConferenceController.remove(connection);
}
Log.d(this, "Removing connection from IMS conference controller: " + connection);
mImsConferenceController.remove(connection);
