Merge changes Ifd7abd8a,I39cab8ff,I6b3270d6,Ia28c6abc,If47d7e23 into main

* changes:
  Move the MULTI_SIM_ACTION receiver inline
  Inline registerForCarrierChanges
  Make mThread a local
  Introduce a flag for using the carrier service changed callbacks.
  Have DevSdkIgnoreRunner support @Parameterized parameters

service/src/com/android/server/ConnectivityService.java

@@ -1427,7 +1427,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
         public CarrierPrivilegeAuthenticator makeCarrierPrivilegeAuthenticator(
                 @NonNull final Context context, @NonNull final TelephonyManager tm) {
             if (isAtLeastT()) {
-                return new CarrierPrivilegeAuthenticator(context, tm);
+                return new CarrierPrivilegeAuthenticator(context, this, tm);
             } else {
                 return null;
             }

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

@@ -16,9 +16,10 @@
 
 package com.android.server.connectivity;
 
-import static android.net.NetworkCapabilities.NET_CAPABILITY_CBS;
 import static android.net.NetworkCapabilities.TRANSPORT_CELLULAR;
 
+import static com.android.server.connectivity.ConnectivityFlags.CARRIER_SERVICE_CHANGED_USE_CALLBACK;
+
 import android.annotation.NonNull;
 import android.content.BroadcastReceiver;
 import android.content.Context;
@@ -44,6 +45,7 @@ import com.android.networkstack.apishim.TelephonyManagerShimImpl;
 import com.android.networkstack.apishim.common.TelephonyManagerShim;
 import com.android.networkstack.apishim.common.TelephonyManagerShim.CarrierPrivilegesListenerShim;
 import com.android.networkstack.apishim.common.UnsupportedApiLevelException;
+import com.android.server.ConnectivityService;
 
 import java.util.ArrayList;
 import java.util.List;
@@ -55,7 +57,7 @@ import java.util.concurrent.Executor;
  * carrier privileged app that provides the carrier config
  * @hide
  */
-public class CarrierPrivilegeAuthenticator extends BroadcastReceiver {
+public class CarrierPrivilegeAuthenticator {
     private static final String TAG = CarrierPrivilegeAuthenticator.class.getSimpleName();
     private static final boolean DBG = true;
 
@@ -68,63 +70,58 @@ public class CarrierPrivilegeAuthenticator extends BroadcastReceiver {
     @GuardedBy("mLock")
     private int mModemCount = 0;
     private final Object mLock = new Object();
-    private final HandlerThread mThread;
     private final Handler mHandler;
     @NonNull
+    @GuardedBy("mLock")
     private final List<CarrierPrivilegesListenerShim> mCarrierPrivilegesChangedListeners =
             new ArrayList<>();
+    private final boolean mUseCallbacksForServiceChanged;
 
     public CarrierPrivilegeAuthenticator(@NonNull final Context c,
+            @NonNull final ConnectivityService.Dependencies deps,
             @NonNull final TelephonyManager t,
             @NonNull final TelephonyManagerShim telephonyManagerShim) {
         mContext = c;
         mTelephonyManager = t;
         mTelephonyManagerShim = telephonyManagerShim;
-        mThread = new HandlerThread(TAG);
+        final HandlerThread thread = new HandlerThread(TAG);
-        mThread.start();
+        thread.start();
-        mHandler = new Handler(mThread.getLooper()) {};
+        mHandler = new Handler(thread.getLooper());
+        mUseCallbacksForServiceChanged = deps.isFeatureEnabled(
+                c, CARRIER_SERVICE_CHANGED_USE_CALLBACK);
+        final IntentFilter filter = new IntentFilter();
+        filter.addAction(TelephonyManager.ACTION_MULTI_SIM_CONFIG_CHANGED);
         synchronized (mLock) {
-            mModemCount = mTelephonyManager.getActiveModemCount();
+            // Never unregistered because the system server never stops
-            registerForCarrierChanges();
+            c.registerReceiver(new BroadcastReceiver() {
-            updateCarrierServiceUid();
+                @Override
+                public void onReceive(final Context context, final Intent intent) {
+                    switch (intent.getAction()) {
+                        case TelephonyManager.ACTION_MULTI_SIM_CONFIG_CHANGED:
+                            simConfigChanged();
+                            break;
+                        default:
+                            Log.d(TAG, "Unknown intent received, action: " + intent.getAction());
+                    }
+                }
+            }, filter, null, mHandler);
+            simConfigChanged();
         }
     }
 
     public CarrierPrivilegeAuthenticator(@NonNull final Context c,
+            @NonNull final ConnectivityService.Dependencies deps,
             @NonNull final TelephonyManager t) {
-        this(c, t, TelephonyManagerShimImpl.newInstance(t));
+        this(c, deps, t, TelephonyManagerShimImpl.newInstance(t));
     }
 
-    /**
+    private void simConfigChanged() {
-     * Broadcast receiver for ACTION_MULTI_SIM_CONFIG_CHANGED
-     *
-     * <p>The broadcast receiver is registered with mHandler
-     */
-    @Override
-    public void onReceive(Context context, Intent intent) {
-        switch (intent.getAction()) {
-            case TelephonyManager.ACTION_MULTI_SIM_CONFIG_CHANGED:
-                handleActionMultiSimConfigChanged(context, intent);
-                break;
-            default:
-                Log.d(TAG, "Unknown intent received with action: " + intent.getAction());
-        }
-    }
-
-    private void handleActionMultiSimConfigChanged(Context context, Intent intent) {
-        unregisterCarrierPrivilegesListeners();
         synchronized (mLock) {
+            unregisterCarrierPrivilegesListeners();
             mModemCount = mTelephonyManager.getActiveModemCount();
+            registerCarrierPrivilegesListeners();
+            updateCarrierServiceUid();
         }
-        registerCarrierPrivilegesListeners();
-        updateCarrierServiceUid();
-    }
-
-    private void registerForCarrierChanges() {
-        final IntentFilter filter = new IntentFilter();
-        filter.addAction(TelephonyManager.ACTION_MULTI_SIM_CONFIG_CHANGED);
-        mContext.registerReceiver(this, filter, null, mHandler);
-        registerCarrierPrivilegesListeners();
     }
 
     private void registerCarrierPrivilegesListeners() {

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

@@ -33,6 +33,10 @@ public final class ConnectivityFlags {
     public static final String NO_REMATCH_ALL_REQUESTS_ON_REGISTER =
             "no_rematch_all_requests_on_register";
 
+    @VisibleForTesting
+    public static final String CARRIER_SERVICE_CHANGED_USE_CALLBACK =
+            "carrier_service_changed_use_callback_version";
+
     private boolean mNoRematchAllRequestsOnRegister;
 
     /**

staticlibs/testutils/devicetests/com/android/testutils/DevSdkIgnoreRunner.kt

@@ -19,6 +19,7 @@ package com.android.testutils
 import androidx.test.ext.junit.runners.AndroidJUnit4
 import com.android.testutils.DevSdkIgnoreRule.IgnoreAfter
 import com.android.testutils.DevSdkIgnoreRule.IgnoreUpTo
+import java.lang.reflect.Modifier
 import org.junit.runner.Description
 import org.junit.runner.Runner
 import org.junit.runner.manipulation.Filter
@@ -27,7 +28,7 @@ import org.junit.runner.manipulation.NoTestsRemainException
 import org.junit.runner.manipulation.Sortable
 import org.junit.runner.manipulation.Sorter
 import org.junit.runner.notification.RunNotifier
-import kotlin.jvm.Throws
+import org.junit.runners.Parameterized
 
 /**
  * A runner that can skip tests based on the development SDK as defined in [DevSdkIgnoreRule].
@@ -41,6 +42,9 @@ import kotlin.jvm.Throws
  * the whole class if they do not match the development SDK as defined in [DevSdkIgnoreRule].
  * Otherwise, it will delegate to [AndroidJUnit4] to run the test as usual.
  *
+ * This class automatically uses the Parameterized runner as its base runner, so the
+ * @Parameterized.Parameters annotation and its friends can be used in tests using this runner.
+ *
  * Example usage:
  *
  *     @RunWith(DevSdkIgnoreRunner::class)
@@ -48,13 +52,34 @@ import kotlin.jvm.Throws
  *     class MyTestClass { ... }
  */
 class DevSdkIgnoreRunner(private val klass: Class<*>) : Runner(), Filterable, Sortable {
-    private val baseRunner = klass.let {
+    // Inference correctly infers Runner & Filterable & Sortable for |baseRunner|, but the
+    // Java bytecode doesn't have a way to express this. Give this type a name by wrapping it.
+    private class RunnerWrapper<T>(private val wrapped: T) :
+            Runner(), Filterable by wrapped, Sortable by wrapped
+            where T : Runner, T : Filterable, T : Sortable {
+        override fun getDescription(): Description = wrapped.description
+        override fun run(notifier: RunNotifier?) = wrapped.run(notifier)
+    }
+
+    private val baseRunner: RunnerWrapper<*>? = klass.let {
         val ignoreAfter = it.getAnnotation(IgnoreAfter::class.java)
         val ignoreUpTo = it.getAnnotation(IgnoreUpTo::class.java)
 
-        if (isDevSdkInRange(ignoreUpTo, ignoreAfter)) AndroidJUnit4(klass) else null
+        if (!isDevSdkInRange(ignoreUpTo, ignoreAfter)) {
+            null
+        } else if (it.hasParameterizedMethod()) {
+            // Parameterized throws if there is no static method annotated with @Parameters, which
+            // isn't too useful. Use if it there are, otherwise use its base AndroidJUnit4 runner.
+            RunnerWrapper(Parameterized(klass))
+        } else {
+            RunnerWrapper(AndroidJUnit4(klass))
+        }
     }
 
+    private fun <T> Class<T>.hasParameterizedMethod(): Boolean = methods.any {
+        Modifier.isStatic(it.modifiers) &&
+                it.isAnnotationPresent(Parameterized.Parameters::class.java) }
+
     override fun run(notifier: RunNotifier) {
         if (baseRunner != null) {
             baseRunner.run(notifier)
@@ -88,4 +113,4 @@ class DevSdkIgnoreRunner(private val klass: Class<*>) : Runner(), Filterable, So
     override fun sort(sorter: Sorter?) {
         baseRunner?.sort(sorter)
     }
-}
+}

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

@@ -30,6 +30,7 @@ import static android.Manifest.permission.NETWORK_SETTINGS;
 import static android.Manifest.permission.NETWORK_SETUP_WIZARD;
 import static android.Manifest.permission.NETWORK_STACK;
 import static android.Manifest.permission.PACKET_KEEPALIVE_OFFLOAD;
+import static android.Manifest.permission.READ_DEVICE_CONFIG;
 import static android.app.ActivityManager.UidFrozenStateChangedCallback.UID_FROZEN_STATE_FROZEN;
 import static android.app.ActivityManager.UidFrozenStateChangedCallback.UID_FROZEN_STATE_UNFROZEN;
 import static android.app.PendingIntent.FLAG_IMMUTABLE;
@@ -153,6 +154,7 @@ import static android.os.Process.INVALID_UID;
 import static android.system.OsConstants.IPPROTO_TCP;
 
 import static com.android.server.ConnectivityService.DELAY_DESTROY_FROZEN_SOCKETS_VERSION;
+import static com.android.net.module.util.DeviceConfigUtils.TETHERING_MODULE_NAME;
 import static com.android.server.ConnectivityService.KEY_DESTROY_FROZEN_SOCKETS_VERSION;
 import static com.android.server.ConnectivityService.MAX_NETWORK_REQUESTS_PER_SYSTEM_UID;
 import static com.android.server.ConnectivityService.PREFERENCE_ORDER_MOBILE_DATA_PREFERERRED;
@@ -251,6 +253,7 @@ import android.content.Context;
 import android.content.Intent;
 import android.content.IntentFilter;
 import android.content.pm.ApplicationInfo;
+import android.content.pm.ModuleInfo;
 import android.content.pm.PackageInfo;
 import android.content.pm.PackageManager;
 import android.content.pm.ResolveInfo;
@@ -1904,6 +1907,7 @@ public class ConnectivityServiceTest {
         mServiceContext.setPermission(CONTROL_OEM_PAID_NETWORK_PREFERENCE, PERMISSION_GRANTED);
         mServiceContext.setPermission(PACKET_KEEPALIVE_OFFLOAD, PERMISSION_GRANTED);
         mServiceContext.setPermission(CONNECTIVITY_USE_RESTRICTED_NETWORKS, PERMISSION_GRANTED);
+        mServiceContext.setPermission(READ_DEVICE_CONFIG, PERMISSION_GRANTED);
 
         mAlarmManagerThread = new HandlerThread("TestAlarmManager");
         mAlarmManagerThread.start();
@@ -2059,7 +2063,8 @@ public class ConnectivityServiceTest {
 
         @Override
         public CarrierPrivilegeAuthenticator makeCarrierPrivilegeAuthenticator(
-                @NonNull final Context context, @NonNull final TelephonyManager tm) {
+                @NonNull final Context context,
+                @NonNull final TelephonyManager tm) {
             return mDeps.isAtLeastT() ? mCarrierPrivilegeAuthenticator : null;
         }
 
@@ -2151,6 +2156,7 @@ public class ConnectivityServiceTest {
         public boolean isFeatureEnabled(Context context, String name) {
             switch (name) {
                 case ConnectivityFlags.NO_REMATCH_ALL_REQUESTS_ON_REGISTER:
+                case ConnectivityFlags.CARRIER_SERVICE_CHANGED_USE_CALLBACK:
                     return true;
                 case KEY_DESTROY_FROZEN_SOCKETS_VERSION:
                     return true;
@@ -2405,6 +2411,7 @@ public class ConnectivityServiceTest {
         final String myPackageName = mContext.getPackageName();
         final PackageInfo myPackageInfo = mContext.getPackageManager().getPackageInfo(
                 myPackageName, PackageManager.GET_PERMISSIONS);
+        myPackageInfo.setLongVersionCode(9_999_999L);
         doReturn(new String[] {myPackageName}).when(mPackageManager)
                 .getPackagesForUid(Binder.getCallingUid());
         doReturn(myPackageInfo).when(mPackageManager).getPackageInfoAsUser(
@@ -2416,6 +2423,13 @@ public class ConnectivityServiceTest {
                 buildPackageInfo(/* SYSTEM */ false, VPN_UID)
         })).when(mPackageManager).getInstalledPackagesAsUser(eq(GET_PERMISSIONS), anyInt());
 
+        final ModuleInfo moduleInfo = new ModuleInfo();
+        moduleInfo.setPackageName(TETHERING_MODULE_NAME);
+        doReturn(moduleInfo).when(mPackageManager)
+                .getModuleInfo(TETHERING_MODULE_NAME, PackageManager.MODULE_APEX_NAME);
+        doReturn(myPackageInfo).when(mPackageManager)
+                .getPackageInfo(TETHERING_MODULE_NAME, PackageManager.MATCH_APEX);
+
         // Create a fake always-on VPN package.
         final int userId = UserHandle.getCallingUserId();
         final ApplicationInfo applicationInfo = new ApplicationInfo();

tests/unit/java/com/android/server/connectivity/CarrierPrivilegeAuthenticatorTest.java

@@ -20,12 +20,15 @@ import static android.net.NetworkCapabilities.TRANSPORT_CELLULAR;
 import static android.net.NetworkCapabilities.TRANSPORT_WIFI;
 import static android.telephony.TelephonyManager.ACTION_MULTI_SIM_CONFIG_CHANGED;
 
+import static com.android.server.connectivity.ConnectivityFlags.CARRIER_SERVICE_CHANGED_USE_CALLBACK;
+
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertTrue;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.anyInt;
+import static org.mockito.ArgumentMatchers.argThat;
 import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.atLeastOnce;
 import static org.mockito.Mockito.clearInvocations;
@@ -34,9 +37,9 @@ import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.verify;
 
 import android.annotation.NonNull;
+import android.content.BroadcastReceiver;
 import android.content.Context;
 import android.content.Intent;
-import android.content.IntentFilter;
 import android.content.pm.ApplicationInfo;
 import android.content.pm.PackageManager;
 import android.net.NetworkCapabilities;
@@ -49,14 +52,17 @@ import com.android.net.module.util.CollectionUtils;
 import com.android.networkstack.apishim.TelephonyManagerShimImpl;
 import com.android.networkstack.apishim.common.TelephonyManagerShim.CarrierPrivilegesListenerShim;
 import com.android.networkstack.apishim.common.UnsupportedApiLevelException;
+import com.android.server.ConnectivityService;
 import com.android.testutils.DevSdkIgnoreRule.IgnoreUpTo;
 import com.android.testutils.DevSdkIgnoreRunner;
 
-import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
 import org.mockito.ArgumentCaptor;
 
+import java.util.Arrays;
+import java.util.Collection;
 import java.util.Collections;
 import java.util.Map;
 
@@ -79,11 +85,13 @@ public class CarrierPrivilegeAuthenticatorTest {
     @NonNull private TestCarrierPrivilegeAuthenticator mCarrierPrivilegeAuthenticator;
     private final int mCarrierConfigPkgUid = 12345;
     private final String mTestPkg = "com.android.server.connectivity.test";
+    private final BroadcastReceiver mMultiSimBroadcastReceiver;
 
     public class TestCarrierPrivilegeAuthenticator extends CarrierPrivilegeAuthenticator {
         TestCarrierPrivilegeAuthenticator(@NonNull final Context c,
+                @NonNull final ConnectivityService.Dependencies deps,
                 @NonNull final TelephonyManager t) {
-            super(c, t, mTelephonyManagerShim);
+            super(c, deps, t, mTelephonyManagerShim);
         }
         @Override
         protected int getSlotIndex(int subId) {
@@ -92,15 +100,20 @@ public class CarrierPrivilegeAuthenticatorTest {
         }
     }
 
-    public CarrierPrivilegeAuthenticatorTest() {
+    /** Parameters to test both using callbacks or the old broadcast */
+    @Parameterized.Parameters
+    public static Collection<Boolean> shouldUseCallbacks() {
+        return Arrays.asList(true, false);
+    }
+
+    public CarrierPrivilegeAuthenticatorTest(final boolean useCallbacks) throws Exception {
         mContext = mock(Context.class);
         mTelephonyManager = mock(TelephonyManager.class);
         mTelephonyManagerShim = mock(TelephonyManagerShimImpl.class);
         mPackageManager = mock(PackageManager.class);
-    }
+        final ConnectivityService.Dependencies deps = mock(ConnectivityService.Dependencies.class);
-
+        doReturn(useCallbacks).when(deps).isFeatureEnabled(any() /* context */,
-    @Before
+                eq(CARRIER_SERVICE_CHANGED_USE_CALLBACK));
-    public void setUp() throws Exception {
         doReturn(SUBSCRIPTION_COUNT).when(mTelephonyManager).getActiveModemCount();
         doReturn(mTestPkg).when(mTelephonyManagerShim)
                 .getCarrierServicePackageNameForLogicalSlot(anyInt());
@@ -109,13 +122,13 @@ public class CarrierPrivilegeAuthenticatorTest {
         applicationInfo.uid = mCarrierConfigPkgUid;
         doReturn(applicationInfo).when(mPackageManager).getApplicationInfo(eq(mTestPkg), anyInt());
         mCarrierPrivilegeAuthenticator =
-                new TestCarrierPrivilegeAuthenticator(mContext, mTelephonyManager);
+                new TestCarrierPrivilegeAuthenticator(mContext, deps, mTelephonyManager);
-    }
+        final ArgumentCaptor<BroadcastReceiver> receiverCaptor =
-
+                ArgumentCaptor.forClass(BroadcastReceiver.class);
-    private IntentFilter getIntentFilter() {
+        verify(mContext).registerReceiver(receiverCaptor.capture(), argThat(filter ->
-        final ArgumentCaptor<IntentFilter> captor = ArgumentCaptor.forClass(IntentFilter.class);
+                filter.getAction(0).equals(ACTION_MULTI_SIM_CONFIG_CHANGED)
-        verify(mContext).registerReceiver(any(), captor.capture(), any(), any());
+        ), any() /* broadcast permissions */, any() /* handler */);
-        return captor.getValue();
+        mMultiSimBroadcastReceiver = receiverCaptor.getValue();
     }
 
     private Map<Integer, CarrierPrivilegesListenerShim> getCarrierPrivilegesListeners() {
@@ -138,15 +151,6 @@ public class CarrierPrivilegeAuthenticatorTest {
     }
     @Test
     public void testConstructor() throws Exception {
-        verify(mContext).registerReceiver(
-                        eq(mCarrierPrivilegeAuthenticator),
-                        any(IntentFilter.class),
-                        any(),
-                        any());
-        final IntentFilter filter = getIntentFilter();
-        assertEquals(1, filter.countActions());
-        assertTrue(filter.hasAction(ACTION_MULTI_SIM_CONFIG_CHANGED));
-
         // Two listeners originally registered, one for slot 0 and one for slot 1
         final Map<Integer, CarrierPrivilegesListenerShim> initialListeners =
                 getCarrierPrivilegesListeners();
@@ -174,8 +178,11 @@ public class CarrierPrivilegeAuthenticatorTest {
         assertEquals(2, initialListeners.size());
 
         doReturn(1).when(mTelephonyManager).getActiveModemCount();
-        mCarrierPrivilegeAuthenticator.onReceive(
+
-                mContext, buildTestMultiSimConfigBroadcastIntent());
+        // This is a little bit cavalier in that the call to onReceive is not on the handler
+        // thread that was specified in registerReceiver.
+        // TODO : capture the handler and call this on it if this causes flakiness.
+        mMultiSimBroadcastReceiver.onReceive(mContext, buildTestMultiSimConfigBroadcastIntent());
         // Check all listeners have been removed
         for (CarrierPrivilegesListenerShim listener : initialListeners.values()) {
             verify(mTelephonyManagerShim).removeCarrierPrivilegesListener(eq(listener));

tests/unit/java/com/android/server/connectivityservice/base/CSTest.kt

@@ -135,8 +135,10 @@ open class CSTest {
         override fun makeHandlerThread() = csHandlerThread
         override fun makeProxyTracker(context: Context, connServiceHandler: Handler) = proxyTracker
 
-        override fun makeCarrierPrivilegeAuthenticator(context: Context, tm: TelephonyManager) =
+        override fun makeCarrierPrivilegeAuthenticator(
-                if (SdkLevel.isAtLeastT()) mock<CarrierPrivilegeAuthenticator>() else null
+                context: Context,
+                tm: TelephonyManager
+        ) = if (SdkLevel.isAtLeastT()) mock<CarrierPrivilegeAuthenticator>() else null
 
         private inner class AOOKTDeps(c: Context) : AutomaticOnOffKeepaliveTracker.Dependencies(c) {
             override fun isTetheringFeatureNotChickenedOut(name: String): Boolean {