Update batching APIs

After API council feedback, use List<Location> in public APIs rather
than LocationResult.

Bug: 173712888
Test: atest CtsLocationFineTestCases
Change-Id: I02caa1f9c7164b52ef55c71ca14fb2bd7e311a85

7 files changed, 134 insertions, 68 deletions

diff --git a/location/java/android/location/ILocationListener.aidl b/location/java/android/location/ILocationListener.aidl
index ce92661cb87c..40d8615b95f7 100644
--- a/location/java/android/location/ILocationListener.aidl
+++ b/location/java/android/location/ILocationListener.aidl
@@ -16,7 +16,7 @@
 
 package android.location;
 
-import android.location.LocationResult;
+import android.location.Location;
 import android.os.IRemoteCallback;
 
 /**
@@ -24,7 +24,7 @@ import android.os.IRemoteCallback;
  */
 oneway interface ILocationListener
 {
-    void onLocationChanged(in LocationResult locationResult, in @nullable IRemoteCallback onCompleteCallback);
+    void onLocationChanged(in List<Location> locations, in @nullable IRemoteCallback onCompleteCallback);
     void onProviderEnabledChanged(String provider, boolean enabled);
     void onFlushComplete(int requestCode);
 }
diff --git a/location/java/android/location/LocationListener.java b/location/java/android/location/LocationListener.java
index 523117b39762..35a40910e373 100644
--- a/location/java/android/location/LocationListener.java
+++ b/location/java/android/location/LocationListener.java
@@ -19,6 +19,7 @@ package android.location;
 import android.annotation.NonNull;
 import android.os.Bundle;
 
+import java.util.List;
 import java.util.concurrent.Executor;
 
 /**
@@ -48,15 +49,18 @@ public interface LocationListener {
     /**
      * Called when the location has changed and locations are being delivered in batches. The
      * default implementation calls through to ({@link #onLocationChanged(Location)} with all
-     * locations in the batch, from earliest to latest.
+     * locations in the batch. The list of locations is always guaranteed to be non-empty, and is
+     * always guaranteed to be ordered from earliest location to latest location (so that the
+     * earliest location in the batch is at index 0 in the list, and the latest location in the
+     * batch is at index size-1 in the list).
      *
      * @see LocationRequest#getMaxUpdateDelayMillis()
-     * @param locationResult the location result list
+     * @param locations the location list
      */
