Merge "Clean up ImsFeature status callbacks correctly in FeatureConnection" into rvc-dev

3 files changed, 63 insertions, 12 deletions

diff --git a/src/java/com/android/ims/FeatureConnection.java b/src/java/com/android/ims/FeatureConnection.java
index 158c55af..f6668b53 100644
--- a/src/java/com/android/ims/FeatureConnection.java
+++ b/src/java/com/android/ims/FeatureConnection.java
@@ -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.
@@ -269,7 +263,36 @@ public abstract class FeatureConnection {
     }
 
     /**
+     * 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();
 }
diff --git a/src/java/com/android/ims/MmTelFeatureConnection.java b/src/java/com/android/ims/MmTelFeatureConnection.java
index 40075bcb..4d5a1799 100644
--- a/src/java/com/android/ims/MmTelFeatureConnection.java
+++ b/src/java/com/android/ims/MmTelFeatureConnection.java
@@ -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);
     }
diff --git a/src/java/com/android/ims/RcsFeatureConnection.java b/src/java/com/android/ims/RcsFeatureConnection.java
index e9b751e8..98e5576f 100644
--- a/src/java/com/android/ims/RcsFeatureConnection.java
+++ b/src/java/com/android/ims/RcsFeatureConnection.java
@@ -144,7 +144,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);
@@ -158,12 +158,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) {
@@ -303,6 +311,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);