diff --git a/service/src/com/android/server/ConnectivityService.java b/service/src/com/android/server/ConnectivityService.java index 6fc63df9b2..b5dbf9618a 100755 --- a/service/src/com/android/server/ConnectivityService.java +++ b/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; } diff --git a/service/src/com/android/server/connectivity/CarrierPrivilegeAuthenticator.java b/service/src/com/android/server/connectivity/CarrierPrivilegeAuthenticator.java index 88aa329885..ef07201abd 100644 --- a/service/src/com/android/server/connectivity/CarrierPrivilegeAuthenticator.java +++ b/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 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); - mThread.start(); - mHandler = new Handler(mThread.getLooper()) {}; + final HandlerThread thread = new HandlerThread(TAG); + thread.start(); + 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(); - registerForCarrierChanges(); - updateCarrierServiceUid(); + // Never unregistered because the system server never stops + c.registerReceiver(new BroadcastReceiver() { + @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)); } - /** - * Broadcast receiver for ACTION_MULTI_SIM_CONFIG_CHANGED - * - *

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(); + private void simConfigChanged() { 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() { diff --git a/service/src/com/android/server/connectivity/ConnectivityFlags.java b/service/src/com/android/server/connectivity/ConnectivityFlags.java index 9039a14f86..5aac8f1e4c 100644 --- a/service/src/com/android/server/connectivity/ConnectivityFlags.java +++ b/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; /** diff --git a/staticlibs/testutils/devicetests/com/android/testutils/DevSdkIgnoreRunner.kt b/staticlibs/testutils/devicetests/com/android/testutils/DevSdkIgnoreRunner.kt index 2e73666bf4..950107cad1 100644 --- a/staticlibs/testutils/devicetests/com/android/testutils/DevSdkIgnoreRunner.kt +++ b/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(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 Class.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) } -} \ No newline at end of file +} diff --git a/tests/unit/java/com/android/server/ConnectivityServiceTest.java b/tests/unit/java/com/android/server/ConnectivityServiceTest.java index 83e5c6700b..eb031575e5 100755 --- a/tests/unit/java/com/android/server/ConnectivityServiceTest.java +++ b/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(); diff --git a/tests/unit/java/com/android/server/connectivity/CarrierPrivilegeAuthenticatorTest.java b/tests/unit/java/com/android/server/connectivity/CarrierPrivilegeAuthenticatorTest.java index 3849e49dfe..2ecd2a14c4 100644 --- a/tests/unit/java/com/android/server/connectivity/CarrierPrivilegeAuthenticatorTest.java +++ b/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 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); - } - - @Before - public void setUp() throws Exception { + final ConnectivityService.Dependencies deps = mock(ConnectivityService.Dependencies.class); + doReturn(useCallbacks).when(deps).isFeatureEnabled(any() /* context */, + eq(CARRIER_SERVICE_CHANGED_USE_CALLBACK)); 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); - } - - private IntentFilter getIntentFilter() { - final ArgumentCaptor captor = ArgumentCaptor.forClass(IntentFilter.class); - verify(mContext).registerReceiver(any(), captor.capture(), any(), any()); - return captor.getValue(); + new TestCarrierPrivilegeAuthenticator(mContext, deps, mTelephonyManager); + final ArgumentCaptor receiverCaptor = + ArgumentCaptor.forClass(BroadcastReceiver.class); + verify(mContext).registerReceiver(receiverCaptor.capture(), argThat(filter -> + filter.getAction(0).equals(ACTION_MULTI_SIM_CONFIG_CHANGED) + ), any() /* broadcast permissions */, any() /* handler */); + mMultiSimBroadcastReceiver = receiverCaptor.getValue(); } private Map 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 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)); 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 93324356d8..b11878d4ec 100644 --- a/tests/unit/java/com/android/server/connectivityservice/base/CSTest.kt +++ b/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) = - if (SdkLevel.isAtLeastT()) mock() else null + override fun makeCarrierPrivilegeAuthenticator( + context: Context, + tm: TelephonyManager + ) = if (SdkLevel.isAtLeastT()) mock() else null private inner class AOOKTDeps(c: Context) : AutomaticOnOffKeepaliveTracker.Dependencies(c) { override fun isTetheringFeatureNotChickenedOut(name: String): Boolean {