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

Commit 32237a07 authored by Thomas Stuart's avatar Thomas Stuart
Browse files

add transactional call state verifier 

A bugreport came in that demonstrated if a self-managed call does not
call Connection#setOnHold in Connection#hold, a new sim call can drop.
Telecom does not verify call state changes for ConnectionService calls
and it's assumed that self-managed implementations are correct.
However, since the consequence of a bad implmentation results in a
dropped sim call there is now a need to verify call state changes.

This change adds the capability for Telecom to verify callback call
state changes. This specific change verifies that when Telecom requests
a hold, the app hosting the call has 2 seconds to call setOnHold or the
call will be disconnected.

Test: 2 unit tests + 2 manual tests:
        Test fail case:
	(1) comment out setOnHold() in SelfManagedConnection#onHold()
        (2) start a SM call in the Self-Managed Sample APP
        (3) set the call active
        (4) open the Test InCall UI activity
        (5) click the hold button
        expect: The call to be disconnected b/c setOnHold is not called
	Test pass case:
        (1) start a SM call in the Self-Managed Sample APP
        (2) set the call active
        (3) open the Test InCall UI activity
        (4) click the hold button
        expect: The call to be held and not disconnected. Also
           The underlying transaction to be successful and listener
	   to be removed.
Fixes: b/267234374
Change-Id: I30908fdce852112bd4ccbe6162bbd541d546e8a6
parent 7f80950c

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

+53 −0
Original line number Diff line number Diff line
@@ -30,6 +30,7 @@ import android.os.Build; 
import android.os.Bundle;

import android.os.Handler;

import android.os.Looper;

import android.os.OutcomeReceiver;

import android.os.ParcelFileDescriptor;

import android.os.Parcelable;

import android.os.RemoteException;
@@ -43,6 +44,7 @@ import android.telecom.CallAttributes; 
import android.telecom.CallAudioState;

import android.telecom.CallDiagnosticService;

import android.telecom.CallDiagnostics;

import android.telecom.CallException;

import android.telecom.CallerInfo;

import android.telecom.Conference;

import android.telecom.Connection;
@@ -73,6 +75,9 @@ import com.android.internal.util.Preconditions; 
import com.android.server.telecom.stats.CallFailureCause;

import com.android.server.telecom.stats.CallStateChangedAtomWriter;

import com.android.server.telecom.ui.ToastFactory;

import com.android.server.telecom.voip.TransactionManager;

import com.android.server.telecom.voip.VerifyCallStateChangeTransaction;

import com.android.server.telecom.voip.VoipCallTransactionResult;



import java.io.IOException;

import java.text.SimpleDateFormat;
@@ -118,6 +123,24 @@ public class Call implements CreateConnectionResponse, EventManager.Loggable, 


    private static final char NO_DTMF_TONE = '\0';





    /**

     * Listener for CallState changes which can be leveraged by a Transaction.

     */

    public interface CallStateListener {

        void onCallStateChanged(int newCallState);

    }



    public List<CallStateListener> mCallStateListeners = new ArrayList<>();



    public void addCallStateListener(CallStateListener newListener) {

        mCallStateListeners.add(newListener);

    }



    public boolean removeCallStateListener(CallStateListener newListener) {

        return mCallStateListeners.remove(newListener);

    }



    /**

     * Listener for events on the call.

     */
@@ -1328,6 +1351,10 @@ public class Call implements CreateConnectionResponse, EventManager.Loggable, 
                Log.addEvent(this, event, stringData);

            }



            for (CallStateListener listener : mCallStateListeners) {

                listener.onCallStateChanged(newState);

            }



            mCallStateChangedAtomWriter

                    .setDisconnectCause(getDisconnectCause())

                    .setSelfManaged(isSelfManaged())
