Merge "Handle Dead Objects from Phone Crashes better"android-p-preview-5android-p-preview-4android-o-mr1-iot-release-1.0.2android-n-iot-release-smart-display-r2android-n-iot-release-smart-display

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));
     }