Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 1a7ea7ec authored by Brad Ebinger's avatar Brad Ebinger
Browse files

Correctly apply subId changes in ImsServiceController when SIMs change 

In DSDS mode, we were incorrectly calculating which slot to apply subId
changes to when slot 0 had no SIM card and slot 1 moved from having a
SIM card to not having a sim card. In this condition, we would never
clear the sub ID of slot 1, causing a mismatch until re-evaluation of
subscriptions occurred.

This fix corrects this bug by taking into account the condition where
multiple sims have a subId of -1.

Bug: 234562356
Test: atest FrameworksTelephonyTests:ImsServiceControllerTest
Change-Id: I828c4fc59024e84acd5e0be6c4059836aabac6ff
parent 3865c72f

src/java/com/android/internal/telephony/ims/ImsServiceController.java

+23 −23
Original line number Original line Diff line number Diff line
@@ -52,7 +52,6 @@ import com.android.internal.telephony.ExponentialBackoff; 
import com.android.internal.telephony.util.TelephonyUtils;

import com.android.internal.telephony.util.TelephonyUtils;





import java.io.PrintWriter;

import java.io.PrintWriter;

import java.util.ArrayList;

import java.util.HashSet;

import java.util.HashSet;

import java.util.List;

import java.util.List;

import java.util.Set;

import java.util.Set;
@@ -452,19 +451,24 @@ public class ImsServiceController { 
                    SparseIntArray  slotIdToSubIdMap) throws RemoteException {

                    SparseIntArray  slotIdToSubIdMap) throws RemoteException {

        sanitizeFeatureConfig(newImsFeatures);

        sanitizeFeatureConfig(newImsFeatures);

        synchronized (mLock) {

        synchronized (mLock) {

            HashSet<Integer> slotIDs =  new HashSet<>();

            HashSet<Integer> slotIDs = newImsFeatures.stream().map(e -> e.slotId).collect(

            slotIDs.addAll(newImsFeatures.stream().map(e -> e.slotId).collect(Collectors.toSet()));

                    Collectors.toCollection(HashSet::new));

            ArrayList<Integer> changedSubIds = new ArrayList<Integer>();

            // detect which subIds have changed on a per-slot basis

            SparseIntArray changedSubIds = new SparseIntArray(slotIDs.size());

            for (Integer slotID : slotIDs) {

            for (Integer slotID : slotIDs) {

                if (mSlotIdToSubIdMap.get(slotID, PLACEHOLDER_SUBSCRIPTION_ID_BASE)

                int oldSubId = mSlotIdToSubIdMap.get(slotID, PLACEHOLDER_SUBSCRIPTION_ID_BASE);

                        != slotIdToSubIdMap.get(slotID)) {

                int newSubId = slotIdToSubIdMap.get(slotID);

                    changedSubIds.add(slotIdToSubIdMap.get(slotID));

                if (oldSubId != newSubId) {

                    mLocalLog.log("changed sub IDs: " + changedSubIds);

                    changedSubIds.put(slotID, newSubId);

                    Log.i(LOG_TAG, "changed sub IDs: " + changedSubIds);

                    mLocalLog.log("subId changed for slot: " + slotID + ", " + oldSubId + " -> "

                            + newSubId);

                    Log.i(LOG_TAG, "subId changed for slot: " + slotID + ", " + oldSubId + " -> "

                            + newSubId);

                }

                }

            }

            }

            mSlotIdToSubIdMap = slotIdToSubIdMap;

            mSlotIdToSubIdMap = slotIdToSubIdMap;

            if (mImsFeatures.equals(newImsFeatures) && !isSubIdChanged(changedSubIds)) {

            // no change, return early.

            if (mImsFeatures.equals(newImsFeatures) && changedSubIds.size() == 0) {

                return;

                return;

            }

            }

            mLocalLog.log("Features (" + mImsFeatures + "->" + newImsFeatures + ")");

            mLocalLog.log("Features (" + mImsFeatures + "->" + newImsFeatures + ")");
@@ -496,22 +500,22 @@ public class ImsServiceController { 
                        new HashSet<>(mImsFeatures);

                        new HashSet<>(mImsFeatures);

                unchangedFeatures.removeAll(oldFeatures);

                unchangedFeatures.removeAll(oldFeatures);

                unchangedFeatures.removeAll(newFeatures);

                unchangedFeatures.removeAll(newFeatures);

                // ensure remove and add unchanged features that have a slot ID associated with

                // Go through ImsFeatures whose associated subId have changed and recreate them.

                // the new subscription ID.

                if (changedSubIds.size() > 0) {

                if (isSubIdChanged(changedSubIds)) {

                    for (int slotId : changedSubIds.copyKeys()) {

                    for (Integer changedSubId : changedSubIds) {

                        int subId = changedSubIds.get(slotId,

                        int slotId = mSlotIdToSubIdMap.indexOfValue(changedSubId);

                                SubscriptionManager.INVALID_SUBSCRIPTION_ID);

                        HashSet<ImsFeatureConfiguration.FeatureSlotPair>

                        HashSet<ImsFeatureConfiguration.FeatureSlotPair>

                                removeAddFeatures = new HashSet<>();

                                removeAddFeatures = unchangedFeatures.stream()

                        removeAddFeatures.addAll(unchangedFeatures.stream()

                                .filter(e -> e.slotId == slotId).collect(

                                .filter(e -> e.slotId == slotId).collect(Collectors.toSet()));

                                        Collectors.toCollection(HashSet::new));

                        for (ImsFeatureConfiguration.FeatureSlotPair i : removeAddFeatures) {

                        for (ImsFeatureConfiguration.FeatureSlotPair i : removeAddFeatures) {

                            removeImsServiceFeature(i, true);

                            removeImsServiceFeature(i, true);

                        }

                        }

                        for (ImsFeatureConfiguration.FeatureSlotPair i : removeAddFeatures) {

                        for (ImsFeatureConfiguration.FeatureSlotPair i : removeAddFeatures) {

                            long caps = modifyCapabiltiesForSlot(mImsFeatures, i.slotId,

                            long caps = modifyCapabiltiesForSlot(mImsFeatures, i.slotId,

                                    mServiceCapabilities);

                                    mServiceCapabilities);

                            addImsServiceFeature(i, caps, changedSubId);

                            addImsServiceFeature(i, caps, subId);

                        }

                        }

                        unchangedFeatures.removeAll(removeAddFeatures);

                        unchangedFeatures.removeAll(removeAddFeatures);

                    }

                    }
@@ -919,10 +923,6 @@ public class ImsServiceController { 
        AnomalyReporter.reportAnomaly(mAnomalyUUID, message);

        AnomalyReporter.reportAnomaly(mAnomalyUUID, message);

    }

    }





    private boolean isSubIdChanged(ArrayList<Integer> changedSubIds) {

        return !changedSubIds.isEmpty();

    }



    @Override

    @Override

    public String toString() {

    public String toString() {

        synchronized (mLock) {

        synchronized (mLock) {

tests/telephonytests/src/com/android/internal/telephony/ims/ImsServiceControllerTest.java

+62 −0
Original line number Original line Diff line number Diff line
@@ -381,6 +381,68 @@ public class ImsServiceControllerTest extends ImsTestBase { 
        validateMmTelFeatureContainerExists(SLOT_1);

        validateMmTelFeatureContainerExists(SLOT_1);

    }

    }





    /**

     * 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.

     * Tests ImsServiceController keeps SIP delegate creation flags if MMTEL and RCS are supported.

     */

     */