diff --git a/location/java/android/location/AbstractListenerManager.java b/location/java/android/location/AbstractListenerManager.java index f075a53829ce0..3dc7cfce2d921 100644 --- a/location/java/android/location/AbstractListenerManager.java +++ b/location/java/android/location/AbstractListenerManager.java @@ -16,6 +16,8 @@ package android.location; +import static com.android.internal.util.function.pooled.PooledLambda.obtainRunnable; + import android.annotation.NonNull; import android.annotation.Nullable; import android.os.Binder; @@ -62,20 +64,24 @@ abstract class AbstractListenerManager { } private void execute(Consumer operation) { - mExecutor.execute(() -> { - TListener listener = mListener; - if (listener == null) { - return; - } + mExecutor.execute( + obtainRunnable(Registration::accept, this, operation) + .recycleOnUse()); + } - // we may be under the binder identity if a direct executor is used - long identity = Binder.clearCallingIdentity(); - try { - operation.accept(listener); - } finally { - Binder.restoreCallingIdentity(identity); - } - }); + private void accept(Consumer operation) { + TListener listener = mListener; + if (listener == null) { + return; + } + + // we may be under the binder identity if a direct executor is used + long identity = Binder.clearCallingIdentity(); + try { + operation.accept(listener); + } finally { + Binder.restoreCallingIdentity(identity); + } } } diff --git a/location/java/android/location/LocationManager.java b/location/java/android/location/LocationManager.java index 3d0765bb08557..03e1c758feb39 100644 --- a/location/java/android/location/LocationManager.java +++ b/location/java/android/location/LocationManager.java @@ -22,6 +22,8 @@ import static android.Manifest.permission.LOCATION_HARDWARE; import static android.Manifest.permission.WRITE_SECURE_SETTINGS; import static android.app.AlarmManager.ELAPSED_REALTIME; +import static com.android.internal.util.function.pooled.PooledLambda.obtainRunnable; + import android.Manifest; import android.annotation.CallbackExecutor; import android.annotation.NonNull; @@ -57,6 +59,7 @@ import android.util.ArrayMap; import com.android.internal.annotations.GuardedBy; import com.android.internal.location.ProviderProperties; import com.android.internal.util.Preconditions; +import com.android.internal.util.function.pooled.PooledRunnable; import java.util.Collections; import java.util.List; @@ -81,8 +84,6 @@ import java.util.function.Consumer; @RequiresFeature(PackageManager.FEATURE_LOCATION) public class LocationManager { - private static final String TAG = "LocationManager"; - /** * For apps targeting Android K and above, supplied {@link PendingIntent}s must be targeted to a * specific package. @@ -91,7 +92,7 @@ public class LocationManager { */ @ChangeId @EnabledAfter(targetSdkVersion = Build.VERSION_CODES.JELLY_BEAN) - public static final long TARGETED_PENDING_INTENT = 148963590L; + private static final long TARGETED_PENDING_INTENT = 148963590L; /** * For apps targeting Android K and above, incomplete locations may not be passed to @@ -2604,18 +2605,28 @@ public class LocationManager { return; } - mExecutor.execute(() -> { - Consumer consumer; - synchronized (GetCurrentLocationTransport.this) { - if (mConsumer == null) { - return; - } - consumer = mConsumer; - cancel(); - } + PooledRunnable runnable = + obtainRunnable(GetCurrentLocationTransport::acceptResult, this, location) + .recycleOnUse(); + try { + mExecutor.execute(runnable); + } catch (RejectedExecutionException e) { + runnable.recycle(); + throw e; + } + } - consumer.accept(location); - }); + private void acceptResult(Location location) { + Consumer consumer; + synchronized (this) { + if (mConsumer == null) { + return; + } + consumer = mConsumer; + cancel(); + } + + consumer.accept(location); } } @@ -2649,30 +2660,36 @@ public class LocationManager { return; } + PooledRunnable runnable = + obtainRunnable(LocationListenerTransport::acceptLocation, this, currentExecutor, + location).recycleOnUse(); try { - currentExecutor.execute(() -> { - try { - if (currentExecutor != mExecutor) { - return; - } - - // we may be under the binder identity if a direct executor is used - long identity = Binder.clearCallingIdentity(); - try { - mListener.onLocationChanged(location); - } finally { - Binder.restoreCallingIdentity(identity); - } - } finally { - locationCallbackFinished(); - } - }); + currentExecutor.execute(runnable); } catch (RejectedExecutionException e) { + runnable.recycle(); locationCallbackFinished(); throw e; } } + private void acceptLocation(Executor currentExecutor, Location location) { + try { + if (currentExecutor != mExecutor) { + return; + } + + // we may be under the binder identity if a direct executor is used + long identity = Binder.clearCallingIdentity(); + try { + mListener.onLocationChanged(location); + } finally { + Binder.restoreCallingIdentity(identity); + } + } finally { + locationCallbackFinished(); + } + } + @Override public void onProviderEnabled(String provider) { Executor currentExecutor = mExecutor; @@ -2680,25 +2697,13 @@ public class LocationManager { return; } + PooledRunnable runnable = + obtainRunnable(LocationListenerTransport::acceptProviderChange, this, + currentExecutor, provider, true).recycleOnUse(); try { - currentExecutor.execute(() -> { - try { - if (currentExecutor != mExecutor) { - return; - } - - // we may be under the binder identity if a direct executor is used - long identity = Binder.clearCallingIdentity(); - try { - mListener.onProviderEnabled(provider); - } finally { - Binder.restoreCallingIdentity(identity); - } - } finally { - locationCallbackFinished(); - } - }); + currentExecutor.execute(runnable); } catch (RejectedExecutionException e) { + runnable.recycle(); locationCallbackFinished(); throw e; } @@ -2711,30 +2716,41 @@ public class LocationManager { return; } + PooledRunnable runnable = + obtainRunnable(LocationListenerTransport::acceptProviderChange, this, + currentExecutor, provider, false).recycleOnUse(); try { - currentExecutor.execute(() -> { - try { - if (currentExecutor != mExecutor) { - return; - } - - // we may be under the binder identity if a direct executor is used - long identity = Binder.clearCallingIdentity(); - try { - mListener.onProviderDisabled(provider); - } finally { - Binder.restoreCallingIdentity(identity); - } - } finally { - locationCallbackFinished(); - } - }); + currentExecutor.execute(runnable); } catch (RejectedExecutionException e) { + runnable.recycle(); locationCallbackFinished(); throw e; } } + private void acceptProviderChange(Executor currentExecutor, String provider, + boolean enabled) { + try { + if (currentExecutor != mExecutor) { + return; + } + + // we may be under the binder identity if a direct executor is used + long identity = Binder.clearCallingIdentity(); + try { + if (enabled) { + mListener.onProviderEnabled(provider); + } else { + mListener.onProviderDisabled(provider); + } + } finally { + Binder.restoreCallingIdentity(identity); + } + } finally { + locationCallbackFinished(); + } + } + @Override public void onRemoved() { // TODO: onRemoved is necessary to GC hanging listeners, but introduces some interesting diff --git a/services/core/java/com/android/server/location/AbstractLocationProvider.java b/services/core/java/com/android/server/location/AbstractLocationProvider.java index 997f21c9b6a3d..433ec435c8d1d 100644 --- a/services/core/java/com/android/server/location/AbstractLocationProvider.java +++ b/services/core/java/com/android/server/location/AbstractLocationProvider.java @@ -16,6 +16,8 @@ package com.android.server.location; +import static com.android.internal.util.function.pooled.PooledLambda.obtainRunnable; + import android.annotation.Nullable; import android.content.Context; import android.location.Location; @@ -335,7 +337,8 @@ public abstract class AbstractLocationProvider { */ public final void setRequest(ProviderRequest request) { // all calls into the provider must be moved onto the provider thread to prevent deadlock - mExecutor.execute(() -> onSetRequest(request)); + mExecutor.execute(obtainRunnable(AbstractLocationProvider::onSetRequest, this, request) + .recycleOnUse()); } /** @@ -348,7 +351,13 @@ public abstract class AbstractLocationProvider { */ 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 - mExecutor.execute(() -> onExtraCommand(uid, pid, command, extras)); + + // 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()); } /** @@ -361,7 +370,9 @@ public abstract class AbstractLocationProvider { */ public final void requestSetAllowed(boolean allowed) { // all calls into the provider must be moved onto the provider thread to prevent deadlock - mExecutor.execute(() -> onRequestSetAllowed(allowed)); + mExecutor.execute( + obtainRunnable(AbstractLocationProvider::onRequestSetAllowed, this, allowed) + .recycleOnUse()); } /** diff --git a/services/core/java/com/android/server/location/SettingsHelper.java b/services/core/java/com/android/server/location/SettingsHelper.java index 370a3f762d32d..5fe21bdf89500 100644 --- a/services/core/java/com/android/server/location/SettingsHelper.java +++ b/services/core/java/com/android/server/location/SettingsHelper.java @@ -27,6 +27,9 @@ import static android.provider.Settings.Secure.LOCATION_COARSE_ACCURACY_M; import static android.provider.Settings.Secure.LOCATION_MODE; import static android.provider.Settings.Secure.LOCATION_MODE_OFF; +import static com.android.server.LocationManagerService.D; +import static com.android.server.LocationManagerService.TAG; + import android.app.ActivityManager; import android.content.Context; import android.database.ContentObserver; @@ -37,6 +40,7 @@ import android.os.UserHandle; import android.provider.Settings; import android.text.TextUtils; import android.util.ArraySet; +import android.util.Log; import com.android.internal.annotations.GuardedBy; import com.android.internal.util.IndentingPrintWriter; @@ -423,6 +427,10 @@ public class SettingsHelper { @Override public void onChange(boolean selfChange, Uri uri, int userId) { + if (D) { + Log.d(TAG, "location setting changed [u" + userId + "]: " + uri); + } + for (UserSettingChangedListener listener : mListeners) { listener.onSettingChanged(userId); } diff --git a/services/core/java/com/android/server/location/UserInfoHelper.java b/services/core/java/com/android/server/location/UserInfoHelper.java index a33e2da4eb87d..28f3f476847b9 100644 --- a/services/core/java/com/android/server/location/UserInfoHelper.java +++ b/services/core/java/com/android/server/location/UserInfoHelper.java @@ -18,6 +18,9 @@ package com.android.server.location; import static android.os.UserManager.DISALLOW_SHARE_LOCATION; +import static com.android.server.LocationManagerService.D; +import static com.android.server.LocationManagerService.TAG; + import android.annotation.IntDef; import android.annotation.Nullable; import android.annotation.UserIdInt; @@ -30,6 +33,7 @@ import android.os.Binder; import android.os.Build; import android.os.UserHandle; import android.os.UserManager; +import android.util.Log; import com.android.internal.annotations.GuardedBy; import com.android.internal.util.ArrayUtils; @@ -122,13 +126,13 @@ public class UserInfoHelper { case Intent.ACTION_USER_STARTED: userId = intent.getIntExtra(Intent.EXTRA_USER_HANDLE, UserHandle.USER_NULL); if (userId != UserHandle.USER_NULL) { - onUserChanged(userId, UserListener.USER_STARTED); + onUserStarted(userId); } break; case Intent.ACTION_USER_STOPPED: userId = intent.getIntExtra(Intent.EXTRA_USER_HANDLE, UserHandle.USER_NULL); if (userId != UserHandle.USER_NULL) { - onUserChanged(userId, UserListener.USER_STOPPED); + onUserStopped(userId); } break; case Intent.ACTION_MANAGED_PROFILE_ADDED: @@ -161,6 +165,10 @@ public class UserInfoHelper { return; } + if (D) { + Log.d(TAG, "current user switched from u" + mCurrentUserId + " to u" + newUserId); + } + int oldUserId = mCurrentUserId; mCurrentUserId = newUserId; @@ -168,6 +176,22 @@ public class UserInfoHelper { onUserChanged(newUserId, UserListener.USER_SWITCHED); } + private void onUserStarted(@UserIdInt int userId) { + if (D) { + Log.d(TAG, "u" + userId + " started"); + } + + onUserChanged(userId, UserListener.USER_STARTED); + } + + private void onUserStopped(@UserIdInt int userId) { + if (D) { + Log.d(TAG, "u" + userId + " stopped"); + } + + onUserChanged(userId, UserListener.USER_STOPPED); + } + private void onUserChanged(@UserIdInt int userId, @UserListener.UserChange int change) { for (UserListener listener : mListeners) { listener.onUserChanged(userId, change);