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

Commit 5bad3188 authored by Brad Ebinger's avatar Brad Ebinger
Browse files

Remove redundant DeathRecipient for ImsServiceController 

ImsServiceController was listening to both
ServiceConnection#onServiceDied and DeathRecipient#binderDied.
This is redundant, so remove DeathRecipient and move it to
where it is needed, ImsServiceControllerStaticCompat.

Also, fix a bug in ImsResolver where overriding the device
ImsService was not working properly (for manual testing).

Bug: 120151611
Test: atest FrameworksTelephonyTests:ImsServiceControllerTest
Change-Id: Ia02eb5b9708373107392e370ea06664e5de72687
parent a9b5a3eb

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

+5 −2
Original line number Diff line number Diff line
@@ -382,9 +382,12 @@ public class ImsResolver implements ImsServiceController.ImsServiceControllerCal 
                    maybeRebindService(slotId, packageName);

                } else {

                    Log.i(TAG, "overriding device ImsService -  packageName=" + packageName);

                    if (packageName == null || packageName.isEmpty()) {

                        unbindImsService(getImsServiceInfoFromCache(mDeviceService));

                    if (TextUtils.equals(mDeviceService, packageName)) {

                        // No change in device service.

                        break;

                    }

                    // Unbind from the previous ImsService before binding to the new one.

                    unbindImsService(getImsServiceInfoFromCache(mDeviceService));

                    mDeviceService = packageName;

                    ImsServiceInfo deviceInfo = getImsServiceInfoFromCache(mDeviceService);

                    if (deviceInfo == null) {

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

+9 −42
Original line number Diff line number Diff line
@@ -58,32 +58,11 @@ import java.util.concurrent.ConcurrentHashMap; 
 * listener that the ImsService now supports that feature.

 *

 * When {@link #changeImsServiceFeatures} is called with a set of features that is different from

 * the original set, create and {@link IImsServiceController#removeImsFeature} will be called for

 * each feature that is created/removed.

 * the original set, create*Feature and {@link IImsServiceController#removeImsFeature} will be

 * called for each feature that is created/removed.

 */

public class ImsServiceController {



    class ImsDeathRecipient implements IBinder.DeathRecipient {



        private ComponentName mComponentName;



        ImsDeathRecipient(ComponentName name) {

            mComponentName = name;

        }



        @Override

        public void binderDied() {

            Log.e(LOG_TAG, "ImsService(" + mComponentName + ") died. Restarting...");

            synchronized (mLock) {

                mIsBinding = false;

                mIsBound = false;

            }

            cleanupAllFeatures();

            cleanUpService();

            startDelayedRebindToService();

        }

    }



    class ImsServiceConnection implements ServiceConnection {



        @Override
@@ -95,10 +74,7 @@ public class ImsServiceController { 
                Log.d(LOG_TAG, "ImsService(" + name + "): onServiceConnected with binder: "

                        + service);

                if (service != null) {

                    mImsDeathRecipient = new ImsDeathRecipient(name);

                    try {

                        service.linkToDeath(mImsDeathRecipient, 0);

                        mImsServiceControllerBinder = service;

                        setServiceController(service);

                        notifyImsServiceReady();

                        // create all associated features in the ImsService
@@ -109,9 +85,8 @@ public class ImsServiceController { 
                        mIsBound = false;

                        mIsBinding = false;

                        // Remote exception means that the binder already died.

                        if (mImsDeathRecipient != null) {

                            mImsDeathRecipient.binderDied();

                        }

                        cleanupConnection();

                        startDelayedRebindToService();

                        Log.e(LOG_TAG, "ImsService(" + name + ") RemoteException:"

                                + e.getMessage());

                    }
@@ -141,9 +116,6 @@ public class ImsServiceController { 
        }



        private void cleanupConnection() {

            if (isServiceControllerAvailable()) {

                mImsServiceControllerBinder.unlinkToDeath(mImsDeathRecipient, 0);

            }

            cleanupAllFeatures();

            cleanUpService();

        }
@@ -215,9 +187,7 @@ public class ImsServiceController { 
    // Binder interfaces to the features set in mImsFeatures;

    private HashSet<ImsFeatureContainer> mImsFeatureBinders = new HashSet<>();

    private IImsServiceController mIImsServiceController;

    private IBinder mImsServiceControllerBinder;

    private ImsServiceConnection mImsServiceConnection;

    private ImsDeathRecipient mImsDeathRecipient;

    private Set<IImsServiceFeatureCallback> mImsStatusCallbacks = ConcurrentHashMap.newKeySet();

    // Only added or removed, never accessed on purpose.

    private Set<ImsFeatureStatusCallback> mFeatureStatusCallbacks = new HashSet<>();
@@ -406,13 +376,12 @@ public class ImsServiceController { 
    public void unbind() throws RemoteException {

        synchronized (mLock) {

            mBackoff.stop();

            if (mImsServiceConnection == null || mImsDeathRecipient == null) {

            if (mImsServiceConnection == null) {

                return;

            }

            // Clean up all features

            changeImsServiceFeatures(new HashSet<>());

            removeImsServiceFeatureCallbacks();

            mImsServiceControllerBinder.unlinkToDeath(mImsDeathRecipient, 0);

            Log.i(LOG_TAG, "Unbinding ImsService: " + mComponentName);

            mContext.unbindService(mImsServiceConnection);

            cleanUpService();
@@ -458,13 +427,13 @@ public class ImsServiceController { 
    }



    @VisibleForTesting

    public IBinder getImsServiceControllerBinder() {

        return mImsServiceControllerBinder;

    public long getRebindDelay() {

        return mBackoff.getCurrentDelay();

    }



    @VisibleForTesting

    public long getRebindDelay() {

        return mBackoff.getCurrentDelay();

    public void stopBackoffTimerForTesting() {

        mBackoff.stop();

    }



    public ComponentName getComponentName() {
@@ -794,9 +763,7 @@ public class ImsServiceController { 


    private void cleanUpService() {

        synchronized (mLock) {

            mImsDeathRecipient = null;

            mImsServiceConnection = null;

            mImsServiceControllerBinder = null;

            setServiceController(null);

        }

    }

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

+2 −2
Original line number Diff line number Diff line
@@ -40,9 +40,9 @@ import com.android.ims.internal.IImsServiceController; 
 * ImsService will support.

 *

 * Compatibility interface for interacting with older implementations of ImsService. The older

 * ImsService implementation is contained within the android.telephony.ims.compat.* namspace.

 * ImsService implementation is contained within the android.telephony.ims.compat.* namespace.

 * Newer implementations of ImsService should use the current APIs contained in

 * android.telephony.ims.compat.*.

 * android.telephony.ims.*.

 */

public class ImsServiceControllerCompat extends ImsServiceController {

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

+40 −2
Original line number Diff line number Diff line
@@ -19,7 +19,9 @@ package com.android.internal.telephony.ims; 
import android.content.ComponentName;

import android.content.Context;

import android.content.Intent;

import android.content.ServiceConnection;

import android.os.IBinder;

import android.os.RemoteException;

import android.os.ServiceManager;

import android.util.Log;
@@ -37,7 +39,27 @@ public class ImsServiceControllerStaticCompat extends ImsServiceControllerCompat 


    private static final String IMS_SERVICE_NAME = "ims";



    private class ImsDeathRecipient implements IBinder.DeathRecipient {



        private ComponentName mComponentName;

        private ServiceConnection mServiceConnection;



        ImsDeathRecipient(ComponentName name, ServiceConnection conn) {

            mComponentName = name;

            mServiceConnection = conn;

        }



        @Override

        public void binderDied() {

            Log.e(TAG, "ImsService(" + mComponentName + ") died. Restarting...");

            // This is hacky... ImsServiceController uses the traditional service binding procedure,

            // so we have to emulate it when using a persistent service.

            mServiceConnection.onBindingDied(mComponentName);

        }

    }



    private IImsService mImsServiceCompat = null;

    private ImsDeathRecipient mImsDeathRecipient = null;



    public ImsServiceControllerStaticCompat(Context context, ComponentName componentName,

            ImsServiceController.ImsServiceControllerCallbacks callbacks) {
@@ -54,13 +76,29 @@ public class ImsServiceControllerStaticCompat extends ImsServiceControllerCompat 
        // This is a little hacky, but we are going to call the onServiceConnected to "pretend" like

        // bindService has completed here, which will pass the binder to setServiceController and

        // set up all supporting structures.

        connection.onServiceConnected(new ComponentName(mContext,

                ImsServiceControllerStaticCompat.class), binder);

        ComponentName name = new ComponentName(mContext, ImsServiceControllerStaticCompat.class);

        connection.onServiceConnected(name, binder);

        try {

            mImsDeathRecipient = new ImsDeathRecipient(name, connection);

            binder.linkToDeath(mImsDeathRecipient, 0);

        } catch (RemoteException e) {

            // The binder connection is already dead.. signal to the ImsServiceController to retry.

            mImsDeathRecipient.binderDied();

            mImsDeathRecipient = null;

        }

        return true;

    }



    @Override

    protected void setServiceController(IBinder serviceController) {

        if (serviceController == null) {

            // The service controller has been set to null, meaning it has been unbound or died.

            // Unlink if needed.

            if (mImsServiceCompat != null) {

                mImsServiceCompat.asBinder().unlinkToDeath(mImsDeathRecipient, 0);

            }

            mImsDeathRecipient = null;

        }

        mImsServiceCompat = IImsService.Stub.asInterface(serviceController);

    }

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

+55 −77

File changed.

Preview size limit exceeded, changes collapsed.