Use connectivityHandler in CarrierPrivilegeAuthenticator
When request_restricted_wifi is enabled CarrierPrivilegeAuthenticator
calls CarrierPrivilegesLost callback passed from ConnectivityService.
But CarrierPrivilegeAuthenticator was using its own handler thread to
receive broadcast and callbacks so CarrierPrivilegesLost callback needed
to post a message to the CS handler.
This CL updates CarrierPrivilegeAuthenticator to use CS handler if
request_restricted_wifi is enabled, and remove the message post in
ConnectivityService.
Test: TH
Change-Id: Ib41304a30e3dd366b0f6f1d7df81145f0e05ece1
diff --git a/service/src/com/android/server/ConnectivityService.java b/service/src/com/android/server/ConnectivityService.java
index 30440f5..a5d1645 100755
--- a/service/src/com/android/server/ConnectivityService.java
+++ b/service/src/com/android/server/ConnectivityService.java
@@ -845,11 +845,6 @@
private static final int EVENT_UID_FROZEN_STATE_CHANGED = 61;
/**
- * Event to inform the ConnectivityService handler when a uid has lost carrier privileges.
- */
- private static final int EVENT_UID_CARRIER_PRIVILEGES_LOST = 62;
-
- /**
* Argument for {@link #EVENT_PROVISIONING_NOTIFICATION} to indicate that the notification
* should be shown.
*/
@@ -1289,14 +1284,6 @@
}
private final LegacyTypeTracker mLegacyTypeTracker = new LegacyTypeTracker(this);
- @VisibleForTesting
- void onCarrierPrivilegesLost(Integer uid, Integer subId) {
- if (mRequestRestrictedWifiEnabled) {
- mHandler.sendMessage(mHandler.obtainMessage(
- EVENT_UID_CARRIER_PRIVILEGES_LOST, uid, subId));
- }
- }
-
final LocalPriorityDump mPriorityDumper = new LocalPriorityDump();
/**
* Helper class which parses out priority arguments and dumps sections according to their
@@ -1518,10 +1505,11 @@
@NonNull final Context context,
@NonNull final TelephonyManager tm,
boolean requestRestrictedWifiEnabled,
- @NonNull BiConsumer<Integer, Integer> listener) {
+ @NonNull BiConsumer<Integer, Integer> listener,
+ @NonNull final Handler connectivityServiceHandler) {
if (isAtLeastT()) {
- return new CarrierPrivilegeAuthenticator(
- context, tm, requestRestrictedWifiEnabled, listener);
+ return new CarrierPrivilegeAuthenticator(context, tm, requestRestrictedWifiEnabled,
+ listener, connectivityServiceHandler);
} else {
return null;
}
@@ -1806,7 +1794,7 @@
&& mDeps.isFeatureEnabled(context, REQUEST_RESTRICTED_WIFI);
mCarrierPrivilegeAuthenticator = mDeps.makeCarrierPrivilegeAuthenticator(
mContext, mTelephonyManager, mRequestRestrictedWifiEnabled,
- this::onCarrierPrivilegesLost);
+ this::handleUidCarrierPrivilegesLost, mHandler);
if (mDeps.isAtLeastU()
&& mDeps
@@ -3823,6 +3811,10 @@
mSatelliteAccessController.start();
}
+ if (mCarrierPrivilegeAuthenticator != null) {
+ mCarrierPrivilegeAuthenticator.start();
+ }
+
// On T+ devices, register callback for statsd to pull NETWORK_BPF_MAP_INFO atom
if (mDeps.isAtLeastT()) {
mBpfNetMaps.setPullAtomCallback(mContext);
@@ -6510,9 +6502,6 @@
UidFrozenStateChangedArgs args = (UidFrozenStateChangedArgs) msg.obj;
handleFrozenUids(args.mUids, args.mFrozenStates);
break;
- case EVENT_UID_CARRIER_PRIVILEGES_LOST:
- handleUidCarrierPrivilegesLost(msg.arg1, msg.arg2);
- break;
}
}
}
@@ -9168,6 +9157,9 @@
}
private void handleUidCarrierPrivilegesLost(int uid, int subId) {
+ if (!mRequestRestrictedWifiEnabled) {
+ return;
+ }
ensureRunningOnConnectivityServiceThread();
// A NetworkRequest needs to be revoked when all the conditions are met
// 1. It requests restricted network
diff --git a/service/src/com/android/server/connectivity/CarrierPrivilegeAuthenticator.java b/service/src/com/android/server/connectivity/CarrierPrivilegeAuthenticator.java
index 04d0fc1..f5fa4fb 100644
--- a/service/src/com/android/server/connectivity/CarrierPrivilegeAuthenticator.java
+++ b/service/src/com/android/server/connectivity/CarrierPrivilegeAuthenticator.java
@@ -91,42 +91,62 @@
@NonNull final TelephonyManager t,
@NonNull final TelephonyManagerShim telephonyManagerShim,
final boolean requestRestrictedWifiEnabled,
- @NonNull BiConsumer<Integer, Integer> listener) {
+ @NonNull BiConsumer<Integer, Integer> listener,
+ @NonNull final Handler connectivityServiceHandler) {
mContext = c;
mTelephonyManager = t;
mTelephonyManagerShim = telephonyManagerShim;
- final HandlerThread thread = deps.makeHandlerThread();
- thread.start();
- mHandler = new Handler(thread.getLooper());
mUseCallbacksForServiceChanged = deps.isFeatureEnabled(
c, CARRIER_SERVICE_CHANGED_USE_CALLBACK);
mRequestRestrictedWifiEnabled = requestRestrictedWifiEnabled;
mListener = listener;
+ if (mRequestRestrictedWifiEnabled) {
+ mHandler = connectivityServiceHandler;
+ } else {
+ final HandlerThread thread = deps.makeHandlerThread();
+ thread.start();
+ mHandler = new Handler(thread.getLooper());
+ synchronized (mLock) {
+ registerSimConfigChangedReceiver();
+ simConfigChanged();
+ }
+ }
+ }
+
+ private void registerSimConfigChangedReceiver() {
final IntentFilter filter = new IntentFilter();
filter.addAction(TelephonyManager.ACTION_MULTI_SIM_CONFIG_CHANGED);
- synchronized (mLock) {
- // 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());
- }
+ // Never unregistered because the system server never stops
+ mContext.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();
+ }
+ }, filter, null, mHandler);
+ }
+
+ /**
+ * Start CarrierPrivilegeAuthenticator
+ */
+ public void start() {
+ if (mRequestRestrictedWifiEnabled) {
+ registerSimConfigChangedReceiver();
+ mHandler.post(this::simConfigChanged);
}
}
public CarrierPrivilegeAuthenticator(@NonNull final Context c,
@NonNull final TelephonyManager t, final boolean requestRestrictedWifiEnabled,
- @NonNull BiConsumer<Integer, Integer> listener) {
+ @NonNull BiConsumer<Integer, Integer> listener,
+ @NonNull final Handler connectivityServiceHandler) {
this(c, new Dependencies(), t, TelephonyManagerShimImpl.newInstance(t),
- requestRestrictedWifiEnabled, listener);
+ requestRestrictedWifiEnabled, listener, connectivityServiceHandler);
}
public static class Dependencies {
@@ -146,6 +166,10 @@
}
private void simConfigChanged() {
+ // If mRequestRestrictedWifiEnabled is false, constructor calls simConfigChanged
+ if (mRequestRestrictedWifiEnabled) {
+ ensureRunningOnHandlerThread();
+ }
synchronized (mLock) {
unregisterCarrierPrivilegesListeners();
mModemCount = mTelephonyManager.getActiveModemCount();
@@ -188,6 +212,7 @@
public void onCarrierPrivilegesChanged(
@NonNull List<String> privilegedPackageNames,
@NonNull int[] privilegedUids) {
+ ensureRunningOnHandlerThread();
if (mUseCallbacksForServiceChanged) return;
// Re-trigger the synchronous check (which is also very cheap due
// to caching in CarrierPrivilegesTracker). This allows consistency
@@ -198,6 +223,7 @@
@Override
public void onCarrierServiceChanged(@Nullable final String carrierServicePackageName,
final int carrierServiceUid) {
+ ensureRunningOnHandlerThread();
if (!mUseCallbacksForServiceChanged) {
// Re-trigger the synchronous check (which is also very cheap due
// to caching in CarrierPrivilegesTracker). This allows consistency
@@ -439,6 +465,13 @@
}
}
+ private void ensureRunningOnHandlerThread() {
+ if (mHandler.getLooper().getThread() != Thread.currentThread()) {
+ throw new IllegalStateException(
+ "Not running on handler thread: " + Thread.currentThread().getName());
+ }
+ }
+
public void dump(IndentingPrintWriter pw) {
pw.println("CarrierPrivilegeAuthenticator:");
pw.println("mRequestRestrictedWifiEnabled = " + mRequestRestrictedWifiEnabled);
diff --git a/tests/integration/src/com/android/server/net/integrationtests/ConnectivityServiceIntegrationTest.kt b/tests/integration/src/com/android/server/net/integrationtests/ConnectivityServiceIntegrationTest.kt
index 035f607..d2e46af 100644
--- a/tests/integration/src/com/android/server/net/integrationtests/ConnectivityServiceIntegrationTest.kt
+++ b/tests/integration/src/com/android/server/net/integrationtests/ConnectivityServiceIntegrationTest.kt
@@ -243,18 +243,19 @@
super.makeHandlerThread(tag).also { handlerThreads.add(it) }
override fun makeCarrierPrivilegeAuthenticator(
- context: Context,
- tm: TelephonyManager,
- requestRestrictedWifiEnabled: Boolean,
- listener: BiConsumer<Int, Int>
+ context: Context,
+ tm: TelephonyManager,
+ requestRestrictedWifiEnabled: Boolean,
+ listener: BiConsumer<Int, Int>,
+ handler: Handler
): CarrierPrivilegeAuthenticator {
return CarrierPrivilegeAuthenticator(context,
- object : CarrierPrivilegeAuthenticator.Dependencies() {
- override fun makeHandlerThread(): HandlerThread =
- super.makeHandlerThread().also { handlerThreads.add(it) }
- },
- tm, TelephonyManagerShimImpl.newInstance(tm),
- requestRestrictedWifiEnabled, listener)
+ object : CarrierPrivilegeAuthenticator.Dependencies() {
+ override fun makeHandlerThread(): HandlerThread =
+ super.makeHandlerThread().also { handlerThreads.add(it) }
+ },
+ tm, TelephonyManagerShimImpl.newInstance(tm),
+ requestRestrictedWifiEnabled, listener, handler)
}
override fun makeSatelliteAccessController(
diff --git a/tests/unit/java/com/android/server/ConnectivityServiceTest.java b/tests/unit/java/com/android/server/ConnectivityServiceTest.java
index e09a2cb..98ba91f 100755
--- a/tests/unit/java/com/android/server/ConnectivityServiceTest.java
+++ b/tests/unit/java/com/android/server/ConnectivityServiceTest.java
@@ -2054,12 +2054,16 @@
};
}
+ private BiConsumer<Integer, Integer> mCarrierPrivilegesLostListener;
+
@Override
public CarrierPrivilegeAuthenticator makeCarrierPrivilegeAuthenticator(
@NonNull final Context context,
@NonNull final TelephonyManager tm,
final boolean requestRestrictedWifiEnabled,
- BiConsumer<Integer, Integer> listener) {
+ BiConsumer<Integer, Integer> listener,
+ @NonNull final Handler handler) {
+ mCarrierPrivilegesLostListener = listener;
return mDeps.isAtLeastT() ? mCarrierPrivilegeAuthenticator : null;
}
@@ -17446,7 +17450,10 @@
.isCarrierServiceUidForNetworkCapabilities(eq(Process.myUid()), any());
doReturn(TEST_SUBSCRIPTION_ID).when(mCarrierPrivilegeAuthenticator)
.getSubIdFromNetworkCapabilities(any());
- mService.onCarrierPrivilegesLost(lostPrivilegeUid, lostPrivilegeSubId);
+
+ visibleOnHandlerThread(mCsHandlerThread.getThreadHandler(), () -> {
+ mDeps.mCarrierPrivilegesLostListener.accept(lostPrivilegeUid, lostPrivilegeSubId);
+ });
waitForIdle();
if (expectCapChanged) {
diff --git a/tests/unit/java/com/android/server/connectivity/CarrierPrivilegeAuthenticatorTest.java b/tests/unit/java/com/android/server/connectivity/CarrierPrivilegeAuthenticatorTest.java
index 7bd2b56..ab81abc 100644
--- a/tests/unit/java/com/android/server/connectivity/CarrierPrivilegeAuthenticatorTest.java
+++ b/tests/unit/java/com/android/server/connectivity/CarrierPrivilegeAuthenticatorTest.java
@@ -21,6 +21,7 @@
import static android.telephony.TelephonyManager.ACTION_MULTI_SIM_CONFIG_CHANGED;
import static com.android.server.connectivity.ConnectivityFlags.CARRIER_SERVICE_CHANGED_USE_CALLBACK;
+import static com.android.testutils.HandlerUtils.visibleOnHandlerThread;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
@@ -45,6 +46,7 @@
import android.net.NetworkCapabilities;
import android.net.TelephonyNetworkSpecifier;
import android.os.Build;
+import android.os.Handler;
import android.os.HandlerThread;
import android.telephony.TelephonyManager;
@@ -56,6 +58,7 @@
import com.android.testutils.DevSdkIgnoreRule;
import com.android.testutils.DevSdkIgnoreRule.IgnoreUpTo;
import com.android.testutils.DevSdkIgnoreRunner;
+import com.android.testutils.HandlerUtils;
import org.junit.After;
import org.junit.Rule;
@@ -85,6 +88,7 @@
private static final int SUBSCRIPTION_COUNT = 2;
private static final int TEST_SUBSCRIPTION_ID = 1;
+ private static final int TIMEOUT_MS = 1_000;
@NonNull private final Context mContext;
@NonNull private final TelephonyManager mTelephonyManager;
@@ -97,13 +101,16 @@
private final String mTestPkg = "com.android.server.connectivity.test";
private final BroadcastReceiver mMultiSimBroadcastReceiver;
@NonNull private final HandlerThread mHandlerThread;
+ @NonNull private final Handler mCsHandler;
+ @NonNull private final HandlerThread mCsHandlerThread;
public class TestCarrierPrivilegeAuthenticator extends CarrierPrivilegeAuthenticator {
TestCarrierPrivilegeAuthenticator(@NonNull final Context c,
@NonNull final Dependencies deps,
- @NonNull final TelephonyManager t) {
+ @NonNull final TelephonyManager t,
+ @NonNull final Handler handler) {
super(c, deps, t, mTelephonyManagerShim, true /* requestRestrictedWifiEnabled */,
- mListener);
+ mListener, handler);
}
@Override
protected int getSubId(int slotIndex) {
@@ -112,8 +119,11 @@
}
@After
- public void tearDown() {
+ public void tearDown() throws Exception {
mHandlerThread.quit();
+ mHandlerThread.join();
+ mCsHandlerThread.quit();
+ mCsHandlerThread.join();
}
/** Parameters to test both using callbacks or the old broadcast */
@@ -141,8 +151,14 @@
final ApplicationInfo applicationInfo = new ApplicationInfo();
applicationInfo.uid = mCarrierConfigPkgUid;
doReturn(applicationInfo).when(mPackageManager).getApplicationInfo(eq(mTestPkg), anyInt());
- mCarrierPrivilegeAuthenticator =
- new TestCarrierPrivilegeAuthenticator(mContext, deps, mTelephonyManager);
+ mCsHandlerThread = new HandlerThread(
+ CarrierPrivilegeAuthenticatorTest.class.getSimpleName() + "-CsHandlerThread");
+ mCsHandlerThread.start();
+ mCsHandler = new Handler(mCsHandlerThread.getLooper());
+ mCarrierPrivilegeAuthenticator = new TestCarrierPrivilegeAuthenticator(mContext, deps,
+ mTelephonyManager, mCsHandler);
+ mCarrierPrivilegeAuthenticator.start();
+ HandlerUtils.waitForIdle(mCsHandlerThread, TIMEOUT_MS);
final ArgumentCaptor<BroadcastReceiver> receiverCaptor =
ArgumentCaptor.forClass(BroadcastReceiver.class);
verify(mContext).registerReceiver(receiverCaptor.capture(), argThat(filter ->
@@ -178,7 +194,9 @@
assertNotNull(initialListeners.get(1));
assertEquals(2, initialListeners.size());
- initialListeners.get(0).onCarrierServiceChanged(null, mCarrierConfigPkgUid);
+ visibleOnHandlerThread(mCsHandler, () -> {
+ initialListeners.get(0).onCarrierServiceChanged(null, mCarrierConfigPkgUid);
+ });
final NetworkCapabilities.Builder ncBuilder = new NetworkCapabilities.Builder()
.addTransportType(TRANSPORT_CELLULAR)
@@ -201,10 +219,10 @@
doReturn(1).when(mTelephonyManager).getActiveModemCount();
- // 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());
+ visibleOnHandlerThread(mCsHandler, () -> {
+ mMultiSimBroadcastReceiver.onReceive(mContext,
+ buildTestMultiSimConfigBroadcastIntent());
+ });
// Check all listeners have been removed
for (CarrierPrivilegesListenerShim listener : initialListeners.values()) {
verify(mTelephonyManagerShim).removeCarrierPrivilegesListener(eq(listener));
@@ -216,7 +234,9 @@
assertNotNull(newListeners.get(0));
assertEquals(1, newListeners.size());
- newListeners.get(0).onCarrierServiceChanged(null, mCarrierConfigPkgUid);
+ visibleOnHandlerThread(mCsHandler, () -> {
+ newListeners.get(0).onCarrierServiceChanged(null, mCarrierConfigPkgUid);
+ });
final TelephonyNetworkSpecifier specifier =
new TelephonyNetworkSpecifier(TEST_SUBSCRIPTION_ID);
@@ -235,12 +255,17 @@
public void testCarrierPrivilegesLostDueToCarrierServiceUpdate() throws Exception {
final CarrierPrivilegesListenerShim l = getCarrierPrivilegesListeners().get(0);
- l.onCarrierServiceChanged(null, mCarrierConfigPkgUid);
- l.onCarrierServiceChanged(null, mCarrierConfigPkgUid + 1);
+ visibleOnHandlerThread(mCsHandler, () -> {
+ l.onCarrierServiceChanged(null, mCarrierConfigPkgUid);
+ l.onCarrierServiceChanged(null, mCarrierConfigPkgUid + 1);
+ });
if (mUseCallbacks) {
verify(mListener).accept(eq(mCarrierConfigPkgUid), eq(TEST_SUBSCRIPTION_ID));
}
- l.onCarrierServiceChanged(null, mCarrierConfigPkgUid + 2);
+
+ visibleOnHandlerThread(mCsHandler, () -> {
+ l.onCarrierServiceChanged(null, mCarrierConfigPkgUid + 2);
+ });
if (mUseCallbacks) {
verify(mListener).accept(eq(mCarrierConfigPkgUid + 1), eq(TEST_SUBSCRIPTION_ID));
}
@@ -260,8 +285,10 @@
final ApplicationInfo applicationInfo = new ApplicationInfo();
applicationInfo.uid = mCarrierConfigPkgUid + 1;
doReturn(applicationInfo).when(mPackageManager).getApplicationInfo(eq(mTestPkg), anyInt());
- listener.onCarrierPrivilegesChanged(Collections.emptyList(), new int[] {});
- listener.onCarrierServiceChanged(null, applicationInfo.uid);
+ visibleOnHandlerThread(mCsHandler, () -> {
+ listener.onCarrierPrivilegesChanged(Collections.emptyList(), new int[]{});
+ listener.onCarrierServiceChanged(null, applicationInfo.uid);
+ });
assertFalse(mCarrierPrivilegeAuthenticator.isCarrierServiceUidForNetworkCapabilities(
mCarrierConfigPkgUid, nc));
@@ -272,7 +299,9 @@
@Test
public void testDefaultSubscription() throws Exception {
final CarrierPrivilegesListenerShim listener = getCarrierPrivilegesListeners().get(0);
- listener.onCarrierServiceChanged(null, mCarrierConfigPkgUid);
+ visibleOnHandlerThread(mCsHandler, () -> {
+ listener.onCarrierServiceChanged(null, mCarrierConfigPkgUid);
+ });
final NetworkCapabilities.Builder ncBuilder = new NetworkCapabilities.Builder();
ncBuilder.addTransportType(TRANSPORT_CELLULAR);
@@ -297,7 +326,9 @@
@IgnoreUpTo(Build.VERSION_CODES.TIRAMISU)
public void testNetworkCapabilitiesContainOneSubId() throws Exception {
final CarrierPrivilegesListenerShim listener = getCarrierPrivilegesListeners().get(0);
- listener.onCarrierServiceChanged(null, mCarrierConfigPkgUid);
+ visibleOnHandlerThread(mCsHandler, () -> {
+ listener.onCarrierServiceChanged(null, mCarrierConfigPkgUid);
+ });
final NetworkCapabilities.Builder ncBuilder = new NetworkCapabilities.Builder();
ncBuilder.addTransportType(TRANSPORT_WIFI);
@@ -311,7 +342,9 @@
@IgnoreUpTo(Build.VERSION_CODES.TIRAMISU)
public void testNetworkCapabilitiesContainTwoSubIds() throws Exception {
final CarrierPrivilegesListenerShim listener = getCarrierPrivilegesListeners().get(0);
- listener.onCarrierServiceChanged(null, mCarrierConfigPkgUid);
+ visibleOnHandlerThread(mCsHandler, () -> {
+ listener.onCarrierServiceChanged(null, mCarrierConfigPkgUid);
+ });
final NetworkCapabilities.Builder ncBuilder = new NetworkCapabilities.Builder();
ncBuilder.addTransportType(TRANSPORT_WIFI);
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 b0fa7ff..3fc57b4 100644
--- a/tests/unit/java/com/android/server/connectivityservice/base/CSTest.kt
+++ b/tests/unit/java/com/android/server/connectivityservice/base/CSTest.kt
@@ -234,7 +234,8 @@
context: Context,
tm: TelephonyManager,
requestRestrictedWifiEnabled: Boolean,
- listener: BiConsumer<Int, Int>
+ listener: BiConsumer<Int, Int>,
+ handler: Handler
) = if (SdkLevel.isAtLeastT()) mock<CarrierPrivilegeAuthenticator>() else null
var satelliteNetworkFallbackUidUpdate: Consumer<Set<Int>>? = null