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

Commit 99c78ed9 authored by Brad Ebinger's avatar Brad Ebinger
Browse files

ImsSmsDispatcher should use the instance returned to it by the connector

Instead of refreshing every time, it should use the ImsManager given to
it.

This is a step in a series of cleaning steps to make IMS more reliable.

Test: atest FrameworksTelephonyTests
Change-Id: I35fd26b9986abf0ab1dcd36a172b532b8cfa4efb
parent f1d3b4dc
Loading
Loading
Loading
Loading
+24 −4
Original line number Diff line number Diff line
@@ -56,6 +56,11 @@ public class ImsSmsDispatcher extends SMSDispatcher {

    private static final String TAG = "ImsSmsDispatcher";

    public interface FeatureConnectorFactory {
        FeatureConnector<ImsManager> create(Context context, int phoneId,
                FeatureConnector.Listener<ImsManager> listener, String logPrefix);
    }

    @VisibleForTesting
    public Map<Integer, SmsTracker> mTrackers = new ConcurrentHashMap<>();
    @VisibleForTesting
@@ -67,6 +72,8 @@ public class ImsSmsDispatcher extends SMSDispatcher {
    private final FeatureConnector<ImsManager> mImsManagerConnector;
    /** Telephony metrics instance for logging metrics event */
    private TelephonyMetrics mMetrics = TelephonyMetrics.getInstance();
    private ImsManager mImsManager;
    private FeatureConnectorFactory mConnectorFactory;

    /**
     * Listen to the IMS service state change
@@ -234,10 +241,12 @@ public class ImsSmsDispatcher extends SMSDispatcher {
        }
    };

    public ImsSmsDispatcher(Phone phone, SmsDispatchersController smsDispatchersController) {
    public ImsSmsDispatcher(Phone phone, SmsDispatchersController smsDispatchersController,
            FeatureConnectorFactory factory) {
        super(phone, smsDispatchersController);
        mConnectorFactory = factory;

        mImsManagerConnector = new FeatureConnector<>(mContext, mPhone.getPhoneId(),
        mImsManagerConnector = mConnectorFactory.create(mContext, mPhone.getPhoneId(),
                new FeatureConnector.Listener<ImsManager>() {
                    @Override
                    public ImsManager getFeatureManager() {
@@ -248,6 +257,7 @@ public class ImsSmsDispatcher extends SMSDispatcher {
                    public void connectionReady(ImsManager manager) throws ImsException {
                        logd("ImsManager: connection ready.");
                        synchronized (mLock) {
                            mImsManager = manager;
                            setListeners();
                            mIsImsServiceUp = true;
                        }
@@ -257,6 +267,7 @@ public class ImsSmsDispatcher extends SMSDispatcher {
                    public void connectionUnavailable() {
                        logd("ImsManager: connection unavailable.");
                        synchronized (mLock) {
                            mImsManager = null;
                            mIsImsServiceUp = false;
                        }
                    }
@@ -331,6 +342,9 @@ public class ImsSmsDispatcher extends SMSDispatcher {

    @Override
    protected String getFormat() {
        // This is called in the constructor before ImsSmsDispatcher has a chance to initialize
        // mLock. ImsManager will not be up anyway at this point, so report UNKNOWN.
        if (mLock == null) return SmsConstants.FORMAT_UNKNOWN;
        try {
            return getImsManager().getSmsFormat();
        } catch (ImsException e) {
@@ -415,8 +429,14 @@ public class ImsSmsDispatcher extends SMSDispatcher {
        }
    }

    private ImsManager getImsManager() {
        return ImsManager.getInstance(mContext, mPhone.getPhoneId());
    private ImsManager getImsManager() throws ImsException {
        synchronized (mLock) {
            if (mImsManager == null) {
                throw new ImsException("ImsManager not up",
                        ImsReasonInfo.CODE_LOCAL_IMS_SERVICE_DOWN);
            }
            return mImsManager;
        }
    }

    @VisibleForTesting
+2 −1
Original line number Diff line number Diff line
@@ -40,6 +40,7 @@ import android.telephony.ServiceState;
import android.telephony.SmsManager;
import android.telephony.SmsMessage;

import com.android.ims.FeatureConnector;
import com.android.internal.annotations.VisibleForTesting;
import com.android.internal.telephony.cdma.CdmaInboundSmsHandler;
import com.android.internal.telephony.cdma.CdmaSMSDispatcher;
@@ -139,7 +140,7 @@ public class SmsDispatchersController extends Handler {

        // Create dispatchers, inbound SMS handlers and
        // broadcast undelivered messages in raw table.
        mImsSmsDispatcher = new ImsSmsDispatcher(phone, this);
        mImsSmsDispatcher = new ImsSmsDispatcher(phone, this, FeatureConnector::new);
        mCdmaDispatcher = new CdmaSMSDispatcher(phone, this);
        mGsmInboundSmsHandler = GsmInboundSmsHandler.makeInboundSmsHandler(phone.getContext(),
                storageMonitor, phone);
+18 −3
Original line number Diff line number Diff line
@@ -21,9 +21,10 @@ import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertTrue;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyInt;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.ArgumentMatchers.nullable;
import static org.mockito.Mockito.spy;
import static org.mockito.Mockito.doAnswer;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

@@ -33,6 +34,8 @@ import android.telephony.SmsMessage;
import android.telephony.ims.stub.ImsSmsImplBase;
import android.test.suitebuilder.annotation.SmallTest;

import com.android.ims.FeatureConnector;
import com.android.ims.ImsManager;
import com.android.internal.util.HexDump;

import org.junit.After;
@@ -40,12 +43,16 @@ import org.junit.Before;
import org.junit.Test;
import org.mockito.ArgumentCaptor;
import org.mockito.Mock;
import org.mockito.stubbing.Answer;

import java.util.HashMap;

public class ImsSmsDispatcherTest extends TelephonyTest {
    @Mock private SmsDispatchersController mSmsDispatchersController;
    @Mock private SMSDispatcher.SmsTracker mSmsTracker;
    @Mock private ImsSmsDispatcher.FeatureConnectorFactory mConnectorFactory;
    @Mock private FeatureConnector<ImsManager> mMockConnector;
    private FeatureConnector.Listener<ImsManager> mImsManagerListener;
    private HashMap<String, Object> mTrackerData;
    private ImsSmsDispatcher mImsSmsDispatcher;

@@ -56,9 +63,17 @@ public class ImsSmsDispatcherTest extends TelephonyTest {
            Looper.prepare();
        }

        mImsSmsDispatcher = spy(new ImsSmsDispatcher(mPhone, mSmsDispatchersController));
        doAnswer((Answer<FeatureConnector<ImsManager>>) invocation -> {
            mImsManagerListener =
                    (FeatureConnector.Listener<ImsManager>) invocation.getArguments()[2];
            return mMockConnector;
        }).when(mConnectorFactory).create(any(), anyInt(), any(), anyString());
        mImsSmsDispatcher = new ImsSmsDispatcher(mPhone, mSmsDispatchersController,
                mConnectorFactory);
        // set the ImsManager instance
        verify(mMockConnector).connect();
        mImsManagerListener.connectionReady(mImsManager);
        when(mSmsDispatchersController.isIms()).thenReturn(true);

        mTrackerData = new HashMap<>(1);
        when(mSmsTracker.getData()).thenReturn(mTrackerData);
    }