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

Commit c41c0c29 authored by Brad Ebinger's avatar Brad Ebinger
Browse files

Clean up ImsFeature status callbacks correctly in FeatureConnection 

1) ImsFeatureStatusCallbacks were not being deregistered when the
MmTelFeatureConnection was removed, causing callbacks to still call
into that FeatureConnection. This caused a memory leak as well as
duplicate callbacks into ImsManager.

2) Small refactor to better decouple the abstract FeatureConnection
from the MmTelFeatureConnection

Bug: 156893040
Fixes: 157948894
Test: atest FrameworksTelephonyTests CtsTelephonyTestCases:ImsServiceTest
Merged-In: Iedf820f9da10088c845b34c2a08658d7204241b3
Change-Id: Iedf820f9da10088c845b34c2a08658d7204241b3
parent 881b57b1

src/java/com/android/ims/FeatureConnection.java

+33 −10
Original line number Diff line number Diff line
@@ -57,7 +57,6 @@ public abstract class FeatureConnection { 
    protected static boolean sImsSupportedOnDevice = true;



    protected final int mSlotId;

    protected final int mFeatureType;

    protected Context mContext;

    protected IBinder mBinder;

    @VisibleForTesting
@@ -71,10 +70,9 @@ public abstract class FeatureConnection { 
    protected IImsRegistration mRegistrationBinder;

    protected final Object mLock = new Object();



    public FeatureConnection(Context context, int slotId, int featureType) {

    public FeatureConnection(Context context, int slotId) {

        mSlotId = slotId;

        mContext = context;

        mFeatureType = featureType;



        // Callbacks should be scheduled on the main thread.

        if (context.getMainLooper() != null) {
@@ -135,6 +133,8 @@ public abstract class FeatureConnection { 
                if (mStatusCallback != null) {

                    Log.d(TAG, "onRemovedOrDied: notifyUnavailable");

                    mStatusCallback.notifyUnavailable();

                    // Unlink because this FeatureConnection should no longer send callbacks.

                    mStatusCallback = null;

                }

            }

        }
@@ -178,10 +178,6 @@ public abstract class FeatureConnection { 
            }

        };



    protected abstract void handleImsFeatureCreatedCallback(int slotId, int feature);

    protected abstract void handleImsFeatureRemovedCallback(int slotId, int feature);

    protected abstract void handleImsStatusChangedCallback(int slotId, int feature, int status);



    public @ImsRegistrationImplBase.ImsRegistrationTech int getRegistrationTech()

            throws RemoteException {

        IImsRegistration registration = getRegistration();
@@ -200,10 +196,8 @@ public abstract class FeatureConnection { 
                return mRegistrationBinder;

            }

        }

        TelephonyManager tm = getTelephonyManager();

        // We don't want to synchronize on a binder call to another process.

        IImsRegistration regBinder = tm != null

            ? tm.getImsRegistration(mSlotId, mFeatureType) : null;

        IImsRegistration regBinder = getRegistrationBinder();

        synchronized (mLock) {

            // mRegistrationBinder may have changed while we tried to get the registration

            // interface.
@@ -268,8 +262,37 @@ public abstract class FeatureConnection { 
        return state;

    }



    /**

     * An ImsFeature has been created for this FeatureConnection for the associated

     * {@link ImsFeature.FeatureType}.

     * @param slotId The slot ID associated with the event.

     * @param feature The {@link ImsFeature.FeatureType} associated with the event.

     */

    protected abstract void handleImsFeatureCreatedCallback(int slotId, int feature);



    /**

     * An ImsFeature has been removed for this FeatureConnection for the associated

     * {@link ImsFeature.FeatureType}.

     * @param slotId The slot ID associated with the event.

     * @param feature The {@link ImsFeature.FeatureType} associated with the event.

     */

    protected abstract void handleImsFeatureRemovedCallback(int slotId, int feature);



    /**

     * The status of an ImsFeature has changed for the associated {@link ImsFeature.FeatureType}.

     * @param slotId The slot ID associated with the event.

     * @param feature The {@link ImsFeature.FeatureType} associated with the event.

     * @param status The new {@link ImsFeature.ImsState} associated with the ImsFeature

     */

    protected abstract void handleImsStatusChangedCallback(int slotId, int feature, int status);



    /**

     * Internal method used to retrieve the feature status from the corresponding ImsService.

     */

    protected abstract Integer retrieveFeatureState();



    /**

     * @return The ImsRegistration instance associated with the FeatureConnection.

     */

    protected abstract IImsRegistration getRegistrationBinder();

}

src/java/com/android/ims/MmTelFeatureConnection.java

+15 −1
Original line number Diff line number Diff line
@@ -230,7 +230,7 @@ public class MmTelFeatureConnection extends FeatureConnection { 
    }



    public MmTelFeatureConnection(Context context, int slotId) {

        super(context, slotId, ImsFeature.FEATURE_MMTEL);

        super(context, slotId);



        mRegistrationCallbackManager = new ImsRegistrationCallbackAdapter(context, mLock);

        mCapabilityCallbackManager = new CapabilityCallbackManager(context, mLock);
@@ -239,6 +239,7 @@ public class MmTelFeatureConnection extends FeatureConnection { 


    @Override

    protected void onRemovedOrDied() {

        removeImsFeatureCallback();

        synchronized (mLock) {

            super.onRemovedOrDied();

            mRegistrationCallbackManager.close();
@@ -248,6 +249,13 @@ public class MmTelFeatureConnection extends FeatureConnection { 
        }

    }



    private void removeImsFeatureCallback() {

        TelephonyManager tm = getTelephonyManager();

        if (tm != null) {

            tm.unregisterImsFeatureCallback(mSlotId, ImsFeature.FEATURE_MMTEL, getListener());

        }

    }



    private IImsConfig getConfig() {

        synchronized (mLock) {

            // null if cache is invalid;
@@ -553,6 +561,12 @@ public class MmTelFeatureConnection extends FeatureConnection { 
        return null;

    }



    @Override

    protected IImsRegistration getRegistrationBinder() {

        TelephonyManager tm = getTelephonyManager();

        return  tm != null ? tm.getImsRegistration(mSlotId, ImsFeature.FEATURE_MMTEL) : null;

    }



    private IImsMmTelFeature getServiceInterface(IBinder b) {

        return IImsMmTelFeature.Stub.asInterface(b);

    }

src/java/com/android/ims/RcsFeatureConnection.java

+15 −1
Original line number Diff line number Diff line
@@ -147,7 +147,7 @@ public class RcsFeatureConnection extends FeatureConnection { 
    public RegistrationCallbackManager mRegistrationCallbackManager;



    private RcsFeatureConnection(Context context, int slotId, IFeatureUpdate callback) {

        super(context, slotId, ImsFeature.FEATURE_RCS);

        super(context, slotId);

        setStatusCallback(callback);

        mAvailabilityCallbackManager = new AvailabilityCallbackManager(mContext);

        mRegistrationCallbackManager = new RegistrationCallbackManager(mContext);
@@ -161,12 +161,20 @@ public class RcsFeatureConnection extends FeatureConnection { 


    @Override

    protected void onRemovedOrDied() {

        removeImsFeatureCallback();

        super.onRemovedOrDied();

        synchronized (mLock) {

            close();

        }

    }



    private void removeImsFeatureCallback() {

        TelephonyManager tm = getTelephonyManager();

        if (tm != null) {

            tm.unregisterImsFeatureCallback(mSlotId, ImsFeature.FEATURE_RCS, getListener());

        }

    }



    @Override

    @VisibleForTesting

    public void handleImsFeatureCreatedCallback(int slotId, int feature) {
@@ -321,6 +329,12 @@ public class RcsFeatureConnection extends FeatureConnection { 
        return null;

    }



    @Override

    protected IImsRegistration getRegistrationBinder() {

        TelephonyManager tm = getTelephonyManager();

        return  tm != null ? tm.getImsRegistration(mSlotId, ImsFeature.FEATURE_RCS) : null;

    }



    @VisibleForTesting

    public IImsRcsFeature getServiceInterface(IBinder b) {

        return IImsRcsFeature.Stub.asInterface(b);