From aecb2b65bacfa7f4620c2db188ab26e600d36acf Mon Sep 17 00:00:00 2001 From: Jimi Chen Date: Tue, 1 Jul 2025 02:24:00 +0000 Subject: [PATCH] Fix onActiveDataSubscriptionIdChanged is not received in multi SIM mode Flag: EXEMPT mainline Bug: 428673855 Test: manually Change-Id: I0d7c2538ec33402dba64949c3fb34519605d843e --- .../MultinetworkPolicyTracker.java | 60 +++++++++++++++++-- .../server/ConnectivityServiceTest.java | 5 ++ .../MultinetworkPolicyTrackerTest.kt | 8 ++- .../server/connectivityservice/base/CSTest.kt | 5 ++ 4 files changed, 72 insertions(+), 6 deletions(-) diff --git a/service/src/com/android/server/connectivity/MultinetworkPolicyTracker.java b/service/src/com/android/server/connectivity/MultinetworkPolicyTracker.java index cd17bbde26..a614cc4008 100644 --- a/service/src/com/android/server/connectivity/MultinetworkPolicyTracker.java +++ b/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 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()); } diff --git a/tests/unit/java/com/android/server/ConnectivityServiceTest.java b/tests/unit/java/com/android/server/ConnectivityServiceTest.java index 08be2ecf03..4783d28aac 100644 --- a/tests/unit/java/com/android/server/ConnectivityServiceTest.java +++ b/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(), diff --git a/tests/unit/java/com/android/server/connectivity/MultinetworkPolicyTrackerTest.kt b/tests/unit/java/com/android/server/connectivity/MultinetworkPolicyTrackerTest.kt index 75349cf1b2..372bd88c3e 100644 --- a/tests/unit/java/com/android/server/connectivity/MultinetworkPolicyTrackerTest.kt +++ b/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()) } diff --git a/tests/unit/java/com/android/server/connectivityservice/base/CSTest.kt b/tests/unit/java/com/android/server/connectivityservice/base/CSTest.kt index 1a97e3a9c0..82cba718fd 100644 --- a/tests/unit/java/com/android/server/connectivityservice/base/CSTest.kt +++ b/tests/unit/java/com/android/server/connectivityservice/base/CSTest.kt @@ -229,6 +229,11 @@ open class CSTest { val appOpsManager = mock() val telephonyManager = mock().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() val bluetoothManager = mock()