-    default void onLocationChanged(@NonNull LocationResult locationResult) {
-        final int size = locationResult.size();
-        for (int i = 0; i < size; ++i) {
-            onLocationChanged(locationResult.get(i));
+    default void onLocationChanged(@NonNull List<Location> locations) {
+        final int size = locations.size();
+        for (int i = 0; i < size; i++) {
+            onLocationChanged(locations.get(i));
         }
     }
 
diff --git a/location/java/android/location/LocationManager.java b/location/java/android/location/LocationManager.java
index fdb044d0dcf6..add3d31e9435 100644
--- a/location/java/android/location/LocationManager.java
+++ b/location/java/android/location/LocationManager.java
@@ -231,19 +231,26 @@ public class LocationManager {
 
     /**
      * Key used for an extra holding a {@link Location} value when a location change is sent using
-     * a PendingIntent.
+     * a PendingIntent. If the location change includes a list of batched locations via
+     * {@link #KEY_LOCATIONS} then this key will still be present, and will hold the last location
+     * in the batch. Use {@link Intent#getParcelableExtra(String)} to retrieve the location.
      *
      * @see #requestLocationUpdates(String, LocationRequest, PendingIntent)
      */
     public static final String KEY_LOCATION_CHANGED = "location";
 
     /**
-     * Key used for an extra holding a {@link LocationResult} value when a location change is sent
-     * using a PendingIntent.
+     * Key used for an extra holding a array of {@link Location}s when a location change is sent
+     * using a PendingIntent. This key will only be present if the location change includes
+     * multiple (ie, batched) locations, otherwise only {@link #KEY_LOCATION_CHANGED} will be
+     * present. Use {@link Intent#getParcelableArrayExtra(String)} to retrieve the locations.
+     *
+     * <p>The array of locations will never be empty, and will ordered from earliest location to
+     * latest location, the same as with {@link LocationListener#onLocationChanged(List)}.
      *
      * @see #requestLocationUpdates(String, LocationRequest, PendingIntent)
      */
-    public static final String KEY_LOCATION_RESULT = "locationResult";
+    public static final String KEY_LOCATIONS = "locations";
 
     /**
      * Key used for an extra holding an integer request code when location flush completion is sent
@@ -2969,12 +2976,12 @@ public class LocationManager {
         }
 
         @Override
-        public void onLocationChanged(LocationResult locationResult,
+        public void onLocationChanged(List<Location> locations,
                 @Nullable IRemoteCallback onCompleteCallback) {
             executeSafely(mExecutor, () -> mListener, new ListenerOperation<LocationListener>() {
                 @Override
                 public void operate(LocationListener listener) {
-                    listener.onLocationChanged(locationResult);
+                    listener.onLocationChanged(locations);
                 }
 
                 @Override
@@ -3321,8 +3328,8 @@ public class LocationManager {
         }
 
         @Override
-        public void onLocationChanged(@NonNull LocationResult locationResult) {
-            mCallback.onLocationBatch(locationResult.asList());
+        public void onLocationChanged(@NonNull List<Location> locations) {
+            mCallback.onLocationBatch(locations);
         }
     }
 
diff --git a/location/java/android/location/LocationRequest.java b/location/java/android/location/LocationRequest.java
index 323e740ee8e3..cb56ee5318ee 100644
--- a/location/java/android/location/LocationRequest.java
+++ b/location/java/android/location/LocationRequest.java
@@ -605,7 +605,7 @@ public final class LocationRequest implements Parcelable {
      * When available, batching can provide substantial power savings to the device, and clients are
      * encouraged to take advantage where appropriate for the use case.
      *
-     * @see LocationListener#onLocationChanged(LocationResult)
+     * @see LocationListener#onLocationChanged(java.util.List)
      * @return the maximum time by which a location update may be delayed
      */
     public @IntRange(from = 0) long getMaxUpdateDelayMillis() {
diff --git a/location/java/android/location/LocationResult.java b/location/java/android/location/LocationResult.java
index 79a000c23484..8423000b6276 100644
--- a/location/java/android/location/LocationResult.java
+++ b/location/java/android/location/LocationResult.java
@@ -19,7 +19,6 @@ package android.location;
 import android.annotation.IntRange;
 import android.annotation.NonNull;
 import android.annotation.Nullable;
-import android.annotation.SystemApi;
 import android.os.Parcel;
 import android.os.Parcelable;
 
@@ -34,25 +33,31 @@ import java.util.function.Predicate;
 
 /**
  * A location result representing a list of locations, ordered from earliest to latest.
+ *
+ * @hide
  */
 public final class LocationResult implements Parcelable {
 
     /**
-     * Creates a new LocationResult from the given location.
+     * Creates a new LocationResult from the given locations, making a copy of each location.
+     * Locations must be ordered in the same order they were derived (earliest to latest).
      */
-    public static @NonNull LocationResult create(@NonNull Location location) {
-        ArrayList<Location> locations = new ArrayList<>(1);
-        locations.add(new Location(Objects.requireNonNull(location)));
-        return new LocationResult(locations);
+    public static @NonNull LocationResult create(@NonNull List<Location> locations) {
+        Preconditions.checkArgument(!locations.isEmpty());
+        ArrayList<Location> locationsCopy = new ArrayList<>(locations.size());
+        for (Location location : locations) {
+            locationsCopy.add(new Location(Objects.requireNonNull(location)));
+        }
+        return new LocationResult(locationsCopy);
     }
 
     /**
-     * Creates a new LocationResult from the given locations. Locations must be ordered in the same
-     * order they were derived (earliest to latest).
+     * Creates a new LocationResult from the given locations, making a copy of each location.
+     * Locations must be ordered in the same order they were derived (earliest to latest).
      */
-    public static @NonNull LocationResult create(@NonNull List<Location> locations) {
-        Preconditions.checkArgument(!locations.isEmpty());
-        ArrayList<Location> locationsCopy = new ArrayList<>(locations.size());
+    public static @NonNull LocationResult create(@NonNull Location... locations) {
+        Preconditions.checkArgument(locations.length > 0);
+        ArrayList<Location> locationsCopy = new ArrayList<>(locations.length);
         for (Location location : locations) {
             locationsCopy.add(new Location(Objects.requireNonNull(location)));
         }
@@ -60,16 +65,27 @@ public final class LocationResult implements Parcelable {
     }
 
     /**
-     * Creates a new LocationResult that takes ownership of the given location without copying it.
-     * Callers must ensure the given location is never mutated after this method is called.
-     *
-     * @hide
+     * Creates a new LocationResult that takes ownership of the given locations without copying
+     * them. Callers must ensure the given locations are never mutated after this method is called.
+     * Locations must be ordered in the same order they were derived (earliest to latest).
+     */
+    public static @NonNull LocationResult wrap(@NonNull List<Location> locations) {
+        Preconditions.checkArgument(!locations.isEmpty());
+        return new LocationResult(new ArrayList<>(locations));
+    }
+
+    /**
+     * Creates a new LocationResult that takes ownership of the given locations without copying
+     * them. Callers must ensure the given locations are never mutated after this method is called.
+     * Locations must be ordered in the same order they were derived (earliest to latest).
      */
-    @SystemApi
-    public static @NonNull LocationResult wrap(@NonNull Location location) {
-        ArrayList<Location> locations = new ArrayList<>(1);
-        locations.add(Objects.requireNonNull(location));
-        return new LocationResult(locations);
+    public static @NonNull LocationResult wrap(@NonNull Location... locations) {
+        Preconditions.checkArgument(locations.length > 0);
+        ArrayList<Location> newLocations = new ArrayList<>(locations.length);
+        for (Location location : locations) {
+            newLocations.add(Objects.requireNonNull(location));
+        }
+        return new LocationResult(newLocations);
     }
 
     private final ArrayList<Location> mLocations;
@@ -112,7 +128,7 @@ public final class LocationResult implements Parcelable {
     }
 
     /**
-     * Returns the numer of locations in this location result.
+     * Returns the number of locations in this location result.
      */
     public @IntRange(from = 1) int size() {
         return mLocations.size();
@@ -139,9 +155,9 @@ public final class LocationResult implements Parcelable {
      * @hide
      */
     public @NonNull LocationResult deepCopy() {
-        ArrayList<Location> copy = new ArrayList<>(mLocations.size());
         final int size = mLocations.size();
-        for (int i = 0; i < size; ++i) {
+        ArrayList<Location> copy = new ArrayList<>(size);
+        for (int i = 0; i < size; i++) {
             copy.add(new Location(mLocations.get(i)));
         }
         return new LocationResult(copy);
@@ -164,7 +180,7 @@ public final class LocationResult implements Parcelable {
      * Returns a LocationResult with only locations that pass the given predicate. This
      * implementation will avoid allocations when no locations are filtered out. The predicate is
      * guaranteed to be invoked once per location, in order from earliest to latest. If all
-     * locations are filtered out a null value is returned instead of an empty LocationResult.
+     * locations are filtered out a null value is returned.
      *
      * @hide
      */
diff --git a/location/java/android/location/provider/ILocationProviderManager.aidl b/location/java/android/location/provider/ILocationProviderManager.aidl
index e3f51d9a23e1..50ed046f09cd 100644
--- a/location/java/android/location/provider/ILocationProviderManager.aidl
+++ b/location/java/android/location/provider/ILocationProviderManager.aidl
@@ -16,7 +16,7 @@
 
 package android.location.provider;
 
-import android.location.LocationResult;
+import android.location.Location;
 import android.location.provider.ProviderProperties;
 
 /**
@@ -28,6 +28,7 @@ interface ILocationProviderManager {
     void onSetAllowed(boolean allowed);
     void onSetProperties(in ProviderProperties properties);
 
-    void onReportLocation(in LocationResult locationResult);
+    void onReportLocation(in Location location);
+    void onReportLocations(in List<Location> locations);
     void onFlushComplete();
 }
diff --git a/location/java/android/location/provider/LocationProviderBase.java b/location/java/android/location/provider/LocationProviderBase.java
index 8f455cd7df07..ae6395d5d12d 100644
--- a/location/java/android/location/provider/LocationProviderBase.java
+++ b/location/java/android/location/provider/LocationProviderBase.java
@@ -25,12 +25,13 @@ import android.annotation.SystemApi;
 import android.content.Context;
 import android.content.Intent;
 import android.location.Location;
-import android.location.LocationResult;
 import android.os.Bundle;
 import android.os.IBinder;
 import android.os.RemoteException;
 import android.util.Log;
 
+import java.util.ArrayList;
+import java.util.List;
 import java.util.Objects;
 
 /**
@@ -200,35 +201,29 @@ public abstract class LocationProviderBase {
      * Reports a new location from this provider.
      */
     public void reportLocation(@NonNull Location location) {
-        reportLocation(LocationResult.create(location));
+        ILocationProviderManager manager = mManager;
+        if (manager != null) {
+            try {
+                manager.onReportLocation(stripExtras(location));
+            } catch (RemoteException e) {
+                throw e.rethrowFromSystemServer();
+            } catch (RuntimeException e) {
+                Log.w(mTag, e);
+            }
+        }
     }
 
     /**
-     * Reports a new location result from this provider.
-     *
-     * <p>May only be used from Android S onwards.
+     * Reports a new batch of locations from this provider. Locations must be ordered in the list
+     * from earliest first to latest last.
      */
-    public void reportLocation(@NonNull LocationResult locationResult) {
+    public void reportLocations(@NonNull List<Location> locations) {
         ILocationProviderManager manager = mManager;
         if (manager != null) {
-            locationResult = locationResult.map(location -> {
-                // remove deprecated extras to save on serialization costs
-                Bundle extras = location.getExtras();
-                if (extras != null && (extras.containsKey(EXTRA_NO_GPS_LOCATION)
-                        || extras.containsKey("coarseLocation"))) {
-                    location = new Location(location);
-                    extras = location.getExtras();
-                    extras.remove(EXTRA_NO_GPS_LOCATION);
-                    extras.remove("coarseLocation");
-                    if (extras.isEmpty()) {
-                        location.setExtras(null);
-                    }
-                }
-                return location;
-            });
+
 
             try {
-                manager.onReportLocation(locationResult);
+                manager.onReportLocations(stripExtras(locations));
             } catch (RemoteException e) {
                 throw e.rethrowFromSystemServer();
             } catch (RuntimeException e) {
@@ -246,9 +241,9 @@ public abstract class LocationProviderBase {
 
     /**
      * Requests a flush of any pending batched locations. The callback must always be invoked once
-     * per invocation, and should be invoked after {@link #reportLocation(LocationResult)} has been
-     * invoked with any flushed locations. The callback may be invoked immediately if no locations
-     * are flushed.
+     * per invocation, and should be invoked after {@link #reportLocation(Location)} or
+     * {@link #reportLocations(List)} has been invoked with any flushed locations. The callback may
+     * be invoked immediately if no locations are flushed.
      */
     public abstract void onFlush(@NonNull OnFlushCompleteCallback callback);
 
@@ -259,6 +254,49 @@ public abstract class LocationProviderBase {
             @SuppressLint("NullableCollection")
             @Nullable Bundle extras);
 
+    private static Location stripExtras(Location location) {
+        Bundle extras = location.getExtras();
+        if (extras != null && (extras.containsKey(EXTRA_NO_GPS_LOCATION)
+                || extras.containsKey("indoorProbability")
+                || extras.containsKey("coarseLocation"))) {
+            location = new Location(location);
+            extras = location.getExtras();
+            extras.remove(EXTRA_NO_GPS_LOCATION);
+            extras.remove("indoorProbability");
+            extras.remove("coarseLocation");
+            if (extras.isEmpty()) {
+                location.setExtras(null);
+            }
+        }
+        return location;
+    }
+
+    private static List<Location> stripExtras(List<Location> locations) {
+        List<Location> mapped = locations;
+        final int size = locations.size();
+        int i = 0;
+        for (Location location : locations) {
+            Location newLocation = stripExtras(location);
+            if (mapped != locations) {
+                mapped.add(newLocation);
+            } else if (newLocation != location) {
+                mapped = new ArrayList<>(size);
+                int j = 0;
+                for (Location copiedLocation : locations) {
+                    if (j >= i) {
+                        break;
+                    }
+                    mapped.add(copiedLocation);
+                    j++;
+                }
+                mapped.add(newLocation);
+            }
+            i++;
+        }
+
+        return mapped;
+    }
+
     private final class Service extends ILocationProvider.Stub {
 
         Service() {}