Decouple the ImsFeature status listener from creating the feature

Add the listener after the ImsFeature has been created to ensure
that listeners do not miss the state update due to the feature not
being tracked internally yet.

(This is part 1 of a two part process to clean up ImsFeature
connecting).

Test: atest CtsTelephonyTestCases FrameworksTelephonyTests ImsCommonTests
Change-Id: I6a0cf4a4dfae2593ea77d3b098cd7a93325ea740

4 files changed, 130 insertions, 51 deletions

diff --git a/telephony/java/android/telephony/ims/ImsService.java b/telephony/java/android/telephony/ims/ImsService.java
index 2b3072eefe2e..da7311c08307 100644
--- a/telephony/java/android/telephony/ims/ImsService.java
+++ b/telephony/java/android/telephony/ims/ImsService.java
@@ -137,18 +137,30 @@ public class ImsService extends Service {
         }
 
         @Override
-        public IImsMmTelFeature createMmTelFeature(int slotId, IImsFeatureStatusCallback c) {
-            return createMmTelFeatureInternal(slotId, c);
+        public IImsMmTelFeature createMmTelFeature(int slotId) {
+            return createMmTelFeatureInternal(slotId);
         }
 
         @Override
-        public IImsRcsFeature createRcsFeature(int slotId, IImsFeatureStatusCallback c) {
-            return createRcsFeatureInternal(slotId, c);
+        public IImsRcsFeature createRcsFeature(int slotId) {
+            return createRcsFeatureInternal(slotId);
         }
 
         @Override
-        public void removeImsFeature(int slotId, int featureType, IImsFeatureStatusCallback c) {
-            ImsService.this.removeImsFeature(slotId, featureType, c);
+        public void addFeatureStatusCallback(int slotId, int featureType,
+                IImsFeatureStatusCallback c) {
+            ImsService.this.addImsFeatureStatusCallback(slotId, featureType, c);
+        }
+
+        @Override
+        public void removeFeatureStatusCallback(int slotId, int featureType,
+                IImsFeatureStatusCallback c) {
+            ImsService.this.removeImsFeatureStatusCallback(slotId, featureType, c);
+        }
+
+        @Override
+        public void removeImsFeature(int slotId, int featureType) {
+            ImsService.this.removeImsFeature(slotId, featureType);
         }
 
         @Override
@@ -204,11 +216,10 @@ public class ImsService extends Service {
         return mFeaturesBySlot.get(slotId);
     }
 
-    private IImsMmTelFeature createMmTelFeatureInternal(int slotId,
-            IImsFeatureStatusCallback c) {
+    private IImsMmTelFeature createMmTelFeatureInternal(int slotId) {
         MmTelFeature f = createMmTelFeature(slotId);
         if (f != null) {
-            setupFeature(f, slotId, ImsFeature.FEATURE_MMTEL, c);
+            setupFeature(f, slotId, ImsFeature.FEATURE_MMTEL);
             return f.getBinder();
         } else {
             Log.e(LOG_TAG, "createMmTelFeatureInternal: null feature returned.");
@@ -216,11 +227,10 @@ public class ImsService extends Service {
         }
     }
 
-    private IImsRcsFeature createRcsFeatureInternal(int slotId,
-            IImsFeatureStatusCallback c) {
+    private IImsRcsFeature createRcsFeatureInternal(int slotId) {
         RcsFeature f = createRcsFeature(slotId);
         if (f != null) {
-            setupFeature(f, slotId, ImsFeature.FEATURE_RCS, c);
+            setupFeature(f, slotId, ImsFeature.FEATURE_RCS);
             return f.getBinder();
         } else {
             Log.e(LOG_TAG, "createRcsFeatureInternal: null feature returned.");
@@ -228,13 +238,45 @@ public class ImsService extends Service {
         }
     }
 
-    private void setupFeature(ImsFeature f, int slotId, int featureType,
-            IImsFeatureStatusCallback c) {
+    private void setupFeature(ImsFeature f, int slotId, int featureType) {
         f.initialize(this, slotId);
-        f.addImsFeatureStatusCallback(c);
         addImsFeature(slotId, featureType, f);
     }
 
+    private void addImsFeatureStatusCallback(int slotId, int featureType,
+            IImsFeatureStatusCallback c) {
+        synchronized (mFeaturesBySlot) {
+            // get ImsFeature associated with the slot/feature
+            SparseArray<ImsFeature> features = mFeaturesBySlot.get(slotId);
+            if (features == null) {
+                Log.w(LOG_TAG, "Can not add ImsFeatureStatusCallback - no features on slot "
+                        + slotId);
+                return;
+            }
+            ImsFeature f = features.get(featureType);
+            if (f != null) {
+                f.addImsFeatureStatusCallback(c);
+            }
+        }
+    }
+
+    private void removeImsFeatureStatusCallback(int slotId, int featureType,
+            IImsFeatureStatusCallback c) {
+        synchronized (mFeaturesBySlot) {
+            // get ImsFeature associated with the slot/feature
+            SparseArray<ImsFeature> features = mFeaturesBySlot.get(slotId);
+            if (features == null) {
+                Log.w(LOG_TAG, "Can not remove ImsFeatureStatusCallback - no features on slot "
+                        + slotId);
+                return;
+            }
+            ImsFeature f = features.get(featureType);
+            if (f != null) {
+                f.removeImsFeatureStatusCallback(c);
+            }
+        }
+    }
+
     private void addImsFeature(int slotId, int featureType, ImsFeature f) {
         synchronized (mFeaturesBySlot) {
             // Get SparseArray for Features, by querying slot Id
@@ -248,8 +290,7 @@ public class ImsService extends Service {
         }
     }
 
-    private void removeImsFeature(int slotId, int featureType,
-            IImsFeatureStatusCallback c) {
+    private void removeImsFeature(int slotId, int featureType) {
         synchronized (mFeaturesBySlot) {
             // get ImsFeature associated with the slot/feature
             SparseArray<ImsFeature> features = mFeaturesBySlot.get(slotId);
@@ -264,7 +305,6 @@ public class ImsService extends Service {
                         + featureType + " exists on slot " + slotId);
                 return;
             }
-            f.removeImsFeatureStatusCallback(c);
             f.onFeatureRemoved();
             features.remove(featureType);
         }
diff --git a/telephony/java/android/telephony/ims/aidl/IImsServiceController.aidl b/telephony/java/android/telephony/ims/aidl/IImsServiceController.aidl
index c7da681b86a3..c956cbcc816c 100644
--- a/telephony/java/android/telephony/ims/aidl/IImsServiceController.aidl
+++ b/telephony/java/android/telephony/ims/aidl/IImsServiceController.aidl
@@ -31,12 +31,14 @@ import com.android.ims.internal.IImsFeatureStatusCallback;
  */
 interface IImsServiceController {
     void setListener(IImsServiceControllerListener l);
-    IImsMmTelFeature createMmTelFeature(int slotId, in IImsFeatureStatusCallback c);
-    IImsRcsFeature createRcsFeature(int slotId, in IImsFeatureStatusCallback c);
+    IImsMmTelFeature createMmTelFeature(int slotId);
+    IImsRcsFeature createRcsFeature(int slotId);
     ImsFeatureConfiguration querySupportedImsFeatures();
+    void addFeatureStatusCallback(int slotId, int featureType, in IImsFeatureStatusCallback c);
+    void removeFeatureStatusCallback(int slotId, int featureType, in IImsFeatureStatusCallback c);
     // Synchronous call to ensure the ImsService is ready before continuing with feature creation.
     void notifyImsServiceReadyForFeatureCreation();
-    void removeImsFeature(int slotId, int featureType, in IImsFeatureStatusCallback c);
+    void removeImsFeature(int slotId, int featureType);
     IImsConfig getConfig(int slotId);
     IImsRegistration getRegistration(int slotId);
     oneway void enableIms(int slotId);
diff --git a/telephony/java/android/telephony/ims/compat/ImsService.java b/telephony/java/android/telephony/ims/compat/ImsService.java
index eafbb14539f5..41d1d726b3f4 100644
--- a/telephony/java/android/telephony/ims/compat/ImsService.java
+++ b/telephony/java/android/telephony/ims/compat/ImsService.java
@@ -21,7 +21,6 @@ import android.app.Service;
 import android.compat.annotation.UnsupportedAppUsage;
 import android.content.Intent;
 import android.os.IBinder;
-import android.os.RemoteException;
 import android.telephony.CarrierConfigManager;
 import android.telephony.ims.compat.feature.ImsFeature;
 import android.telephony.ims.compat.feature.MMTelFeature;
@@ -91,25 +90,35 @@ public class ImsService extends Service {
     protected final IBinder mImsServiceController = new IImsServiceController.Stub() {
 
         @Override
-        public IImsMMTelFeature createEmergencyMMTelFeature(int slotId,
-                IImsFeatureStatusCallback c) {
-            return createEmergencyMMTelFeatureInternal(slotId, c);
+        public IImsMMTelFeature createEmergencyMMTelFeature(int slotId) {
+            return createEmergencyMMTelFeatureInternal(slotId);
         }
 
         @Override
-        public IImsMMTelFeature createMMTelFeature(int slotId, IImsFeatureStatusCallback c) {
-            return createMMTelFeatureInternal(slotId, c);
+        public IImsMMTelFeature createMMTelFeature(int slotId) {
+            return createMMTelFeatureInternal(slotId);
         }
 
         @Override
-        public IImsRcsFeature createRcsFeature(int slotId, IImsFeatureStatusCallback c) {
-            return createRcsFeatureInternal(slotId, c);
+        public IImsRcsFeature createRcsFeature(int slotId) {
+            return createRcsFeatureInternal(slotId);
         }
 
         @Override
-        public void removeImsFeature(int slotId, int featureType, IImsFeatureStatusCallback c)
-                throws RemoteException {
-            ImsService.this.removeImsFeature(slotId, featureType, c);
+        public void removeImsFeature(int slotId, int featureType) {
+            ImsService.this.removeImsFeature(slotId, featureType);
+        }
+
+        @Override
+        public void addFeatureStatusCallback(int slotId, int featureType,
+                IImsFeatureStatusCallback c) {
+            addImsFeatureStatusCallback(slotId, featureType, c);
+        }
+
+        @Override
+        public void removeFeatureStatusCallback(int slotId, int featureType,
+                IImsFeatureStatusCallback c) {
+            removeImsFeatureStatusCallback(slotId, featureType, c);
         }
     };
 
@@ -137,46 +146,40 @@ public class ImsService extends Service {
         return mFeaturesBySlot.get(slotId);
     }
 
-    private IImsMMTelFeature createEmergencyMMTelFeatureInternal(int slotId,
-            IImsFeatureStatusCallback c) {
+    private IImsMMTelFeature createEmergencyMMTelFeatureInternal(int slotId) {
         MMTelFeature f = onCreateEmergencyMMTelImsFeature(slotId);
         if (f != null) {
-            setupFeature(f, slotId, ImsFeature.EMERGENCY_MMTEL, c);
+            setupFeature(f, slotId, ImsFeature.EMERGENCY_MMTEL);
             return f.getBinder();
         } else {
             return null;
         }
     }
 
-    private IImsMMTelFeature createMMTelFeatureInternal(int slotId,
-            IImsFeatureStatusCallback c) {
+    private IImsMMTelFeature createMMTelFeatureInternal(int slotId) {
         MMTelFeature f = onCreateMMTelImsFeature(slotId);
         if (f != null) {
-            setupFeature(f, slotId, ImsFeature.MMTEL, c);
+            setupFeature(f, slotId, ImsFeature.MMTEL);
             return f.getBinder();
         } else {
             return null;
         }
     }
 
-    private IImsRcsFeature createRcsFeatureInternal(int slotId,
-            IImsFeatureStatusCallback c) {
+    private IImsRcsFeature createRcsFeatureInternal(int slotId) {
         RcsFeature f = onCreateRcsFeature(slotId);
         if (f != null) {
-            setupFeature(f, slotId, ImsFeature.RCS, c);
+            setupFeature(f, slotId, ImsFeature.RCS);
             return f.getBinder();
         } else {
             return null;
         }
     }
 
-    private void setupFeature(ImsFeature f, int slotId, int featureType,
-            IImsFeatureStatusCallback c) {
+    private void setupFeature(ImsFeature f, int slotId, int featureType) {
         f.setContext(this);
         f.setSlotId(slotId);
-        f.addImsFeatureStatusCallback(c);
         addImsFeature(slotId, featureType, f);
-        // TODO: Remove once new onFeatureReady AIDL is merged in.
         f.onFeatureReady();
     }
 
@@ -193,12 +196,45 @@ public class ImsService extends Service {
         }
     }
 
-    private void removeImsFeature(int slotId, int featureType,
+    private void addImsFeatureStatusCallback(int slotId, int featureType,
+            IImsFeatureStatusCallback c) {
+        synchronized (mFeaturesBySlot) {
+            // get ImsFeature associated with the slot/feature
+            SparseArray<ImsFeature> features = mFeaturesBySlot.get(slotId);
+            if (features == null) {
+                Log.w(LOG_TAG, "Can not add ImsFeatureStatusCallback. No ImsFeatures exist on"
+                        + " slot " + slotId);
+                return;
+            }
+            ImsFeature f = features.get(featureType);
+            if (f != null) {
+                f.addImsFeatureStatusCallback(c);
+            }
+        }
+    }
+
+    private void removeImsFeatureStatusCallback(int slotId, int featureType,
             IImsFeatureStatusCallback c) {
         synchronized (mFeaturesBySlot) {
             // get ImsFeature associated with the slot/feature
             SparseArray<ImsFeature> features = mFeaturesBySlot.get(slotId);
             if (features == null) {
+                Log.w(LOG_TAG, "Can not remove ImsFeatureStatusCallback. No ImsFeatures exist on"
+                        + " slot " + slotId);
+                return;
+            }
+            ImsFeature f = features.get(featureType);
+            if (f != null) {
+                f.removeImsFeatureStatusCallback(c);
+            }
+        }
+    }
+
+    private void removeImsFeature(int slotId, int featureType) {
+        synchronized (mFeaturesBySlot) {
+            // get ImsFeature associated with the slot/feature
+            SparseArray<ImsFeature> features = mFeaturesBySlot.get(slotId);
+            if (features == null) {
                 Log.w(LOG_TAG, "Can not remove ImsFeature. No ImsFeatures exist on slot "
                         + slotId);
                 return;
@@ -209,7 +245,6 @@ public class ImsService extends Service {
                         + featureType + " exists on slot " + slotId);
                 return;
             }
-            f.removeImsFeatureStatusCallback(c);
             f.onFeatureRemoved();
             features.remove(featureType);
         }
diff --git a/telephony/java/com/android/ims/internal/IImsServiceController.aidl b/telephony/java/com/android/ims/internal/IImsServiceController.aidl
index 857089fac33a..e9528f474082 100644
--- a/telephony/java/com/android/ims/internal/IImsServiceController.aidl
+++ b/telephony/java/com/android/ims/internal/IImsServiceController.aidl
@@ -25,8 +25,10 @@ import com.android.ims.internal.IImsRcsFeature;
  * {@hide}
  */
 interface IImsServiceController {
-    IImsMMTelFeature createEmergencyMMTelFeature(int slotId, in IImsFeatureStatusCallback c);
-    IImsMMTelFeature createMMTelFeature(int slotId, in IImsFeatureStatusCallback c);
-    IImsRcsFeature createRcsFeature(int slotId, in IImsFeatureStatusCallback c);
-    void removeImsFeature(int slotId, int featureType, in IImsFeatureStatusCallback c);
+    IImsMMTelFeature createEmergencyMMTelFeature(int slotId);
+    IImsMMTelFeature createMMTelFeature(int slotId);
+    IImsRcsFeature createRcsFeature(int slotId);
+    void removeImsFeature(int slotId, int featureType);
+    void addFeatureStatusCallback(int slotId, int featureType, in IImsFeatureStatusCallback c);
+    void removeFeatureStatusCallback(int slotId, int featureType, in IImsFeatureStatusCallback c);
 }