Merge "Don't throw in FullScore#policyNameOf." am: 644fe232cd

Original change: https://android-review.googlesource.com/c/platform/packages/modules/Connectivity/+/2041564

Change-Id: I7143d220336b50b106cad79d890f49b77654a51c
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>

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

@@ -29,6 +29,7 @@ import android.net.NetworkAgentConfig;
 import android.net.NetworkCapabilities;
 import android.net.NetworkScore;
 import android.net.NetworkScore.KeepConnectedReason;
+import android.util.Log;
 import android.util.SparseArray;
 
 import com.android.internal.annotations.VisibleForTesting;
@@ -46,6 +47,8 @@ import java.util.StringJoiner;
  * they are handling a score that had the CS-managed bits set.
  */
 public class FullScore {
+    private static final String TAG = FullScore.class.getSimpleName();
+
     // This will be removed soon. Do *NOT* depend on it for any new code that is not part of
     // a migration.
     private final int mLegacyInt;
@@ -126,7 +129,15 @@ public class FullScore {
     @VisibleForTesting
     static @NonNull String policyNameOf(final int policy) {
         final String name = sMessageNames.get(policy);
-        if (name == null) throw new IllegalArgumentException("Unknown policy: " + policy);
+        if (name == null) {
+            // Don't throw here because name might be null due to proguard stripping out the
+            // POLICY_* constants, potentially causing a crash only on user builds because proguard
+            // does not run on userdebug builds.
+            // TODO: make MessageUtils safer by not returning the array and instead storing it
+            // internally and providing a getter (that does not throw) for individual values.
+            Log.wtf(TAG, "Unknown policy: " + policy);
+            return Integer.toString(policy);
+        }
         return name.substring("POLICY_".length());
     }
 

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

@@ -22,6 +22,7 @@ import android.net.NetworkScore.KEEP_CONNECTED_NONE
 import android.os.Build
 import android.text.TextUtils
 import android.util.ArraySet
+import android.util.Log
 import androidx.test.filters.SmallTest
 import com.android.server.connectivity.FullScore.MAX_CS_MANAGED_POLICY
 import com.android.server.connectivity.FullScore.POLICY_ACCEPT_UNVALIDATED
@@ -32,11 +33,12 @@ import com.android.server.connectivity.FullScore.POLICY_IS_VALIDATED
 import com.android.server.connectivity.FullScore.POLICY_IS_VPN
 import com.android.testutils.DevSdkIgnoreRule
 import com.android.testutils.DevSdkIgnoreRunner
+import org.junit.After
+import org.junit.Before
 import org.junit.Test
 import org.junit.runner.RunWith
 import kotlin.reflect.full.staticProperties
 import kotlin.test.assertEquals
-import kotlin.test.assertFailsWith
 import kotlin.test.assertFalse
 import kotlin.test.assertTrue
 
@@ -63,6 +65,23 @@ class FullScoreTest {
         return mixInScore(nc, nac, validated, false /* yieldToBadWifi */, destroyed)
     }
 
+    private val TAG = this::class.simpleName
+
+    private var wtfHandler: Log.TerribleFailureHandler? = null
+
+    @Before
+    fun setUp() {
+        // policyNameOf will call Log.wtf if passed an invalid policy.
+        wtfHandler = Log.setWtfHandler() { tagString, what, system ->
+            Log.d(TAG, "WTF captured, ignoring: $tagString $what")
+        }
+    }
+
+    @After
+    fun tearDown() {
+        Log.setWtfHandler(wtfHandler)
+    }
+
     @Test
     fun testGetLegacyInt() {
         val ns = FullScore(50, 0L /* policy */, KEEP_CONNECTED_NONE)
@@ -101,10 +120,9 @@ class FullScoreTest {
             assertFalse(foundNames.contains(name))
             foundNames.add(name)
         }
-        assertFailsWith<IllegalArgumentException> {
-            FullScore.policyNameOf(MAX_CS_MANAGED_POLICY + 1)
-        }
         assertEquals("IS_UNMETERED", FullScore.policyNameOf(POLICY_IS_UNMETERED))
+        val invalidPolicy = MAX_CS_MANAGED_POLICY + 1
+        assertEquals(Integer.toString(invalidPolicy), FullScore.policyNameOf(invalidPolicy))
     }
 
     fun getAllPolicies() = Regex("POLICY_.*").let { nameRegex ->