From 7b7cfa45a673a52a7623956ba475f57ce3c1eab2 Mon Sep 17 00:00:00 2001 From: "Priyanka Advani (xWF)" Date: Tue, 20 May 2025 11:19:03 -0700 Subject: [PATCH] Revert "Check whether CarrierConfigManager is null" This reverts commit b936f0c5fd7b636063cf14fc38376f3f00ace890. Reason for revert: Droidmonitor created revert due to b/418898788. Will be verifying through ABTD before submission. Fix: 418898788 Change-Id: Ia47f0cc9a33ed33f9d53064878992081a792f1ad --- .../MultinetworkPolicyTracker.java | 134 ++++++++---------- .../MultinetworkPolicyTrackerTest.kt | 31 +--- ...ltinetworkPolicyTrackerTestDependencies.kt | 4 +- 3 files changed, 69 insertions(+), 100 deletions(-) diff --git a/service/src/com/android/server/connectivity/MultinetworkPolicyTracker.java b/service/src/com/android/server/connectivity/MultinetworkPolicyTracker.java index 9b15728754..60d27c353d 100644 --- a/service/src/com/android/server/connectivity/MultinetworkPolicyTracker.java +++ b/service/src/com/android/server/connectivity/MultinetworkPolicyTracker.java @@ -20,7 +20,6 @@ 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; @@ -53,8 +52,6 @@ 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; @@ -90,23 +87,10 @@ public class MultinetworkPolicyTracker { } } - /** - * 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 enum AvoidBadWifiSource { + FROM_RESOURCE, + FROM_CARRIER_CONFIG, + } private static String TAG = MultinetworkPolicyTracker.class.getSimpleName(); @@ -121,7 +105,8 @@ public class MultinetworkPolicyTracker { private final ContentResolver mResolver; private final SettingObserver mSettingObserver; private final BroadcastReceiver mBroadcastReceiver; - private final @AvoidBadWifiSource int mAvoidBadWifiSource; + private final boolean mAvoidBadWifiFromCarrierConfigFeature; + private final boolean mHasTelephonySubscription; // 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; @@ -177,19 +162,15 @@ public class MultinetworkPolicyTracker { @VisibleForTesting protected boolean readAvoidBadWifiFromCarrierConfig( - @Nullable final CarrierConfigManager ccm, final int subId) { + @NonNull final Context context, 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) { return defaultConfig; } - // It implies the FEATURE_TELEPHONY_SUBSCRIPTION is not supported, return default config - if (ccm == null) { - return defaultConfig; - } - PersistableBundle config = null; + final CarrierConfigManager ccm = context.getSystemService(CarrierConfigManager.class); try { config = ccm.getConfigForSubId(subId, @@ -290,17 +271,18 @@ public class MultinetworkPolicyTracker { } }; - 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; + mHasTelephonySubscription = + mContext.getPackageManager().hasSystemFeature(FEATURE_TELEPHONY_SUBSCRIPTION); + mAvoidBadWifiFromCarrierConfigFeature = mDeps.getAvoidBadWifiFromCarrierConfigFeature(); + mCarrierConfigManager = mAvoidBadWifiFromCarrierConfigFeature + ? mContext.getSystemService(CarrierConfigManager.class) + : null; + + mCarrierConfigChangeListener = mAvoidBadWifiFromCarrierConfigFeature + ? new CarrierConfigChangeListener() + : null; + + if (!mAvoidBadWifiFromCarrierConfigFeature) { updateAvoidBadWifi(); updateMeteredMultipathPreference(); } @@ -322,7 +304,7 @@ public class MultinetworkPolicyTracker { mContext.getSystemService(TelephonyManager.class).registerTelephonyCallback( handlerExecutor, new ActiveDataSubscriptionIdListener()); - if (mCarrierConfigManager != null) { + if (mAvoidBadWifiFromCarrierConfigFeature) { mCarrierConfigManager.registerCarrierConfigChangeListener( BackgroundThread.getExecutor(), mCarrierConfigChangeListener ); @@ -343,7 +325,7 @@ public class MultinetworkPolicyTracker { mResolver.unregisterContentObserver(mSettingObserver); mContext.unregisterReceiver(mBroadcastReceiver); - if (mCarrierConfigManager != null) { + if (mAvoidBadWifiFromCarrierConfigFeature) { mCarrierConfigManager.unregisterCarrierConfigChangeListener( mCarrierConfigChangeListener ); @@ -480,6 +462,18 @@ 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. @@ -508,8 +502,8 @@ public class MultinetworkPolicyTracker { // CarrierConfigManager#getConfigForSubId() is supported // only when system has FEATURE_TELEPHONY_SUBSCRIPTION - final boolean config = - mDeps.readAvoidBadWifiFromCarrierConfig(mCarrierConfigManager, subId); + final boolean config = !mHasTelephonySubscription + || mDeps.readAvoidBadWifiFromCarrierConfig(mContext, subId); mHandler.post(() -> updateAvoidBadWifiFromCarrierConfig(subId, config)); } @@ -534,35 +528,33 @@ public class MultinetworkPolicyTracker { */ public boolean updateAvoidBadWifi() { final boolean prevAvoid = mAvoidBadWifi; - 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; + 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; } } diff --git a/tests/unit/java/com/android/server/connectivity/MultinetworkPolicyTrackerTest.kt b/tests/unit/java/com/android/server/connectivity/MultinetworkPolicyTrackerTest.kt index ca0a136227..95a6d8c8bb 100644 --- a/tests/unit/java/com/android/server/connectivity/MultinetworkPolicyTrackerTest.kt +++ b/tests/unit/java/com/android/server/connectivity/MultinetworkPolicyTrackerTest.kt @@ -49,19 +49,16 @@ import org.junit.After import org.junit.Assert.assertEquals import org.junit.Assert.assertFalse import org.junit.Assert.assertTrue -import org.junit.Assume.assumeNotNull 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 import org.mockito.Mockito.doCallRealMethod import org.mockito.Mockito.doReturn import org.mockito.Mockito.mock -import org.mockito.Mockito.reset import org.mockito.Mockito.times import org.mockito.Mockito.verify @@ -77,17 +74,6 @@ 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( - null, - mock(CarrierConfigManager::class.java) - ) - } - } - // 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( @@ -111,11 +97,6 @@ class MultinetworkPolicyTrackerTest { private val featureFlags = HashSet() - // Indicates where carrierConfigManager is supported. - @Parameterized.Parameter(0) - @JvmField - var carrierConfigManager: CarrierConfigManager? = null - // This will set feature flags from @FeatureFlag annotations // into the map before setUp() runs. @get:Rule @@ -134,6 +115,7 @@ 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()) } @@ -153,6 +135,8 @@ 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()) @@ -222,8 +206,6 @@ class MultinetworkPolicyTrackerTest { featureFlags.contains(FLAG_AVOID_BAD_WIFI_FROM_CARRIER_CONFIG) ) - doReturn(carrierConfigManager).`when`(context) - .getSystemService(CarrierConfigManager::class.java) trackerDependencies.setBackgroundThreadHandler(bgHandler) tracker = MultinetworkPolicyTracker( context, @@ -237,9 +219,6 @@ class MultinetworkPolicyTrackerTest { @After fun tearDown() { ConnectivityResources.setResourcesContextForTest(null) - carrierConfigManager?.let { ccm -> - reset(ccm) - } trackerDependencies.resetAvoidBadWifiCarrierConfigForSubIdMap() } @@ -347,8 +326,6 @@ class MultinetworkPolicyTrackerTest { @FeatureFlag(name = FLAG_AVOID_BAD_WIFI_FROM_CARRIER_CONFIG, true) @DevSdkIgnoreRule.IgnoreUpTo(Build.VERSION_CODES.BAKLAVA) fun testUpdateAvoidBadWifiOnCarrierConfigChange() { - assumeNotNull("skip test if carrierConfigManager is not supported", carrierConfigManager) - // Mock the initial global setting to null Settings.Global.putString(resolver, NETWORK_AVOID_BAD_WIFI, null) @@ -369,7 +346,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 dc2ace4984..7285cbe32b 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( - ccm: CarrierConfigManager?, + context: Context, subId: Int ): Boolean = avoidBadWifiCarrierConfigForSubIdMap.getOrDefault(subId, true)