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
This commit is contained in:
Soonil Nagarkar
2020-09-30 13:46:49 -07:00
parent f20b5f0303
commit e963789b7f
5 changed files with 60 additions and 70 deletions

View File

@@ -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.

View File

@@ -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 ");

View File

@@ -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());

View File

@@ -260,7 +260,7 @@ public abstract class GnssListenerMultiplexer<TRequest, TListener extends IInter
CallerIdentity identity = registration.getIdentity();
return registration.isPermitted()
&& (registration.isForeground() || isBackgroundRestrictionExempt(identity))
&& mUserInfoHelper.isCurrentUserId(identity.getUserId())
&& (identity.isSystem() || mUserInfoHelper.isCurrentUserId(identity.getUserId()))
&& mLocationManagerInternal.isProviderEnabledForUser(GPS_PROVIDER,
identity.getUserId())
&& !mSettingsHelper.isLocationPackageBlacklisted(identity.getUserId(),

View File

@@ -202,22 +202,20 @@ public class LocationProviderManagerTest {
@Test
public void testIsEnabled() {
assertThat(mManager.isEnabled(CURRENT_USER)).isTrue();
assertThat(mManager.isEnabled(OTHER_USER)).isTrue();
mInjector.getSettingsHelper().setLocationEnabled(false, CURRENT_USER);
assertThat(mManager.isEnabled(CURRENT_USER)).isFalse();
assertThat(mManager.isEnabled(OTHER_USER)).isTrue();
mInjector.getSettingsHelper().setLocationEnabled(true, CURRENT_USER);
mProvider.setAllowed(false);
assertThat(mManager.isEnabled(CURRENT_USER)).isFalse();
assertThat(mManager.isEnabled(OTHER_USER)).isFalse();
mProvider.setAllowed(true);
mInjector.getUserInfoHelper().setCurrentUserId(OTHER_USER);
assertThat(mManager.isEnabled(CURRENT_USER)).isFalse();
assertThat(mManager.isEnabled(OTHER_USER)).isTrue();
mInjector.getUserInfoHelper().setCurrentUserId(CURRENT_USER);
assertThat(mManager.isEnabled(CURRENT_USER)).isTrue();
assertThat(mManager.isEnabled(OTHER_USER)).isFalse();
assertThat(mManager.isEnabled(OTHER_USER)).isTrue();
}
@Test
@@ -237,23 +235,15 @@ public class LocationProviderManagerTest {
mProvider.setAllowed(false);
verify(listener, timeout(TIMEOUT_MS).times(2)).onProviderEnabledChanged(NAME, CURRENT_USER,
false);
verify(listener, timeout(TIMEOUT_MS).times(1)).onProviderEnabledChanged(NAME, OTHER_USER,
false);
mProvider.setAllowed(true);
verify(listener, timeout(TIMEOUT_MS).times(2)).onProviderEnabledChanged(NAME, CURRENT_USER,
true);
mInjector.getUserInfoHelper().setCurrentUserId(OTHER_USER);
verify(listener, timeout(TIMEOUT_MS).times(3)).onProviderEnabledChanged(NAME, CURRENT_USER,
false);
verify(listener, timeout(TIMEOUT_MS).times(1)).onProviderEnabledChanged(NAME, OTHER_USER,
true);
mInjector.getUserInfoHelper().setCurrentUserId(CURRENT_USER);
verify(listener, timeout(TIMEOUT_MS).times(3)).onProviderEnabledChanged(NAME, CURRENT_USER,
true);
verify(listener, timeout(TIMEOUT_MS).times(1)).onProviderEnabledChanged(NAME, OTHER_USER,
false);
mManager.removeEnabledListener(listener);
mInjector.getSettingsHelper().setLocationEnabled(false, CURRENT_USER);
verifyNoMoreInteractions(listener);
@@ -397,16 +387,6 @@ public class LocationProviderManagerTest {
mProvider.setAllowed(true);
verify(listener, timeout(TIMEOUT_MS).times(2)).onProviderEnabledChanged(NAME, true);
mInjector.getUserInfoHelper().setCurrentUserId(OTHER_USER);
verify(listener, timeout(TIMEOUT_MS).times(3)).onProviderEnabledChanged(NAME, false);
loc = createLocation(NAME, mRandom);
mProvider.setProviderLocation(loc);
verify(listener, times(1)).onLocationChanged(any(Location.class),
nullable(IRemoteCallback.class));
mInjector.getUserInfoHelper().setCurrentUserId(CURRENT_USER);
verify(listener, timeout(TIMEOUT_MS).times(3)).onProviderEnabledChanged(NAME, true);
loc = createLocation(NAME, mRandom);
mProvider.setProviderLocation(loc);
verify(listener, times(2)).onLocationChanged(locationCaptor.capture(),