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

Commit 87e95239 authored by Hakjun Choi's avatar Hakjun Choi Committed by Sungcheol Ahn
Browse files

Separate the executor for the ServiceConnection of ImsServiceController 

separate the executor of ServiceConnection for ImsServiceController from the binder thread to prevent potential ANR cases

Bug: 227221941
Test: ImsServiceTest / E2E call regression test 271378397
Test: atest ImsServiceControllerTest

Change-Id: I9ca39033cffe10e633f1fbf371c3e399d950c16c
parent 816e6132

src/java/com/android/internal/telephony/ims/ImsServiceController.java

+53 −17
Original line number Diff line number Diff line
@@ -85,14 +85,49 @@ public class ImsServiceController { 


        @Override

        public void onServiceConnected(ComponentName name, IBinder service) {

            if (mHandler.getLooper().isCurrentThread()) {

                onServiceConnectedInternal(name, service);

            } else {

                mHandler.post(() -> onServiceConnectedInternal(name, service));

            }

        }



        @Override

        public void onServiceDisconnected(ComponentName name) {

            if (mHandler.getLooper().isCurrentThread()) {

                onServiceDisconnectedInternal(name);

            } else {

                mHandler.post(() -> onServiceDisconnectedInternal(name));

            }

        }



        @Override

        public void onBindingDied(ComponentName name) {

            if (mHandler.getLooper().isCurrentThread()) {

                onBindingDiedInternal(name);

            } else {

                mHandler.post(() -> onBindingDiedInternal(name));

            }

        }



        @Override

        public void onNullBinding(ComponentName name) {

            if (mHandler.getLooper().isCurrentThread()) {

                onNullBindingInternal(name);

            } else {

                mHandler.post(() -> onNullBindingInternal(name));

            }

        }



        private void onServiceConnectedInternal(ComponentName name, IBinder service) {

            synchronized (mLock) {

                mBackoff.stop();

                mIsBound = true;

                mIsBinding = false;

                try {

                    mLocalLog.log("onServiceConnected");

                    Log.d(LOG_TAG, "ImsService(" + name + "): onServiceConnected with binder: "

                            + service);

                    mLocalLog.log("onServiceConnectedInternal");

                    Log.d(LOG_TAG, "ImsService(" + name

                            + "): onServiceConnectedInternal with binder: " + service);

                    setServiceController(service);

                    notifyImsServiceReady();

                    retrieveStaticImsServiceCapabilities();
@@ -118,20 +153,18 @@ public class ImsServiceController { 
            }

        }



        @Override

        public void onServiceDisconnected(ComponentName name) {

        private void onServiceDisconnectedInternal(ComponentName name) {

            synchronized (mLock) {

                mIsBinding = false;

                cleanupConnection();

            }

            mLocalLog.log("onServiceDisconnected");

            Log.w(LOG_TAG, "ImsService(" + name + "): onServiceDisconnected. Waiting...");

            mLocalLog.log("onServiceDisconnectedInternal");

            Log.w(LOG_TAG, "ImsService(" + name + "): onServiceDisconnectedInternal. Waiting...");

            // Service disconnected, but we are still technically bound. Waiting for reconnect.

            checkAndReportAnomaly(name);

        }



        @Override

        public void onBindingDied(ComponentName name) {

        private void onBindingDiedInternal(ComponentName name) {

            mIsServiceConnectionDead = true;

            synchronized (mLock) {

                mIsBinding = false;
@@ -141,15 +174,15 @@ public class ImsServiceController { 
                unbindService();

                startDelayedRebindToService();

            }

            Log.w(LOG_TAG, "ImsService(" + name + "): onBindingDied. Starting rebind...");

            mLocalLog.log("onBindingDied, retrying in " + mBackoff.getCurrentDelay() + " mS");

            Log.w(LOG_TAG, "ImsService(" + name + "): onBindingDiedInternal. Starting rebind...");

            mLocalLog.log("onBindingDiedInternal, retrying in "

                    + mBackoff.getCurrentDelay() + " mS");

        }



        @Override

        public void onNullBinding(ComponentName name) {

            Log.w(LOG_TAG, "ImsService(" + name + "): onNullBinding. Is service dead = "

        private void onNullBindingInternal(ComponentName name) {

            Log.w(LOG_TAG, "ImsService(" + name + "): onNullBindingInternal. Is service dead = "

                    + mIsServiceConnectionDead);

            mLocalLog.log("onNullBinding, is service dead = " + mIsServiceConnectionDead);

            mLocalLog.log("onNullBindingInternal, is service dead = " + mIsServiceConnectionDead);

            // onNullBinding will happen after onBindingDied. In this case, we should not

            // permanently unbind and instead let the automatic rebind occur.

            if (mIsServiceConnectionDead) return;
@@ -230,6 +263,7 @@ public class ImsServiceController { 
    private static final boolean ENFORCE_SINGLE_SERVICE_FOR_SIP_TRANSPORT = false;

    private final ComponentName mComponentName;

    private final HandlerThread mHandlerThread = new HandlerThread("ImsServiceControllerHandler");

    private final Handler mHandler;

    private final LegacyPermissionManager mPermissionManager;

    private ImsFeatureBinderRepository mRepo;

    private ImsServiceControllerCallbacks mCallbacks;
@@ -324,11 +358,12 @@ public class ImsServiceController { 
        mComponentName = componentName;

        mCallbacks = callbacks;

        mHandlerThread.start();

        mHandler = new Handler(mHandlerThread.getLooper());

        mBackoff = new ExponentialBackoff(

                mRebindRetry.getStartDelay(),

                mRebindRetry.getMaximumDelay(),

                2, /* multiplier */

                mHandlerThread.getLooper(),

                mHandler,

                mRestartImsServiceRunnable);

        mPermissionManager = (LegacyPermissionManager) mContext.getSystemService(

                Context.LEGACY_PERMISSION_SERVICE);
@@ -352,6 +387,7 @@ public class ImsServiceController { 
        mContext = context;

        mComponentName = componentName;

        mCallbacks = callbacks;

        mHandler = handler;

        mBackoff = new ExponentialBackoff(

                rebindRetry.getStartDelay(),

                rebindRetry.getMaximumDelay(),

tests/telephonytests/src/com/android/internal/telephony/ims/ImsServiceControllerCompatTest.java

+4 −0
Original line number Diff line number Diff line
@@ -140,6 +140,8 @@ public class ImsServiceControllerCompatTest extends ImsTestBase { 
        SparseIntArray slotIdToSubIdMap = new SparseIntArray();

        slotIdToSubIdMap.put(SLOT_0, SUB_1);

        ServiceConnection conn = bindAndConnectService(slotIdToSubIdMap);

        long delay = mTestImsServiceController.getRebindDelay();

        waitForHandlerActionDelayed(mHandler, delay, 2 * delay);

        // add the MMTelFeature

        verify(mMockServiceControllerBinder).createMMTelFeature(SLOT_0);

        verify(mMockServiceControllerBinder).addFeatureStatusCallback(eq(SLOT_0),
@@ -149,6 +151,8 @@ public class ImsServiceControllerCompatTest extends ImsTestBase { 
        validateMmTelFeatureContainerExists(SLOT_0);

        // Remove the feature

        conn.onBindingDied(mTestComponentName);

        delay = REBIND_RETRY.getStartDelay();

        waitForHandlerActionDelayed(mHandler, delay, 2 * delay);

        verify(mMmTelCompatAdapterSpy).onFeatureRemoved();

        verify(mMockServiceControllerBinder).removeImsFeature(eq(SLOT_0),

                eq(ImsFeature.FEATURE_MMTEL));

tests/telephonytests/src/com/android/internal/telephony/ims/ImsServiceControllerTest.java

+16 −4
Original line number Diff line number Diff line
@@ -634,6 +634,8 @@ public class ImsServiceControllerTest extends ImsTestBase { 


        conn.onServiceDisconnected(mTestComponentName);



        long delay = mTestImsServiceController.getRebindDelay();

        waitForHandlerActionDelayed(mHandler, delay, 2 * delay);

        verify(mMockCallbacks).imsServiceFeatureRemoved(eq(SLOT_0), eq(ImsFeature.FEATURE_MMTEL),

                eq(mTestImsServiceController));

        verify(mMockCallbacks).imsServiceFeatureRemoved(eq(SLOT_0), eq(ImsFeature.FEATURE_RCS),
@@ -660,6 +662,8 @@ public class ImsServiceControllerTest extends ImsTestBase { 


        conn.onServiceDisconnected(mTestComponentName);



        long delay = mTestImsServiceController.getRebindDelay();

        waitForHandlerActionDelayed(mHandler, delay, 2 * delay);

        verify(mMockCallbacks).imsServiceFeatureRemoved(eq(SLOT_0), eq(ImsFeature.FEATURE_MMTEL),

                eq(mTestImsServiceController));

        verify(mMockCallbacks).imsServiceFeatureRemoved(eq(SLOT_0), eq(ImsFeature.FEATURE_RCS),
@@ -762,6 +766,8 @@ public class ImsServiceControllerTest extends ImsTestBase { 


        conn.onBindingDied(null /*null*/);



        long delay = REBIND_RETRY.getStartDelay();

        waitForHandlerActionDelayed(mHandler, delay, 2 * delay);

        verify(mMockContext).unbindService(eq(conn));

        verify(mMockCallbacks).imsServiceFeatureRemoved(eq(SLOT_0), eq(ImsFeature.FEATURE_MMTEL),

                eq(mTestImsServiceController));
@@ -787,6 +793,8 @@ public class ImsServiceControllerTest extends ImsTestBase { 
        slotIdToSubIdMap.put(SLOT_0, SUB_2);

        bindAndNullServiceError(testFeatures, slotIdToSubIdMap.clone());



        long delay = mTestImsServiceController.getRebindDelay();

        waitForHandlerActionDelayed(mHandler, delay, 2 * delay);

        verify(mMockCallbacks, never()).imsServiceFeatureCreated(anyInt(), anyInt(),

                eq(mTestImsServiceController));

        verify(mMockCallbacks).imsServiceBindPermanentError(eq(mTestComponentName));
@@ -1304,7 +1312,7 @@ public class ImsServiceControllerTest extends ImsTestBase { 


        conn.onBindingDied(null /*null*/);



        long delay = mTestImsServiceController.getRebindDelay();

        long delay = REBIND_RETRY.getStartDelay();

        waitForHandlerActionDelayed(mHandler, delay, 2 * delay);

        // The service should autobind after rebind event occurs

        verify(mMockContext, times(2)).bindService(any(), any(), anyInt());
@@ -1330,7 +1338,7 @@ public class ImsServiceControllerTest extends ImsTestBase { 
        // null binding should be ignored in this case.

        conn.onNullBinding(null);



        long delay = mTestImsServiceController.getRebindDelay();

        long delay = REBIND_RETRY.getStartDelay();

        waitForHandlerActionDelayed(mHandler, delay, 2 * delay);

        // The service should autobind after rebind event occurs

        verify(mMockContext, times(2)).bindService(any(), any(), anyInt());
@@ -1398,10 +1406,11 @@ public class ImsServiceControllerTest extends ImsTestBase { 
        slotIdToSubIdMap.put(SLOT_0, SUB_2);

        ServiceConnection conn = bindAndConnectService(testFeatures, slotIdToSubIdMap.clone());

        conn.onBindingDied(null /*null*/);

        mTestImsServiceController.bind(testFeatures, slotIdToSubIdMap.clone());



        long delay = mTestImsServiceController.getRebindDelay();

        long delay = REBIND_RETRY.getStartDelay();

        waitForHandlerActionDelayed(mHandler, delay, 2 * delay);

        mTestImsServiceController.bind(testFeatures, slotIdToSubIdMap.clone());



        // Should only see two binds, not three from the auto rebind that occurs.

        verify(mMockContext, times(2)).bindService(any(), any(), anyInt());

    }
@@ -1481,6 +1490,9 @@ public class ImsServiceControllerTest extends ImsTestBase { 
        IImsServiceController.Stub controllerStub = mock(IImsServiceController.Stub.class);

        when(controllerStub.queryLocalInterface(any())).thenReturn(mMockServiceControllerBinder);

        connection.onServiceConnected(mTestComponentName, controllerStub);



        long delay = mTestImsServiceController.getRebindDelay();

        waitForHandlerActionDelayed(mHandler, delay, 2 * delay);

        return connection;

    }