Handle Dead Objects from Phone Crashes better

1) Check to make sure ImsService is available when calling
changeMmTelCapability.
2) Remove caching of ImsConfig in ImsManager. It is already
cached properly in MmTelFeatureConnection.
3) Create a DeathRecipient for MmTelFeature in
MmTelFeatureConnection that cleans up all caches and notifies
ImsManager that the connection is now unavailable.

Bug: 77941698
Test: Manual
Merged-In: Ib26ce5308ec75113ad02c281b2a19cba5f94cdac
Change-Id: I9c3721fe275a94f7f8841e8e84a2f9d8a3c4823b

4 files changed, 66 insertions, 57 deletions

diff --git a/src/java/com/android/ims/ImsManager.java b/src/java/com/android/ims/ImsManager.java
index 3ad6c72e..a01bbbbd 100644
--- a/src/java/com/android/ims/ImsManager.java
+++ b/src/java/com/android/ims/ImsManager.java
@@ -368,18 +368,16 @@ public class ImsManager {
     private int mPhoneId;
     private final boolean mConfigDynamicBind;
     private @Nullable MmTelFeatureConnection mMmTelFeatureConnection = null;
-    private ImsServiceDeathRecipient mDeathRecipient = new ImsServiceDeathRecipient();
-    // Ut interface for the supplementary service configuration
-    private ImsUt mUt = null;
-    // Interface to get/set ims config items
-    private ImsConfig mConfig = null;
     private boolean mConfigUpdated = false;
 
     private ImsConfigListener mImsConfigListener;
 
+    //TODO: Move these caches into the MmTelFeature Connection and restrict their lifetimes to the
+    // lifetime of the MmTelFeature.
+    // Ut interface for the supplementary service configuration
+    private ImsUt mUt = null;
     // ECBM interface
     private ImsEcbm mEcbm = null;
-
     private ImsMultiEndpoint mMultiEndpoint = null;
 
     private Set<MmTelFeatureConnection.IFeatureUpdate> mStatusCallbacks =
@@ -774,7 +772,7 @@ public class ImsManager {
                 log("setVtSetting(b) : imsServiceAllowTurnOff -> turnOffIms");
                 turnOffIms();
             }
-        } catch (ImsException | RemoteException e) {
+        } catch (ImsException e) {
             // The ImsService is down. Since the SubscriptionManager already recorded the user's
             // preference, it will be resent in updateImsServiceConfig when the ImsPhoneCallTracker
             // reconnects.
@@ -902,7 +900,7 @@ public class ImsManager {
             }
 
             setWfcModeInternal(imsWfcModeFeatureValue);
-        } catch (ImsException | RemoteException e) {
+        } catch (ImsException e) {
             loge("setWfcSetting(): ", e);
         }
     }
@@ -1301,7 +1299,7 @@ public class ImsManager {
                 }
 
                 mConfigUpdated = true;
-            } catch (ImsException | RemoteException e) {
+            } catch (ImsException e) {
                 loge("updateImsServiceConfig: ", e);
                 mConfigUpdated = false;
             }