@@ -2898,11 +2925,16 @@ public class Call implements CreateConnectionResponse, EventManager.Loggable, 
        hold(null /* reason */);

    }



    /**

     * This method requests the ConnectionService or TransactionalService hosting the call to put

     * the call on hold

     */

    public void hold(String reason) {

        if (mState == CallState.ACTIVE) {

            if (mTransactionalService != null) {

                mTransactionalService.onSetInactive(this);

            } else if (mConnectionService != null) {

                awaitCallStateChangeAndMaybeDisconnectCall(CallState.ON_HOLD, isSelfManaged(), "hold");

                mConnectionService.hold(this);

            } else {

                Log.e(this, new NullPointerException(),
@@ -2912,6 +2944,27 @@ public class Call implements CreateConnectionResponse, EventManager.Loggable, 
        }

    }



    /**

     * helper that can be used for any callback that requests a call state change and wants to

     * verify the change

     */

    public void awaitCallStateChangeAndMaybeDisconnectCall(int targetCallState,

            boolean shouldDisconnectUponTimeout, String callingMethod) {

        TransactionManager tm = TransactionManager.getInstance();

        tm.addTransaction(new VerifyCallStateChangeTransaction(mCallsManager,

                this, targetCallState, shouldDisconnectUponTimeout), new OutcomeReceiver<>() {

            @Override

            public void onResult(VoipCallTransactionResult result) {

            }



            @Override

            public void onError(CallException e) {

                Log.i(this, "awaitCallStateChangeAndMaybeDisconnectCall: %s: onError"

                        + " due to CallException=[%s]", callingMethod, e);

            }

        });

    }



    /**

     * Releases the call from hold if it is currently active.

     */

src/com/android/server/telecom/voip/VerifyCallStateChangeTransaction.java

0 → 100644
+147 −0
Original line number Diff line number Diff line 
/*

 * Copyright (C) 2023 The Android Open Source Project

 *

 * Licensed under the Apache License, Version 2.0 (the "License");

 * you may not use this file except in compliance with the License.

 * You may obtain a copy of the License at

 *

 *      http://www.apache.org/licenses/LICENSE-2.0

 *

 * Unless required by applicable law or agreed to in writing, software

 * distributed under the License is distributed on an "AS IS" BASIS,

 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.

 * See the License for the specific language governing permissions and

 * limitations under the License.

 */



package com.android.server.telecom.voip;



import com.android.internal.annotations.VisibleForTesting;

import com.android.server.telecom.Call;

import com.android.server.telecom.CallsManager;



import android.telecom.DisconnectCause;

import android.telecom.Log;



import java.util.concurrent.CompletableFuture;

import java.util.concurrent.CompletionStage;

import java.util.concurrent.TimeUnit;



/**

 * VerifyCallStateChangeTransaction is a transaction that verifies a CallState change and has

 * the ability to disconnect if the CallState is not changed within the timeout window.

 * <p>

 * Note: This transaction has a timeout of 2 seconds.

 */

public class VerifyCallStateChangeTransaction extends VoipCallTransaction {

    private static final String TAG = VerifyCallStateChangeTransaction.class.getSimpleName();

    public static final int FAILURE_CODE = 0;

    public static final int SUCCESS_CODE = 1;

    public static final int TIMEOUT_SECONDS = 2;

    private final Call mCall;

    private final CallsManager mCallsManager;

    private final int mTargetCallState;

    private final boolean mShouldDisconnectUponFailure;

    private final CompletableFuture<Integer> mCallStateOrTimeoutResult = new CompletableFuture<>();

    private final CompletableFuture<VoipCallTransactionResult> mTransactionResult =

            new CompletableFuture<>();



    @VisibleForTesting

    public Call.CallStateListener mCallStateListenerImpl = new Call.CallStateListener() {

        @Override

        public void onCallStateChanged(int newCallState) {

            Log.d(TAG, "newState=[%d], expectedState=[%d]", newCallState, mTargetCallState);

            if (newCallState == mTargetCallState) {

                mCallStateOrTimeoutResult.complete(SUCCESS_CODE);

            }

            // NOTE:: keep listening to the call state until the timeout is reached. It's possible

            // another call state is reached in between...

        }

    };



    public VerifyCallStateChangeTransaction(CallsManager callsManager, Call call,

            int targetCallState, boolean shouldDisconnectUponFailure) {

        super(callsManager.getLock());

        mCallsManager = callsManager;

        mCall = call;

        mTargetCallState = targetCallState;

        mShouldDisconnectUponFailure = shouldDisconnectUponFailure;

    }



    @Override

    public CompletionStage<VoipCallTransactionResult> processTransaction(Void v) {

        Log.d(TAG, "processTransaction:");

        // It's possible the Call is already in the expected call state

        if (isNewCallStateTargetCallState()) {

            mTransactionResult.complete(

                    new VoipCallTransactionResult(VoipCallTransactionResult.RESULT_SUCCEED,

                            TAG));

            return mTransactionResult;

        }

        initCallStateListenerOnTimeout();

        // At this point, the mCallStateOrTimeoutResult has been completed. There are 2 scenarios:

        // (1) newCallState == targetCallState --> the transaction is successful

        // (2) timeout is reached --> evaluate the current call state and complete the t accordingly

        // also need to do cleanup for the transaction

        evaluateCallStateUponChangeOrTimeout();



        return mTransactionResult;

    }



    private boolean isNewCallStateTargetCallState() {

        return mCall.getState() == mTargetCallState;

    }



    private void initCallStateListenerOnTimeout() {

        mCall.addCallStateListener(mCallStateListenerImpl);

        mCallStateOrTimeoutResult.completeOnTimeout(FAILURE_CODE, TIMEOUT_SECONDS,

                TimeUnit.SECONDS);

    }



    private void evaluateCallStateUponChangeOrTimeout() {

        mCallStateOrTimeoutResult.thenAcceptAsync((result) -> {

            Log.i(TAG, "processTransaction: thenAcceptAsync: result=[%s]", result);

            mCall.removeCallStateListener(mCallStateListenerImpl);

            if (isNewCallStateTargetCallState()) {

                mTransactionResult.complete(

                        new VoipCallTransactionResult(VoipCallTransactionResult.RESULT_SUCCEED,

                                TAG));

            } else {

                maybeDisconnectCall();

                mTransactionResult.complete(

                        new VoipCallTransactionResult(VoipCallTransactionResult.RESULT_FAILED,

                                TAG));

            }

        }).exceptionally(exception -> {

            Log.i(TAG, "hit exception=[%s] while completing future", exception);

            mTransactionResult.complete(

                    new VoipCallTransactionResult(VoipCallTransactionResult.RESULT_FAILED,

                            TAG));

            return null;

        });

    }



    private void maybeDisconnectCall() {

        if (mShouldDisconnectUponFailure) {

            mCallsManager.markCallAsDisconnected(mCall,

                    new DisconnectCause(DisconnectCause.ERROR,

                            "did not hold in timeout window"));

            mCallsManager.markCallAsRemoved(mCall);

        }

    }



    @VisibleForTesting

    public CompletableFuture<Integer> getCallStateOrTimeoutResult() {

        return mCallStateOrTimeoutResult;

    }



    @VisibleForTesting

    public CompletableFuture<VoipCallTransactionResult> getTransactionResult() {

        return mTransactionResult;

    }



    @VisibleForTesting

    public Call.CallStateListener getCallStateListenerImpl() {

        return mCallStateListenerImpl;

    }

}

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

+79 −0
Original line number Diff line number Diff line
@@ -16,7 +16,11 @@ 


package com.android.server.telecom.tests;



import static org.mockito.ArgumentMatchers.any;

import static org.mockito.ArgumentMatchers.eq;

import static org.mockito.Mockito.atLeastOnce;

import static org.mockito.Mockito.doReturn;

import static org.mockito.Mockito.mock;

import static org.mockito.Mockito.never;

import static org.mockito.Mockito.times;

import static org.mockito.Mockito.verify;
@@ -39,11 +43,14 @@ import android.telecom.CallAttributes; 
import android.telecom.DisconnectCause;

import android.telecom.PhoneAccountHandle;



import androidx.test.filters.SmallTest;



import com.android.server.telecom.Call;

import com.android.server.telecom.CallState;

import com.android.server.telecom.CallerInfoLookupHelper;

import com.android.server.telecom.CallsManager;

import com.android.server.telecom.ClockProxy;

import com.android.server.telecom.ConnectionServiceWrapper;

import com.android.server.telecom.PhoneNumberUtilsAdapter;

import com.android.server.telecom.TelecomSystem;

import com.android.server.telecom.ui.ToastFactory;
@@ -53,6 +60,8 @@ import com.android.server.telecom.voip.IncomingCallTransaction; 
import com.android.server.telecom.voip.OutgoingCallTransaction;

import com.android.server.telecom.voip.MaybeHoldCallForNewCallTransaction;

import com.android.server.telecom.voip.RequestNewActiveCallTransaction;

import com.android.server.telecom.voip.VerifyCallStateChangeTransaction;

import com.android.server.telecom.voip.VoipCallTransactionResult;



import org.junit.After;

import org.junit.Before;
@@ -62,6 +71,11 @@ import org.mockito.Mock; 
import org.mockito.Mockito;

import org.mockito.MockitoAnnotations;



import java.util.concurrent.CompletableFuture;

import java.util.concurrent.ExecutionException;

import java.util.concurrent.TimeUnit;

import java.util.concurrent.TimeoutException;





public class TransactionTests extends TelecomTestCase {
@@ -250,6 +264,63 @@ public class TransactionTests extends TelecomTestCase { 
                        isA(Boolean.class));

    }



    /**

     * This test verifies if the ConnectionService call is NOT transitioned to the desired call

     * state (within timeout period), Telecom will disconnect the call.

     */

    @SmallTest

    @Test

    public void testCallStateChangeTimesOut()

            throws ExecutionException, InterruptedException, TimeoutException {

        VerifyCallStateChangeTransaction t = new VerifyCallStateChangeTransaction(mCallsManager,

                mMockCall1, CallState.ON_HOLD, true);

        // WHEN

        setupHoldableCall();



        // simulate the transaction being processed and the CompletableFuture timing out

        t.processTransaction(null);

        CompletableFuture<Integer> timeoutFuture = t.getCallStateOrTimeoutResult();

        timeoutFuture.complete(VerifyCallStateChangeTransaction.FAILURE_CODE);



        // THEN

        verify(mMockCall1, times(1)).addCallStateListener(t.getCallStateListenerImpl());

        assertEquals(timeoutFuture.get().intValue(), VerifyCallStateChangeTransaction.FAILURE_CODE);

        assertEquals(VoipCallTransactionResult.RESULT_FAILED,

                t.getTransactionResult().get(2, TimeUnit.SECONDS).getResult());

        verify(mMockCall1, atLeastOnce()).removeCallStateListener(any());

        verify(mCallsManager, times(1)).markCallAsDisconnected(eq(mMockCall1), any());

        verify(mCallsManager, times(1)).markCallAsRemoved(eq(mMockCall1));

    }



    /**

     * This test verifies that when an application transitions a call to the requested state,

     * Telecom does not disconnect the call and transaction completes successfully.

     */

    @SmallTest

    @Test

    public void testCallStateIsSuccessfullyChanged()

            throws ExecutionException, InterruptedException, TimeoutException {

        VerifyCallStateChangeTransaction t = new VerifyCallStateChangeTransaction(mCallsManager,

                mMockCall1, CallState.ON_HOLD, true);

        // WHEN

        setupHoldableCall();



        // simulate the transaction being processed and the setOnHold() being called / state change

        t.processTransaction(null);

        t.getCallStateListenerImpl().onCallStateChanged(CallState.ON_HOLD);

        when(mMockCall1.getState()).thenReturn(CallState.ON_HOLD);



        // THEN

        verify(mMockCall1, times(1)).addCallStateListener(t.getCallStateListenerImpl());

        assertEquals(t.getCallStateOrTimeoutResult().get().intValue(),

                VerifyCallStateChangeTransaction.SUCCESS_CODE);

        assertEquals(VoipCallTransactionResult.RESULT_SUCCEED,

                t.getTransactionResult().get(2, TimeUnit.SECONDS).getResult());

        verify(mMockCall1, atLeastOnce()).removeCallStateListener(any());

        verify(mCallsManager, never()).markCallAsDisconnected(eq(mMockCall1), any());

        verify(mCallsManager, never()).markCallAsRemoved(eq(mMockCall1));

    }



    private Call createSpyCall(PhoneAccountHandle targetPhoneAccount, int initialState, String id) {

        when(mCallsManager.getCallerInfoLookupHelper()).thenReturn(mCallerInfoLookupHelper);
@@ -280,4 +351,12 @@ public class TransactionTests extends TelecomTestCase { 


        return callSpy;

    }



    private void setupHoldableCall(){

        when(mMockCall1.getState()).thenReturn(CallState.ACTIVE);

        when(mMockCall1.getConnectionServiceWrapper()).thenReturn(

                mock(ConnectionServiceWrapper.class));

        doNothing().when(mMockCall1).addCallStateListener(any());

        doReturn(true).when(mMockCall1).removeCallStateListener(any());

    }

}
 
 No newline at end of file