Fix cache by userId bugs
Three bugs were present here:
1) isLocationEnabled() will return incorrect results for USER_CURRENT
and USER_CURRENT_OR_SELF
2) Disabling the location enabled cache does not work properly in the
system process, we only disable the cache for a single instance of
LocationManager.
3) isLocationEnabled() was not respecting the user as defined by the
context used to create the LocationManager.
Bug: 185401689
Test: atest CtsLocationFineTestCases
Change-Id: I84bff02604a4b2d84494a0e099e172c4ffb400a2
diff --git a/location/java/android/location/LocationManager.java b/location/java/android/location/LocationManager.java
index c1d6725..5952479 100644
--- a/location/java/android/location/LocationManager.java
+++ b/location/java/android/location/LocationManager.java
@@ -360,6 +360,9 @@
}
}
+ private static volatile LocationEnabledCache sLocationEnabledCache =
+ new LocationEnabledCache(4);
+
@GuardedBy("sLocationListeners")
private static final WeakHashMap<LocationListener, WeakReference<LocationListenerTransport>>
sLocationListeners = new WeakHashMap<>();
@@ -386,20 +389,6 @@
final Context mContext;
final ILocationManager mService;
- private volatile PropertyInvalidatedCache<Integer, Boolean> mLocationEnabledCache =
- new PropertyInvalidatedCache<Integer, Boolean>(
- 4,
- CACHE_KEY_LOCATION_ENABLED_PROPERTY) {
- @Override
- protected Boolean recompute(Integer userHandle) {
- try {
- return mService.isLocationEnabledForUser(userHandle);
- } catch (RemoteException e) {
- throw e.rethrowFromSystemServer();
- }
- }
- };
-
/**
* @hide
*/
@@ -533,7 +522,7 @@
* @return true if location is enabled and false if location is disabled.
*/
public boolean isLocationEnabled() {
- return isLocationEnabledForUser(Process.myUserHandle());
+ return isLocationEnabledForUser(mContext.getUser());
}
/**
@@ -546,12 +535,17 @@
*/
@SystemApi
public boolean isLocationEnabledForUser(@NonNull UserHandle userHandle) {
- PropertyInvalidatedCache<Integer, Boolean> cache = mLocationEnabledCache;
- if (cache != null) {
- return cache.query(userHandle.getIdentifier());
+ // skip the cache for any "special" user ids - special ids like CURRENT_USER may change
+ // their meaning over time and should never be in the cache. we could resolve the special
+ // user ids here, but that would require an x-process call anyways, and the whole point of
+ // the cache is to avoid x-process calls.
+ if (userHandle.getIdentifier() >= 0) {
+ PropertyInvalidatedCache<Integer, Boolean> cache = sLocationEnabledCache;
+ if (cache != null) {
+ return cache.query(userHandle.getIdentifier());
+ }
}
- // fallback if cache is disabled
try {
return mService.isLocationEnabledForUser(userHandle.getIdentifier());
} catch (RemoteException e) {
@@ -3004,7 +2998,7 @@
ListenerExecutor, CancellationSignal.OnCancelListener {
private final Executor mExecutor;
- private volatile @Nullable Consumer<Location> mConsumer;
+ volatile @Nullable Consumer<Location> mConsumer;
GetCurrentLocationTransport(Executor executor, Consumer<Location> consumer,
@Nullable CancellationSignal cancellationSignal) {
@@ -3465,6 +3459,37 @@
}
}
+ private static class LocationEnabledCache extends PropertyInvalidatedCache<Integer, Boolean> {
+
+ // this is not loaded immediately because this class is created as soon as LocationManager
+ // is referenced for the first time, and within the system server, the ILocationManager
+ // service may not have been loaded yet at that time.
+ private @Nullable ILocationManager mManager;
+
+ LocationEnabledCache(int numEntries) {
+ super(numEntries, CACHE_KEY_LOCATION_ENABLED_PROPERTY);
+ }
+
+ @Override
+ protected Boolean recompute(Integer userId) {
+ Preconditions.checkArgument(userId >= 0);
+
+ if (mManager == null) {
+ try {
+ mManager = getService();
+ } catch (RemoteException e) {
+ e.rethrowFromSystemServer();
+ }
+ }
+
+ try {
+ return mManager.isLocationEnabledForUser(userId);
+ } catch (RemoteException e) {
+ throw e.rethrowFromSystemServer();
+ }
+ }
+ }
+
/**
* @hide
*/
@@ -3475,7 +3500,7 @@
/**
* @hide
*/
- public void disableLocalLocationEnabledCaches() {
- mLocationEnabledCache = null;
+ public static void disableLocalLocationEnabledCaches() {
+ sLocationEnabledCache = null;
}
}
diff --git a/services/core/java/com/android/server/location/LocationManagerService.java b/services/core/java/com/android/server/location/LocationManagerService.java
index 864aa33..172a68a 100644
--- a/services/core/java/com/android/server/location/LocationManagerService.java
+++ b/services/core/java/com/android/server/location/LocationManagerService.java
@@ -171,8 +171,7 @@
// client caching behavior is only enabled after seeing the first invalidate
LocationManager.invalidateLocalLocationEnabledCaches();
// disable caching for our own process
- Objects.requireNonNull(getContext().getSystemService(LocationManager.class))
- .disableLocalLocationEnabledCaches();
+ LocationManager.disableLocalLocationEnabledCaches();
}
@Override