Refactor ListenerMultiplexer with new capabilities
-Store tag string in multiplexer rather than duplicating in every
registration.
-Add support for failure callbacks at listener operation submission
time, in addition to within the listener operation itself.
-Extract request out of ListenerRegistration and into subclass, so that
the multiplexer does not need to have any knowledge of requests.
-New multiplexer specific execute and failure callbacks within
registrations objects, with strongly typed listener operation objects.
-Fixed resetService() to work properly even if the service is still
registered.
-Added registration info as an argument to registerWithService() so that
this doesn't need to be duplicated if a client wants access to this
information.
-Narrow exception handling so that only the proper exception
(RemoteException for binder calls for instance) results in registration
removal, and other unexpected checked exceptions should still crash the
process.
-Various other minor fixes.
Test: manual + presubmit
Change-Id: I6fdec313222ff1ef01a1aa4d27d8ee56b4be25d4
diff --git a/core/java/com/android/internal/listeners/ListenerExecutor.java b/core/java/com/android/internal/listeners/ListenerExecutor.java
index 9979e60..7d9b9c9 100644
--- a/core/java/com/android/internal/listeners/ListenerExecutor.java
+++ b/core/java/com/android/internal/listeners/ListenerExecutor.java
@@ -38,71 +38,86 @@
void operate(TListener listener) throws Exception;
/**
- * Called before this operation is to be run. Some operations may be canceled before they
- * are run, in which case this method may not be called. {@link #onPostExecute(boolean)}
- * will only be run if this method was run. This callback is invoked on the calling thread.
+ * Called before this operation is to be run. An operation may be canceled before it is run,
+ * in which case this method may not be invoked. {@link #onPostExecute(boolean)} will only
+ * be invoked if this method was previously invoked. This callback is invoked on the
+ * calling thread.
*/
default void onPreExecute() {}
/**
* Called if the operation fails while running. Will not be invoked in the event of a
- * RuntimeException, which will propagate normally. Implementations of
- * {@link ListenerExecutor} have the option to override
- * {@link ListenerExecutor#onOperationFailure(ListenerOperation, Exception)} instead to
- * intercept failures at the class level. This callback is invoked on the executor thread.
+ * unchecked exception, which will propagate normally. This callback is invoked on the
+ * executor thread.
*/
- default void onFailure(Exception e) {
- // implementations should handle any exceptions that may be thrown
- throw new AssertionError(e);
- }
+ default void onFailure(Exception e) {}
/**
- * Called after the operation is run. This method will always be called if
- * {@link #onPreExecute()} is called. Success implies that the operation was run to
- * completion with no failures. This callback may be invoked on the calling thread or
- * executor thread.
+ * Called after the operation may have been run. Will always be invoked for every operation
+ * which has previously had {@link #onPreExecute()} invoked. Success implies that the
+ * operation was run to completion with no failures. If {@code success} is true, this
+ * callback will always be invoked on the executor thread. If {@code success} is false, this
+ * callback may be invoked on the calling thread or executor thread.
*/
default void onPostExecute(boolean success) {}
/**
- * Called after this operation is complete (which does not imply that it was necessarily
- * run). Will always be called once per operation, no matter if the operation was run or
- * not. Success implies that the operation was run to completion with no failures. This
- * callback may be invoked on the calling thread or executor thread.
+ * Will always be called once for every operation submitted to
+ * {@link #executeSafely(Executor, Supplier, ListenerOperation)}, no matter if the operation
+ * was run or not. This method is always invoked last, after every other callback. Success
+ * implies that the operation was run to completion with no failures. If {@code success}
+ * is true, this callback will always be invoked on the executor thread. If {@code success}
+ * is false, this callback may be invoked on the calling thread or executor thread.
*/
default void onComplete(boolean success) {}
}
/**
- * May be override to handle operation failures at a class level. Will not be invoked in the
- * event of a RuntimeException, which will propagate normally. This callback is invoked on the
- * executor thread.
+ * An callback for listener operation failure.
+ *
+ * @param <TListenerOperation> listener operation type
*/
- default <TListener> void onOperationFailure(ListenerOperation<TListener> operation,
- Exception exception) {
- operation.onFailure(exception);
+ interface FailureCallback<TListenerOperation extends ListenerOperation<?>> {
+
+ /**
+ * Called if a listener operation fails while running with a checked exception. This
+ * callback is invoked on the executor thread.
+ */
+ void onFailure(TListenerOperation operation, Exception exception);
+ }
+
+ /**
+ * See {@link #executeSafely(Executor, Supplier, ListenerOperation, FailureCallback)}.
+ */
+ default <TListener> void executeSafely(Executor executor, Supplier<TListener> listenerSupplier,
+ @Nullable ListenerOperation<TListener> operation) {
+ executeSafely(executor, listenerSupplier, operation, null);
}
/**
* Executes the given listener operation on the given executor, using the provided listener
* supplier. If the supplier returns a null value, or a value during the operation that does not
* match the value prior to the operation, then the operation is considered canceled. If a null
- * operation is supplied, nothing happens.
+ * operation is supplied, nothing happens. If a failure callback is supplied, this will be
+ * invoked on the executor thread in the event a checked exception is thrown from the listener
+ * operation.
*/
- default <TListener> void executeSafely(Executor executor, Supplier<TListener> listenerSupplier,
- @Nullable ListenerOperation<TListener> operation) {
+ default <TListener, TListenerOperation extends ListenerOperation<TListener>> void executeSafely(
+ Executor executor, Supplier<TListener> listenerSupplier,
+ @Nullable TListenerOperation operation,
+ @Nullable FailureCallback<TListenerOperation> failureCallback) {
if (operation == null) {
return;
}
+ TListener listener = listenerSupplier.get();
+ if (listener == null) {
+ return;
+ }
+
boolean executing = false;
boolean preexecute = false;
try {
- TListener listener = listenerSupplier.get();
- if (listener == null) {
- return;
- }
-
operation.onPreExecute();
preexecute = true;
executor.execute(() -> {
@@ -116,7 +131,10 @@
if (e instanceof RuntimeException) {
throw (RuntimeException) e;
} else {
- onOperationFailure(operation, e);
+ operation.onFailure(e);
+ if (failureCallback != null) {
+ failureCallback.onFailure(operation, e);
+ }
}
} finally {
operation.onPostExecute(success);
diff --git a/location/java/android/location/LocationRequest.java b/location/java/android/location/LocationRequest.java
index 911b9ab..0521b10 100644
--- a/location/java/android/location/LocationRequest.java
+++ b/location/java/android/location/LocationRequest.java
@@ -475,7 +475,7 @@
/**
* Returns the minimum update interval. If location updates are available faster than the
- * request interval then locations will only be delivered if the minimum update interval has
+ * request interval then locations will only be updated if the minimum update interval has
* expired since the last location update.
*
* <p class=note><strong>Note:</strong> Some allowance for jitter is already built into the
@@ -951,9 +951,9 @@
/**
* Sets an explicit minimum update interval. If location updates are available faster than
- * the request interval then locations will only be delivered if the minimum update interval
- * has expired since the last location update. Defaults to no explicit minimum update
- * interval set, which means the minimum update interval is the same as the interval.
+ * the request interval then an update will only occur if the minimum update interval has
+ * expired since the last location update. Defaults to no explicit minimum update interval
+ * set, which means the minimum update interval is the same as the interval.
*
* <p class=note><strong>Note:</strong> Some allowance for jitter is already built into the
* minimum update interval, so you need not worry about updates blocked simply because they
@@ -972,7 +972,7 @@
/**
* Clears an explicitly set minimum update interval and reverts to an implicit minimum
- * update interval (ie, the minimum update interval is same value as the interval).
+ * update interval (ie, the minimum update interval is the same value as the interval).
*/
public @NonNull Builder clearMinUpdateIntervalMillis() {
mMinUpdateIntervalMillis = IMPLICIT_MIN_UPDATE_INTERVAL;
@@ -980,8 +980,8 @@
}
/**
- * Sets the minimum update distance between delivered locations. If a potential location
- * update is closer to the last delivered location than the minimum update distance, then
+ * Sets the minimum update distance between location updates. If a potential location
+ * update is closer to the last location update than the minimum update distance, then
* the potential location update will not occur. Defaults to 0, which represents no minimum
* update distance.
*/
diff --git a/services/core/java/com/android/server/location/AbstractLocationProvider.java b/services/core/java/com/android/server/location/AbstractLocationProvider.java
index 06e7a76..56982a8 100644
--- a/services/core/java/com/android/server/location/AbstractLocationProvider.java
+++ b/services/core/java/com/android/server/location/AbstractLocationProvider.java
@@ -16,8 +16,6 @@
package com.android.server.location;
-import static com.android.internal.util.function.pooled.PooledLambda.obtainRunnable;
-
import android.annotation.Nullable;
import android.location.Location;
import android.location.util.identity.CallerIdentity;
@@ -342,8 +340,7 @@
*/
public final void setRequest(ProviderRequest request) {
// all calls into the provider must be moved onto the provider thread to prevent deadlock
- mExecutor.execute(obtainRunnable(AbstractLocationProvider::onSetRequest, this, request)
- .recycleOnUse());
+ mExecutor.execute(() -> onSetRequest(request));
}
/**
@@ -356,13 +353,7 @@
*/
public final void sendExtraCommand(int uid, int pid, String command, Bundle extras) {
// all calls into the provider must be moved onto the provider thread to prevent deadlock
-
- // the integer boxing done here likely cancels out any gains from removing lambda
- // allocation, but since this an infrequently used api with no real performance needs, we
- // we use pooled lambdas anyways for consistency.
- mExecutor.execute(
- obtainRunnable(AbstractLocationProvider::onExtraCommand, this, uid, pid, command,
- extras).recycleOnUse());
+ mExecutor.execute(() -> onExtraCommand(uid, pid, command, extras));
}
/**
diff --git a/services/core/java/com/android/server/location/LocationProviderManager.java b/services/core/java/com/android/server/location/LocationProviderManager.java
index e224a9d..2a5e035 100644
--- a/services/core/java/com/android/server/location/LocationProviderManager.java
+++ b/services/core/java/com/android/server/location/LocationProviderManager.java
@@ -29,6 +29,7 @@
import static android.os.PowerManager.LOCATION_MODE_GPS_DISABLED_WHEN_SCREEN_OFF;
import static android.os.PowerManager.LOCATION_MODE_THROTTLE_REQUESTS_WHEN_SCREEN_OFF;
+import static com.android.internal.location.ProviderRequest.EMPTY_REQUEST;
import static com.android.server.location.LocationManagerService.D;
import static com.android.server.location.LocationManagerService.TAG;
import static com.android.server.location.LocationPermissions.PERMISSION_COARSE;
@@ -77,6 +78,7 @@
import android.util.TimeUtils;
import com.android.internal.annotations.GuardedBy;
+import com.android.internal.listeners.ListenerExecutor.ListenerOperation;
import com.android.internal.location.ProviderProperties;
import com.android.internal.location.ProviderRequest;
import com.android.internal.util.Preconditions;
@@ -111,7 +113,8 @@
import java.util.Objects;
class LocationProviderManager extends
- ListenerMultiplexer<Object, LocationRequest, LocationProviderManager.LocationTransport,
+ ListenerMultiplexer<Object, LocationProviderManager.LocationTransport,
+ LocationProviderManager.LocationListenerOperation,
LocationProviderManager.Registration, ProviderRequest> implements
AbstractLocationProvider.Listener {
@@ -121,16 +124,16 @@
private static final String WAKELOCK_TAG = "*location*";
private static final long WAKELOCK_TIMEOUT_MS = 30 * 1000;
- // maximum interval to be considered "high power" request
+ // max interval to be considered "high power" request
private static final long MAX_HIGH_POWER_INTERVAL_MS = 5 * 60 * 1000;
- // maximum age of a location before it is no longer considered "current"
+ // max age of a location before it is no longer considered "current"
private static final long MAX_CURRENT_LOCATION_AGE_MS = 10 * 1000;
// max timeout allowed for getting the current location
private static final long GET_CURRENT_LOCATION_MAX_TIMEOUT_MS = 30 * 1000;
- // maximum jitter allowed for fastest interval evaluation
+ // max jitter allowed for fastest interval evaluation
private static final int MAX_FASTEST_INTERVAL_JITTER_MS = 100;
protected interface LocationTransport {
@@ -214,8 +217,15 @@
}
}
- protected abstract class Registration extends
- RemoteListenerRegistration<LocationRequest, LocationTransport> {
+ protected interface LocationListenerOperation extends ListenerOperation<LocationTransport> {
+ /**
+ * Must be implemented to return the location this operation intends to deliver.
+ */
+ Location getLocation();
+ }
+
+ protected abstract class Registration extends RemoteListenerRegistration<LocationRequest,
+ LocationTransport, LocationListenerOperation> {
@PermissionLevel protected final int mPermissionLevel;
private final WorkSource mWorkSource;
@@ -226,9 +236,11 @@
private LocationRequest mProviderLocationRequest;
private boolean mIsUsingHighPower;
+ @Nullable private Location mLastLocation = null;
+
protected Registration(LocationRequest request, CallerIdentity identity,
LocationTransport transport, @PermissionLevel int permissionLevel) {
- super(TAG, Objects.requireNonNull(request), identity, transport);
+ super(Objects.requireNonNull(request), identity, transport);
Preconditions.checkArgument(permissionLevel > PERMISSION_NONE);
mPermissionLevel = permissionLevel;
@@ -291,7 +303,11 @@
protected void onProviderListenerUnregister() {}
@Override
- protected final ListenerOperation<LocationTransport> onActive() {
+ protected final LocationListenerOperation onActive() {
+ if (Build.IS_DEBUGGABLE) {
+ Preconditions.checkState(Thread.holdsLock(mLock));
+ }
+
if (!getRequest().isHiddenFromAppOps()) {
mLocationAttributionHelper.reportLocationStart(getIdentity(), getName(), getKey());
}
@@ -300,7 +316,11 @@
}
@Override
- protected final ListenerOperation<LocationTransport> onInactive() {
+ protected final LocationListenerOperation onInactive() {
+ if (Build.IS_DEBUGGABLE) {
+ Preconditions.checkState(Thread.holdsLock(mLock));
+ }
+
onHighPowerUsageChanged();
if (!getRequest().isHiddenFromAppOps()) {
mLocationAttributionHelper.reportLocationStop(getIdentity(), getName(), getKey());
@@ -309,18 +329,14 @@
}
@Override
- public final <Listener> void onOperationFailure(ListenerOperation<Listener> operation,
- Exception e) {
- synchronized (mLock) {
- super.onOperationFailure(operation, e);
- }
- }
-
- @Override
public final LocationRequest getRequest() {
return mProviderLocationRequest;
}
+ public final Location getLastDeliveredLocation() {
+ return mLastLocation;
+ }
+
public final boolean isForeground() {
return mForeground;
}
@@ -480,8 +496,17 @@
return mLocationManagerInternal.isProvider(null, getIdentity());
}
+ @GuardedBy("mLock")
+ @Override
+ protected final LocationListenerOperation onExecuteOperation(
+ LocationListenerOperation operation) {
+ mLastLocation = operation.getLocation();
+ return super.onExecuteOperation(operation);
+ }
+
+ @GuardedBy("mLock")
@Nullable
- abstract ListenerOperation<LocationTransport> acceptLocationChange(Location fineLocation);
+ abstract LocationListenerOperation acceptLocationChange(Location fineLocation);
@Override
public String toString() {
@@ -514,7 +539,6 @@
private final PowerManager.WakeLock mWakeLock;
private volatile ProviderTransport mProviderTransport;
- @Nullable private Location mLastLocation = null;
private int mNumLocationsDelivered = 0;
private long mExpirationRealtimeMs = Long.MAX_VALUE;
@@ -606,7 +630,7 @@
@GuardedBy("mLock")
@Nullable
@Override
- ListenerOperation<LocationTransport> acceptLocationChange(Location fineLocation) {
+ LocationListenerOperation acceptLocationChange(Location fineLocation) {
if (Build.IS_DEBUGGABLE) {
Preconditions.checkState(Thread.holdsLock(mLock));
}
@@ -631,11 +655,12 @@
throw new AssertionError();
}
- if (mLastLocation != null) {
+ Location lastDeliveredLocation = getLastDeliveredLocation();
+ if (lastDeliveredLocation != null) {
// check fastest interval
long deltaMs = NANOSECONDS.toMillis(
location.getElapsedRealtimeNanos()
- - mLastLocation.getElapsedRealtimeNanos());
+ - lastDeliveredLocation.getElapsedRealtimeNanos());
if (deltaMs < getRequest().getMinUpdateIntervalMillis()
- MAX_FASTEST_INTERVAL_JITTER_MS) {
return null;
@@ -643,7 +668,7 @@
// check smallest displacement
double smallestDisplacementM = getRequest().getMinUpdateDistanceMeters();
- if (smallestDisplacementM > 0.0 && location.distanceTo(mLastLocation)
+ if (smallestDisplacementM > 0.0 && location.distanceTo(lastDeliveredLocation)
<= smallestDisplacementM) {
return null;
}
@@ -658,10 +683,12 @@
return null;
}
- // update last location
- mLastLocation = location;
+ return new LocationListenerOperation() {
+ @Override
+ public Location getLocation() {
+ return location;
+ }
- return new ListenerOperation<LocationTransport>() {
@Override
public void onPreExecute() {
// don't acquire a wakelock for mock locations to prevent abuse
@@ -720,8 +747,12 @@
// we choose not to hold a wakelock for provider enabled changed events
executeSafely(getExecutor(), () -> mProviderTransport,
- listener -> listener.deliverOnProviderEnabledChanged(mName, enabled));
+ listener -> listener.deliverOnProviderEnabledChanged(mName, enabled),
+ this::onProviderOperationFailure);
}
+
+ protected abstract void onProviderOperationFailure(
+ ListenerOperation<ProviderTransport> operation, Exception exception);
}
protected final class LocationListenerRegistration extends LocationRegistration implements
@@ -749,6 +780,28 @@
}
@Override
+ protected void onProviderOperationFailure(ListenerOperation<ProviderTransport> operation,
+ Exception exception) {
+ onTransportFailure(exception);
+ }
+
+ @Override
+ public void onOperationFailure(LocationListenerOperation operation, Exception exception) {
+ onTransportFailure(exception);
+ }
+
+ private void onTransportFailure(Exception exception) {
+ if (exception instanceof RemoteException) {
+ Log.w(TAG, "registration " + this + " removed", exception);
+ synchronized (mLock) {
+ remove();
+ }
+ } else {
+ throw new AssertionError(exception);
+ }
+ }
+
+ @Override
public void binderDied() {
try {
if (D) {
@@ -788,6 +841,28 @@
}
@Override
+ protected void onProviderOperationFailure(ListenerOperation<ProviderTransport> operation,
+ Exception exception) {
+ onTransportFailure(exception);
+ }
+
+ @Override
+ public void onOperationFailure(LocationListenerOperation operation, Exception exception) {
+ onTransportFailure(exception);
+ }
+
+ private void onTransportFailure(Exception exception) {
+ if (exception instanceof PendingIntent.CanceledException) {
+ Log.w(TAG, "registration " + this + " removed", exception);
+ synchronized (mLock) {
+ remove();
+ }
+ } else {
+ throw new AssertionError(exception);
+ }
+ }
+
+ @Override
public void onCancelled(PendingIntent intent) {
synchronized (mLock) {
remove();
@@ -838,13 +913,17 @@
0, this, FgThread.getHandler(), getWorkSource());
}
- // start listening for provider enabled/disabled events
- addEnabledListener(this);
+ // if this request is ignoring location settings, then we don't want to immediately fail
+ // it if the provider is disabled or becomes disabled.
+ if (!getRequest().isLocationSettingsIgnored()) {
+ // start listening for provider enabled/disabled events
+ addEnabledListener(this);
- // if the provider is currently disabled fail immediately
- int userId = getIdentity().getUserId();
- if (!getRequest().isLocationSettingsIgnored() && !isEnabled(userId)) {
- deliverLocation(null);
+ // if the provider is currently disabled fail immediately
+ int userId = getIdentity().getUserId();
+ if (!getRequest().isLocationSettingsIgnored() && !isEnabled(userId)) {
+ deliverLocation(null);
+ }
}
}
@@ -880,7 +959,7 @@
@GuardedBy("mLock")
@Nullable
@Override
- ListenerOperation<LocationTransport> acceptLocationChange(@Nullable Location fineLocation) {
+ LocationListenerOperation acceptLocationChange(@Nullable Location fineLocation) {
if (Build.IS_DEBUGGABLE) {
Preconditions.checkState(Thread.holdsLock(mLock));
}
@@ -909,19 +988,35 @@
}
}
- return listener -> {
- // if delivering to the same process, make a copy of the location first (since
- // location is mutable)
- Location deliveryLocation = location;
- if (getIdentity().getPid() == Process.myPid() && location != null) {
- deliveryLocation = new Location(location);
+ return new LocationListenerOperation() {
+ @Override
+ public Location getLocation() {
+ return location;
}
- // we currently don't hold a wakelock for getCurrentLocation deliveries
- listener.deliverOnLocationChanged(deliveryLocation, null);
+ @Override
+ public void operate(LocationTransport listener) {
+ // if delivering to the same process, make a copy of the location first (since
+ // location is mutable)
+ Location deliveryLocation = location;
+ if (getIdentity().getPid() == Process.myPid() && location != null) {
+ deliveryLocation = new Location(location);
+ }
- synchronized (mLock) {
- remove();
+ // we currently don't hold a wakelock for getCurrentLocation deliveries
+ try {
+ listener.deliverOnLocationChanged(deliveryLocation, null);
+ } catch (Exception exception) {
+ if (exception instanceof RemoteException) {
+ Log.w(TAG, "registration " + this + " failed", exception);
+ } else {
+ throw new AssertionError(exception);
+ }
+ }
+
+ synchronized (mLock) {
+ remove();
+ }
}
};
}
@@ -1054,6 +1149,11 @@
mProvider = new MockableLocationProvider(mLock, this);
}
+ @Override
+ public String getTag() {
+ return TAG;
+ }
+
public void startManager() {
synchronized (mLock) {
mStarted = true;
@@ -1260,8 +1360,7 @@
/**
* This function does not perform any permissions or safety checks, by calling it you are
- * committing to performing all applicable checks yourself. Prefer
- * {@link #getLastLocation(CallerIdentity, int, boolean)} where possible.
+ * committing to performing all applicable checks yourself.
*/
@Nullable
public Location getLastLocationUnsafe(int userId, @PermissionLevel int permissionLevel,
@@ -1554,7 +1653,8 @@
@GuardedBy("mLock")
@Override
- protected boolean registerWithService(ProviderRequest mergedRequest) {
+ protected boolean registerWithService(ProviderRequest mergedRequest,
+ Collection<Registration> registrations) {
if (Build.IS_DEBUGGABLE) {
Preconditions.checkState(Thread.holdsLock(mLock));
}
@@ -1566,8 +1666,8 @@
@GuardedBy("mLock")
@Override
protected boolean reregisterWithService(ProviderRequest oldRequest,
- ProviderRequest newRequest) {
- return registerWithService(newRequest);
+ ProviderRequest newRequest, Collection<Registration> registrations) {
+ return registerWithService(newRequest, registrations);
}
@GuardedBy("mLock")
@@ -1576,7 +1676,8 @@
if (Build.IS_DEBUGGABLE) {
Preconditions.checkState(Thread.holdsLock(mLock));
}
- mProvider.setRequest(ProviderRequest.EMPTY_REQUEST);
+
+ mProvider.setRequest(EMPTY_REQUEST);
}
@GuardedBy("mLock")
@@ -1628,7 +1729,7 @@
@GuardedBy("mLock")
@Override
- protected ProviderRequest mergeRequests(Collection<Registration> registrations) {
+ protected ProviderRequest mergeRegistrations(Collection<Registration> registrations) {
if (Build.IS_DEBUGGABLE) {
Preconditions.checkState(Thread.holdsLock(mLock));
}
@@ -1814,9 +1915,7 @@
}
// attempt listener delivery
- deliverToListeners(registration -> {
- return registration.acceptLocationChange(location);
- });
+ deliverToListeners(registration -> registration.acceptLocationChange(location));
}
@GuardedBy("mLock")
@@ -1951,7 +2050,8 @@
public void dump(FileDescriptor fd, IndentingPrintWriter ipw, String[] args) {
synchronized (mLock) {
- ipw.print(mName + " provider");
+ ipw.print(mName);
+ ipw.print(" provider");
if (mProvider.isMock()) {
ipw.print(" [mock]");
}
@@ -1963,12 +2063,15 @@
int[] userIds = mUserInfoHelper.getRunningUserIds();
for (int userId : userIds) {
if (userIds.length != 1) {
- ipw.println("user " + userId + ":");
+ ipw.print("user ");
+ ipw.print(userId);
+ ipw.println(":");
ipw.increaseIndent();
}
- ipw.println(
- "last location=" + getLastLocationUnsafe(userId, PERMISSION_FINE, false));
- ipw.println("enabled=" + isEnabled(userId));
+ ipw.print("last location=");
+ ipw.println(getLastLocationUnsafe(userId, PERMISSION_FINE, false));
+ ipw.print("enabled=");
+ ipw.println(isEnabled(userId));
if (userIds.length != 1) {
ipw.decreaseIndent();
}
diff --git a/services/core/java/com/android/server/location/PassiveLocationProviderManager.java b/services/core/java/com/android/server/location/PassiveLocationProviderManager.java
index 5133683..afeb644 100644
--- a/services/core/java/com/android/server/location/PassiveLocationProviderManager.java
+++ b/services/core/java/com/android/server/location/PassiveLocationProviderManager.java
@@ -64,7 +64,7 @@
}
@Override
- protected ProviderRequest mergeRequests(Collection<Registration> registrations) {
+ protected ProviderRequest mergeRegistrations(Collection<Registration> registrations) {
ProviderRequest.Builder providerRequest = new ProviderRequest.Builder()
.setIntervalMillis(0);
diff --git a/services/core/java/com/android/server/location/geofence/GeofenceManager.java b/services/core/java/com/android/server/location/geofence/GeofenceManager.java
index 7f02b2a..58e6d59 100644
--- a/services/core/java/com/android/server/location/geofence/GeofenceManager.java
+++ b/services/core/java/com/android/server/location/geofence/GeofenceManager.java
@@ -41,6 +41,7 @@
import android.util.ArraySet;
import com.android.internal.annotations.GuardedBy;
+import com.android.internal.listeners.ListenerExecutor.ListenerOperation;
import com.android.server.PendingIntentUtils;
import com.android.server.location.LocationPermissions;
import com.android.server.location.listeners.ListenerMultiplexer;
@@ -59,7 +60,7 @@
* Manages all geofences.
*/
public class GeofenceManager extends
- ListenerMultiplexer<GeofenceKey, Geofence, PendingIntent,
+ ListenerMultiplexer<GeofenceKey, PendingIntent, ListenerOperation<PendingIntent>,
GeofenceManager.GeofenceRegistration, LocationRequest> implements
LocationListener {
@@ -94,7 +95,7 @@
protected GeofenceRegistration(Geofence geofence, CallerIdentity identity,
PendingIntent pendingIntent) {
- super(TAG, geofence, identity, pendingIntent);
+ super(geofence, identity, pendingIntent);
mCenter = new Location("");
mCenter.setLatitude(geofence.getLatitude());
@@ -273,6 +274,11 @@
mLocationUsageLogger = injector.getLocationUsageLogger();
}
+ @Override
+ public String getTag() {
+ return TAG;
+ }
+
private LocationManager getLocationManager() {
synchronized (mLock) {
if (mLocationManager == null) {
@@ -372,7 +378,8 @@
}
@Override
- protected boolean registerWithService(LocationRequest locationRequest) {
+ protected boolean registerWithService(LocationRequest locationRequest,
+ Collection<GeofenceRegistration> registrations) {
getLocationManager().requestLocationUpdates(FUSED_PROVIDER, locationRequest,
DIRECT_EXECUTOR, this);
return true;
@@ -387,7 +394,7 @@
}
@Override
- protected LocationRequest mergeRequests(Collection<GeofenceRegistration> registrations) {
+ protected LocationRequest mergeRegistrations(Collection<GeofenceRegistration> registrations) {
Location location = getLastLocation();
long realtimeMs = SystemClock.elapsedRealtime();
diff --git a/services/core/java/com/android/server/location/gnss/GnssAntennaInfoProvider.java b/services/core/java/com/android/server/location/gnss/GnssAntennaInfoProvider.java
index 5ddc6a1..b99e269 100644
--- a/services/core/java/com/android/server/location/gnss/GnssAntennaInfoProvider.java
+++ b/services/core/java/com/android/server/location/gnss/GnssAntennaInfoProvider.java
@@ -28,6 +28,7 @@
import com.android.internal.util.Preconditions;
import com.android.server.location.util.Injector;
+import java.util.Collection;
import java.util.List;
/**
@@ -59,7 +60,8 @@
}
@Override
- protected boolean registerWithService(Void ignored) {
+ protected boolean registerWithService(Void ignored,
+ Collection<GnssListenerRegistration> registrations) {
Preconditions.checkState(mNative.isAntennaInfoSupported());
if (mNative.startAntennaInfoListening()) {
diff --git a/services/core/java/com/android/server/location/gnss/GnssListenerMultiplexer.java b/services/core/java/com/android/server/location/gnss/GnssListenerMultiplexer.java
index a9fdacc..a830e2f 100644
--- a/services/core/java/com/android/server/location/gnss/GnssListenerMultiplexer.java
+++ b/services/core/java/com/android/server/location/gnss/GnssListenerMultiplexer.java
@@ -26,11 +26,13 @@
import android.location.LocationManagerInternal.ProviderEnabledListener;
import android.location.util.identity.CallerIdentity;
import android.os.Binder;
+import android.os.Build;
import android.os.IBinder;
import android.os.IInterface;
import android.os.Process;
import android.util.ArraySet;
+import com.android.internal.listeners.ListenerExecutor.ListenerOperation;
import com.android.internal.util.Preconditions;
import com.android.server.LocalServices;
import com.android.server.location.listeners.BinderListenerRegistration;
@@ -43,6 +45,7 @@
import com.android.server.location.util.UserInfoHelper.UserListener;
import java.io.PrintWriter;
+import java.util.Collection;
import java.util.Objects;
/**
@@ -52,14 +55,15 @@
* Listeners must be registered with the associated IBinder as the key, if the IBinder dies, the
* registration will automatically be removed.
*
- * @param <TRequest> request type
- * @param <TListener> listener type
- * @param <TMergedRequest> merged request type
+ * @param <TRequest> request type
+ * @param <TListener> listener type
+ * @param <TMergedRegistration> merged registration type
*/
public abstract class GnssListenerMultiplexer<TRequest, TListener extends IInterface,
- TMergedRequest> extends
- ListenerMultiplexer<IBinder, TRequest, TListener, GnssListenerMultiplexer<TRequest,
- TListener, TMergedRequest>.GnssListenerRegistration, TMergedRequest> {
+ TMergedRegistration> extends
+ ListenerMultiplexer<IBinder, TListener, ListenerOperation<TListener>,
+ GnssListenerMultiplexer<TRequest, TListener, TMergedRegistration>
+ .GnssListenerRegistration, TMergedRegistration> {
/**
* Registration object for GNSS listeners.
@@ -74,11 +78,11 @@
protected GnssListenerRegistration(@Nullable TRequest request,
CallerIdentity callerIdentity, TListener listener) {
- super(TAG, request, callerIdentity, listener);
+ super(request, callerIdentity, listener);
}
@Override
- protected GnssListenerMultiplexer<TRequest, TListener, TMergedRequest> getOwner() {
+ protected GnssListenerMultiplexer<TRequest, TListener, TMergedRegistration> getOwner() {
return GnssListenerMultiplexer.this;
}
@@ -199,6 +203,11 @@
LocalServices.getService(LocationManagerInternal.class));
}
+ @Override
+ public String getTag() {
+ return TAG;
+ }
+
/**
* May be overridden by subclasses to return whether the service is supported or not. This value
* should never change for the lifetime of the multiplexer. If the service is unsupported, all
@@ -271,6 +280,21 @@
return mLocationManagerInternal.isProvider(null, identity);
}
+ // this provides a default implementation for all further subclasses which assumes that there is
+ // never an associated request object, and thus nothing interesting to merge. the majority of
+ // gnss listener multiplexers do not current have associated requests, and the ones that do can
+ // override this implementation.
+ protected TMergedRegistration mergeRegistrations(
+ Collection<GnssListenerRegistration> gnssListenerRegistrations) {
+ if (Build.IS_DEBUGGABLE) {
+ for (GnssListenerRegistration registration : gnssListenerRegistrations) {
+ Preconditions.checkState(registration.getRequest() == null);
+ }
+ }
+
+ return null;
+ }
+
@Override
protected void onRegister() {
if (!isServiceSupported()) {
diff --git a/services/core/java/com/android/server/location/gnss/GnssMeasurementsProvider.java b/services/core/java/com/android/server/location/gnss/GnssMeasurementsProvider.java
index 37db023..2faa15f 100644
--- a/services/core/java/com/android/server/location/gnss/GnssMeasurementsProvider.java
+++ b/services/core/java/com/android/server/location/gnss/GnssMeasurementsProvider.java
@@ -76,7 +76,8 @@
}
@Override
- protected boolean registerWithService(Boolean fullTrackingRequest) {
+ protected boolean registerWithService(Boolean fullTrackingRequest,
+ Collection<GnssListenerRegistration> registrations) {
Preconditions.checkState(mNative.isMeasurementSupported());
if (mNative.startMeasurementCollection(fullTrackingRequest)) {
@@ -121,7 +122,7 @@
}
@Override
- protected Boolean mergeRequests(Collection<GnssListenerRegistration> registrations) {
+ protected Boolean mergeRegistrations(Collection<GnssListenerRegistration> registrations) {
if (mSettingsHelper.isGnssMeasurementsFullTrackingEnabled()) {
return true;
}
diff --git a/services/core/java/com/android/server/location/gnss/GnssNavigationMessageProvider.java b/services/core/java/com/android/server/location/gnss/GnssNavigationMessageProvider.java
index 7dcffc6..861abc3 100644
--- a/services/core/java/com/android/server/location/gnss/GnssNavigationMessageProvider.java
+++ b/services/core/java/com/android/server/location/gnss/GnssNavigationMessageProvider.java
@@ -30,6 +30,8 @@
import com.android.server.location.util.AppOpsHelper;
import com.android.server.location.util.Injector;
+import java.util.Collection;
+
/**
* An base implementation for GPS navigation messages provider.
* It abstracts out the responsibility of handling listeners, while still allowing technology
@@ -61,7 +63,8 @@
}
@Override
- protected boolean registerWithService(Void ignored) {
+ protected boolean registerWithService(Void ignored,
+ Collection<GnssListenerRegistration> registrations) {
Preconditions.checkState(mNative.isNavigationMessageSupported());
if (mNative.startNavigationMessageCollection()) {
diff --git a/services/core/java/com/android/server/location/gnss/GnssStatusProvider.java b/services/core/java/com/android/server/location/gnss/GnssStatusProvider.java
index 68813b3..248d38a 100644
--- a/services/core/java/com/android/server/location/gnss/GnssStatusProvider.java
+++ b/services/core/java/com/android/server/location/gnss/GnssStatusProvider.java
@@ -31,6 +31,8 @@
import com.android.server.location.util.Injector;
import com.android.server.location.util.LocationUsageLogger;
+import java.util.Collection;
+
/**
* Implementation of a handler for {@link IGnssStatusListener}.
*/
@@ -51,7 +53,8 @@
}
@Override
- protected boolean registerWithService(Void ignored) {
+ protected boolean registerWithService(Void ignored,
+ Collection<GnssListenerRegistration> registrations) {
if (D) {
Log.d(TAG, "starting gnss status");
}
diff --git a/services/core/java/com/android/server/location/listeners/BinderListenerRegistration.java b/services/core/java/com/android/server/location/listeners/BinderListenerRegistration.java
index 58aabda..bc675ce 100644
--- a/services/core/java/com/android/server/location/listeners/BinderListenerRegistration.java
+++ b/services/core/java/com/android/server/location/listeners/BinderListenerRegistration.java
@@ -23,6 +23,8 @@
import android.os.RemoteException;
import android.util.Log;
+import com.android.internal.listeners.ListenerExecutor.ListenerOperation;
+
/**
* A registration that works with IBinder keys, and registers a DeathListener to automatically
* remove the registration if the binder dies. The key for this registration must either be an
@@ -32,7 +34,8 @@
* @param <TListener> listener type
*/
public abstract class BinderListenerRegistration<TRequest, TListener> extends
- RemoteListenerRegistration<TRequest, TListener> implements Binder.DeathRecipient {
+ RemoteListenerRegistration<TRequest, TListener, ListenerOperation<TListener>> implements
+ Binder.DeathRecipient {
/**
* Interface to allow binder retrieval when keys are not themselves IBinders.
@@ -44,9 +47,9 @@
IBinder getBinder();
}
- protected BinderListenerRegistration(String tag, @Nullable TRequest request,
- CallerIdentity callerIdentity, TListener listener) {
- super(tag, request, callerIdentity, listener);
+ protected BinderListenerRegistration(@Nullable TRequest request, CallerIdentity callerIdentity,
+ TListener listener) {
+ super(request, callerIdentity, listener);
}
@Override
@@ -78,10 +81,20 @@
protected void onBinderListenerUnregister() {}
@Override
+ public void onOperationFailure(ListenerOperation<TListener> operation, Exception e) {
+ if (e instanceof RemoteException) {
+ Log.w(getOwner().getTag(), "registration " + this + " removed", e);
+ remove();
+ } else {
+ super.onOperationFailure(operation, e);
+ }
+ }
+
+ @Override
public void binderDied() {
try {
- if (Log.isLoggable(mTag, Log.DEBUG)) {
- Log.d(mTag, "binder registration " + getIdentity() + " died");
+ if (Log.isLoggable(getOwner().getTag(), Log.DEBUG)) {
+ Log.d(getOwner().getTag(), "binder registration " + getIdentity() + " died");
}
remove();
} catch (RuntimeException e) {
diff --git a/services/core/java/com/android/server/location/listeners/ListenerMultiplexer.java b/services/core/java/com/android/server/location/listeners/ListenerMultiplexer.java
index feb4fbd..87d668a 100644
--- a/services/core/java/com/android/server/location/listeners/ListenerMultiplexer.java
+++ b/services/core/java/com/android/server/location/listeners/ListenerMultiplexer.java
@@ -37,12 +37,20 @@
import java.util.function.Predicate;
/**
- * A base class to multiplex client listener registrations within system server. Registrations are
- * divided into two categories, active registrations and inactive registrations, as defined by
- * {@link #isActive(ListenerRegistration)}. If a registration's active state changes,
+ * A base class to multiplex client listener registrations within system server. Every listener is
+ * represented by a registration object which stores all required state for a listener. Keys are
+ * used to uniquely identify every registration. Listener operations may be executed on
+ * registrations in order to invoke the represented listener.
+ *
+ * Registrations are divided into two categories, active registrations and inactive registrations,
+ * as defined by {@link #isActive(ListenerRegistration)}. If a registration's active state changes,
* {@link #updateRegistrations(Predicate)} must be invoked and return true for any registration
* whose active state may have changed. Listeners will only be invoked for active registrations.
*
+ * The set of active registrations is combined into a single merged registration, which is submitted
+ * to the backing service when necessary in order to register the service. The merged registration
+ * is updated whenever the set of active registration changes.
+ *
* Callbacks invoked for various changes will always be ordered according to this lifecycle list:
*
* <ul>
@@ -63,14 +71,16 @@
* {@link #removeRegistration(Object, ListenerRegistration)}, not via any other removal method. This
* ensures re-entrant removal does not accidentally remove the incorrect registration.
*
- * @param <TKey> key type
- * @param <TRequest> request type
- * @param <TListener> listener type
- * @param <TRegistration> registration type
- * @param <TMergedRequest> merged request type
+ * @param <TKey> key type
+ * @param <TListener> listener type
+ * @param <TListenerOperation> listener operation type
+ * @param <TRegistration> registration type
+ * @param <TMergedRegistration> merged registration type
*/
-public abstract class ListenerMultiplexer<TKey, TRequest, TListener,
- TRegistration extends ListenerRegistration<TRequest, TListener>, TMergedRequest> {
+public abstract class ListenerMultiplexer<TKey, TListener,
+ TListenerOperation extends ListenerOperation<TListener>,
+ TRegistration extends ListenerRegistration<TListener, TListenerOperation>,
+ TMergedRegistration> {
@GuardedBy("mRegistrations")
private final ArrayMap<TKey, TRegistration> mRegistrations = new ArrayMap<>();
@@ -88,25 +98,42 @@
private boolean mServiceRegistered = false;
@GuardedBy("mRegistrations")
- @Nullable private TMergedRequest mCurrentRequest;
+ @Nullable private TMergedRegistration mMerged;
/**
- * Should be implemented to register with the backing service with the given merged request, and
- * should return true if a matching call to {@link #unregisterWithService()} is required to
- * unregister (ie, if registration succeeds).
+ * Should be implemented to return a unique identifying tag that may be used for logging, etc...
+ */
+ public abstract @NonNull String getTag();
+
+ /**
+ * Should be implemented to register with the backing service with the given merged
+ * registration, and should return true if a matching call to {@link #unregisterWithService()}
+ * is required to unregister (ie, if registration succeeds). The set of registrations passed in
+ * is the same set passed into {@link #mergeRegistrations(Collection)} to generate the merged
+ * registration.
*
- * @see #reregisterWithService(Object, Object)
+ * <p class="note">It may seem redundant to pass in the set of active registrations when they
+ * have already been used to generate the merged request, and indeed, for many implementations
+ * this parameter can likely simply be ignored. However, some implementations may require access
+ * to the set of registrations used to generate the merged requestion for further logic even
+ * after the merged registration has been generated.
+ *
+ * @see #mergeRegistrations(Collection)
+ * @see #reregisterWithService(Object, Object, Collection)
*/
- protected abstract boolean registerWithService(TMergedRequest newRequest);
+ protected abstract boolean registerWithService(TMergedRegistration merged,
+ @NonNull Collection<TRegistration> registrations);
/**
- * Invoked when the service already has a request, and it is being replaced with a new request.
- * The default implementation unregisters first, then registers with the new merged request, but
- * this may be overridden by subclasses in order to reregister more efficiently.
+ * Invoked when the service has already been registered with some merged registration, and is
+ * now being registered with a different merged registration. The default implementation simply
+ * invokes {@link #registerWithService(Object, Collection)}.
+ *
+ * @see #registerWithService(Object, Collection)
*/
- protected boolean reregisterWithService(TMergedRequest oldRequest, TMergedRequest newRequest) {
- unregisterWithService();
- return registerWithService(newRequest);
+ protected boolean reregisterWithService(TMergedRegistration oldMerged,
+ TMergedRegistration newMerged, @NonNull Collection<TRegistration> registrations) {
+ return registerWithService(newMerged, registrations);
}
/**
@@ -116,28 +143,23 @@
/**
* Defines whether a registration is currently active or not. Only active registrations will be
- * considered within {@link #mergeRequests(Collection)} to calculate the merged request, and
- * listener invocations will only be delivered to active requests. If a registration's active
- * state changes, {@link #updateRegistrations(Predicate)} must be invoked with a function that
- * returns true for any registrations that may have changed their active state.
+ * forwarded to {@link #registerWithService(Object, Collection)}, and listener invocations will
+ * only be delivered to active requests. If a registration's active state changes,
+ * {@link #updateRegistrations(Predicate)} must be invoked with a function that returns true for
+ * any registrations that may have changed their active state.
*/
protected abstract boolean isActive(@NonNull TRegistration registration);
/**
- * Called in order to generate a merged request from the given registrations. The list of
- * registrations will never be empty.
+ * Called in order to generate a merged registration from the given set of active registrations.
+ * The list of registrations will never be empty. If the resulting merged registration is equal
+ * to the currently registered merged registration, nothing further will happen. If the merged
+ * registration differs, {@link #registerWithService(Object, Collection)} or
+ * {@link #reregisterWithService(Object, Object, Collection)} will be invoked with the new
+ * merged registration so that the backing service can be updated.
*/
- protected @Nullable TMergedRequest mergeRequests(
- @NonNull Collection<TRegistration> registrations) {
- if (Build.IS_DEBUGGABLE) {
- for (TRegistration registration : registrations) {
- // if using non-null requests then implementations must override this method
- Preconditions.checkState(registration.getRequest() == null);
- }
- }
-
- return null;
- }
+ protected abstract @Nullable TMergedRegistration mergeRegistrations(
+ @NonNull Collection<TRegistration> registrations);
/**
* Invoked when the multiplexer goes from having no registrations to having some registrations.
@@ -348,41 +370,42 @@
}
if (actives.isEmpty()) {
- mCurrentRequest = null;
if (mServiceRegistered) {
+ mMerged = null;
mServiceRegistered = false;
- mCurrentRequest = null;
unregisterWithService();
}
return;
}
- TMergedRequest merged = mergeRequests(actives);
- if (!mServiceRegistered || !Objects.equals(merged, mCurrentRequest)) {
+ TMergedRegistration merged = mergeRegistrations(actives);
+ if (!mServiceRegistered || !Objects.equals(merged, mMerged)) {
if (mServiceRegistered) {
- mServiceRegistered = reregisterWithService(mCurrentRequest, merged);
+ mServiceRegistered = reregisterWithService(mMerged, merged, actives);
} else {
- mServiceRegistered = registerWithService(merged);
+ mServiceRegistered = registerWithService(merged, actives);
}
- if (mServiceRegistered) {
- mCurrentRequest = merged;
- } else {
- mCurrentRequest = null;
- }
+ mMerged = mServiceRegistered ? merged : null;
}
}
}
/**
- * Clears currently stored service state, and invokes {@link #updateService()} to force a new
- * call to {@link #registerWithService(Object)} if necessary. This is useful, for instance, if
- * the backing service has crashed or otherwise lost state, and needs to be re-initialized.
+ * If the service is currently registered, unregisters it and then calls
+ * {@link #updateService()} so that {@link #registerWithService(Object, Collection)} will be
+ * re-invoked. This is useful, for instance, if the backing service has crashed or otherwise
+ * lost state, and needs to be re-initialized. Because this unregisters first, this is safe to
+ * use even if there is a possibility the backing server has not crashed, or has already been
+ * reinitialized.
*/
protected final void resetService() {
synchronized (mRegistrations) {
- mServiceRegistered = false;
- mCurrentRequest = null;
- updateService();
+ if (mServiceRegistered) {
+ mMerged = null;
+ mServiceRegistered = false;
+ unregisterWithService();
+ updateService();
+ }
}
}
@@ -435,12 +458,12 @@
if (++mActiveRegistrationsCount == 1) {
onActive();
}
- ListenerOperation<TListener> operation = registration.onActive();
+ TListenerOperation operation = registration.onActive();
if (operation != null) {
execute(registration, operation);
}
} else {
- ListenerOperation<TListener> operation = registration.onInactive();
+ TListenerOperation operation = registration.onInactive();
if (operation != null) {
execute(registration, operation);
}
@@ -459,14 +482,14 @@
* change the active state of the registration.
*/
protected final void deliverToListeners(
- @NonNull Function<TRegistration, ListenerOperation<TListener>> function) {
+ @NonNull Function<TRegistration, TListenerOperation> function) {
synchronized (mRegistrations) {
try (ReentrancyGuard ignored = mReentrancyGuard.acquire()) {
final int size = mRegistrations.size();
for (int i = 0; i < size; i++) {
TRegistration registration = mRegistrations.valueAt(i);
if (registration.isActive()) {
- ListenerOperation<TListener> operation = function.apply(registration);
+ TListenerOperation operation = function.apply(registration);
if (operation != null) {
execute(registration, operation);
}
@@ -483,7 +506,7 @@
* deliverToListeners(registration -> operation);
* </pre>
*/
- protected final void deliverToListeners(@NonNull ListenerOperation<TListener> operation) {
+ protected final void deliverToListeners(@NonNull TListenerOperation operation) {
synchronized (mRegistrations) {
try (ReentrancyGuard ignored = mReentrancyGuard.acquire()) {
final int size = mRegistrations.size();
@@ -502,7 +525,7 @@
onRegistrationActiveChanged(registration);
}
- private void execute(TRegistration registration, ListenerOperation<TListener> operation) {
+ private void execute(TRegistration registration, TListenerOperation operation) {
registration.executeInternal(operation);
}
@@ -539,10 +562,11 @@
*/
protected void dumpServiceState(PrintWriter pw) {
if (mServiceRegistered) {
- if (mCurrentRequest == null) {
- pw.print("registered");
- } else {
- pw.print("registered with " + mCurrentRequest);
+ pw.print("registered");
+ if (mMerged != null) {
+ pw.print(" [");
+ pw.print(mMerged);
+ pw.print("]");
}
} else {
pw.print("unregistered");
diff --git a/services/core/java/com/android/server/location/listeners/ListenerRegistration.java b/services/core/java/com/android/server/location/listeners/ListenerRegistration.java
index deb9660..d7ecbcb 100644
--- a/services/core/java/com/android/server/location/listeners/ListenerRegistration.java
+++ b/services/core/java/com/android/server/location/listeners/ListenerRegistration.java
@@ -21,6 +21,7 @@
import android.annotation.Nullable;
import com.android.internal.listeners.ListenerExecutor;
+import com.android.internal.listeners.ListenerExecutor.ListenerOperation;
import java.util.Objects;
import java.util.concurrent.Executor;
@@ -29,22 +30,21 @@
* A listener registration object which holds data associated with the listener, such as an optional
* request, and an executor responsible for listener invocations.
*
- * @param <TRequest> request type
- * @param <TListener> listener type
+ * @param <TListener> listener type
+ * @param <TListenerOperation> listener operation type
*/
-public class ListenerRegistration<TRequest, TListener> implements ListenerExecutor {
+public class ListenerRegistration<TListener,
+ TListenerOperation extends ListenerOperation<TListener>> implements
+ ListenerExecutor {
private final Executor mExecutor;
- private final @Nullable TRequest mRequest;
private boolean mActive;
private volatile @Nullable TListener mListener;
- protected ListenerRegistration(Executor executor, @Nullable TRequest request,
- TListener listener) {
+ protected ListenerRegistration(Executor executor, TListener listener) {
mExecutor = Objects.requireNonNull(executor);
- mRequest = request;
mActive = false;
mListener = Objects.requireNonNull(listener);
}
@@ -54,13 +54,6 @@
}
/**
- * Returns the request associated with this listener, or null if one wasn't supplied.
- */
- public @Nullable TRequest getRequest() {
- return mRequest;
- }
-
- /**
* May be overridden by subclasses. Invoked when registration occurs. Invoked while holding the
* owning multiplexer's internal lock.
*/
@@ -77,7 +70,7 @@
* returns a non-null operation, that operation will be invoked for the listener. Invoked
* while holding the owning multiplexer's internal lock.
*/
- protected @Nullable ListenerOperation<TListener> onActive() {
+ protected @Nullable TListenerOperation onActive() {
return null;
}
@@ -86,7 +79,7 @@
* a non-null operation, that operation will be invoked for the listener. Invoked while holding
* the owning multiplexer's internal lock.
*/
- protected @Nullable ListenerOperation<TListener> onInactive() {
+ protected @Nullable TListenerOperation onInactive() {
return null;
}
@@ -115,21 +108,38 @@
/**
* May be overridden by subclasses, however should rarely be needed. Invoked when the listener
* associated with this registration is unregistered, which may occur before the registration
- * itself is unregistered. This immediately prevents the listener from being further invoked.
+ * itself is unregistered. This immediately prevents the listener from being further invoked
+ * until the registration itself can be finalized and unregistered completely.
*/
- protected void onListenerUnregister() {};
+ protected void onListenerUnregister() {}
- final void executeInternal(@NonNull ListenerOperation<TListener> operation) {
- executeSafely(mExecutor, () -> mListener, operation);
+ /**
+ * May be overridden by subclasses, however should rarely be needed. Invoked whenever a listener
+ * operation is submitted for execution, and allows the registration a chance to replace the
+ * listener operation or perform related bookkeeping. There is no guarantee a listener operation
+ * submitted or returned here will ever be invoked. Will always be invoked on the calling
+ * thread.
+ */
+ protected TListenerOperation onExecuteOperation(@NonNull TListenerOperation operation) {
+ return operation;
+ }
+
+ /**
+ * May be overridden by subclasses to handle listener operation failures. The default behavior
+ * is to further propagate any exceptions. Will always be invoked on the executor thread.
+ */
+ protected void onOperationFailure(TListenerOperation operation, Exception exception) {
+ throw new AssertionError(exception);
+ }
+
+ final void executeInternal(@NonNull TListenerOperation operation) {
+ executeSafely(mExecutor, () -> mListener,
+ onExecuteOperation(Objects.requireNonNull(operation)), this::onOperationFailure);
}
@Override
public String toString() {
- if (mRequest == null) {
- return "[]";
- } else {
- return mRequest.toString();
- }
+ return "[]";
}
@Override
diff --git a/services/core/java/com/android/server/location/listeners/PendingIntentListenerRegistration.java b/services/core/java/com/android/server/location/listeners/PendingIntentListenerRegistration.java
index 7b6154e..e57b532 100644
--- a/services/core/java/com/android/server/location/listeners/PendingIntentListenerRegistration.java
+++ b/services/core/java/com/android/server/location/listeners/PendingIntentListenerRegistration.java
@@ -21,6 +21,8 @@
import android.location.util.identity.CallerIdentity;
import android.util.Log;
+import com.android.internal.listeners.ListenerExecutor.ListenerOperation;
+
/**
* A registration that works with PendingIntent keys, and registers a CancelListener to
* automatically remove the registration if the PendingIntent is canceled. The key for this
@@ -30,7 +32,8 @@
* @param <TListener> listener type
*/
public abstract class PendingIntentListenerRegistration<TRequest, TListener> extends
- RemoteListenerRegistration<TRequest, TListener> implements PendingIntent.CancelListener {
+ RemoteListenerRegistration<TRequest, TListener, ListenerOperation<TListener>> implements
+ PendingIntent.CancelListener {
/**
* Interface to allowed pending intent retrieval when keys are not themselves PendingIntents.
@@ -42,9 +45,9 @@
PendingIntent getPendingIntent();
}
- protected PendingIntentListenerRegistration(String tag, @Nullable TRequest request,
+ protected PendingIntentListenerRegistration(@Nullable TRequest request,
CallerIdentity callerIdentity, TListener listener) {
- super(tag, request, callerIdentity, listener);
+ super(request, callerIdentity, listener);
}
@Override
@@ -70,9 +73,20 @@
protected void onPendingIntentListenerUnregister() {}
@Override
+ public void onOperationFailure(ListenerOperation<TListener> operation, Exception e) {
+ if (e instanceof PendingIntent.CanceledException) {
+ Log.w(getOwner().getTag(), "registration " + this + " removed", e);
+ remove();
+ } else {
+ super.onOperationFailure(operation, e);
+ }
+ }
+
+ @Override
public void onCancelled(PendingIntent intent) {
- if (Log.isLoggable(mTag, Log.DEBUG)) {
- Log.d(mTag, "pending intent registration " + getIdentity() + " canceled");
+ if (Log.isLoggable(getOwner().getTag(), Log.DEBUG)) {
+ Log.d(getOwner().getTag(),
+ "pending intent registration " + getIdentity() + " canceled");
}
remove();
diff --git a/services/core/java/com/android/server/location/listeners/RemoteListenerRegistration.java b/services/core/java/com/android/server/location/listeners/RemoteListenerRegistration.java
index e4b0b19..242bf32 100644
--- a/services/core/java/com/android/server/location/listeners/RemoteListenerRegistration.java
+++ b/services/core/java/com/android/server/location/listeners/RemoteListenerRegistration.java
@@ -24,6 +24,7 @@
import android.os.Process;
import com.android.internal.annotations.VisibleForTesting;
+import com.android.internal.listeners.ListenerExecutor.ListenerOperation;
import com.android.server.FgThread;
import java.util.Objects;
@@ -35,11 +36,13 @@
* invocation should already be asynchronous. Listeners from the same process will be run on a
* normal executor, since in-process listener invocation may be synchronous.
*
- * @param <TRequest> request type
- * @param <TListener> listener type
+ * @param <TRequest> request type
+ * @param <TListener> listener type
+ * @param <TListenerOperation> listener operation type
*/
-public abstract class RemoteListenerRegistration<TRequest, TListener> extends
- RemovableListenerRegistration<TRequest, TListener> {
+public abstract class RemoteListenerRegistration<TRequest, TListener,
+ TListenerOperation extends ListenerOperation<TListener>> extends
+ RemovableListenerRegistration<TRequest, TListener, TListenerOperation> {
@VisibleForTesting
public static final Executor IN_PROCESS_EXECUTOR = FgThread.getExecutor();
@@ -59,9 +62,9 @@
private final CallerIdentity mIdentity;
- protected RemoteListenerRegistration(String tag, @Nullable TRequest request,
- CallerIdentity identity, TListener listener) {
- super(tag, chooseExecutor(identity), request, listener);
+ protected RemoteListenerRegistration(@Nullable TRequest request, CallerIdentity identity,
+ TListener listener) {
+ super(chooseExecutor(identity), request, listener);
mIdentity = Objects.requireNonNull(identity);
}
diff --git a/services/core/java/com/android/server/location/listeners/RemovableListenerRegistration.java b/services/core/java/com/android/server/location/listeners/RemovableListenerRegistration.java
index 2383bec..d3b5f66 100644
--- a/services/core/java/com/android/server/location/listeners/RemovableListenerRegistration.java
+++ b/services/core/java/com/android/server/location/listeners/RemovableListenerRegistration.java
@@ -17,7 +17,8 @@
package com.android.server.location.listeners;
import android.annotation.Nullable;
-import android.util.Log;
+
+import com.android.internal.listeners.ListenerExecutor.ListenerOperation;
import java.util.Objects;
import java.util.concurrent.Executor;
@@ -26,20 +27,19 @@
* A listener registration that stores its own key, and thus can remove itself. By default it will
* remove itself if any checked exception occurs on listener execution.
*
- * @param <TRequest> request type
- * @param <TListener> listener type
+ * @param <TRequest> request type
+ * @param <TListener> listener type
+ * @param <TListenerOperation> listener operation type
*/
-public abstract class RemovableListenerRegistration<TRequest, TListener> extends
- ListenerRegistration<TRequest, TListener> {
-
- protected final String mTag;
+public abstract class RemovableListenerRegistration<TRequest, TListener,
+ TListenerOperation extends ListenerOperation<TListener>> extends
+ RequestListenerRegistration<TRequest, TListener, TListenerOperation> {
private volatile @Nullable Object mKey;
- protected RemovableListenerRegistration(String tag, Executor executor,
- @Nullable TRequest request, TListener listener) {
+ protected RemovableListenerRegistration(Executor executor, @Nullable TRequest request,
+ TListener listener) {
super(executor, request, listener);
- mTag = Objects.requireNonNull(tag);
}
/**
@@ -47,7 +47,8 @@
* with. Often this is easiest to accomplish by defining registration subclasses as non-static
* inner classes of the multiplexer they are to be used with.
*/
- protected abstract ListenerMultiplexer<?, ? super TRequest, ? super TListener, ?, ?> getOwner();
+ protected abstract ListenerMultiplexer<?, ? super TListener, ?
+ super TListenerOperation, ?, ?> getOwner();
/**
* Returns the key associated with this registration. May not be invoked before
@@ -69,12 +70,6 @@
}
@Override
- public <Listener> void onOperationFailure(ListenerOperation<Listener> operation, Exception e) {
- Log.w(mTag, "registration " + this + " removed due to unexpected exception", e);
- remove();
- }
-
- @Override
protected final void onRegister(Object key) {
mKey = Objects.requireNonNull(key);
onRemovableListenerRegister();
diff --git a/services/core/java/com/android/server/location/listeners/RequestListenerRegistration.java b/services/core/java/com/android/server/location/listeners/RequestListenerRegistration.java
new file mode 100644
index 0000000..d97abae
--- /dev/null
+++ b/services/core/java/com/android/server/location/listeners/RequestListenerRegistration.java
@@ -0,0 +1,58 @@
+/*
+ * Copyright (C) 2020 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.android.server.location.listeners;
+
+import com.android.internal.listeners.ListenerExecutor.ListenerOperation;
+
+import java.util.concurrent.Executor;
+
+/**
+ * A listener registration object which includes an associated request.
+ *
+ * @param <TRequest> request type
+ * @param <TListener> listener type
+ * @param <TListenerOperation> listener operation type
+ */
+public class RequestListenerRegistration<TRequest, TListener,
+ TListenerOperation extends ListenerOperation<TListener>> extends
+ ListenerRegistration<TListener, TListenerOperation> {
+
+ private final TRequest mRequest;
+
+ protected RequestListenerRegistration(Executor executor, TRequest request,
+ TListener listener) {
+ super(executor, listener);
+ mRequest = request;
+ }
+
+ /**
+ * Returns the request associated with this listener, or null if one wasn't supplied.
+ */
+ public TRequest getRequest() {
+ return mRequest;
+ }
+
+ @Override
+ public String toString() {
+ if (mRequest == null) {
+ return "[]";
+ } else {
+ return mRequest.toString();
+ }
+ }
+}
+
diff --git a/services/tests/mockingservicestests/src/com/android/server/location/listeners/ListenerMultiplexerTest.java b/services/tests/mockingservicestests/src/com/android/server/location/listeners/ListenerMultiplexerTest.java
index 0b5a699..29d3f29 100644
--- a/services/tests/mockingservicestests/src/com/android/server/location/listeners/ListenerMultiplexerTest.java
+++ b/services/tests/mockingservicestests/src/com/android/server/location/listeners/ListenerMultiplexerTest.java
@@ -34,6 +34,7 @@
import androidx.test.filters.SmallTest;
import androidx.test.runner.AndroidJUnit4;
+import com.android.internal.listeners.ListenerExecutor.ListenerOperation;
import com.android.server.location.listeners.ListenerMultiplexer.UpdateServiceLock;
import org.junit.Before;
@@ -318,7 +319,8 @@
}
private static class TestListenerRegistration extends
- ListenerRegistration<Integer, Consumer<TestListenerRegistration>> {
+ RequestListenerRegistration<Integer, Consumer<TestListenerRegistration>,
+ ListenerOperation<Consumer<TestListenerRegistration>>> {
boolean mActive = true;
@@ -329,8 +331,10 @@
}
private static class TestMultiplexer extends
- ListenerMultiplexer<Consumer<TestListenerRegistration>, Integer,
- Consumer<TestListenerRegistration>, TestListenerRegistration, Integer> {
+ ListenerMultiplexer<Consumer<TestListenerRegistration>,
+ Consumer<TestListenerRegistration>,
+ ListenerOperation<Consumer<TestListenerRegistration>>, TestListenerRegistration,
+ Integer> {
boolean mRegistered;
int mMergedRequest;
@@ -341,6 +345,11 @@
mCallbacks = callbacks;
}
+ @Override
+ public String getTag() {
+ return "TestMultiplexer";
+ }
+
public void addListener(Integer request, Consumer<TestListenerRegistration> consumer) {
addRegistration(consumer, new TestListenerRegistration(request, consumer));
}
@@ -369,7 +378,8 @@
}
@Override
- protected boolean registerWithService(Integer mergedRequest) {
+ protected boolean registerWithService(Integer mergedRequest,
+ Collection<TestListenerRegistration> registrations) {
mRegistered = true;
mMergedRequest = mergedRequest;
return true;
@@ -418,7 +428,8 @@
}
@Override
- protected Integer mergeRequests(Collection<TestListenerRegistration> testRegistrations) {
+ protected Integer mergeRegistrations(
+ Collection<TestListenerRegistration> testRegistrations) {
int max = Integer.MIN_VALUE;
for (TestListenerRegistration registration : testRegistrations) {
if (registration.getRequest() > max) {
