From e963789b7f8651bda5a6c502d09eb711a22efc36 Mon Sep 17 00:00:00 2001 From: Soonil Nagarkar Date: Wed, 30 Sep 2020 13:46:49 -0700 Subject: [PATCH] Refactor background/system user behavior This is a breaking change so that providers are no longer disabled for non-current (ie background users). Instead, location requests from background users are now considered inactive. This brings provider behavior more in line with other APIs (such as GNSS and geofencing), and allows us to special case requests from the system UID. Without this change, the system UID would see incorrect provider enabled/disabled notifications. Impact on clients is expected to be minimal, as by far the vast majority of android applications run on single user devices. For multiuser devices, applications will no longer see enabled/disabled notifications in response to current user changes. Bug: 157682495 Test: manual + presubmit Change-Id: I0edb15f0792bf1426855baefcda8477ba3a32b28 --- .../util/identity/CallerIdentity.java | 5 ++ .../location/LocationProviderManager.java | 89 ++++++++++--------- .../location/geofence/GeofenceManager.java | 2 +- .../gnss/GnssListenerMultiplexer.java | 2 +- .../location/LocationProviderManagerTest.java | 32 ++----- 5 files changed, 60 insertions(+), 70 deletions(-) diff --git a/location/java/android/location/util/identity/CallerIdentity.java b/location/java/android/location/util/identity/CallerIdentity.java index c32970f1bb04d..e023aa1dcd224 100644 --- a/location/java/android/location/util/identity/CallerIdentity.java +++ b/location/java/android/location/util/identity/CallerIdentity.java @@ -150,6 +150,11 @@ public final class CallerIdentity { return mListenerId; } + /** Returns true if this represents a system identity. */ + public boolean isSystem() { + return mUid == Process.SYSTEM_UID; + } + /** * Adds this identity to the worksource supplied, or if not worksource is supplied, creates a * new worksource representing this identity. diff --git a/services/core/java/com/android/server/location/LocationProviderManager.java b/services/core/java/com/android/server/location/LocationProviderManager.java index cd8bf4a0154d3..016a18e1f7941 100644 --- a/services/core/java/com/android/server/location/LocationProviderManager.java +++ b/services/core/java/com/android/server/location/LocationProviderManager.java @@ -1004,7 +1004,7 @@ class LocationProviderManager extends // if the provider is currently disabled fail immediately int userId = getIdentity().getUserId(); - if (!getRequest().isLocationSettingsIgnored() && !isEnabled(userId)) { + if (!isEnabled(userId)) { deliverLocation(null); } } @@ -1165,7 +1165,7 @@ class LocationProviderManager extends protected final LocationManagerInternal mLocationManagerInternal; protected final SettingsHelper mSettingsHelper; - protected final UserInfoHelper mUserInfoHelper; + protected final UserInfoHelper mUserHelper; protected final AlarmHelper mAlarmHelper; protected final AppOpsHelper mAppOpsHelper; protected final LocationPermissionsHelper mLocationPermissionsHelper; @@ -1227,7 +1227,7 @@ class LocationProviderManager extends mLocationManagerInternal = Objects.requireNonNull( LocalServices.getService(LocationManagerInternal.class)); mSettingsHelper = injector.getSettingsHelper(); - mUserInfoHelper = injector.getUserInfoHelper(); + mUserHelper = injector.getUserInfoHelper(); mAlarmHelper = injector.getAlarmHelper(); mAppOpsHelper = injector.getAppOpsHelper(); mLocationPermissionsHelper = injector.getLocationPermissionsHelper(); @@ -1252,7 +1252,7 @@ class LocationProviderManager extends synchronized (mLock) { mStarted = true; - mUserInfoHelper.addListener(mUserChangedListener); + mUserHelper.addListener(mUserChangedListener); mSettingsHelper.addOnLocationEnabledChangedListener(mLocationEnabledChangedListener); long identity = Binder.clearCallingIdentity(); @@ -1267,20 +1267,19 @@ class LocationProviderManager extends public void stopManager() { synchronized (mLock) { - mUserInfoHelper.removeListener(mUserChangedListener); + mUserHelper.removeListener(mUserChangedListener); mSettingsHelper.removeOnLocationEnabledChangedListener(mLocationEnabledChangedListener); - // notify and remove all listeners + mStarted = false; + long identity = Binder.clearCallingIdentity(); try { - onUserStopped(UserHandle.USER_ALL); + onEnabledChanged(UserHandle.USER_ALL); removeRegistrationIf(key -> true); + mEnabledListeners.clear(); } finally { Binder.restoreCallingIdentity(identity); } - - mEnabledListeners.clear(); - mStarted = false; } } @@ -1430,11 +1429,13 @@ class LocationProviderManager extends identity.getPackageName())) { return null; } - if (!mUserInfoHelper.isCurrentUserId(identity.getUserId())) { - return null; - } - if (!ignoreLocationSettings && !isEnabled(identity.getUserId())) { - return null; + if (!ignoreLocationSettings) { + if (!isEnabled(identity.getUserId())) { + return null; + } + if (!identity.isSystem() && !mUserHelper.isCurrentUserId(identity.getUserId())) { + return null; + } } Location location = getLastLocationUnsafe(identity.getUserId(), permissionLevel, @@ -1462,7 +1463,7 @@ class LocationProviderManager extends if (userId == UserHandle.USER_ALL) { // find the most recent location across all users Location lastLocation = null; - final int[] runningUserIds = mUserInfoHelper.getRunningUserIds(); + final int[] runningUserIds = mUserHelper.getRunningUserIds(); for (int i = 0; i < runningUserIds.length; i++) { Location next = getLastLocationUnsafe(runningUserIds[i], permissionLevel, ignoreLocationSettings, maximumAgeMs); @@ -1507,7 +1508,7 @@ class LocationProviderManager extends private void setLastLocation(Location location, int userId) { if (userId == UserHandle.USER_ALL) { - final int[] runningUserIds = mUserInfoHelper.getRunningUserIds(); + final int[] runningUserIds = mUserHelper.getRunningUserIds(); for (int i = 0; i < runningUserIds.length; i++) { setLastLocation(location, runningUserIds[i]); } @@ -1533,7 +1534,7 @@ class LocationProviderManager extends @Nullable public ICancellationSignal getCurrentLocation(LocationRequest request, - CallerIdentity callerIdentity, int permissionLevel, ILocationCallback callback) { + CallerIdentity identity, int permissionLevel, ILocationCallback callback) { if (request.getDurationMillis() > GET_CURRENT_LOCATION_MAX_TIMEOUT_MS) { request = new LocationRequest.Builder(request) .setDurationMillis(GET_CURRENT_LOCATION_MAX_TIMEOUT_MS) @@ -1543,27 +1544,31 @@ class LocationProviderManager extends GetCurrentLocationListenerRegistration registration = new GetCurrentLocationListenerRegistration( request, - callerIdentity, + identity, new GetCurrentLocationTransport(callback), permissionLevel); synchronized (mLock) { - if (mSettingsHelper.isLocationPackageBlacklisted(callerIdentity.getUserId(), - callerIdentity.getPackageName())) { + // shortcut various failure conditions so that we can return immediately rather than + // waiting for location to timeout + if (mSettingsHelper.isLocationPackageBlacklisted(identity.getUserId(), + identity.getPackageName())) { registration.deliverLocation(null); return null; } - if (!mUserInfoHelper.isCurrentUserId(callerIdentity.getUserId())) { - registration.deliverLocation(null); - return null; - } - if (!request.isLocationSettingsIgnored() && !isEnabled(callerIdentity.getUserId())) { - registration.deliverLocation(null); - return null; + if (!request.isLocationSettingsIgnored()) { + if (!isEnabled(identity.getUserId())) { + registration.deliverLocation(null); + return null; + } + if (!identity.isSystem() && !mUserHelper.isCurrentUserId(identity.getUserId())) { + registration.deliverLocation(null); + return null; + } } Location lastLocation = getLastLocationUnsafe( - callerIdentity.getUserId(), + identity.getUserId(), permissionLevel, request.isLocationSettingsIgnored(), MAX_CURRENT_LOCATION_AGE_MS); @@ -1573,11 +1578,11 @@ class LocationProviderManager extends } // if last location isn't good enough then we add a location request - long identity = Binder.clearCallingIdentity(); + long ident = Binder.clearCallingIdentity(); try { addRegistration(callback.asBinder(), registration); } finally { - Binder.restoreCallingIdentity(identity); + Binder.restoreCallingIdentity(ident); } } @@ -1601,20 +1606,20 @@ class LocationProviderManager extends } } - public void registerLocationRequest(LocationRequest request, CallerIdentity callerIdentity, + public void registerLocationRequest(LocationRequest request, CallerIdentity identity, @PermissionLevel int permissionLevel, ILocationListener listener) { synchronized (mLock) { - long identity = Binder.clearCallingIdentity(); + long ident = Binder.clearCallingIdentity(); try { addRegistration( listener.asBinder(), new LocationListenerRegistration( request, - callerIdentity, + identity, new LocationListenerTransport(listener), permissionLevel)); } finally { - Binder.restoreCallingIdentity(identity); + Binder.restoreCallingIdentity(ident); } } } @@ -1845,6 +1850,9 @@ class LocationProviderManager extends if (!isEnabled(identity.getUserId())) { return false; } + if (!identity.isSystem() && !mUserHelper.isCurrentUserId(identity.getUserId())) { + return false; + } switch (mLocationPowerSaveModeHelper.getLocationPowerSaveMode()) { case LOCATION_MODE_FOREGROUND_ONLY: @@ -1973,7 +1981,8 @@ class LocationProviderManager extends synchronized (mLock) { switch (change) { case UserListener.CURRENT_USER_CHANGED: - onEnabledChanged(userId); + updateRegistrations( + registration -> registration.getIdentity().getUserId() == userId); break; case UserListener.USER_STARTED: onUserStarted(userId); @@ -2149,13 +2158,10 @@ class LocationProviderManager extends } if (userId == UserHandle.USER_ALL) { - onEnabledChanged(UserHandle.USER_ALL); mEnabled.clear(); mLastLocations.clear(); } else { Preconditions.checkArgument(userId >= 0); - - onEnabledChanged(userId); mEnabled.delete(userId); mLastLocations.remove(userId); } @@ -2172,7 +2178,7 @@ class LocationProviderManager extends // settings for instance) do not support the null user return; } else if (userId == UserHandle.USER_ALL) { - final int[] runningUserIds = mUserInfoHelper.getRunningUserIds(); + final int[] runningUserIds = mUserHelper.getRunningUserIds(); for (int i = 0; i < runningUserIds.length; i++) { onEnabledChanged(runningUserIds[i]); } @@ -2183,7 +2189,6 @@ class LocationProviderManager extends boolean enabled = mStarted && mProvider.getState().allowed - && mUserInfoHelper.isCurrentUserId(userId) && mSettingsHelper.isLocationEnabled(userId); int index = mEnabled.indexOfKey(userId); @@ -2247,7 +2252,7 @@ class LocationProviderManager extends super.dump(fd, ipw, args); - int[] userIds = mUserInfoHelper.getRunningUserIds(); + int[] userIds = mUserHelper.getRunningUserIds(); for (int userId : userIds) { if (userIds.length != 1) { ipw.print("user "); diff --git a/services/core/java/com/android/server/location/geofence/GeofenceManager.java b/services/core/java/com/android/server/location/geofence/GeofenceManager.java index 58e6d593ed2d3..baa01b1506afd 100644 --- a/services/core/java/com/android/server/location/geofence/GeofenceManager.java +++ b/services/core/java/com/android/server/location/geofence/GeofenceManager.java @@ -327,7 +327,7 @@ public class GeofenceManager extends protected boolean isActive(GeofenceRegistration registration) { CallerIdentity identity = registration.getIdentity(); return registration.isPermitted() - && mUserInfoHelper.isCurrentUserId(identity.getUserId()) + && (identity.isSystem() || mUserInfoHelper.isCurrentUserId(identity.getUserId())) && mSettingsHelper.isLocationEnabled(identity.getUserId()) && !mSettingsHelper.isLocationPackageBlacklisted(identity.getUserId(), identity.getPackageName()); diff --git a/services/core/java/com/android/server/location/gnss/GnssListenerMultiplexer.java b/services/core/java/com/android/server/location/gnss/GnssListenerMultiplexer.java index a830e2fe79739..73e6b8211cd71 100644 --- a/services/core/java/com/android/server/location/gnss/GnssListenerMultiplexer.java +++ b/services/core/java/com/android/server/location/gnss/GnssListenerMultiplexer.java @@ -260,7 +260,7 @@ public abstract class GnssListenerMultiplexer