Merge "Fix cache by userId bugs" into sc-dev

location/java/android/location/LocationManager.java

@@ -360,6 +360,9 @@ public class LocationManager {
         }
     }
 
+    private static volatile LocationEnabledCache sLocationEnabledCache =
+            new LocationEnabledCache(4);
+
     @GuardedBy("sLocationListeners")
     private static final WeakHashMap<LocationListener, WeakReference<LocationListenerTransport>>
             sLocationListeners = new WeakHashMap<>();
@@ -386,20 +389,6 @@ public class LocationManager {
     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 @@ public class LocationManager {
      * @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 @@ public class LocationManager {
      */
     @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 @@ public class LocationManager {
             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 @@ public class LocationManager {
         }
     }
 
+    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 @@ public class LocationManager {
     /**
      * @hide
      */
-    public void disableLocalLocationEnabledCaches() {
-        mLocationEnabledCache = null;
+    public static void disableLocalLocationEnabledCaches() {
+        sLocationEnabledCache = null;
     }
 }

services/core/java/com/android/server/location/LocationManagerService.java

@@ -171,8 +171,7 @@ public class LocationManagerService extends ILocationManager.Stub {
             // 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