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

Commit 6af866bd authored by Tyler Gunn's avatar Tyler Gunn
Browse files

Correct call swapping logic to account for connection manager. 

Some connection managers have been known to add connections using their
own phone accounts other than the telephony ones. This causes a problem
when trying to unhold one call and hold another because one call could be
from telephony, and another from the connection manager.

Further, there was an issue where existing connections and conferences
were not being assigned a connection manager.

This bug:
1. Redefines what it means to be from the same "source" as another call.
In the context of telecom we assume that the target phone accounts
are the same package (existing behavior) they're the same source.  We
also assume if the calls have the same package for their connection
manager, they're from the same source.  This allows for scenarios like
the one described above.
2. Ensures existing connections and conferences are assigned a connection
manager phone account.  This is inferred using existing logic in the
PHoneAccountRegistrar which can look up a sim call manager based on the
target phone account for a call.  The logic is enhanced so that if a
target phone account is in fact already a connection manager, it is found
as well.
3. Updates the telecom event logging to log the connection manager, and
also updates telecom Call toString to better log details on the calls.

Test: Performed manual live network testing with connection manager to
ensure correct assignment of connection mgr phone accounts in dumpsys.
Test: Wrote unit test to verify same source matching.
Test: Wrote unit test to verify that the hold/unhold swap logic works when
the connection manager matches but the target phone account does not.
Fixes: 151221531

Change-Id: I263e706e8b6a99a9a79fb63dfc7dc10f4b271eb5
parent 41dbedc8

src/com/android/server/telecom/Call.java

+10 −9
Original line number Diff line number Diff line
@@ -802,21 +802,18 @@ public class Call implements CreateConnectionResponse, EventManager.Loggable, 
    /** {@inheritDoc} */

    @Override

    public String toString() {

        String component = null;

        if (mConnectionService != null && mConnectionService.getComponentName() != null) {

            component = mConnectionService.getComponentName().flattenToShortString();

        }



        return String.format(Locale.US, "[%s, %s, %s, %s, %s, childs(%d), has_parent(%b), %s, %s]",

        return String.format(Locale.US, "[Call id=%s, state=%s, tpac=%s, cmgr=%s, handle=%s, "

                        + "vidst=%s, childs(%d), has_parent(%b), cap=%s, prop=%s]",

                mId,

                CallState.toString(mState),

                component,

                getTargetPhoneAccount(),

                getConnectionManagerPhoneAccount(),

                Log.piiHandle(mHandle),

                getVideoStateDescription(getVideoState()),

                getChildCalls().size(),

                getParentCall() != null,

                Connection.capabilitiesToString(getConnectionCapabilities()),

                Connection.propertiesToString(getConnectionProperties()));

                Connection.capabilitiesToStringShort(getConnectionCapabilities()),

                Connection.propertiesToStringShort(getConnectionProperties()));

    }



    @Override
@@ -844,6 +841,10 @@ public class Call implements CreateConnectionResponse, EventManager.Loggable, 
        } else {

            s.append("not set");

        }

        if (getConnectionManagerPhoneAccount() != null) {

            s.append("\n\tConn mgr: ");

            s.append(getConnectionManagerPhoneAccount());

        }



        s.append("\n\tTo address: ");

        s.append(Log.piiHandle(getHandle()));

src/com/android/server/telecom/CallsManager.java

