Merge "Correctly apply subId changes in ImsServiceController when SIMs change" into tm-dev

2 files changed, 85 insertions, 23 deletions

diff --git a/src/java/com/android/internal/telephony/ims/ImsServiceController.java b/src/java/com/android/internal/telephony/ims/ImsServiceController.java
index 9ae40e1c23..92e7d7117c 100644
--- a/src/java/com/android/internal/telephony/ims/ImsServiceController.java
+++ b/src/java/com/android/internal/telephony/ims/ImsServiceController.java
@@ -52,7 +52,6 @@ import com.android.internal.telephony.ExponentialBackoff;
 import com.android.internal.telephony.util.TelephonyUtils;
 
 import java.io.PrintWriter;
-import java.util.ArrayList;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
@@ -452,19 +451,24 @@ public class ImsServiceController {
                     SparseIntArray  slotIdToSubIdMap) throws RemoteException {
         sanitizeFeatureConfig(newImsFeatures);
         synchronized (mLock) {
-            HashSet<Integer> slotIDs =  new HashSet<>();
-            slotIDs.addAll(newImsFeatures.stream().map(e -> e.slotId).collect(Collectors.toSet()));
-            ArrayList<Integer> changedSubIds = new ArrayList<Integer>();
+            HashSet<Integer> slotIDs = newImsFeatures.stream().map(e -> e.slotId).collect(
+                    Collectors.toCollection(HashSet::new));
+            // detect which subIds have changed on a per-slot basis
+            SparseIntArray changedSubIds = new SparseIntArray(slotIDs.size());
             for (Integer slotID : slotIDs) {
-                if (mSlotIdToSubIdMap.get(slotID, PLACEHOLDER_SUBSCRIPTION_ID_BASE)
-                        != slotIdToSubIdMap.get(slotID)) {
-                    changedSubIds.add(slotIdToSubIdMap.get(slotID));
-                    mLocalLog.log("changed sub IDs: " + changedSubIds);
-                    Log.i(LOG_TAG, "changed sub IDs: " + changedSubIds);
+                int oldSubId = mSlotIdToSubIdMap.get(slotID, PLACEHOLDER_SUBSCRIPTION_ID_BASE);
+                int newSubId = slotIdToSubIdMap.get(slotID);
+                if (oldSubId != newSubId) {
+                    changedSubIds.put(slotID, newSubId);
+                    mLocalLog.log("subId changed for slot: " + slotID + ", " + oldSubId + " -> "
+                            + newSubId);
+                    Log.i(LOG_TAG, "subId changed for slot: " + slotID + ", " + oldSubId + " -> "
+                            + newSubId);
                 }
             }
             mSlotIdToSubIdMap = slotIdToSubIdMap;
-            if (mImsFeatures.equals(newImsFeatures) && !isSubIdChanged(changedSubIds)) {
+            // no change, return early.
+            if (mImsFeatures.equals(newImsFeatures) && changedSubIds.size() == 0) {
                 return;
             }
             mLocalLog.log("Features (" + mImsFeatures + "->" + newImsFeatures + ")");
@@ -496,22 +500,22 @@ public class ImsServiceController {
                         new HashSet<>(mImsFeatures);
                 unchangedFeatures.removeAll(oldFeatures);
                 unchangedFeatures.removeAll(newFeatures);
-                // ensure remove and add unchanged features that have a slot ID associated with
-                // the new subscription ID.
-                if (isSubIdChanged(changedSubIds)) {
-                    for (Integer changedSubId : changedSubIds) {
-                        int slotId = mSlotIdToSubIdMap.indexOfValue(changedSubId);
+                // Go through ImsFeatures whose associated subId have changed and recreate them.
+                if (changedSubIds.size() > 0) {
+                    for (int slotId : changedSubIds.copyKeys()) {
+                        int subId = changedSubIds.get(slotId,
+                                SubscriptionManager.INVALID_SUBSCRIPTION_ID);
                         HashSet<ImsFeatureConfiguration.FeatureSlotPair>
-                                removeAddFeatures = new HashSet<>();
-                        removeAddFeatures.addAll(unchangedFeatures.stream()
-                                .filter(e -> e.slotId == slotId).collect(Collectors.toSet()));
+                                removeAddFeatures = unchangedFeatures.stream()
+                                .filter(e -> e.slotId == slotId).collect(
+                                        Collectors.toCollection(HashSet::new));
                         for (ImsFeatureConfiguration.FeatureSlotPair i : removeAddFeatures) {
                             removeImsServiceFeature(i, true);
                         }
                         for (ImsFeatureConfiguration.FeatureSlotPair i : removeAddFeatures) {
                             long caps = modifyCapabiltiesForSlot(mImsFeatures, i.slotId,
                                     mServiceCapabilities);
-                            addImsServiceFeature(i, caps, changedSubId);
+                            addImsServiceFeature(i, caps, subId);
                         }
                         unchangedFeatures.removeAll(removeAddFeatures);
                     }
@@ -919,10 +923,6 @@ public class ImsServiceController {
         AnomalyReporter.reportAnomaly(mAnomalyUUID, message);
     }
 
-    private boolean isSubIdChanged(ArrayList<Integer> changedSubIds) {
-        return !changedSubIds.isEmpty();
-    }
-
     @Override
     public String toString() {
         synchronized (mLock) {
diff --git a/tests/telephonytests/src/com/android/internal/telephony/ims/ImsServiceControllerTest.java b/tests/telephonytests/src/com/android/internal/telephony/ims/ImsServiceControllerTest.java
index 083fac98be..113829f88b 100644
--- a/tests/telephonytests/src/com/android/internal/telephony/ims/ImsServiceControllerTest.java
+++ b/tests/telephonytests/src/com/android/internal/telephony/ims/ImsServiceControllerTest.java
@@ -382,6 +382,68 @@ public class ImsServiceControllerTest extends ImsTestBase {
     }
 
     /**
+     * Ensures ImsServiceController correctly removes the existing MmTelFeature and creates an
+     * emergency only MmTelFeature when slot 0 has no subscription and the sim card is removed for
+     * slot 1.
+     */
+    @SmallTest
+    @Test
+    public void testCallChangeWithNoNewFeaturesWithSlot1SubIdChanged()
+            throws RemoteException {
+        HashSet<ImsFeatureConfiguration.FeatureSlotPair> testFeatures = new HashSet<>();
+        testFeatures.add(new ImsFeatureConfiguration.FeatureSlotPair(SLOT_0,
+                ImsFeature.FEATURE_EMERGENCY_MMTEL));
+        testFeatures.add(new ImsFeatureConfiguration.FeatureSlotPair(SLOT_0,
+                ImsFeature.FEATURE_MMTEL));
+        testFeatures.add(new ImsFeatureConfiguration.FeatureSlotPair(SLOT_1,
+                ImsFeature.FEATURE_EMERGENCY_MMTEL));
+        testFeatures.add(new ImsFeatureConfiguration.FeatureSlotPair(SLOT_1,
+                ImsFeature.FEATURE_MMTEL));
+        SparseIntArray slotIdToSubIdMap = new SparseIntArray();
+        // invalid subid in slot 0
+        slotIdToSubIdMap.put(SLOT_0, SubscriptionManager.INVALID_SUBSCRIPTION_ID);
+        // valid subId in slot 1
+        slotIdToSubIdMap.put(SLOT_1, SUB_3);
+        bindAndConnectService(testFeatures, slotIdToSubIdMap.clone());
+        verify(mMockServiceControllerBinder).createEmergencyOnlyMmTelFeature(SLOT_0);
+        verify(mMockServiceControllerBinder).addFeatureStatusCallback(eq(SLOT_0),
+                eq(ImsFeature.FEATURE_MMTEL), any());
+        verify(mMockCallbacks).imsServiceFeatureCreated(eq(SLOT_0), eq(ImsFeature.FEATURE_MMTEL),
+                eq(mTestImsServiceController));
+        verify(mMockServiceControllerBinder).createMmTelFeature(SLOT_1, SUB_3);
+        verify(mMockServiceControllerBinder).addFeatureStatusCallback(eq(SLOT_1),
+                eq(ImsFeature.FEATURE_MMTEL), any());
+        verify(mMockCallbacks).imsServiceFeatureCreated(eq(SLOT_1), eq(ImsFeature.FEATURE_MMTEL),
+                eq(mTestImsServiceController));
+        validateMmTelFeatureContainerExistsWithEmergency(SLOT_0);
+        validateMmTelFeatureContainerExistsWithEmergency(SLOT_1);
+
+        slotIdToSubIdMap.put(SLOT_0, SubscriptionManager.INVALID_SUBSCRIPTION_ID);
+        slotIdToSubIdMap.put(SLOT_1, SubscriptionManager.INVALID_SUBSCRIPTION_ID);
+
+        // ensure only slot 1 gets replaced with emergency only MmTelFeature.
+        mTestImsServiceController.changeImsServiceFeatures(testFeatures,
+                slotIdToSubIdMap.clone());
+        verify(mMockServiceControllerBinder).removeImsFeature(eq(SLOT_1),
+                eq(ImsFeature.FEATURE_MMTEL), eq(true));
+        verify(mMockServiceControllerBinder).removeFeatureStatusCallback(eq(SLOT_1),
+                eq(ImsFeature.FEATURE_MMTEL), any());
+        verify(mMockCallbacks).imsServiceFeatureRemoved(eq(SLOT_1), eq(ImsFeature.FEATURE_MMTEL),
+                eq(mTestImsServiceController));
+
+        verify(mMockServiceControllerBinder).createEmergencyOnlyMmTelFeature(SLOT_1);
+        verify(mMockServiceControllerBinder, times(2)).addFeatureStatusCallback(eq(SLOT_1),
+                eq(ImsFeature.FEATURE_MMTEL), any());
+        verify(mMockCallbacks, times(2)).imsServiceFeatureCreated(eq(SLOT_1),
+                eq(ImsFeature.FEATURE_MMTEL), eq(mTestImsServiceController));
+        validateMmTelFeatureContainerExistsWithEmergency(SLOT_0);
+        validateMmTelFeatureContainerExistsWithEmergency(SLOT_1);
+
+        // this should not have been called again since it did not change (times = 1)
+        verify(mMockServiceControllerBinder, times(1)).createEmergencyOnlyMmTelFeature(SLOT_0);
+    }
+
+    /**
      * Tests ImsServiceController keeps SIP delegate creation flags if MMTEL and RCS are supported.
      */
     @SmallTest