@@ -1313,7 +1311,7 @@ public class ImsManager {
      * @return whether feature is On
      * @throws ImsException
      */
-    private boolean updateVolteFeatureValue() throws RemoteException {
+    private boolean updateVolteFeatureValue() throws ImsException {
         boolean available = isVolteEnabledByPlatform();
         boolean enabled = isEnhanced4gLteModeSettingEnabledByUser();
         boolean isNonTty = isNonTtyOrTtyOnVolteEnabled();
@@ -1334,7 +1332,7 @@ public class ImsManager {
      * @return whether feature is On
      * @throws ImsException
      */
-    private boolean updateVideoCallFeatureValue() throws RemoteException {
+    private boolean updateVideoCallFeatureValue() throws ImsException {
         boolean available = isVtEnabledByPlatform();
         boolean enabled = isVtEnabledByUser();
         boolean isNonTty = isNonTtyOrTtyOnVolteEnabled();
@@ -1361,7 +1359,7 @@ public class ImsManager {
      * @return whether feature is On
      * @throws ImsException
      */
-    private boolean updateWfcFeatureAndProvisionedValues() throws RemoteException {
+    private boolean updateWfcFeatureAndProvisionedValues() throws ImsException {
         TelephonyManager tm = new TelephonyManager(mContext, getSubId());
         boolean isNetworkRoaming = tm.isNetworkRoaming();
         boolean available = isWfcEnabledByPlatform();
@@ -1652,7 +1650,6 @@ public class ImsManager {
             mMmTelFeatureConnection.closeConnection();
         }
         mUt = null;
-        mConfig = null;
         mEcbm = null;
         mMultiEndpoint = null;
     }
@@ -1801,41 +1798,44 @@ public class ImsManager {
      * @throws ImsException if getting the setting interface results in an error.
      */
     public ImsConfig getConfigInterface() throws ImsException {
-        if (mConfig != null && mConfig.isBinderAlive()) {
-            return mConfig;
-        }
-
         checkAndThrowExceptionIfServiceUnavailable();
+
         try {
             IImsConfig config = mMmTelFeatureConnection.getConfigInterface();
             if (config == null) {
                 throw new ImsException("getConfigInterface()",
                         ImsReasonInfo.CODE_LOCAL_SERVICE_UNAVAILABLE);
             }
-            mConfig = new ImsConfig(config, mContext);
+            return new ImsConfig(config);
         } catch (RemoteException e) {
             throw new ImsException("getConfigInterface()", e,
                     ImsReasonInfo.CODE_LOCAL_IMS_SERVICE_DOWN);
         }
-        return mConfig;
     }
 
     public void changeMmTelCapability(
             @MmTelFeature.MmTelCapabilities.MmTelCapability int capability,
             @ImsRegistrationImplBase.ImsRegistrationTech int radioTech,
-            boolean isEnabled) throws RemoteException {
+            boolean isEnabled) throws ImsException {
+        checkAndThrowExceptionIfServiceUnavailable();
+
         CapabilityChangeRequest request = new CapabilityChangeRequest();
         if (isEnabled) {
             request.addCapabilitiesToEnableForTech(capability, radioTech);
         } else {
             request.addCapabilitiesToDisableForTech(capability, radioTech);
         }
-        mMmTelFeatureConnection.changeEnabledCapabilities(request, null);
-        if (mImsConfigListener != null) {
-            mImsConfigListener.onSetFeatureResponse(capability,
-                    mMmTelFeatureConnection.getRegistrationTech(),
-                    isEnabled ? ImsConfig.FeatureValueConstants.ON
-                            : ImsConfig.FeatureValueConstants.OFF, -1);
+        try {
+            mMmTelFeatureConnection.changeEnabledCapabilities(request, null);
+            if (mImsConfigListener != null) {
+                mImsConfigListener.onSetFeatureResponse(capability,
+                        mMmTelFeatureConnection.getRegistrationTech(),
+                        isEnabled ? ImsConfig.FeatureValueConstants.ON
+                                : ImsConfig.FeatureValueConstants.OFF, -1);
+            }
+        } catch (RemoteException e) {
+            throw new ImsException("changeMmTelCapability()", e,
+                    ImsReasonInfo.CODE_LOCAL_IMS_SERVICE_DOWN);
         }
     }
 
@@ -2150,20 +2150,6 @@ public class ImsManager {
     }
 
     /**
-     * Death recipient class for monitoring IMS service.
-     */
-    private class ImsServiceDeathRecipient implements IBinder.DeathRecipient {
-        @Override
-        public void binderDied() {
-            mMmTelFeatureConnection = null;
-            mUt = null;
-            mConfig = null;
-            mEcbm = null;
-            mMultiEndpoint = null;
-        }
-    }
-
-    /**
      * Gets the ECBM interface to request ECBM exit.
      *
      * @return the ECBM interface instance
diff --git a/src/java/com/android/ims/MmTelFeatureConnection.java b/src/java/com/android/ims/MmTelFeatureConnection.java
index 07069219..d6954f3f 100644
--- a/src/java/com/android/ims/MmTelFeatureConnection.java
+++ b/src/java/com/android/ims/MmTelFeatureConnection.java
@@ -75,6 +75,11 @@ public class MmTelFeatureConnection {
     private IImsRegistration mRegistrationBinder;
     private IImsConfig mConfigBinder;
 
+    private IBinder.DeathRecipient mDeathRecipient = () -> {
+            Log.w(TAG, "DeathRecipient triggered, binder died.");
+            onRemovedOrDied();
+    };
+
     private abstract class CallbackAdapterManager<T> {
         private static final String TAG = "CallbackAdapterManager";
 
@@ -337,14 +342,8 @@ public class MmTelFeatureConnection {
                 }
                 switch (feature) {
                     case ImsFeature.FEATURE_MMTEL: {
-                        if (mIsAvailable) {
-                            Log.i(TAG, "MmTel disabled on slotId: " + slotId);
-                            mIsAvailable = false;
-                            mmTelFeatureRemoved();
-                            if (mStatusCallback != null) {
-                                mStatusCallback.notifyUnavailable();
-                            }
-                        }
+                        Log.i(TAG, "MmTel removed on slotId: " + slotId);
+                        onRemovedOrDied();
                         break;
                     }
                     case ImsFeature.FEATURE_EMERGENCY_MMTEL : {
@@ -376,12 +375,23 @@ public class MmTelFeatureConnection {
         mContext = context;
     }
 
-    // Called when the MmTelFeatureConnection has received an unavailable notification.
-    private void mmTelFeatureRemoved() {
+    /**
+     * Called when the MmTelFeature has either been removed by Telephony or crashed.
+     */
+    private void onRemovedOrDied() {
         synchronized (mLock) {
-            // invalidate caches.
-            mRegistrationBinder = null;
-            mConfigBinder = null;
+            if (mIsAvailable) {
+                mIsAvailable = false;
+                // invalidate caches.
+                mRegistrationBinder = null;
+                mConfigBinder = null;
+                if (mBinder != null) {
+                    mBinder.unlinkToDeath(mDeathRecipient, 0);
+                }
+                if (mStatusCallback != null) {
+                    mStatusCallback.notifyUnavailable();
+                }
+            }
         }
     }
 
@@ -434,7 +444,16 @@ public class MmTelFeatureConnection {
     }
 
     public void setBinder(IBinder binder) {
-        mBinder = binder;
+        synchronized (mLock) {
+            mBinder = binder;
+            try {
+                if (mBinder != null) {
+                    mBinder.linkToDeath(mDeathRecipient, 0);
+                }
+            } catch (RemoteException e) {
+                // No need to do anything if the binder is already dead.
+            }
+        }
     }
 
     /**
diff --git a/tests/Android.mk b/tests/Android.mk
index e07cab5c..a0b998b8 100644
--- a/tests/Android.mk
+++ b/tests/Android.mk
@@ -25,7 +25,9 @@ LOCAL_CERTIFICATE := platform
 
 LOCAL_MODULE_TAGS := tests
 
-LOCAL_JAVA_LIBRARIES := ims-common android.test.runner
+LOCAL_JAVA_LIBRARIES := ims-common \
+        android.test.runner \
+        android.test.base
 
 LOCAL_STATIC_JAVA_LIBRARIES := \
         android-support-test \
diff --git a/tests/src/com/android/ims/ImsConfigTest.java b/tests/src/com/android/ims/ImsConfigTest.java
index 4cf7a926..31d9bcb0 100644
--- a/tests/src/com/android/ims/ImsConfigTest.java
+++ b/tests/src/com/android/ims/ImsConfigTest.java
@@ -18,6 +18,7 @@ package com.android.ims;
 
 import android.support.test.runner.AndroidJUnit4;
 import android.telephony.ims.aidl.IImsConfig;
+import android.test.suitebuilder.annotation.SmallTest;
 
 import org.junit.After;
 import org.junit.Before;
@@ -42,7 +43,7 @@ public class ImsConfigTest extends ImsTestBase {
     @Override
     public void setUp() throws Exception {
         super.setUp();
-        mTestImsConfig = new ImsConfig(mMockImsConfigInterface, mContext);
+        mTestImsConfig = new ImsConfig(mMockImsConfigInterface);
     }
 
     @After
@@ -53,10 +54,11 @@ public class ImsConfigTest extends ImsTestBase {
     }
 
     @Test
+    @SmallTest
     public void testImsConfigGetProvisionedValue() throws Exception {
         int testItem = 0;
 
-        mTestImsConfig.getProvisionedValue(testItem);
+        mTestImsConfig.getConfigInt(testItem);
 
         verify(mMockImsConfigInterface).getConfigInt(eq(testItem));
     }