Callbacks into phone for IMS feature state should be on main thread

When IMS feature state callbacks update the IMS feature state,
they should be on the main thread to ensure that new callback
executors that are registered are not on the binder thread.

Bug: 123305476
Test: manual - crash phone process, atest FrameworksTelephonyTests
Change-Id: I26292022e9274676d751ebe3e0ec63f50e5a200a

2 files changed, 99 insertions, 72 deletions

diff --git a/src/java/com/android/ims/ImsManager.java b/src/java/com/android/ims/ImsManager.java
index 073b5f96..0b540e82 100644
--- a/src/java/com/android/ims/ImsManager.java
+++ b/src/java/com/android/ims/ImsManager.java
@@ -228,45 +228,50 @@ public class ImsManager {
                 new MmTelFeatureConnection.IFeatureUpdate() {
                     @Override
                     public void notifyStateChanged() {
-                        try {
-                            int status = ImsFeature.STATE_UNAVAILABLE;
-                            synchronized (mLock) {
-                                if (mImsManager != null) {
-                                    status = mImsManager.getImsServiceState();
+                        mExecutor.execute(() -> {
+                            try {
+                                int status = ImsFeature.STATE_UNAVAILABLE;
+                                synchronized (mLock) {
+                                    if (mImsManager != null) {
+                                        status = mImsManager.getImsServiceState();
+                                    }
                                 }
-                            }
-                            switch (status) {
-                                case ImsFeature.STATE_READY: {
-                                    notifyReady();
-                                    break;
-                                }
-                                case ImsFeature.STATE_INITIALIZING:
-                                    // fall through
-                                case ImsFeature.STATE_UNAVAILABLE: {
-                                    notifyNotReady();
-                                    break;
-                                }
-                                default: {
-                                    Log.w(TAG, "Unexpected State!");
+                                switch (status) {
+                                    case ImsFeature.STATE_READY: {
+                                        notifyReady();
+                                        break;
+                                    }
+                                    case ImsFeature.STATE_INITIALIZING:
+                                        // fall through
+                                    case ImsFeature.STATE_UNAVAILABLE: {
+                                        notifyNotReady();
+                                        break;
+                                    }
+                                    default: {
+                                        Log.w(TAG, "Unexpected State!");
+                                    }
                                 }
+                            } catch (ImsException e) {
+                                // Could not get the ImsService, retry!
+                                notifyNotReady();
+                                retryGetImsService();
                             }
-                        } catch (ImsException e) {
-                            // Could not get the ImsService, retry!
-                            notifyNotReady();
-                            retryGetImsService();
-                        }
+                        });
                     }
 
                     @Override
                     public void notifyUnavailable() {
-                        notifyNotReady();
-                        retryGetImsService();
+                        mExecutor.execute(() -> {
+                            notifyNotReady();
+                            retryGetImsService();
+                        });
                     }
                 };
 
         private final Context mContext;
         private final int mPhoneId;
         private final Listener mListener;
+        private final Executor mExecutor;
         private final Object mLock = new Object();
 
         private int mRetryCount = 0;
@@ -287,15 +292,18 @@ public class ImsManager {
             mContext = context;
             mPhoneId = phoneId;
             mListener = listener;
+            mExecutor = new HandlerExecutor(this);
         }
 
-        public Connector(Context context, int phoneId, Listener listener, Looper looper) {
-            super(looper);
+        @VisibleForTesting
+        public Connector(Context context, int phoneId, Listener listener, Executor executor) {
             mContext = context;
             mPhoneId = phoneId;
             mListener= listener;
+            mExecutor = executor;
         }
 
+
         /**
          * Start the creation of a connection to the underlying ImsService implementation. When the
          * service is connected, {@link Listener#connectionReady(ImsManager)} will be called with
diff --git a/src/java/com/android/ims/MmTelFeatureConnection.java b/src/java/com/android/ims/MmTelFeatureConnection.java
index 12f55ef6..86ef8760 100644
--- a/src/java/com/android/ims/MmTelFeatureConnection.java
+++ b/src/java/com/android/ims/MmTelFeatureConnection.java
@@ -18,6 +18,8 @@ package com.android.ims;
 
 import android.annotation.Nullable;
 import android.content.Context;
+import android.os.Handler;
+import android.os.HandlerExecutor;
 import android.os.IBinder;
 import android.os.IInterface;
 import android.os.Looper;
@@ -56,6 +58,7 @@ import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
 import java.util.Set;
+import java.util.concurrent.Executor;
 import java.util.stream.Collectors;
 
 /**
@@ -417,6 +420,7 @@ public class MmTelFeatureConnection {
     protected final int mSlotId;
     protected IBinder mBinder;
     private Context mContext;
+    private Executor mExecutor;
 
     private volatile boolean mIsAvailable = false;
     // ImsFeature Status from the ImsService. Cached.
@@ -492,70 +496,85 @@ public class MmTelFeatureConnection {
             new IImsServiceFeatureCallback.Stub() {
 
         @Override
-        public void imsFeatureCreated(int slotId, int feature) throws RemoteException {
-            // The feature has been enabled. This happens when the feature is first created and may
-            // happen when the feature is re-enabled.
-            synchronized (mLock) {
-                if(mSlotId != slotId) {
-                    return;
-                }
-                switch (feature) {
-                    case ImsFeature.FEATURE_MMTEL: {
-                        if (!mIsAvailable) {
-                            Log.i(TAG, "MmTel enabled on slotId: " + slotId);
-                            mIsAvailable = true;
-                        }
-                        break;
+        public void imsFeatureCreated(int slotId, int feature) {
+                mExecutor.execute(() -> {
+                // The feature has been enabled. This happens when the feature is first created and
+                // may happen when the feature is re-enabled.
+                synchronized (mLock) {
+                    if(mSlotId != slotId) {
+                        return;
                     }
-                    case ImsFeature.FEATURE_EMERGENCY_MMTEL: {
-                        mSupportsEmergencyCalling = true;
-                        Log.i(TAG, "Emergency calling enabled on slotId: " + slotId);
-                        break;
+                    switch (feature) {
+                        case ImsFeature.FEATURE_MMTEL: {
+                            if (!mIsAvailable) {
+                                Log.i(TAG, "MmTel enabled on slotId: " + slotId);
+                                mIsAvailable = true;
+                            }
+                            break;
+                        }
+                        case ImsFeature.FEATURE_EMERGENCY_MMTEL: {
+                            mSupportsEmergencyCalling = true;
+                            Log.i(TAG, "Emergency calling enabled on slotId: " + slotId);
+                            break;
+                        }
                     }
                 }
-
-            }
+            });
         }
 
         @Override
-        public void imsFeatureRemoved(int slotId, int feature) throws RemoteException {
-            synchronized (mLock) {
-                if(mSlotId != slotId) {
-                    return;
-                }
-                switch (feature) {
-                    case ImsFeature.FEATURE_MMTEL: {
-                        Log.i(TAG, "MmTel removed on slotId: " + slotId);
-                        onRemovedOrDied();
-                        break;
+        public void imsFeatureRemoved(int slotId, int feature) {
+            mExecutor.execute(() -> {
+                synchronized (mLock) {
+                    if (mSlotId != slotId) {
+                        return;
                     }
-                    case ImsFeature.FEATURE_EMERGENCY_MMTEL : {
-                        mSupportsEmergencyCalling = false;
-                        Log.i(TAG, "Emergency calling disabled on slotId: " + slotId);
-                        break;
+                    switch (feature) {
+                        case ImsFeature.FEATURE_MMTEL: {
+                            Log.i(TAG, "MmTel removed on slotId: " + slotId);
+                            onRemovedOrDied();
+                            break;
+                        }
+                        case ImsFeature.FEATURE_EMERGENCY_MMTEL: {
+                            mSupportsEmergencyCalling = false;
+                            Log.i(TAG, "Emergency calling disabled on slotId: " + slotId);
+                            break;
+                        }
                     }
                 }
-            }
+            });
         }
 
         @Override
-        public void imsStatusChanged(int slotId, int feature, int status) throws RemoteException {
-            synchronized (mLock) {
-                Log.i(TAG, "imsStatusChanged: slot: " + slotId + " feature: " + feature +
-                        " status: " + status);
-                if (mSlotId == slotId && feature == ImsFeature.FEATURE_MMTEL) {
-                    mFeatureStateCached = status;
-                    if (mStatusCallback != null) {
-                        mStatusCallback.notifyStateChanged();
+        public void imsStatusChanged(int slotId, int feature, int status) {
+            mExecutor.execute(() -> {
+                synchronized (mLock) {
+                    Log.i(TAG, "imsStatusChanged: slot: " + slotId + " feature: " + feature +
+                            " status: " + status);
+                    if (mSlotId == slotId && feature == ImsFeature.FEATURE_MMTEL) {
+                        mFeatureStateCached = status;
+                        if (mStatusCallback != null) {
+                            mStatusCallback.notifyStateChanged();
+                        }
                     }
                 }
-            }
+            });
         }
     };
 
     public MmTelFeatureConnection(Context context, int slotId) {
         mSlotId = slotId;
         mContext = context;
+        // Callbacks should be scheduled on the main thread.
+        if (context.getMainLooper() != null) {
+            mExecutor = context.getMainExecutor();
+        } else {
+            // Fallback to the current thread.
+            if (Looper.myLooper() == null) {
+                Looper.prepare();
+            }
+            mExecutor = new HandlerExecutor(new Handler(Looper.myLooper()));
+        }
         mRegistrationCallbackManager = new ImsRegistrationCallbackAdapter(context, mLock);
         mCapabilityCallbackManager = new CapabilityCallbackManager(context, mLock);
         mProvisioningCallbackManager = new ProvisioningCallbackManager(context, mLock);