Notify ImsService Status Callbacks correctly
Brad Ebinger [Sat, 6 May 2017 00:45:36 +0000 (17:45 -0700)]
Adds support in the ImsServiceController for multiple
Feature listeners for one feature. Also adds support for
signaling the ImsFeature to remove the binder callback when
the feature is removed.

Stops Phone from listening to IMS_SERVUICE_UP/DOWN when
using the ImsResolver.

Bug: 38001858
Test: Unit Tests
Merged-In: I920605b32160fdce9fcd3b4c0100804d3590397f
Change-Id: I920605b32160fdce9fcd3b4c0100804d3590397f

src/java/com/android/internal/telephony/Phone.java
src/java/com/android/internal/telephony/ims/ImsServiceController.java
tests/telephonytests/src/android/telephony/ims/ImsFeatureTest.java
tests/telephonytests/src/android/telephony/ims/ImsServiceTest.java
tests/telephonytests/src/com/android/internal/telephony/ims/TestImsServiceControllerAdapter.java

index 1b2e11d..30d1591 100644 (file)
@@ -543,15 +543,18 @@ public abstract class Phone extends Handler implements PhoneInternalInterface {
 
         synchronized(Phone.lockForRadioTechnologyChange) {
             IntentFilter filter = new IntentFilter();
-            filter.addAction(ImsManager.ACTION_IMS_SERVICE_UP);
-            filter.addAction(ImsManager.ACTION_IMS_SERVICE_DOWN);
+            ImsManager imsManager = ImsManager.getInstance(mContext, getPhoneId());
+            // Don't listen to deprecated intents using the new dynamic binding.
+            if (imsManager != null && !imsManager.isDynamicBinding()) {
+                filter.addAction(ImsManager.ACTION_IMS_SERVICE_UP);
+                filter.addAction(ImsManager.ACTION_IMS_SERVICE_DOWN);
+            }
             filter.addAction(ImsConfig.ACTION_IMS_CONFIG_CHANGED);
             mContext.registerReceiver(mImsIntentReceiver, filter);
 
             // Monitor IMS service - but first poll to see if already up (could miss
             // intent). Also, when using new ImsResolver APIs, the service will be available soon,
             // so start trying to bind.
-            ImsManager imsManager = ImsManager.getInstance(mContext, getPhoneId());
             if (imsManager != null) {
                 // If it is dynamic binding, kick off ImsPhone creation now instead of waiting for
                 // the service to be available.
index 2362348..5257dad 100644 (file)
@@ -284,7 +284,7 @@ public class ImsServiceController {
     }
 
     /**
-     * Calls {@link IImsServiceController#removeImsFeature(int, int)} on all features that the
+     * Calls {@link IImsServiceController#removeImsFeature} on all features that the
      * ImsService supports and then unbinds the service.
      */
     public void unbind() throws RemoteException {
@@ -459,7 +459,15 @@ public class ImsServiceController {
             Log.w(LOG_TAG, "removeImsServiceFeature called with null values.");
             return;
         }
-        mIImsServiceController.removeImsFeature(featurePair.first, featurePair.second);
+        ImsFeatureStatusCallback callbackToRemove = mFeatureStatusCallbacks.stream().filter(c ->
+                c.mSlotId == featurePair.first && c.mFeatureType == featurePair.second)
+                .findFirst().orElse(null);
+        // Remove status callbacks from list.
+        if (callbackToRemove != null) {
+            mFeatureStatusCallbacks.remove(callbackToRemove);
+        }
+        mIImsServiceController.removeImsFeature(featurePair.first, featurePair.second,
+                (callbackToRemove != null ? callbackToRemove.getCallback() : null));
         // Signal ImsResolver to change supported ImsFeatures for this ImsServiceController
         mCallbacks.imsServiceFeatureRemoved(featurePair.first, featurePair.second, this);
         // Send callback to ImsServiceProxy to change supported ImsFeatures
@@ -467,9 +475,6 @@ public class ImsServiceController {
         // ImsManager requests the ImsService while it is being removed in ImsResolver, this
         // callback will clean it up after.
         sendImsFeatureRemovedCallback(featurePair.first, featurePair.second);
-        // Remove status callbacks from list.
-        mFeatureStatusCallbacks.removeIf(c -> c.mSlotId == featurePair.first
-                && c.mFeatureType == featurePair.second);
     }
 
     private void notifyAllFeaturesRemoved() {
index 8248c08..acd4e19 100644 (file)
@@ -43,6 +43,8 @@ public class ImsFeatureTest {
     @Mock
     private IImsFeatureStatusCallback mTestStatusCallback;
     @Mock
+    private IImsFeatureStatusCallback mTestStatusCallback2;
+    @Mock
     private ImsFeature.INotifyFeatureRemoved mTestRemovedCallback;
 
     private class TestImsFeature extends ImsFeature {
@@ -73,19 +75,23 @@ public class ImsFeatureTest {
     @Test
     @SmallTest
     public void testSetCallbackAndNotify() throws Exception {
-        mTestImsService.setImsFeatureStatusCallback(mTestStatusCallback);
+        mTestImsService.addImsFeatureStatusCallback(mTestStatusCallback);
+        mTestImsService.addImsFeatureStatusCallback(mTestStatusCallback2);
 
         verify(mTestStatusCallback).notifyImsFeatureStatus(eq(ImsFeature.STATE_NOT_AVAILABLE));
+        verify(mTestStatusCallback2).notifyImsFeatureStatus(eq(ImsFeature.STATE_NOT_AVAILABLE));
     }
 
     @Test
     @SmallTest
     public void testSetFeatureAndCheckCallback() throws Exception {
-        mTestImsService.setImsFeatureStatusCallback(mTestStatusCallback);
+        mTestImsService.addImsFeatureStatusCallback(mTestStatusCallback);
+        mTestImsService.addImsFeatureStatusCallback(mTestStatusCallback2);
 
         mTestImsService.testSetFeatureState(ImsFeature.STATE_READY);
 
         verify(mTestStatusCallback).notifyImsFeatureStatus(eq(ImsFeature.STATE_READY));
+        verify(mTestStatusCallback2).notifyImsFeatureStatus(eq(ImsFeature.STATE_READY));
         assertEquals(ImsFeature.STATE_READY, mTestImsService.getFeatureState());
     }
 
index 772bb37..3a80fa6 100644 (file)
@@ -101,7 +101,7 @@ public class ImsServiceTest {
                 mTestImsService.getImsFeatureFromType(features, ImsFeature.MMTEL));
         // Verify that upon creating a feature, we assign the callback and get the set feature state
         // when querying it.
-        verify(mTestImsService.mSpyMMTelFeature).setImsFeatureStatusCallback(eq(mTestCallback));
+        verify(mTestImsService.mSpyMMTelFeature).addImsFeatureStatusCallback(eq(mTestCallback));
         assertEquals(ImsFeature.STATE_READY, mTestImsServiceBinder.getFeatureStatus(TEST_SLOT_0,
                 ImsFeature.MMTEL));
     }
@@ -111,10 +111,10 @@ public class ImsServiceTest {
     public void testRemoveMMTelFeature() throws RemoteException {
         mTestImsServiceBinder.createImsFeature(TEST_SLOT_0, ImsFeature.MMTEL, mTestCallback);
 
-        mTestImsServiceBinder.removeImsFeature(TEST_SLOT_0, ImsFeature.MMTEL);
+        mTestImsServiceBinder.removeImsFeature(TEST_SLOT_0, ImsFeature.MMTEL, mTestCallback);
 
         verify(mTestImsService.mSpyMMTelFeature).notifyFeatureRemoved(eq(0));
-        verify(mTestImsService.mSpyMMTelFeature).setImsFeatureStatusCallback(null);
+        verify(mTestImsService.mSpyMMTelFeature).removeImsFeatureStatusCallback(mTestCallback);
         SparseArray<ImsFeature> features = mTestImsService.getImsFeatureMap(TEST_SLOT_0);
         assertNull(mTestImsService.getImsFeatureFromType(features, ImsFeature.MMTEL));
     }
@@ -186,14 +186,11 @@ public class ImsServiceTest {
         mTestImsService.mSpyMMTelFeature.sendSetFeatureState(ImsFeature.STATE_READY);
 
         ArgumentCaptor<Intent> intentCaptor = ArgumentCaptor.forClass(Intent.class);
-        verify(mMockContext, times(2)).sendBroadcast(intentCaptor.capture());
+        verify(mMockContext).sendBroadcast(intentCaptor.capture());
         try {
-            // IMS_SERVICE_DOWN is always sent when createImsFeature completes
-            assertNotNull(intentCaptor.getAllValues().get(0));
-            verifyServiceDownSent(intentCaptor.getAllValues().get(0));
             // Verify IMS_SERVICE_UP is sent
-            assertNotNull(intentCaptor.getAllValues().get(1));
-            verifyServiceUpSent(intentCaptor.getAllValues().get(1));
+            assertNotNull(intentCaptor.getValue());
+            verifyServiceUpSent(intentCaptor.getValue());
         } catch (IndexOutOfBoundsException e) {
             fail("Did not receive all intents");
         }
@@ -211,37 +208,16 @@ public class ImsServiceTest {
         mTestImsService.mSpyMMTelFeature.sendSetFeatureState(ImsFeature.STATE_INITIALIZING);
 
         ArgumentCaptor<Intent> intentCaptor = ArgumentCaptor.forClass(Intent.class);
-        verify(mMockContext, times(2)).sendBroadcast(intentCaptor.capture());
+        verify(mMockContext).sendBroadcast(intentCaptor.capture());
         try {
-            // IMS_SERVICE_DOWN is always sent when createImsFeature completes.
-            assertNotNull(intentCaptor.getAllValues().get(0));
-            verifyServiceDownSent(intentCaptor.getAllValues().get(0));
             // IMS_SERVICE_DOWN is sent when the service is STATE_INITIALIZING.
-            assertNotNull(intentCaptor.getAllValues().get(1));
-            verifyServiceDownSent(intentCaptor.getAllValues().get(1));
+            assertNotNull(intentCaptor.getValue());
+            verifyServiceDownSent(intentCaptor.getValue());
         } catch (IndexOutOfBoundsException e) {
             fail("Did not receive all intents");
         }
     }
 
-    /**
-     * Tests that the new ImsService still sends the IMS_SERVICE_DOWN broadcast when the feature is
-     * set to not available.
-     */
-    @Test
-    @SmallTest
-    public void testImsServiceDownSentCompatNotAvailable() throws RemoteException {
-        mTestImsServiceBinder.createImsFeature(TEST_SLOT_0, ImsFeature.MMTEL, mTestCallback);
-
-        // The ImsService will send the STATE_NOT_AVAILABLE status as soon as the feature is
-        // created.
-
-        ArgumentCaptor<Intent> intentCaptor = ArgumentCaptor.forClass(Intent.class);
-        verify(mMockContext).sendBroadcast(intentCaptor.capture());
-        assertNotNull(intentCaptor.getValue());
-        verifyServiceDownSent(intentCaptor.getValue());
-    }
-
     private void verifyServiceDownSent(Intent testIntent) {
         assertEquals(ImsManager.ACTION_IMS_SERVICE_DOWN, testIntent.getAction());
         assertEquals(TEST_SLOT_0, testIntent.getIntExtra(ImsManager.EXTRA_PHONE_ID, -1));
index 564e284..0e22be1 100644 (file)
@@ -52,7 +52,8 @@ public class TestImsServiceControllerAdapter {
         }
 
         @Override
-        public void removeImsFeature(int slotId, int feature) throws RemoteException {
+        public void removeImsFeature(int slotId, int feature, IImsFeatureStatusCallback c)
+                throws RemoteException {
             TestImsServiceControllerAdapter.this.removeImsFeature(slotId, feature);
         }