Fix creating the UceController instance associated with the invalid subscription ID
Bug: 185322224
Test: atest ImsServiceTest RcsUceAdapterTest
Change-Id: I6eaf69127a8ebe1115b743ffcb29de6e7b2e9970
diff --git a/src/com/android/services/telephony/rcs/RcsFeatureController.java b/src/com/android/services/telephony/rcs/RcsFeatureController.java
index 3eefdb0..7834903 100644
--- a/src/com/android/services/telephony/rcs/RcsFeatureController.java
+++ b/src/com/android/services/telephony/rcs/RcsFeatureController.java
@@ -20,9 +20,7 @@
import android.content.Context;
import android.net.Uri;
import android.telephony.ims.ImsException;
-import android.telephony.ims.ImsRcsManager;
import android.telephony.ims.ImsReasonInfo;
-import android.telephony.ims.RegistrationManager;
import android.telephony.ims.aidl.IImsCapabilityCallback;
import android.telephony.ims.aidl.IImsRegistrationCallback;
import android.telephony.ims.stub.ImsRegistrationImplBase;
@@ -135,6 +133,7 @@
logw("connectionReady returned null RcsFeatureManager");
return;
}
+ logd("connectionReady");
try {
// May throw ImsException if for some reason the connection to the
// ImsService is gone.
@@ -153,6 +152,7 @@
if (reason == FeatureConnector.UNAVAILABLE_REASON_SERVER_UNAVAILABLE) {
loge("unexpected - connectionUnavailable due to server unavailable");
}
+ logd("connectionUnavailable");
// Call before disabling connection to manager.
removeConnectionToService();
updateConnectionStatus(null /*manager*/);
@@ -411,6 +411,7 @@
}
private void setupConnectionToService(RcsFeatureManager manager) throws ImsException {
+ logd("setupConnectionToService");
// Open persistent listener connection, sends RcsFeature#onFeatureReady.
manager.openConnection();
manager.updateCapabilities(mAssociatedSubId);
@@ -418,6 +419,7 @@
}
private void removeConnectionToService() {
+ logd("removeConnectionToService");
RcsFeatureManager manager = getFeatureManager();
if (manager != null) {
manager.unregisterImsRegistrationCallback(
@@ -472,6 +474,10 @@
}
}
+ private void logd(String log) {
+ Log.d(LOG_TAG, getLogPrefix().append(log).toString());
+ }
+
private void logw(String log) {
Log.w(LOG_TAG, getLogPrefix().append(log).toString());
}
diff --git a/src/com/android/services/telephony/rcs/UceControllerManager.java b/src/com/android/services/telephony/rcs/UceControllerManager.java
index cca8330..009700f 100644
--- a/src/com/android/services/telephony/rcs/UceControllerManager.java
+++ b/src/com/android/services/telephony/rcs/UceControllerManager.java
@@ -16,8 +16,10 @@
package com.android.services.telephony.rcs;
+import android.annotation.Nullable;
import android.content.Context;
import android.net.Uri;
+import android.telephony.SubscriptionManager;
import android.telephony.ims.ImsException;
import android.telephony.ims.RcsContactUceCapability;
import android.telephony.ims.RcsUceAdapter;
@@ -52,28 +54,24 @@
private final Context mContext;
private final ExecutorService mExecutorService;
- private volatile int mSubId;
- private volatile UceController mUceController;
- private volatile RcsFeatureManager mRcsFeatureManager;
+ private volatile @Nullable UceController mUceController;
+ private volatile @Nullable RcsFeatureManager mRcsFeatureManager;
public UceControllerManager(Context context, int slotId, int subId) {
Log.d(LOG_TAG, "create: slotId=" + slotId + ", subId=" + subId);
-
mSlotId = slotId;
- mSubId = subId;
mContext = context;
mExecutorService = Executors.newSingleThreadExecutor();
- mUceController = new UceController(mContext, subId);
+ initUceController(subId);
}
/**
* Constructor to inject dependencies for testing.
*/
@VisibleForTesting
- public UceControllerManager(Context context, int slotId, int subId, ExecutorService executor,
+ public UceControllerManager(Context context, int slotId, ExecutorService executor,
UceController uceController) {
mSlotId = slotId;
- mSubId = subId;
mContext = context;
mExecutorService = executor;
mUceController = uceController;
@@ -83,7 +81,11 @@
public void onRcsConnected(RcsFeatureManager manager) {
mExecutorService.submit(() -> {
mRcsFeatureManager = manager;
- mUceController.onRcsConnected(manager);
+ if (mUceController != null) {
+ mUceController.onRcsConnected(manager);
+ } else {
+ Log.d(LOG_TAG, "onRcsConnected: UceController is null");
+ }
});
}
@@ -91,14 +93,22 @@
public void onRcsDisconnected() {
mExecutorService.submit(() -> {
mRcsFeatureManager = null;
- mUceController.onRcsDisconnected();
+ if (mUceController != null) {
+ mUceController.onRcsDisconnected();
+ } else {
+ Log.d(LOG_TAG, "onRcsDisconnected: UceController is null");
+ }
});
}
@Override
public void onDestroy() {
- Log.d(LOG_TAG, "onDestroy");
- mExecutorService.submit(() -> mUceController.onDestroy());
+ mExecutorService.submit(() -> {
+ Log.d(LOG_TAG, "onDestroy");
+ if (mUceController != null) {
+ mUceController.onDestroy();
+ }
+ });
// When the shutdown is called, it will refuse any new tasks and let existing tasks finish.
mExecutorService.shutdown();
}
@@ -108,22 +118,17 @@
* changed.
*/
@Override
- public void onAssociatedSubscriptionUpdated(int subId) {
+ public void onAssociatedSubscriptionUpdated(int newSubId) {
mExecutorService.submit(() -> {
Log.i(LOG_TAG, "onAssociatedSubscriptionUpdated: slotId=" + mSlotId
- + ", subId=" + mSubId + ", newSubId=" + subId);
- if (mSubId == subId) {
- Log.w(LOG_TAG, "onAssociatedSubscriptionUpdated called with the same subId");
- return;
- }
- mSubId = subId;
- // Destroy existing UceController and create a new one.
- mUceController.onDestroy();
- mUceController = new UceController(mContext, subId);
+ + ", newSubId=" + newSubId);
+
+ // Check and create the UceController with the new updated subscription ID.
+ initUceController(newSubId);
// The RCS should be connected when the mRcsFeatureManager is not null. Set it to the
// new UceController instance.
- if (mRcsFeatureManager != null) {
+ if (mUceController != null && mRcsFeatureManager != null) {
mUceController.onRcsConnected(mRcsFeatureManager);
}
});
@@ -136,8 +141,12 @@
@Override
public void onCarrierConfigChanged() {
mExecutorService.submit(() -> {
- Log.i(LOG_TAG, "onCarrierConfigChanged: subId=" + mSubId);
- mUceController.onCarrierConfigChanged();
+ Log.i(LOG_TAG, "onCarrierConfigChanged");
+ if (mUceController != null) {
+ mUceController.onCarrierConfigChanged();
+ } else {
+ Log.d(LOG_TAG, "onCarrierConfigChanged: UceController is null");
+ }
});
}
@@ -390,6 +399,31 @@
}
}
+ /**
+ * Initialize the UceController instance associated with the given subscription ID.
+ * The existing UceController will be destroyed if the original subscription ID is different
+ * from the new subscription ID.
+ * If the new subscription ID is invalid, the UceController instance will be null.
+ */
+ private void initUceController(int newSubId) {
+ Log.d(LOG_TAG, "initUceController: newSubId=" + newSubId + ", current UceController subId="
+ + ((mUceController == null) ? "null" : mUceController.getSubId()));
+ if (mUceController == null) {
+ // Create new UceController only when the subscription ID is valid.
+ if (SubscriptionManager.isValidSubscriptionId(newSubId)) {
+ mUceController = new UceController(mContext, newSubId);
+ }
+ } else if (mUceController.getSubId() != newSubId) {
+ // The subscription ID is updated. Remove the old UceController instance.
+ mUceController.onDestroy();
+ mUceController = null;
+ // Create new UceController only when the subscription ID is valid.
+ if (SubscriptionManager.isValidSubscriptionId(newSubId)) {
+ mUceController = new UceController(mContext, newSubId);
+ }
+ }
+ }
+
private boolean checkUceControllerState() throws ImsException {
if (mUceController == null || mUceController.isUnavailable()) {
throw new ImsException("UCE controller is unavailable",
@@ -398,13 +432,26 @@
return true;
}
+ /**
+ * Get the UceController instance.
+ * <p>
+ * Used for testing ONLY.
+ */
+ @VisibleForTesting
+ public UceController getUceController() {
+ return mUceController;
+ }
@Override
public void dump(PrintWriter printWriter) {
IndentingPrintWriter pw = new IndentingPrintWriter(printWriter, " ");
pw.println("UceControllerManager" + "[" + mSlotId + "]:");
pw.increaseIndent();
- mUceController.dump(pw);
+ if (mUceController != null) {
+ mUceController.dump(pw);
+ } else {
+ pw.println("UceController is null.");
+ }
pw.decreaseIndent();
}
}
diff --git a/tests/src/com/android/services/telephony/rcs/UceControllerManagerTest.java b/tests/src/com/android/services/telephony/rcs/UceControllerManagerTest.java
index 75ddb96..8d719fd 100644
--- a/tests/src/com/android/services/telephony/rcs/UceControllerManagerTest.java
+++ b/tests/src/com/android/services/telephony/rcs/UceControllerManagerTest.java
@@ -17,6 +17,7 @@
package com.android.services.telephony.rcs;
import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNull;
import static org.junit.Assert.fail;
import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.never;
@@ -108,6 +109,21 @@
}
@Test
+ public void testSubIdAndCarrierConfigUpdateWithInvalidSubId() throws Exception {
+ UceControllerManager uceCtrlManager = getUceControllerManager();
+
+ // Updates with the same subId should not destroy the UceController
+ uceCtrlManager.onCarrierConfigChanged();
+ verify(mUceController, never()).onDestroy();
+
+ // Updates with invalid subscription ID
+ uceCtrlManager.onAssociatedSubscriptionUpdated(-1);
+
+ verify(mUceController).onDestroy();
+ assertNull(uceCtrlManager.getUceController());
+ }
+
+ @Test
public void testRequestCapabilitiesWithRcsUnavailable() throws Exception {
UceControllerManager uceCtrlManager = getUceControllerManager();
doReturn(true).when(mUceController).isUnavailable();
@@ -242,7 +258,7 @@
}
private UceControllerManager getUceControllerManager() {
- UceControllerManager manager = new UceControllerManager(mContext, mSlotId, mSubId,
+ UceControllerManager manager = new UceControllerManager(mContext, mSlotId,
mExecutorService, mUceController);
return manager;
}