Fix onActiveDataSubscriptionIdChanged is not received in multi SIM mode

Flag: EXEMPT mainline
Bug: 428673855
Test: manually
Change-Id: I0d7c2538ec33402dba64949c3fb34519605d843e

service/src/com/android/server/connectivity/MultinetworkPolicyTracker.java

@@ -114,6 +114,7 @@ public class MultinetworkPolicyTracker {
     private final Context mContext;
     private final ConnectivityResources mResources;
     private final Handler mHandler;
+    private final HandlerExecutor mExecutor;
     private final Runnable mAvoidBadWifiCallback;
     private final List<Uri> mSettingsUris;
     private final ContentResolver mResolver;
@@ -128,6 +129,8 @@ public class MultinetworkPolicyTracker {
     private volatile int mMeteredMultipathPreference;
     private int mActiveSubId = SubscriptionManager.INVALID_SUBSCRIPTION_ID;
     private volatile long mTestAllowBadWifiUntilMs = 0;
+    // This field should only be accessed on the handler thread
+    private TelephonyCallback mTelephonyCallback;
 
     /**
      * Dependencies for testing
@@ -180,6 +183,36 @@ public class MultinetworkPolicyTracker {
     }
     private final Dependencies mDeps;
 
+    private void unregisterTelephonyCallback(
+            int subId, @NonNull final TelephonyCallback telephonyCallback
+    ) {
+        final TelephonyManager telephonyManager = mContext.getSystemService(TelephonyManager.class);
+        if (null == telephonyManager) return;
+
+        final TelephonyManager telephonyManagerForSubId =
+                telephonyManager.createForSubscriptionId(subId);
+
+        if (null == telephonyManagerForSubId) return;
+
+        Log.d(TAG, "unregisterTelephonyCallback for subId:" + subId);
+        telephonyManagerForSubId.unregisterTelephonyCallback(telephonyCallback);
+    }
+
+    private void registerTelephonyCallback(
+            int subId, @NonNull final TelephonyCallback telephonyCallback
+    ) {
+        final TelephonyManager telephonyManager = mContext.getSystemService(TelephonyManager.class);
+        if (null == telephonyManager) return;
+
+        final TelephonyManager telephonyManagerForSubId =
+                telephonyManager.createForSubscriptionId(subId);
+
+        if (null == telephonyManagerForSubId) return;
+
+        Log.d(TAG, "registerTelephonyCallback for subId:" + subId);
+        telephonyManagerForSubId.registerTelephonyCallback(mExecutor, telephonyCallback);
+    }
+
     /**
      * Whether to prefer bad wifi to a network that yields to bad wifis, even if it never validated
      *
@@ -223,7 +256,22 @@ public class MultinetworkPolicyTracker {
             implements TelephonyCallback.ActiveDataSubscriptionIdListener {
         @Override
         public void onActiveDataSubscriptionIdChanged(final int subId) {
+            if (mActiveSubId == subId) return;
+
+            if (subId == SubscriptionManager.INVALID_SUBSCRIPTION_ID) return;
+
+            Log.d(TAG, "onActiveDataSubscriptionIdChanged from " + mActiveSubId
+                    + " to " + subId);
+            // register new subId callback first to prevent subscription changes in between
+            final int oldSubId = mActiveSubId;
+            final TelephonyCallback oldCallback = mTelephonyCallback;
+
+            mTelephonyCallback =
+                    new MultinetworkPolicyTracker.ActiveDataSubscriptionIdListener();
+            registerTelephonyCallback(subId, mTelephonyCallback); // new
             mActiveSubId = subId;
+
+            unregisterTelephonyCallback(oldSubId, oldCallback); // old
             reevaluateInternal();
         }
     }
@@ -237,6 +285,7 @@ public class MultinetworkPolicyTracker {
         mContext = ctx;
         mResources = new ConnectivityResources(ctx);
         mHandler = handler;
+        mExecutor = new HandlerExecutor(mHandler);
         mAvoidBadWifiCallback = avoidBadWifiCallback;
         mDeps = deps;
         mSettingsUris = Arrays.asList(
@@ -280,19 +329,20 @@ public class MultinetworkPolicyTracker {
         mContext.registerReceiverForAllUsers(mBroadcastReceiver, intentFilter,
                 null /* broadcastPermission */, mHandler);
 
-        final Executor handlerExecutor = new HandlerExecutor(mHandler);
-        mContext.getSystemService(TelephonyManager.class).registerTelephonyCallback(
-                handlerExecutor, new ActiveDataSubscriptionIdListener());
+        mHandler.post(() -> {
+            mTelephonyCallback = new ActiveDataSubscriptionIdListener();
+            registerTelephonyCallback(mActiveSubId, mTelephonyCallback);
+        });
 
         if (mCarrierConfigManager != null) {
             mCarrierConfigManager.registerCarrierConfigChangeListener(
-                    handlerExecutor, mCarrierConfigChangeListener
+                    mExecutor, mCarrierConfigChangeListener
             );
 
             // This ensures the latest carrier configuration is read.
             mHandler.post(() -> reevaluateInternal());
         } else {
-            mDeps.addOnDevicePropertiesChangedListener(handlerExecutor,
+            mDeps.addOnDevicePropertiesChangedListener(mExecutor,
                 properties -> reevaluateInternal());
         }
 

tests/unit/java/com/android/server/ConnectivityServiceTest.java

@@ -1899,6 +1899,11 @@ public class ConnectivityServiceTest {
         mockHasSystemFeature(FEATURE_WIFI_DIRECT, true);
         mockHasSystemFeature(FEATURE_ETHERNET, true);
         doReturn(true).when(mTelephonyManager).isDataCapable();
+        // This will return the same object for all subscription IDs. This is
+        // fine for all tests at the time of this writing, but if the difference
+        // becomes important for all tests, then it may be necessary to create a
+        // new one per subId. See [CSTest#createContextAsUser] above for a model.
+        doReturn(mTelephonyManager).when(mTelephonyManager).createForSubscriptionId(anyInt());
 
         FakeSettingsProvider.clearSettingsProvider();
         mServiceContext = new MockContext(InstrumentationRegistry.getContext(),

tests/unit/java/com/android/server/connectivity/MultinetworkPolicyTrackerTest.kt

@@ -107,7 +107,13 @@ class MultinetworkPolicyTrackerTest(private val supportCarrierConfigManager: Boo
         doReturn(0).`when`(it).getInteger(R.integer.config_activelyPreferBadWifi)
     }
 
-    private val telephonyManager = mock(TelephonyManager::class.java)
+    private val telephonyManager = mock(TelephonyManager::class.java).also {
+        // This will return the same object for all subscription IDs. This is
+        // fine for all tests at the time of this writing, but if the difference
+        // becomes important for all tests, then it may be necessary to create a
+        // new one per subId. See [CSTest#createContextAsUser] above for a model.
+        doReturn(it).`when`(it).createForSubscriptionId(anyInt())
+    }
     private val subscriptionManager = mock(SubscriptionManager::class.java).also {
         doReturn(null).`when`(it).getActiveSubscriptionInfo(anyInt())
     }

tests/unit/java/com/android/server/connectivityservice/base/CSTest.kt

@@ -229,6 +229,11 @@ open class CSTest {
     val appOpsManager = mock<AppOpsManager>()
     val telephonyManager = mock<TelephonyManager>().also {
         doReturn(true).`when`(it).isDataCapable()
+        // This will return the same object for all subscription IDs. This is
+        // fine for all tests at the time of this writing, but if the difference
+        // becomes important for all tests, then it may be necessary to create a
+        // new one per subId. See [createContextAsUser] above for a model.
+        doReturn(it).`when`(it).createForSubscriptionId(any())
     }
     val subscriptionManager = mock<SubscriptionManager>()
     val bluetoothManager = mock<BluetoothManager>()