Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit a44e3df0 authored by TreeHugger Robot's avatar TreeHugger Robot Committed by Android (Google) Code Review
Browse files

Merge "Fix bug in getCurrentLocation cancellation"

parents 45704f31 43a368dc
Loading
Loading
Loading
Loading
+1 −1
Original line number Diff line number Diff line
@@ -47,7 +47,7 @@ import com.android.internal.location.ProviderProperties;
interface ILocationManager
{
    Location getLastLocation(String provider, String packageName, String attributionTag);
    void getCurrentLocation(String provider, in LocationRequest request, in ICancellationSignal cancellationSignal, in ILocationCallback callback, String packageName, String attributionTag, String listenerId);
    @nullable ICancellationSignal getCurrentLocation(String provider, in LocationRequest request, in ILocationCallback callback, String packageName, String attributionTag, String listenerId);

    void registerLocationListener(String provider, in LocationRequest request, in ILocationListener listener, String packageName, String attributionTag, String listenerId);
    void unregisterLocationListener(in ILocationListener listener);
+19 −28
Original line number Diff line number Diff line
@@ -785,22 +785,25 @@ public class LocationManager {
        Preconditions.checkArgument(provider != null, "invalid null provider");
        Preconditions.checkArgument(locationRequest != null, "invalid null location request");

        ICancellationSignal remoteCancellationSignal = CancellationSignal.createTransport();
        GetCurrentLocationTransport transport = new GetCurrentLocationTransport(executor, consumer,
                remoteCancellationSignal);

        if (cancellationSignal != null) {
            cancellationSignal.throwIfCanceled();
            cancellationSignal.setOnCancelListener(transport::cancel);
        }

        GetCurrentLocationTransport transport = new GetCurrentLocationTransport(executor, consumer,
                cancellationSignal);

        ICancellationSignal cancelRemote;
        try {
            mService.getCurrentLocation(provider, locationRequest, remoteCancellationSignal,
                    transport, mContext.getPackageName(), mContext.getAttributionTag(),
                    AppOpsManager.toReceiverId(consumer));
            cancelRemote = mService.getCurrentLocation(provider,
                    locationRequest, transport, mContext.getPackageName(),
                    mContext.getAttributionTag(), AppOpsManager.toReceiverId(consumer));
        } catch (RemoteException e) {
            throw e.rethrowFromSystemServer();
        }

        if (cancellationSignal != null) {
            cancellationSignal.setRemote(cancelRemote);
        }
    }

