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

Commit ed542ea4 authored by Tyler Gunn's avatar Tyler Gunn
Browse files

Suppress state changes in CallAudioManager when state doesn't change.

A fix was made for b/155109531 which to ensure that the disconnecting
state gets communicated to Dialer upon local disconnect initiation.
Due to... reasons.. the DISCONNECTING state is not a proper top level
state in a Call.  It is only sent when parcelling the call info to an
InCallService and otherwise is reflected in Call.isLocallyDisconnecting().

In any case, as a consequence of that change, CallAudioManager was now
getting notificed of a state change from oldState = DIALING to
newState = DIALING (since its locally disconnecting).

CallAudioManager ultimately DOES NOT care about the disconnecting state
and only really cares about top level state changes.  However, the way
this logic was structured we first remove the call from all the bins it
is in, and then re-add it to the new bin.  But we're not actually moving
it out of its current bin since oldState == newState.

As a consequence we'd end up triggering CallaudioModeStateMachine
NEW_ACTIVE_OR_DIALING_CALL, and also stopping and re-starting the
ringback tone, which really we don't want to do.

Also, I saw a little incongruity in the CAllScreeningServiceHelper logs
which meant we were not emitting a log message when we should have, so
fixing that too.

Fixes: 229938369
Test: Manual test on ATT to verify local ringback starts and stops as it
should.
Test: Added unit test to verify that we are not starting and stopping the
tone multiple times.

Change-Id: I21b95f9137fb4dd1da78e4a21fad9e7ea33164e0
parent 026d702d
Loading
Loading
Loading
Loading
+1 −1
Original line number Diff line number Diff line
@@ -1262,7 +1262,7 @@ public class Call implements CreateConnectionResponse, EventManager.Loggable,
        }
    }

    boolean isRingbackRequested() {
    public boolean isRingbackRequested() {
        return mRingbackRequested;
    }

+4 −0
Original line number Diff line number Diff line
@@ -107,6 +107,10 @@ public class CallAudioManager extends CallsManagerListenerBase {
            // No audio management for calls in a conference, or external calls.
            return;
        }
        if (oldState == newState) {
            // State did not change, so no need to do anything.
            return;
        }
        Log.d(LOG_TAG, "Call state changed for TC@%s: %s -> %s", call.getId(),
                CallState.toString(oldState), CallState.toString(newState));

+1 −1
Original line number Diff line number Diff line
@@ -145,9 +145,9 @@ public class CallScreeningServiceHelper {

        if (!bindCallScreeningService(mContext, mUserHandle, mPackageName, serviceConnection)) {
            Log.i(this, "bindAndGetCallIdentification - bind failed");
            Log.addEvent(mCall, LogUtils.Events.BIND_SCREENING, mPackageName);
            mFuture.complete(null);
        }
        Log.addEvent(mCall, LogUtils.Events.BIND_SCREENING, mPackageName);

        // Set up a timeout so that we're not waiting forever for the caller ID information.
        Handler handler = new Handler();
+54 −0
Original line number Diff line number Diff line
@@ -294,6 +294,60 @@ public class CallAudioManagerTest extends TelecomTestCase {
        verifyProperCleanup();
    }

    @MediumTest
    @Test
    public void testRingbackStartStop() {
        Call call = mock(Call.class);
        ArgumentCaptor<CallAudioModeStateMachine.MessageArgs> captor = makeNewCaptor();
        when(call.getState()).thenReturn(CallState.CONNECTING);
        when(call.isRingbackRequested()).thenReturn(true);

        mCallAudioManager.onCallAdded(call);
        assertEquals(call, mCallAudioManager.getForegroundCall());
        verify(mCallAudioRouteStateMachine).sendMessageWithSessionInfo(
                CallAudioRouteStateMachine.UPDATE_SYSTEM_AUDIO_ROUTE);
        verify(mCallAudioModeStateMachine).sendMessageWithArgs(
                eq(CallAudioModeStateMachine.NEW_ACTIVE_OR_DIALING_CALL), captor.capture());
        CallAudioModeStateMachine.MessageArgs expectedArgs =
                new Builder()
                        .setHasActiveOrDialingCalls(true)
                        .setHasRingingCalls(false)
                        .setHasHoldingCalls(false)
                        .setIsTonePlaying(false)
                        .setHasAudioProcessingCalls(false)
                        .setForegroundCallIsVoip(false)
                        .setSession(null)
                        .build();
        assertMessageArgEquality(expectedArgs, captor.getValue());

        when(call.getState()).thenReturn(CallState.DIALING);
        mCallAudioManager.onCallStateChanged(call, CallState.CONNECTING, CallState.DIALING);
        verify(mCallAudioModeStateMachine, times(2)).sendMessageWithArgs(
                eq(CallAudioModeStateMachine.NEW_ACTIVE_OR_DIALING_CALL), captor.capture());
        assertMessageArgEquality(expectedArgs, captor.getValue());
        verify(mCallAudioModeStateMachine, times(2)).sendMessageWithArgs(
                anyInt(), any(CallAudioModeStateMachine.MessageArgs.class));

        // Ensure we started ringback.
        verify(mRingbackPlayer).startRingbackForCall(any(Call.class));

        // Report state change from dialing to dialing, which happens when a call is locally
        // disconnected.
        mCallAudioManager.onCallStateChanged(call, CallState.DIALING, CallState.DIALING);
        // Should not have stopped ringback.
        verify(mRingbackPlayer, never()).stopRingbackForCall(any(Call.class));
        // Should still only have initial ringback start
        verify(mRingbackPlayer, times(1)).startRingbackForCall(any(Call.class));

        // Report state to disconnected
        when(call.getState()).thenReturn(CallState.DISCONNECTED);
        mCallAudioManager.onCallStateChanged(call, CallState.DIALING, CallState.DISCONNECTED);
        // Now we should have stopped ringback.
        verify(mRingbackPlayer).stopRingbackForCall(any(Call.class));
        // Should still only have initial ringback start recorded from before (don't restart it).
        verify(mRingbackPlayer, times(1)).startRingbackForCall(any(Call.class));
    }

    @SmallTest
    @Test
    public void testNewCallGoesToAudioProcessing() {