+46 −14
Original line number Diff line number Diff line
@@ -2607,10 +2607,10 @@ public class CallsManager extends Call.ListenerBase 
                    Log.addEvent(call, LogUtils.Events.SWAP, "From " + activeCall.getId());

                } else {

                    // This call does not support hold. If it is from a different connection

                    // service, then disconnect it, otherwise invoke call.hold() and allow the

                    // connection service to handle the situation.

                    if (!PhoneAccountHandle.areFromSamePackage(activeCall.getTargetPhoneAccount(),

                            call.getTargetPhoneAccount())) {

                    // service or connection manager, then disconnect it, otherwise invoke

                    // call.hold() and allow the connection service or connection manager to handle

                    // the situation.

                    if (!areFromSameSource(activeCall, call)) {

                        if (!activeCall.isEmergencyCall()) {

                            activeCall.disconnect("Swap to " + call.getId());

                        } else {
@@ -2856,11 +2856,11 @@ public class CallsManager extends Call.ListenerBase 
                activeCall.hold();

                return true;

            } else if (supportsHold(activeCall)

                    && PhoneAccountHandle.areFromSamePackage(activeCall.getTargetPhoneAccount(),

                        call.getTargetPhoneAccount())) {

                    && areFromSameSource(activeCall, call)) {



                // Handle the case where the active call and the new call are from the same CS, and

                // the currently active call supports hold but cannot currently be held.

                // Handle the case where the active call and the new call are from the same CS or

                // connection manager, and the currently active call supports hold but cannot

                // currently be held.

                // In this case we'll look for the other held call for this connectionService and

                // disconnect it prior to holding the active call.

                // E.g.
@@ -2882,10 +2882,9 @@ public class CallsManager extends Call.ListenerBase 
                return true;

            } else {

                // This call does not support hold. If it is from a different connection

                // service, then disconnect it, otherwise allow the connection service to

                // figure out the right states.

                if (!PhoneAccountHandle.areFromSamePackage(activeCall.getTargetPhoneAccount(),

                        call.getTargetPhoneAccount())) {

                // service or connection manager, then disconnect it, otherwise allow the connection

                // service or connection manager to figure out the right states.

                if (!areFromSameSource(activeCall, call)) {

                    Log.i(this, "holdActiveCallForNewCall: disconnecting %s so that %s can be "

                            + "made active.", activeCall.getId(), call.getId());

                    if (!activeCall.isEmergencyCall()) {
@@ -3297,6 +3296,9 @@ public class CallsManager extends Call.ListenerBase 
                        Conference.CONNECT_TIME_NOT_SPECIFIED ? 0 :

                        parcelableConference.getConnectElapsedTimeMillis();



        PhoneAccountHandle connectionMgr =

                    mPhoneAccountRegistrar.getSimCallManagerFromHandle(phoneAccount,

                            mCurrentUserHandle);

        Call call = new Call(

                callId,

                mContext,
@@ -3306,7 +3308,7 @@ public class CallsManager extends Call.ListenerBase 
                mPhoneNumberUtilsAdapter,

                null /* handle */,

                null /* gatewayInfo */,

                null /* connectionManagerPhoneAccount */,

                connectionMgr,

                phoneAccount,

                Call.CALL_DIRECTION_UNDEFINED /* callDirection */,

                false /* forceAttachToExistingConnection */,
@@ -4085,6 +4087,10 @@ public class CallsManager extends Call.ListenerBase 
    Call createCallForExistingConnection(String callId, ParcelableConnection connection) {

        boolean isDowngradedConference = (connection.getConnectionProperties()

                & Connection.PROPERTY_IS_DOWNGRADED_CONFERENCE) != 0;



        PhoneAccountHandle connectionMgr =

                mPhoneAccountRegistrar.getSimCallManagerFromHandle(connection.getPhoneAccount(),

                        mCurrentUserHandle);

        Call call = new Call(

                callId,

                mContext,
@@ -4094,7 +4100,7 @@ public class CallsManager extends Call.ListenerBase 
                mPhoneNumberUtilsAdapter,

                connection.getHandle() /* handle */,

                null /* gatewayInfo */,

                null /* connectionManagerPhoneAccount */,

                connectionMgr,

                connection.getPhoneAccount(), /* targetPhoneAccountHandle */

                Call.getRemappedCallDirection(connection.getCallDirection()) /* callDirection */,

                false /* forceAttachToExistingConnection */,
@@ -5110,6 +5116,32 @@ public class CallsManager extends Call.ListenerBase 
                .forEach(c -> c.setVideoCallingSupportedByPhoneAccount(isVideoNowSupported));

    }



    /**

     * Determines if two {@link Call} instances originated from either the same target

     * {@link PhoneAccountHandle} or connection manager {@link PhoneAccountHandle}.

     * @param call1 The first call

     * @param call2 The second call

     * @return {@code true} if both calls are from the same target or connection manager

     * {@link PhoneAccountHandle}.

     */

    public static boolean areFromSameSource(@NonNull Call call1, @NonNull Call call2) {

        PhoneAccountHandle call1ConnectionMgr = call1.getConnectionManagerPhoneAccount();

        PhoneAccountHandle call2ConnectionMgr = call2.getConnectionManagerPhoneAccount();



        if (call1ConnectionMgr != null && call2ConnectionMgr != null

                && PhoneAccountHandle.areFromSamePackage(call1ConnectionMgr, call2ConnectionMgr)) {

            // Both calls share the same connection manager package, so they are from the same

            // source.

            return true;

        }



        PhoneAccountHandle call1TargetAcct = call1.getTargetPhoneAccount();

        PhoneAccountHandle call2TargetAcct = call2.getTargetPhoneAccount();

        // Otherwise if the target phone account for both is the same package, they're the same

        // source.

        return PhoneAccountHandle.areFromSamePackage(call1TargetAcct, call2TargetAcct);

    }



    public LinkedList<HandlerThread> getGraphHandlerThreads() {

        return mGraphHandlerThreads;

    }

src/com/android/server/telecom/PhoneAccountRegistrar.java

+8 −0
Original line number Diff line number Diff line
@@ -508,6 +508,14 @@ public class PhoneAccountRegistrar { 
     */

    public PhoneAccountHandle getSimCallManagerFromHandle(PhoneAccountHandle targetPhoneAccount,

            UserHandle userHandle) {

        // First, check if the specified target phone account handle is a connection manager; if

        // it is, then just return it.

        PhoneAccount phoneAccount = getPhoneAccountUnchecked(targetPhoneAccount);

        if (phoneAccount != null

                && phoneAccount.hasCapabilities(PhoneAccount.CAPABILITY_CONNECTION_MANAGER)) {

            return targetPhoneAccount;

        }



        int subId = getSubscriptionIdForPhoneAccount(targetPhoneAccount);

        if (SubscriptionManager.isValidSubscriptionId(subId)

                 && subId != SubscriptionManager.DEFAULT_SUBSCRIPTION_ID) {

tests/src/com/android/server/telecom/tests/CallsManagerTest.java

+78 −2
Original line number Diff line number Diff line
@@ -120,6 +120,10 @@ public class CallsManagerTest extends TelecomTestCase { 
            ComponentName.unflattenFromString("com.foo/.Blah"), "Sim1");

    private static final PhoneAccountHandle SIM_2_HANDLE = new PhoneAccountHandle(

            ComponentName.unflattenFromString("com.foo/.Blah"), "Sim2");

    private static final PhoneAccountHandle CONNECTION_MGR_1_HANDLE = new PhoneAccountHandle(

            ComponentName.unflattenFromString("com.bar/.Conn"), "Cm1");

    private static final PhoneAccountHandle CONNECTION_MGR_2_HANDLE = new PhoneAccountHandle(

            ComponentName.unflattenFromString("com.spa/.Conn"), "Cm2");

    private static final PhoneAccountHandle VOIP_1_HANDLE = new PhoneAccountHandle(

            ComponentName.unflattenFromString("com.voip/.Stuff"), "Voip1");

    private static final PhoneAccountHandle SELF_MANAGED_HANDLE = new PhoneAccountHandle(
@@ -1239,6 +1243,68 @@ public class CallsManagerTest extends TelecomTestCase { 
        verify(screenedCall).setAudioProcessingRequestingApp(appName);

    }



    /**

     * Verify the behavior of the {@link CallsManager#areFromSameSource(Call, Call)} method.

     * @throws Exception

     */

    @SmallTest

    @Test

    public void testAreFromSameSource() throws Exception {

        Call callSim1 = createCall(SIM_1_HANDLE, null, CallState.ACTIVE);

        Call callSim2 = createCall(SIM_2_HANDLE, null, CallState.ACTIVE);

        Call callVoip1 = createCall(VOIP_1_HANDLE, null, CallState.ACTIVE);

        assertTrue(CallsManager.areFromSameSource(callSim1, callSim1));

        assertTrue(CallsManager.areFromSameSource(callSim1, callSim2));

        assertFalse(CallsManager.areFromSameSource(callSim1, callVoip1));

        assertFalse(CallsManager.areFromSameSource(callSim2, callVoip1));



        Call callSim1ConnectionMgr1 = createCall(SIM_1_HANDLE, CONNECTION_MGR_1_HANDLE,

                CallState.ACTIVE);

        Call callSim2ConnectionMgr2 = createCall(SIM_2_HANDLE, CONNECTION_MGR_2_HANDLE,

                CallState.ACTIVE);

        assertFalse(CallsManager.areFromSameSource(callSim1ConnectionMgr1, callVoip1));

        assertFalse(CallsManager.areFromSameSource(callSim2ConnectionMgr2, callVoip1));

        // Even though the connection manager differs, the underlying telephony CS is the same

        // so hold/swap will still work as expected.

        assertTrue(CallsManager.areFromSameSource(callSim1ConnectionMgr1, callSim2ConnectionMgr2));



        // Sometimes connection managers have been known to also have calls

        Call callConnectionMgr = createCall(CONNECTION_MGR_2_HANDLE, CONNECTION_MGR_2_HANDLE,

                CallState.ACTIVE);

        assertTrue(CallsManager.areFromSameSource(callSim2ConnectionMgr2, callConnectionMgr));

    }



    /**

     * Ensures that if we have two calls hosted by the same connection manager, but with

     * different target phone accounts, we can swap between them.

     * @throws Exception

     */

    @SmallTest

    @Test

    public void testSwapCallsWithSameConnectionMgr() throws Exception {

        // GIVEN a CallsManager with ongoing call, and this call can not be held

        Call ongoingCall = addSpyCall(SIM_1_HANDLE, CONNECTION_MGR_1_HANDLE, CallState.ACTIVE);

        doReturn(false).when(ongoingCall).can(Connection.CAPABILITY_HOLD);

        doReturn(true).when(ongoingCall).can(Connection.CAPABILITY_SUPPORT_HOLD);

        when(mConnectionSvrFocusMgr.getCurrentFocusCall()).thenReturn(ongoingCall);



        // and a held call which has the same connection manager, but a different target phone

        // account.  We have seen cases where a connection mgr adds its own calls and these can

        // be problematic for swapping.

        Call heldCall = addSpyCall(CONNECTION_MGR_1_HANDLE, CONNECTION_MGR_1_HANDLE,

                CallState.ON_HOLD);



        // WHEN unhold the held call

        mCallsManager.unholdCall(heldCall);



        // THEN the ongoing call is held

        verify(ongoingCall).hold(any());

        verifyFocusRequestAndExecuteCallback(heldCall);



        // and held call is unhold now

        verify(heldCall).unhold(any());

    }



    private Call addSpyCall() {

        return addSpyCall(SIM_2_HANDLE, CallState.ACTIVE);

    }
@@ -1248,7 +1314,12 @@ public class CallsManagerTest extends TelecomTestCase { 
    }



    private Call addSpyCall(PhoneAccountHandle targetPhoneAccount, int initialState) {

        Call ongoingCall = createCall(targetPhoneAccount, initialState);

        return addSpyCall(targetPhoneAccount, null, initialState);

    }



    private Call addSpyCall(PhoneAccountHandle targetPhoneAccount,

            PhoneAccountHandle connectionMgrAcct, int initialState) {

        Call ongoingCall = createCall(targetPhoneAccount, connectionMgrAcct, initialState);

        Call callSpy = Mockito.spy(ongoingCall);



        // Mocks some methods to not call the real method.
@@ -1263,6 +1334,11 @@ public class CallsManagerTest extends TelecomTestCase { 
    }



    private Call createCall(PhoneAccountHandle targetPhoneAccount, int initialState) {

        return createCall(targetPhoneAccount, null /* connectionManager */, initialState);

    }



    private Call createCall(PhoneAccountHandle targetPhoneAccount,

            PhoneAccountHandle connectionManagerAccount, int initialState) {

        Call ongoingCall = new Call(String.format("TC@%d", sCallId++), /* callId */

                mContext,

                mCallsManager,
@@ -1271,7 +1347,7 @@ public class CallsManagerTest extends TelecomTestCase { 
                mPhoneNumberUtilsAdapter,

                TEST_ADDRESS,

                null /* GatewayInfo */,

                null /* connectionManagerPhoneAccountHandle */,

                connectionManagerAccount,

                targetPhoneAccount,

                Call.CALL_DIRECTION_INCOMING,

                false /* shouldAttachToExistingConnection*/,