Merge "Make ListenerMultiplexer more flexible"

3 files changed, 41 insertions, 42 deletions

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<String> getAllProviders();
     List<String> 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.
  *
  * <div class="special reference">
  * <h3>Developer Guides</h3>
@@ -32,6 +31,8 @@ import android.os.Bundle;
  * <a href="{@docRoot}guide/topics/location/obtaining-user-location.html">Obtaining User
  * Location</a> developer guide.</p>
  * </div>
+ *
+ * @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<LocationListenerTransport> 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<LocationListenerTransport> 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<LocationListenerTransport> 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<LocationListenerTransport> 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;
         }