From aad92674934a33ab735da915fd85958f525fd41d Mon Sep 17 00:00:00 2001 From: Lorenzo Colitti Date: Thu, 22 May 2025 16:09:58 +0900 Subject: [PATCH 1/2] Support multi-user SettingsObservers in ConnectivityService ConnectivityService uses SettingsObservers to detect when settings change. These are all set on user zero. This is fine for most settings, which are in Settings.Global, but some settings are per-user, such as MOBILE_DATA_PREFERRED_UIDS, which is in Settings.Secure. This CL adds the ability to register SettingsObservers on all users. These observers will fire any time the setting changes for any current user, and any time a user is added or removed. This CL does not add or change any SettingsObservers and should cause no behaviour changes. Bug: 404186833 Flag: EXEMPT refactor Test: covered by existing tests Change-Id: I5954b196eea776414098f1ce4441eb02065438ef --- .../android/server/ConnectivityService.java | 77 +++++++++++++++++-- .../server/ConnectivityServiceTest.java | 7 ++ .../server/connectivityservice/base/CSTest.kt | 4 + 3 files changed, 82 insertions(+), 6 deletions(-) diff --git a/service/src/com/android/server/ConnectivityService.java b/service/src/com/android/server/ConnectivityService.java index 633ab980c3..9acfb7c348 100644 --- a/service/src/com/android/server/ConnectivityService.java +++ b/service/src/com/android/server/ConnectivityService.java @@ -1596,6 +1596,11 @@ public class ConnectivityService extends IConnectivityManager.Stub cr.registerContentObserver(uri, notifyForDescendants, observer); } + public void registerContentObserverAsUser(ContentResolver cr, Uri uri, + boolean notifyForDescendants, ContentObserver observer, UserHandle userHandle) { + cr.registerContentObserverAsUser(uri, notifyForDescendants, observer, userHandle); + } + /** * Get a reference to the ModuleNetworkStackClient. */ @@ -7667,11 +7672,20 @@ public class ConnectivityService extends IConnectivityManager.Stub } private static class SettingsObserver extends ContentObserver { - private final HashMap mUriEventMap; + private final HashMap mUriEventMap; private final Context mContext; private final Handler mHandler; private final Dependencies mDeps; + private static class SettingInfo { + final int what; + final boolean forAllUsers; + SettingInfo(int what, boolean forAllUsers) { + this.what = what; + this.forAllUsers = forAllUsers; + } + } + SettingsObserver(Context context, Handler handler, Dependencies deps) { super(null); mUriEventMap = new HashMap<>(); @@ -7680,9 +7694,50 @@ public class ConnectivityService extends IConnectivityManager.Stub mDeps = deps; } + /** + * Observe settings changes for the specified URI on the current user. + * + * Sends an empty message with the specified {@code what} whenever the setting changes on + * the current user. + * + * @param uri The URI to observe. + * @param what The empty message to send whenever the setting changes. + */ void observe(Uri uri, int what) { - mUriEventMap.put(uri, what); - mDeps.registerContentObserver(mContext.getContentResolver(), uri, false, this); + mUriEventMap.put(uri, new SettingInfo(what, false /* forAllUsers */)); + final ContentResolver resolver = mContext.getContentResolver(); + mDeps.registerContentObserver(resolver, uri, false, this); + } + + /** + * Observe settings changes for the specified URI on all users. + * + * Sends an empty message with the specified {@code what} whenever the setting changes on + * any user, or whenever a user is added or removed. + * + * @param uri The URI to observe. + * @param what The empty message to send whenever the setting changes. + */ + void observeForAllUsers(Uri uri, int what) { + if (!SdkLevel.isAtLeastT()) { + observe(uri, what); + return; + } + mUriEventMap.put(uri, new SettingInfo(what, true /* forAllUsers */)); + final ContentResolver resolver = mContext.getContentResolver(); + mDeps.registerContentObserverAsUser(resolver, uri, false, this, UserHandle.ALL); + } + + void onUsersChanged() { + for (SettingInfo si : mUriEventMap.values()) { + if (si.forAllUsers) { + sendMessage(si.what); + } + } + } + + private void sendMessage(int what) { + mHandler.obtainMessage(what).sendToTarget(); } @Override @@ -7692,13 +7747,21 @@ public class ConnectivityService extends IConnectivityManager.Stub @Override public void onChange(boolean selfChange, Uri uri) { - final Integer what = mUriEventMap.get(uri); - if (what != null) { - mHandler.obtainMessage(what).sendToTarget(); + final SettingInfo si = mUriEventMap.get(uri); + if (si != null) { + sendMessage(si.what); } else { loge("No matching event to send for URI=" + uri); } } + + @Override + public void onChange(boolean selfChange, @NonNull Collection uris, + int flags, UserHandle unused) { + for (Uri uri : uris) { + onChange(selfChange, uri); + } + } } private static void log(String s) { @@ -7998,6 +8061,7 @@ public class ConnectivityService extends IConnectivityManager.Stub if (mSatelliteAccessController != null) { mSatelliteAccessController.onUserAddedWithInstalledPackageList(user, apps); } + mSettingsObserver.onUsersChanged(); } @Override @@ -8016,6 +8080,7 @@ public class ConnectivityService extends IConnectivityManager.Stub if (mSatelliteAccessController != null) { mSatelliteAccessController.onUserRemoved(user); } + mSettingsObserver.onUsersChanged(); } @Override diff --git a/tests/unit/java/com/android/server/ConnectivityServiceTest.java b/tests/unit/java/com/android/server/ConnectivityServiceTest.java index 402c4afa29..6bd4bea963 100644 --- a/tests/unit/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/unit/java/com/android/server/ConnectivityServiceTest.java @@ -2036,6 +2036,13 @@ public class ConnectivityServiceTest { .registerContentObserver(uri, observer); } + @Override + public void registerContentObserverAsUser(ContentResolver cr, Uri uri, + boolean notifyForDescendants, ContentObserver observer, UserHandle userHandle) { + ((ContentResolverWithFakeSettingsProvider) mServiceContext.getContentResolver()) + .registerContentObserverAsUser(uri, observer, userHandle); + } + @Override public NetworkStackClientBase getNetworkStack() { return mNetworkStack; 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 84f72a2151..5fa720c310 100644 --- a/tests/unit/java/com/android/server/connectivityservice/base/CSTest.kt +++ b/tests/unit/java/com/android/server/connectivityservice/base/CSTest.kt @@ -357,6 +357,10 @@ open class CSTest { ) = (cr as ContentResolverWithFakeSettingsProvider).registerContentObserver(uri, observer) + override fun registerContentObserverAsUser(cr: ContentResolver, uri: Uri, + notifyForDescendants: Boolean, observer: ContentObserver, userHandle: UserHandle) = + context.getContentResolver().registerContentObserverAsUser(uri, observer, userHandle) + override fun makeCarrierPrivilegeAuthenticator( context: Context, tm: TelephonyManager, From 2139fcf34542f7e88de0c07d5e55297c87df5a94 Mon Sep 17 00:00:00 2001 From: Lorenzo Colitti Date: Fri, 6 Jun 2025 18:15:54 +0900 Subject: [PATCH 2/2] Fix mobile data preferred on multi-user devices. The mobile data preferred feature stores the list of UIDs in Settings.Secure, which is per-user. But the code only sets, reads and observes the setting for the current user instead of for all users. This does not work on HSUM or automotive. Fix this as follows: - Register the ContentObserver on UserHandle.ALL - Any time the observer fires, read the setting on all current users, and operate on the merged result. - Ensure that the code gets called on ACTION_USER_ADDED and ACTION_USER_REMOVED. Because the code now aggregates the setting for all UIDs, it should just work on single-user devices. It should also work if there are OEM devices that store all UIDs (for all users) in Settings.Secure for user 0. Because the bug is fixed, re-enable the test on automotive. Also fix style nits. Fix: 404186833 Flag: EXEMPT bug fix Test: covered by existing CTS tests Change-Id: I69ad789f1c7c7f3bab04ab8789dfb93ee0de553d --- .../com/android/server/ConnectivityService.java | 15 ++++++++++++--- .../android/net/cts/ConnectivityManagerTest.java | 4 ---- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/service/src/com/android/server/ConnectivityService.java b/service/src/com/android/server/ConnectivityService.java index 9acfb7c348..e9dc6b9768 100644 --- a/service/src/com/android/server/ConnectivityService.java +++ b/service/src/com/android/server/ConnectivityService.java @@ -2516,7 +2516,7 @@ public class ConnectivityService extends IConnectivityManager.Stub EVENT_CONFIGURE_ALWAYS_ON_NETWORKS); // Watch for mobile data preferred uids changes. - mSettingsObserver.observe( + mSettingsObserver.observeForAllUsers( Settings.Secure.getUriFor(ConnectivitySettingsManager.MOBILE_DATA_PREFERRED_UIDS), EVENT_MOBILE_DATA_PREFERRED_UIDS_CHANGED); @@ -4388,7 +4388,7 @@ public class ConnectivityService extends IConnectivityManager.Stub // Note that updating can be skipped here if the list is empty only because no uid // rules are applied before system ready. Normally, the empty uid list means to clear // the uids rules on netd. - if (!ConnectivitySettingsManager.getMobileDataPreferredUids(mContext).isEmpty()) { + if (!getMobileDataPreferredUids().isEmpty()) { mHandler.sendEmptyMessage(EVENT_MOBILE_DATA_PREFERRED_UIDS_CHANGED); } @@ -14971,8 +14971,17 @@ public class ConnectivityService extends IConnectivityManager.Stub return requests; } + private Set getMobileDataPreferredUids() { + Set uids = new ArraySet<>(); + for (UserHandle userHandle : mUserManager.getUserHandles(/* excludeDying= */ true)) { + final Context userContext = mContext.createContextAsUser(userHandle, 0 /* flags */); + uids.addAll(ConnectivitySettingsManager.getMobileDataPreferredUids(userContext)); + } + return uids; + } + private void handleMobileDataPreferredUidsChanged() { - mMobileDataPreferredUids = ConnectivitySettingsManager.getMobileDataPreferredUids(mContext); + mMobileDataPreferredUids = getMobileDataPreferredUids(); removeDefaultNetworkRequestsForPreference(PREFERENCE_ORDER_MOBILE_DATA_PREFERERRED); addPerAppDefaultNetworkRequests( createNrisFromMobileDataPreferredUids(mMobileDataPreferredUids)); diff --git a/tests/cts/net/src/android/net/cts/ConnectivityManagerTest.java b/tests/cts/net/src/android/net/cts/ConnectivityManagerTest.java index e25eddb63d..6d5cc5f6f2 100644 --- a/tests/cts/net/src/android/net/cts/ConnectivityManagerTest.java +++ b/tests/cts/net/src/android/net/cts/ConnectivityManagerTest.java @@ -28,7 +28,6 @@ import static android.Manifest.permission.NETWORK_SETUP_WIZARD; import static android.Manifest.permission.NETWORK_STACK; import static android.Manifest.permission.READ_DEVICE_CONFIG; import static android.Manifest.permission.TETHER_PRIVILEGED; -import static android.content.pm.PackageManager.FEATURE_AUTOMOTIVE; import static android.content.pm.PackageManager.FEATURE_BLUETOOTH; import static android.content.pm.PackageManager.FEATURE_ETHERNET; import static android.content.pm.PackageManager.FEATURE_TELEPHONY; @@ -3416,9 +3415,6 @@ public class ConnectivityManagerTest { && mPackageManager.hasSystemFeature(FEATURE_TELEPHONY); assumeTrue("testMobileDataPreferredUidsWithCallback cannot execute" + " unless device supports both WiFi and telephony", canRunTest); - // TODO(b/404186833): re-enable - assumeFalse("testMobileDataPreferredUids is broken on automotive", - mPackageManager.hasSystemFeature(FEATURE_AUTOMOTIVE)); final int uid = mPackageManager.getPackageUid(mContext.getPackageName(), 0 /* flag */); final Set mobileDataPreferredUids =