From 3a983cb95f4027aa6c9dad4401f893a059de2c08 Mon Sep 17 00:00:00 2001 From: Soonil Nagarkar Date: Fri, 23 Oct 2020 17:04:50 -0700 Subject: Make ListenerMultiplexer more flexible Previously ListenerMultiplexer attempted to be perscriptive about when individual listener invocations should be allowed, but this is not proving flexible enough. So, we allow registrations to invoke listeners at any time without being perscriptive about it instead. Test: manual + presubmit Change-Id: I6ff8c6bd4f3e3af15d6ebc4b484b971df5a8c0e2 --- .../java/android/location/ILocationManager.aidl | 5 +- .../java/android/location/LocationListener.java | 11 ++-- .../java/android/location/LocationManager.java | 67 +++++++++++----------- 3 files changed, 41 insertions(+), 42 deletions(-) (limited to 'location/java') diff --git a/location/java/android/location/ILocationManager.aidl b/location/java/android/location/ILocationManager.aidl index 42cf53b44f1e..3905e0b2b878 100644 --- a/location/java/android/location/ILocationManager.aidl +++ b/location/java/android/location/ILocationManager.aidl @@ -52,9 +52,11 @@ interface ILocationManager void registerLocationListener(String provider, in LocationRequest request, in ILocationListener listener, String packageName, String attributionTag, String listenerId); void unregisterLocationListener(in ILocationListener listener); - void registerLocationPendingIntent(String provider, in LocationRequest request, in PendingIntent intent, String packageName, String attributionTag); + void registerLocationPendingIntent(String provider, in LocationRequest request, in PendingIntent pendingIntent, String packageName, String attributionTag); void unregisterLocationPendingIntent(in PendingIntent intent); + void injectLocation(in Location location); + void requestGeofence(in Geofence geofence, in PendingIntent intent, String packageName, String attributionTag); void removeGeofence(in PendingIntent intent); @@ -89,7 +91,6 @@ interface ILocationManager void startGnssBatch(long periodNanos, boolean wakeOnFifoFull, String packageName, String attributionTag); void flushGnssBatch(); void stopGnssBatch(); - void injectLocation(in Location location); List getAllProviders(); List getProviders(in Criteria criteria, boolean enabledOnly); diff --git a/location/java/android/location/LocationListener.java b/location/java/android/location/LocationListener.java index 2738ff4ff38c..0ff0a723237b 100644 --- a/location/java/android/location/LocationListener.java +++ b/location/java/android/location/LocationListener.java @@ -19,12 +19,11 @@ package android.location; import android.annotation.NonNull; import android.os.Bundle; +import java.util.concurrent.Executor; + /** - * Used for receiving notifications from the LocationManager when - * the location has changed. These methods are called if the - * LocationListener has been registered with the location manager service - * using the {@link LocationManager#requestLocationUpdates(String, long, float, LocationListener)} - * method. + * Used for receiving notifications when the device location has changed. These methods are called + * when the listener has been registered with the LocationManager. * *
*

Developer Guides

@@ -32,6 +31,8 @@ import android.os.Bundle; * Obtaining User * Location developer guide.

*
+ * + * @see LocationManager#requestLocationUpdates(String, LocationRequest, Executor, LocationListener) */ public interface LocationListener { diff --git a/location/java/android/location/LocationManager.java b/location/java/android/location/LocationManager.java index ac775ca05cab..3493693ac67e 100644 --- a/location/java/android/location/LocationManager.java +++ b/location/java/android/location/LocationManager.java @@ -216,15 +216,15 @@ public class LocationManager { * Key used for an extra holding a boolean enabled/disabled status value when a provider * enabled/disabled event is broadcast using a PendingIntent. * - * @see #requestLocationUpdates(String, long, float, PendingIntent) + * @see #requestLocationUpdates(String, LocationRequest, PendingIntent) */ public static final String KEY_PROVIDER_ENABLED = "providerEnabled"; /** - * Key used for an extra holding a {@link Location} value when a location change is broadcast - * using a PendingIntent. + * Key used for an extra holding a {@link Location} value when a location change is sent using + * a PendingIntent. * - * @see #requestLocationUpdates(String, long, float, PendingIntent) + * @see #requestLocationUpdates(String, LocationRequest, PendingIntent) */ public static final String KEY_LOCATION_CHANGED = "location"; @@ -1322,27 +1322,26 @@ public class LocationManager { Preconditions.checkArgument(provider != null, "invalid null provider"); Preconditions.checkArgument(locationRequest != null, "invalid null location request"); - synchronized (sLocationListeners) { - WeakReference reference = sLocationListeners.get(listener); - LocationListenerTransport transport = reference != null ? reference.get() : null; - if (transport == null) { - transport = new LocationListenerTransport(listener, executor); - sLocationListeners.put(listener, new WeakReference<>(transport)); - } else { - transport.setExecutor(executor); - } + try { + synchronized (sLocationListeners) { + WeakReference reference = sLocationListeners.get( + listener); + LocationListenerTransport transport = reference != null ? reference.get() : null; + if (transport == null) { + transport = new LocationListenerTransport(listener, executor); + } else { + Preconditions.checkState(transport.isRegistered()); + transport.setExecutor(executor); + } - try { - // making the service call while under lock is less than ideal since LMS must - // make sure that callbacks are not made on the same thread - however it is the - // easiest way to guarantee that clients will not receive callbacks after - // unregistration is complete. mService.registerLocationListener(provider, locationRequest, transport, mContext.getPackageName(), mContext.getAttributionTag(), AppOpsManager.toReceiverId(listener)); - } catch (RemoteException e) { - throw e.rethrowFromSystemServer(); + + sLocationListeners.put(listener, new WeakReference<>(transport)); } + } catch (RemoteException e) { + throw e.rethrowFromSystemServer(); } } @@ -1429,23 +1428,17 @@ public class LocationManager { public void removeUpdates(@NonNull LocationListener listener) { Preconditions.checkArgument(listener != null, "invalid null listener"); - synchronized (sLocationListeners) { - WeakReference reference = sLocationListeners.remove( - listener); - LocationListenerTransport transport = reference != null ? reference.get() : null; - if (transport != null) { - transport.unregister(); - - try { - // making the service call while under lock is less than ideal since LMS must - // make sure that callbacks are not made on the same thread - however it is the - // easiest way to guarantee that clients will not receive callbacks after - // unregistration is complete. + try { + synchronized (sLocationListeners) { + WeakReference ref = sLocationListeners.remove(listener); + LocationListenerTransport transport = ref != null ? ref.get() : null; + if (transport != null) { + transport.unregister(); mService.unregisterLocationListener(transport); - } catch (RemoteException e) { - throw e.rethrowFromSystemServer(); } } + } catch (RemoteException e) { + throw e.rethrowFromSystemServer(); } } @@ -2568,7 +2561,7 @@ public class LocationManager { @Nullable private volatile LocationListener mListener; LocationListenerTransport(LocationListener listener, Executor executor) { - Preconditions.checkArgument(listener != null, "invalid null listener/callback"); + Preconditions.checkArgument(listener != null, "invalid null listener"); mListener = listener; setExecutor(executor); } @@ -2578,6 +2571,10 @@ public class LocationManager { mExecutor = executor; } + boolean isRegistered() { + return mListener != null; + } + void unregister() { mListener = null; } -- cgit v1.2.3-59-g8ed1b