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

Commit 0a74874a authored by Tyler Gunn's avatar Tyler Gunn
Browse files

Fix issue where redial connection change is notified on wrong phone.

notifyRedialConnectionChanged was being raised on the ImsPhone, which is
incorrect.  The only registrations for redial connection change comes
from TelephonyConnection, which registers on the default phone (e.g.
GsmCdmaPhone).  As a consequence, when a silent redial connection change
occurs, the TelephonyConnection was not being notified and we'd end up
with a stuck call.

Test: Add unit test to verify that the silent redial connection change
event is raised on the default phone, not on the ImsPhone.
Bug: 191607537

Change-Id: I50506dadfe300c3682fe091a452b619ab5ba54f4
parent 632f4e1a
Loading
Loading
Loading
Loading
+25 −11
Original line number Diff line number Diff line
@@ -200,7 +200,8 @@ public abstract class Phone extends Handler implements PhoneInternalInterface {
    // Single Radio Voice Call Continuity
    @VisibleForTesting
    protected static final int EVENT_SRVCC_STATE_CHANGED             = 31;
    private static final int EVENT_INITIATE_SILENT_REDIAL           = 32;
    @VisibleForTesting
    public static final int EVENT_INITIATE_SILENT_REDIAL           = 32;
    private static final int EVENT_RADIO_NOT_AVAILABLE              = 33;
    private static final int EVENT_UNSOL_OEM_HOOK_RAW               = 34;
    protected static final int EVENT_GET_RADIO_CAPABILITY           = 35;
@@ -356,6 +357,11 @@ public abstract class Phone extends Handler implements PhoneInternalInterface {
    private static final boolean LCE_PULL_MODE = true;
    private int mLceStatus = RILConstants.LCE_NOT_AVAILABLE;
    protected TelephonyComponentFactory mTelephonyComponentFactory;
    /**
     * Should ALWAYS be {@code true} in production code.  This is used only used in tests so that we
     * can disable the read checks which interfer with unit testing.
     */
    private boolean mAreThreadChecksEnabled = true;

    //IMS
    /**
@@ -737,7 +743,7 @@ public abstract class Phone extends Handler implements PhoneInternalInterface {
                break;

            case EVENT_INITIATE_SILENT_REDIAL:
                Rlog.d(LOG_TAG, "Event EVENT_INITIATE_SILENT_REDIAL Received");
                Rlog.i(LOG_TAG, "Event EVENT_INITIATE_SILENT_REDIAL Received");
                ar = (AsyncResult) msg.obj;
                if ((ar.exception == null) && (ar.result != null)) {
                    SilentRedialParam result = (SilentRedialParam) ar.result;
@@ -747,16 +753,12 @@ public abstract class Phone extends Handler implements PhoneInternalInterface {
                    if (TextUtils.isEmpty(dialString)) return;
                    try {
                        Connection cn = dialInternal(dialString, dialArgs);
                        Rlog.d(LOG_TAG, "Notify redial connection changed cn: " + cn);
                        if (mImsPhone != null) {
                            // Don't care it is null or not.
                            mImsPhone.notifyRedialConnectionChanged(cn);
                        }
                        Rlog.i(LOG_TAG, "Notify redial connection changed cn: " + cn);
                        notifyRedialConnectionChanged(cn);
                    } catch (CallStateException e) {
                        Rlog.e(LOG_TAG, "silent redial failed: " + e);
                        if (mImsPhone != null) {
                            mImsPhone.notifyRedialConnectionChanged(null);
                        }
                        Rlog.e(LOG_TAG, "Notify redial connection changed - silent redial failed: "
                                + e);
                        notifyRedialConnectionChanged(null);
                    }
                }
                break;
@@ -1762,6 +1764,9 @@ public abstract class Phone extends Handler implements PhoneInternalInterface {
     * the thread that originally obtained this Phone instance.
     */
    private void checkCorrectThread(Handler h) {
        if (!mAreThreadChecksEnabled) {
            return;
        }
        if (h.getLooper() != mLooper) {
            throw new RuntimeException(
                    "com.android.internal.telephony.Phone must be used from within one thread");
@@ -5024,4 +5029,13 @@ public abstract class Phone extends Handler implements PhoneInternalInterface {
    private static String pii(String s) {
        return Rlog.pii(LOG_TAG, s);
    }

    /**
     * Used in unit tests to disable the thread checks.  Should not be used otherwise.
     * @param enabled {@code true} if thread checks are enabled, {@code false} otherwise.
     */
    @VisibleForTesting
    public void setAreThreadChecksEnabled(boolean enabled) {
        mAreThreadChecksEnabled = enabled;
    }
}
+26 −0
Original line number Diff line number Diff line
@@ -112,6 +112,7 @@ public class GsmCdmaPhoneTest extends TelephonyTest {
    private static final int EVENT_EMERGENCY_CALLBACK_MODE_EXIT = 1;
    private static final int EVENT_EMERGENCY_CALL_TOGGLE = 2;
    private static final int EVENT_SET_ICC_LOCK_ENABLED = 3;
    private static final int EVENT_SILENT_REDIAL_CONNECTION_CHANGED = 4;

    private void switchToGsm() {
        mSimulatedCommands.setVoiceRadioTech(ServiceState.RIL_RADIO_TECHNOLOGY_GSM);
@@ -639,6 +640,31 @@ public class GsmCdmaPhoneTest extends TelephonyTest {
        assertFalse(mPhoneUT.handlePinMmi("1234567890"));
    }

    /**
     * Verifies that silent redial connection changes are raised on the phone itself, not on the
     * ImsPhone.
     */
    @Test
    @SmallTest
    public void testSilentRedialNotify() {
        mPhoneUT.setAreThreadChecksEnabled(false);
        mPhoneUT.registerForRedialConnectionChanged(mTestHandler,
                EVENT_SILENT_REDIAL_CONNECTION_CHANGED, null);

        // Raise a silent redial on the GsmCdmaPhone.
        Phone.SilentRedialParam params = new Phone.SilentRedialParam("6505551212", 0, null);
        AsyncResult result = new AsyncResult(null, params, null);
        Message.obtain(mPhoneUT, Phone.EVENT_INITIATE_SILENT_REDIAL, result).sendToTarget();
        processAllMessages();

        // We expect our test handler to have the registered redial connection changed event
        // raised on it.
        ArgumentCaptor<Message> messageCaptor = ArgumentCaptor.forClass(Message.class);
        verify(mTestHandler).sendMessageDelayed(messageCaptor.capture(), anyLong());
        assertEquals(1, messageCaptor.getAllValues().size());
        assertEquals(EVENT_SILENT_REDIAL_CONNECTION_CHANGED, messageCaptor.getValue().what);
    }

    @Test
    @SmallTest
    public void testEmergencySmsMode() {
+3 −0
Original line number Diff line number Diff line
@@ -471,6 +471,9 @@ public abstract class TelephonyTest {
                BlockedNumberContract.AUTHORITY, mFakeBlockedNumberContentProvider);
        mPhone.mCi = mSimulatedCommands;
        mCT.mCi = mSimulatedCommands;
        mCT.mForegroundCall = new GsmCdmaCall(mCT);
        mCT.mBackgroundCall = new GsmCdmaCall(mCT);
        mCT.mRingingCall = new GsmCdmaCall(mCT);
        doReturn(mUiccCard).when(mPhone).getUiccCard();
        doReturn(mUiccProfile).when(mUiccCard).getUiccProfile();