From 232c944de35eaa03b77c7dba33c389bdf237e8cc Mon Sep 17 00:00:00 2001 From: Jimi Chen Date: Tue, 20 May 2025 20:29:30 -0700 Subject: [PATCH] Revert^2 "Check whether CarrierConfigManager is null" This reverts commit 7b7cfa45a673a52a7623956ba475f57ce3c1eab2. Reason for revert: prevents crash with no carrierConfigManager support Test: atest ConnectivityCoverageTests Change-Id: I9e91d2d25d16c0c2a39388f08a49bef7b17fda42 --- .../MultinetworkPolicyTracker.java | 129 +++++++++--------- .../MultinetworkPolicyTrackerTest.kt | 40 +++++- ...ltinetworkPolicyTrackerTestDependencies.kt | 4 +- 3 files changed, 104 insertions(+), 69 deletions(-) diff --git a/service/src/com/android/server/connectivity/MultinetworkPolicyTracker.java b/service/src/com/android/server/connectivity/MultinetworkPolicyTracker.java index 60d27c353d..561a7934aa 100644 --- a/service/src/com/android/server/connectivity/MultinetworkPolicyTracker.java +++ b/service/src/com/android/server/connectivity/MultinetworkPolicyTracker.java @@ -20,6 +20,7 @@ import static android.content.pm.PackageManager.FEATURE_TELEPHONY_SUBSCRIPTION; import static android.net.ConnectivitySettingsManager.NETWORK_AVOID_BAD_WIFI; import static android.net.ConnectivitySettingsManager.NETWORK_METERED_MULTIPATH_PREFERENCE; +import android.annotation.IntDef; import android.annotation.NonNull; import android.annotation.Nullable; import android.annotation.TargetApi; @@ -52,6 +53,8 @@ import com.android.modules.utils.build.SdkLevel; import com.android.net.module.util.DeviceConfigUtils; import com.android.net.module.util.HandlerUtils; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; import java.util.Arrays; import java.util.List; import java.util.concurrent.Executor; @@ -87,10 +90,23 @@ public class MultinetworkPolicyTracker { } } - private enum AvoidBadWifiSource { - FROM_RESOURCE, - FROM_CARRIER_CONFIG, - } + /** + * Indicates that the "Avoid Bad Wi-Fi" setting originates from a resource. + */ + private static final int FROM_RESOURCE = 0; + + /** + * Indicates that the "Avoid Bad Wi-Fi" setting originates from carrier configuration. + */ + private static final int FROM_CARRIER_CONFIG = 1; + + /** + * Defines the set of possible integer constants for AvoidBadWifiSource. + * This annotation provides compile-time type safety. + */ + @IntDef({FROM_RESOURCE, FROM_CARRIER_CONFIG}) + @Retention(RetentionPolicy.SOURCE) + public @interface AvoidBadWifiSource {} private static String TAG = MultinetworkPolicyTracker.class.getSimpleName(); @@ -105,8 +121,7 @@ public class MultinetworkPolicyTracker { private final ContentResolver mResolver; private final SettingObserver mSettingObserver; private final BroadcastReceiver mBroadcastReceiver; - private final boolean mAvoidBadWifiFromCarrierConfigFeature; - private final boolean mHasTelephonySubscription; + private final @AvoidBadWifiSource int mAvoidBadWifiSource; // This will be null if the FLAG_AVOID_BAD_WIFI_FROM_CARRIER_CONFIG is off private final @Nullable CarrierConfigManager mCarrierConfigManager; private final @Nullable CarrierConfigChangeListener mCarrierConfigChangeListener; @@ -162,7 +177,7 @@ public class MultinetworkPolicyTracker { @VisibleForTesting protected boolean readAvoidBadWifiFromCarrierConfig( - @NonNull final Context context, final int subId) { + @NonNull final CarrierConfigManager ccm, final int subId) { // Defaults to true to avoid potentially poor Wi-Fi and improve user experience. final boolean defaultConfig = true; if (subId == SubscriptionManager.INVALID_SUBSCRIPTION_ID) { @@ -170,7 +185,6 @@ public class MultinetworkPolicyTracker { } PersistableBundle config = null; - final CarrierConfigManager ccm = context.getSystemService(CarrierConfigManager.class); try { config = ccm.getConfigForSubId(subId, @@ -271,18 +285,17 @@ public class MultinetworkPolicyTracker { } }; - mHasTelephonySubscription = - mContext.getPackageManager().hasSystemFeature(FEATURE_TELEPHONY_SUBSCRIPTION); - mAvoidBadWifiFromCarrierConfigFeature = mDeps.getAvoidBadWifiFromCarrierConfigFeature(); - mCarrierConfigManager = mAvoidBadWifiFromCarrierConfigFeature - ? mContext.getSystemService(CarrierConfigManager.class) - : null; - - mCarrierConfigChangeListener = mAvoidBadWifiFromCarrierConfigFeature - ? new CarrierConfigChangeListener() - : null; - - if (!mAvoidBadWifiFromCarrierConfigFeature) { + if (mDeps.getAvoidBadWifiFromCarrierConfigFeature()) { + mAvoidBadWifiSource = FROM_CARRIER_CONFIG; + mCarrierConfigManager = + mContext.getPackageManager().hasSystemFeature(FEATURE_TELEPHONY_SUBSCRIPTION) + ? mContext.getSystemService(CarrierConfigManager.class) + : null; + mCarrierConfigChangeListener = new CarrierConfigChangeListener(); + } else { + mAvoidBadWifiSource = FROM_RESOURCE; + mCarrierConfigManager = null; + mCarrierConfigChangeListener = null; updateAvoidBadWifi(); updateMeteredMultipathPreference(); } @@ -304,7 +317,7 @@ public class MultinetworkPolicyTracker { mContext.getSystemService(TelephonyManager.class).registerTelephonyCallback( handlerExecutor, new ActiveDataSubscriptionIdListener()); - if (mAvoidBadWifiFromCarrierConfigFeature) { + if (mCarrierConfigManager != null) { mCarrierConfigManager.registerCarrierConfigChangeListener( BackgroundThread.getExecutor(), mCarrierConfigChangeListener ); @@ -325,7 +338,7 @@ public class MultinetworkPolicyTracker { mResolver.unregisterContentObserver(mSettingObserver); mContext.unregisterReceiver(mBroadcastReceiver); - if (mAvoidBadWifiFromCarrierConfigFeature) { + if (mCarrierConfigManager != null) { mCarrierConfigManager.unregisterCarrierConfigChangeListener( mCarrierConfigChangeListener ); @@ -462,18 +475,6 @@ public class MultinetworkPolicyTracker { } } - /** - * Determines the source of the avoid bad Wi-Fi setting. - * It returns FROM_CARRIER_CONFIG if the carrier config feature is enabled, - * otherwise it returns FROM_RESOURCE. - * - */ - private AvoidBadWifiSource getAvoidBadWifiSource() { - return mAvoidBadWifiFromCarrierConfigFeature - ? AvoidBadWifiSource.FROM_CARRIER_CONFIG - : AvoidBadWifiSource.FROM_RESOURCE; - } - /** * Updates the local cache of the "avoid bad Wi-Fi" setting from the carrier config * for a specific subscription ID. @@ -502,8 +503,8 @@ public class MultinetworkPolicyTracker { // CarrierConfigManager#getConfigForSubId() is supported // only when system has FEATURE_TELEPHONY_SUBSCRIPTION - final boolean config = !mHasTelephonySubscription - || mDeps.readAvoidBadWifiFromCarrierConfig(mContext, subId); + final boolean config = + mDeps.readAvoidBadWifiFromCarrierConfig(mCarrierConfigManager, subId); mHandler.post(() -> updateAvoidBadWifiFromCarrierConfig(subId, config)); } @@ -528,33 +529,35 @@ public class MultinetworkPolicyTracker { */ public boolean updateAvoidBadWifi() { final boolean prevAvoid = mAvoidBadWifi; - if (getAvoidBadWifiSource() == AvoidBadWifiSource.FROM_CARRIER_CONFIG) { - // Force update activelyPreferBadWifi since it will always be true in Android U+, - // and mAvoidBadWifiFromCarrierConfigFeature is a trunk stable flag - // that only exists in 25Q4+ - mActivelyPreferBadWifi = true; - - final String settingAvoidBadWifiStr = readAvoidBadWifiFromSettings(); - if (settingAvoidBadWifiStr != null) { - mAvoidBadWifi = "1".equals(settingAvoidBadWifiStr); - } else { - // Retrieve the avoid bad Wi-Fi setting from the local cache to avoid potential - // issues or blocking from the IPC call getAvoidBadWifiCarrierConfigForSubId(). - mAvoidBadWifi = readAvoidBadWifiFromCache(mActiveSubId); - } - return mAvoidBadWifi != prevAvoid; - } else { - final boolean settingAvoidBadWifi = "1".equals(readAvoidBadWifiFromSettings()); - mAvoidBadWifi = settingAvoidBadWifi || !configRestrictsAvoidBadWifi(); - final boolean prevActive = mActivelyPreferBadWifi; - final Boolean deviceConfigPreferBadWifi = deviceConfigActivelyPreferBadWifi(); - if (null == deviceConfigPreferBadWifi) { - mActivelyPreferBadWifi = configActivelyPrefersBadWifi(); - } else { - mActivelyPreferBadWifi = deviceConfigPreferBadWifi; - } - - return mAvoidBadWifi != prevAvoid || mActivelyPreferBadWifi != prevActive; + switch (mAvoidBadWifiSource) { + case FROM_CARRIER_CONFIG: + // Force update activelyPreferBadWifi since it will always be true in Android U+, + // and mAvoidBadWifiFromCarrierConfigFeature is a trunk stable flag + // that only exists in 25Q4+ + mActivelyPreferBadWifi = true; + final String settingAvoidBadWifiStr = readAvoidBadWifiFromSettings(); + if (settingAvoidBadWifiStr != null) { + mAvoidBadWifi = "1".equals(settingAvoidBadWifiStr); + } else { + // Retrieve the avoid bad Wi-Fi setting from the local cache to avoid potential + // issues or blocking from the IPC call getAvoidBadWifiCarrierConfigForSubId(). + mAvoidBadWifi = readAvoidBadWifiFromCache(mActiveSubId); + } + return mAvoidBadWifi != prevAvoid; + case FROM_RESOURCE: + final boolean settingAvoidBadWifi = "1".equals(readAvoidBadWifiFromSettings()); + mAvoidBadWifi = settingAvoidBadWifi || !configRestrictsAvoidBadWifi(); + final boolean prevActive = mActivelyPreferBadWifi; + final Boolean deviceConfigPreferBadWifi = deviceConfigActivelyPreferBadWifi(); + if (null == deviceConfigPreferBadWifi) { + mActivelyPreferBadWifi = configActivelyPrefersBadWifi(); + } else { + mActivelyPreferBadWifi = deviceConfigPreferBadWifi; + } + return mAvoidBadWifi != prevAvoid || mActivelyPreferBadWifi != prevActive; + default: + Log.wtf(TAG, "Unexpected avoid bad Wi-Fi source: " + mAvoidBadWifiSource); + return false; } } diff --git a/tests/unit/java/com/android/server/connectivity/MultinetworkPolicyTrackerTest.kt b/tests/unit/java/com/android/server/connectivity/MultinetworkPolicyTrackerTest.kt index 95a6d8c8bb..97b3c49cde 100644 --- a/tests/unit/java/com/android/server/connectivity/MultinetworkPolicyTrackerTest.kt +++ b/tests/unit/java/com/android/server/connectivity/MultinetworkPolicyTrackerTest.kt @@ -49,10 +49,12 @@ import org.junit.After import org.junit.Assert.assertEquals import org.junit.Assert.assertFalse import org.junit.Assert.assertTrue +import org.junit.Assume.assumeTrue import org.junit.Before import org.junit.Rule import org.junit.Test import org.junit.runner.RunWith +import org.junit.runners.Parameterized import org.mockito.ArgumentCaptor import org.mockito.ArgumentMatchers.anyInt import org.mockito.Mockito.any @@ -74,6 +76,17 @@ const val HANDLER_TIMEOUT_MS = 400 @SmallTest @DevSdkIgnoreRule.IgnoreUpTo(Build.VERSION_CODES.R) class MultinetworkPolicyTrackerTest { + companion object { + @Parameterized.Parameters + @JvmStatic + fun data(): Iterable { + return mutableListOf( + false, + true + ) + } + } + // This wrapper class prevents JUnit from attempting to load unsupported system classes // that are present in the System Test (S/T) image, which would otherwise cause test failures. private class CarrierConfigChangeRunner( @@ -97,6 +110,14 @@ class MultinetworkPolicyTrackerTest { private val featureFlags = HashSet() + // Indicates where CarrierConfigManager is supported. + // It will be false if FEATURE_TELEPHONY_SUBSCRIPTION is not supported. + @Parameterized.Parameter(0) + @JvmField + var supportCarrierConfigManager: Boolean = false + + var carrierConfigManager: CarrierConfigManager? = null + // This will set feature flags from @FeatureFlag annotations // into the map before setUp() runs. @get:Rule @@ -115,7 +136,6 @@ class MultinetworkPolicyTrackerTest { } private val telephonyManager = mock(TelephonyManager::class.java) - private val carrierConfigManager = mock(CarrierConfigManager::class.java) private val subscriptionManager = mock(SubscriptionManager::class.java).also { doReturn(null).`when`(it).getActiveSubscriptionInfo(anyInt()) } @@ -135,8 +155,6 @@ class MultinetworkPolicyTrackerTest { } doReturn(subscriptionManager).`when`(it) .getSystemService(Context.TELEPHONY_SUBSCRIPTION_SERVICE) - doReturn(carrierConfigManager).`when`(it) - .getSystemService(CarrierConfigManager::class.java) doReturn(resolver).`when`(it).contentResolver doReturn(resources).`when`(it).resources doReturn(it).`when`(it).createConfigurationContext(any()) @@ -206,6 +224,15 @@ class MultinetworkPolicyTrackerTest { featureFlags.contains(FLAG_AVOID_BAD_WIFI_FROM_CARRIER_CONFIG) ) + if (supportCarrierConfigManager) { + carrierConfigManager = mock(CarrierConfigManager::class.java) + } else { + carrierConfigManager = null + } + + doReturn(carrierConfigManager).`when`(context) + .getSystemService(CarrierConfigManager::class.java) + trackerDependencies.setBackgroundThreadHandler(bgHandler) tracker = MultinetworkPolicyTracker( context, @@ -326,6 +353,11 @@ class MultinetworkPolicyTrackerTest { @FeatureFlag(name = FLAG_AVOID_BAD_WIFI_FROM_CARRIER_CONFIG, true) @DevSdkIgnoreRule.IgnoreUpTo(Build.VERSION_CODES.BAKLAVA) fun testUpdateAvoidBadWifiOnCarrierConfigChange() { + assumeTrue( + "skip test if carrierConfigManager is not supported", + supportCarrierConfigManager + ) + // Mock the initial global setting to null Settings.Global.putString(resolver, NETWORK_AVOID_BAD_WIFI, null) @@ -346,7 +378,7 @@ class MultinetworkPolicyTrackerTest { // Mock the initial carrier configuration to return false trackerDependencies.setAvoidBadWifiCarrierConfigForSubId(activeSubId, false) verify(carrierConfigManager, times(1)) - .registerCarrierConfigChangeListener(any(), carrierConfiglistenCaptor.capture()) + ?.registerCarrierConfigChangeListener(any(), carrierConfiglistenCaptor.capture()) val carrierConfiglistener = carrierConfiglistenCaptor.value // dispatch for the first carrier config initialization on the handler thread diff --git a/tests/unit/java/com/android/server/connectivity/MultinetworkPolicyTrackerTestDependencies.kt b/tests/unit/java/com/android/server/connectivity/MultinetworkPolicyTrackerTestDependencies.kt index 7285cbe32b..0103fd906e 100644 --- a/tests/unit/java/com/android/server/connectivity/MultinetworkPolicyTrackerTestDependencies.kt +++ b/tests/unit/java/com/android/server/connectivity/MultinetworkPolicyTrackerTestDependencies.kt @@ -1,11 +1,11 @@ package com.android.server.connectivity -import android.content.Context import android.content.res.Resources import android.os.Handler import android.provider.DeviceConfig import android.provider.DeviceConfig.NAMESPACE_CONNECTIVITY import android.provider.DeviceConfig.OnPropertiesChangedListener +import android.telephony.CarrierConfigManager import com.android.internal.annotations.GuardedBy import com.android.internal.os.BackgroundThread import com.android.server.connectivity.MultinetworkPolicyTracker.CONFIG_ACTIVELY_PREFER_BAD_WIFI @@ -72,7 +72,7 @@ class MultinetworkPolicyTrackerTestDependencies( resources override fun readAvoidBadWifiFromCarrierConfig( - context: Context, + ccm: CarrierConfigManager, subId: Int ): Boolean = avoidBadWifiCarrierConfigForSubIdMap.getOrDefault(subId, true)