Revert "Fix onActiveDataSubscriptionIdChanged is not received in multi SIM mode"

This reverts commit aecb2b65ba.

Reason for revert: Likely culprit for b/430219293 - verifying through ABTD before revert submission. This is part of the standard investigation process, and does not mean your CL will be reverted.

Bug: 430219293
Change-Id: I9b786abf784de8c34b9f517236305330b79b227e

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

@@ -114,7 +114,6 @@ 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;
@@ -129,8 +128,6 @@ 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
@@ -183,36 +180,6 @@ 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
      *
@@ -256,22 +223,7 @@ 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();
         }
     }
@@ -285,7 +237,6 @@ public class MultinetworkPolicyTracker {
         mContext = ctx;
         mResources = new ConnectivityResources(ctx);
         mHandler = handler;
-        mExecutor = new HandlerExecutor(mHandler);
         mAvoidBadWifiCallback = avoidBadWifiCallback;
         mDeps = deps;
         mSettingsUris = Arrays.asList(
@@ -329,20 +280,19 @@ public class MultinetworkPolicyTracker {
         mContext.registerReceiverForAllUsers(mBroadcastReceiver, intentFilter,
                 null /* broadcastPermission */, mHandler);
 
-        mHandler.post(() -> {
-            mTelephonyCallback = new ActiveDataSubscriptionIdListener();
-            registerTelephonyCallback(mActiveSubId, mTelephonyCallback);
-        });
+        final Executor handlerExecutor = new HandlerExecutor(mHandler);
+        mContext.getSystemService(TelephonyManager.class).registerTelephonyCallback(
+                handlerExecutor, new ActiveDataSubscriptionIdListener());
 
         if (mCarrierConfigManager != null) {
             mCarrierConfigManager.registerCarrierConfigChangeListener(
-                    mExecutor, mCarrierConfigChangeListener
+                    handlerExecutor, mCarrierConfigChangeListener
             );
 
             // This ensures the latest carrier configuration is read.
             mHandler.post(() -> reevaluateInternal());
         } else {
-            mDeps.addOnDevicePropertiesChangedListener(mExecutor,
+            mDeps.addOnDevicePropertiesChangedListener(handlerExecutor,
                 properties -> reevaluateInternal());
         }
 

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

@@ -1899,11 +1899,6 @@ 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,13 +107,7 @@ class MultinetworkPolicyTrackerTest(private val supportCarrierConfigManager: Boo
         doReturn(0).`when`(it).getInteger(R.integer.config_activelyPreferBadWifi)
     }
 
-    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 telephonyManager = mock(TelephonyManager::class.java)
     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,11 +229,6 @@ 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>()