    /**
@@ -2511,7 +2514,7 @@ public class LocationManager {
    }

    private static class GetCurrentLocationTransport extends ILocationCallback.Stub implements
            ListenerExecutor {
            ListenerExecutor, CancellationSignal.OnCancelListener {

        private final Executor mExecutor;

@@ -2519,33 +2522,22 @@ public class LocationManager {
        @Nullable
        private Consumer<Location> mConsumer;

        @GuardedBy("this")
        @Nullable
        private ICancellationSignal mRemoteCancellationSignal;

        GetCurrentLocationTransport(Executor executor, Consumer<Location> consumer,
                ICancellationSignal remoteCancellationSignal) {
                @Nullable CancellationSignal cancellationSignal) {
            Preconditions.checkArgument(executor != null, "illegal null executor");
            Preconditions.checkArgument(consumer != null, "illegal null consumer");
            mExecutor = executor;
            mConsumer = consumer;
            mRemoteCancellationSignal = remoteCancellationSignal;

            if (cancellationSignal != null) {
                cancellationSignal.setOnCancelListener(this);
            }
        }

        public void cancel() {
            ICancellationSignal cancellationSignal;
        @Override
        public void onCancel() {
            synchronized (this) {
                cancellationSignal = mRemoteCancellationSignal;
                mConsumer = null;
                mRemoteCancellationSignal = null;
            }

            if (cancellationSignal != null) {
                try {
                    cancellationSignal.cancel();
                } catch (RemoteException e) {
                    throw e.rethrowFromSystemServer();
                }
            }
        }

@@ -2555,7 +2547,6 @@ public class LocationManager {
            synchronized (this) {
                consumer = mConsumer;
                mConsumer = null;
                mRemoteCancellationSignal = null;
            }

            executeSafely(mExecutor, () -> consumer, listener -> listener.accept(location));
+5 −5
Original line number Diff line number Diff line
@@ -701,10 +701,11 @@ public class LocationManagerService extends ILocationManager.Stub {
        return location;
    }

    @Nullable
    @Override
    public void getCurrentLocation(String provider, LocationRequest request,
            ICancellationSignal cancellationTransport, ILocationCallback consumer,
            String packageName, String attributionTag, String listenerId) {
    public ICancellationSignal getCurrentLocation(String provider, LocationRequest request,
            ILocationCallback consumer, String packageName, String attributionTag,
            String listenerId) {
        CallerIdentity identity = CallerIdentity.fromBinder(mContext, packageName, attributionTag,
                listenerId);
        int permissionLevel = LocationPermissions.getPermissionLevel(mContext, identity.getUid(),
@@ -721,8 +722,7 @@ public class LocationManagerService extends ILocationManager.Stub {
        Preconditions.checkArgument(manager != null,
                "provider \"" + provider + "\" does not exist");

        manager.getCurrentLocation(request, identity, permissionLevel, cancellationTransport,
                consumer);
        return manager.getCurrentLocation(request, identity, permissionLevel, consumer);
    }

    @Override
+17 −18
Original line number Diff line number Diff line
@@ -1424,9 +1424,9 @@ class LocationProviderManager extends
        }
    }

    public void getCurrentLocation(LocationRequest request, CallerIdentity callerIdentity,
            int permissionLevel, ICancellationSignal cancellationTransport,
            ILocationCallback callback) {
    @Nullable
    public ICancellationSignal getCurrentLocation(LocationRequest request,
            CallerIdentity callerIdentity, int permissionLevel, ILocationCallback callback) {
        if (request.getDurationMillis() > GET_CURRENT_LOCATION_MAX_TIMEOUT_MS) {
            request = new LocationRequest.Builder(request)
                    .setDurationMillis(GET_CURRENT_LOCATION_MAX_TIMEOUT_MS)
@@ -1444,15 +1444,15 @@ class LocationProviderManager extends
            if (mSettingsHelper.isLocationPackageBlacklisted(callerIdentity.getUserId(),
                    callerIdentity.getPackageName())) {
                registration.deliverLocation(null);
                return;
                return null;
            }
            if (!mUserInfoHelper.isCurrentUserId(callerIdentity.getUserId())) {
                registration.deliverLocation(null);
                return;
                return null;
            }
            if (!request.isLocationSettingsIgnored() && !isEnabled(callerIdentity.getUserId())) {
                registration.deliverLocation(null);
                return;
                return null;
            }

            Location lastLocation = getLastLocationUnsafe(callerIdentity.getUserId(),
@@ -1463,13 +1463,13 @@ class LocationProviderManager extends
                                - lastLocation.getElapsedRealtimeNanos());
                if (locationAgeMs < MAX_CURRENT_LOCATION_AGE_MS) {
                    registration.deliverLocation(lastLocation);
                    return;
                    return null;
                }

                if (!mAppForegroundHelper.isAppForeground(Binder.getCallingUid())
                        && locationAgeMs < mSettingsHelper.getBackgroundThrottleIntervalMs()) {
                    registration.deliverLocation(null);
                    return;
                    return null;
                }
            }

@@ -1482,16 +1482,15 @@ class LocationProviderManager extends
            }
        }

        CancellationSignal cancellationSignal = CancellationSignal.fromTransport(
                cancellationTransport);
        if (cancellationSignal != null) {
        ICancellationSignal cancelTransport = CancellationSignal.createTransport();
        CancellationSignal cancellationSignal = CancellationSignal.fromTransport(cancelTransport);
        cancellationSignal.setOnCancelListener(SingleUseCallback.wrap(
                () -> {
                    synchronized (mLock) {
                        removeRegistration(callback.asBinder(), registration);
                    }
                }));
        }
        return cancelTransport;
    }

    public void sendExtraCommand(int uid, int pid, String command, Bundle extras) {
+7 −19
Original line number Diff line number Diff line
@@ -66,7 +66,6 @@ import android.location.LocationManagerInternal.ProviderEnabledListener;
import android.location.LocationRequest;
import android.location.util.identity.CallerIdentity;
import android.os.Bundle;
import android.os.CancellationSignal;
import android.os.Handler;
import android.os.ICancellationSignal;
import android.os.IRemoteCallback;
@@ -616,9 +615,7 @@ public class LocationProviderManagerTest {

        ILocationCallback listener = createMockGetCurrentLocationListener();
        LocationRequest locationRequest = new LocationRequest.Builder(0).build();
        ICancellationSignal cancellationSignal = CancellationSignal.createTransport();
        mManager.getCurrentLocation(locationRequest, IDENTITY,
                PERMISSION_FINE, cancellationSignal, listener);
        mManager.getCurrentLocation(locationRequest, IDENTITY, PERMISSION_FINE, listener);

        Location loc = createLocation(NAME, mRandom);
        mProvider.setProviderLocation(loc);
@@ -632,9 +629,8 @@ public class LocationProviderManagerTest {
    public void testGetCurrentLocation_Cancel() throws Exception {
        ILocationCallback listener = createMockGetCurrentLocationListener();
        LocationRequest locationRequest = new LocationRequest.Builder(0).build();
        ICancellationSignal cancellationSignal = CancellationSignal.createTransport();
        mManager.getCurrentLocation(locationRequest, IDENTITY,
                PERMISSION_FINE, cancellationSignal, listener);
        ICancellationSignal cancellationSignal = mManager.getCurrentLocation(locationRequest,
                IDENTITY, PERMISSION_FINE, listener);

        cancellationSignal.cancel();
        mProvider.setProviderLocation(createLocation(NAME, mRandom));
@@ -646,9 +642,7 @@ public class LocationProviderManagerTest {
    public void testGetCurrentLocation_ProviderDisabled() throws Exception {
        ILocationCallback listener = createMockGetCurrentLocationListener();
        LocationRequest locationRequest = new LocationRequest.Builder(0).build();
        ICancellationSignal cancellationSignal = CancellationSignal.createTransport();
        mManager.getCurrentLocation(locationRequest, IDENTITY,
                PERMISSION_FINE, cancellationSignal, listener);
        mManager.getCurrentLocation(locationRequest, IDENTITY, PERMISSION_FINE, listener);

        mProvider.setProviderAllowed(false);
        mProvider.setProviderAllowed(true);
@@ -662,9 +656,7 @@ public class LocationProviderManagerTest {

        ILocationCallback listener = createMockGetCurrentLocationListener();
        LocationRequest locationRequest = new LocationRequest.Builder(0).build();
        ICancellationSignal cancellationSignal = CancellationSignal.createTransport();
        mManager.getCurrentLocation(locationRequest, IDENTITY,
                PERMISSION_FINE, cancellationSignal, listener);
        mManager.getCurrentLocation(locationRequest, IDENTITY, PERMISSION_FINE, listener);

        mProvider.setProviderAllowed(true);
        mProvider.setProviderLocation(createLocation(NAME, mRandom));
@@ -680,9 +672,7 @@ public class LocationProviderManagerTest {

        ILocationCallback listener = createMockGetCurrentLocationListener();
        LocationRequest locationRequest = new LocationRequest.Builder(0).build();
        ICancellationSignal cancellationSignal = CancellationSignal.createTransport();
        mManager.getCurrentLocation(locationRequest, IDENTITY,
                PERMISSION_FINE, cancellationSignal, listener);
        mManager.getCurrentLocation(locationRequest, IDENTITY, PERMISSION_FINE, listener);

        verify(listener, times(1)).onLocation(locationCaptor.capture());
        assertThat(locationCaptor.getValue()).isEqualTo(loc);
@@ -692,9 +682,7 @@ public class LocationProviderManagerTest {
    public void testGetCurrentLocation_Timeout() throws Exception {
        ILocationCallback listener = createMockGetCurrentLocationListener();
        LocationRequest locationRequest = new LocationRequest.Builder(0).build();
        ICancellationSignal cancellationSignal = CancellationSignal.createTransport();
        mManager.getCurrentLocation(locationRequest, IDENTITY,
                PERMISSION_FINE, cancellationSignal, listener);
        mManager.getCurrentLocation(locationRequest, IDENTITY, PERMISSION_FINE, listener);

        ArgumentCaptor<OnAlarmListener> listenerCapture = ArgumentCaptor.forClass(
                OnAlarmListener.class);