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

Commit 05f32ddf authored by Soonil Nagarkar's avatar Soonil Nagarkar
Browse files

DO NOT MERGE Fix bug in getCurrentLocation cancellation 

Cancellation was not being propagated to the system server due to a
misunderstanding on how CancellationSignal functions. There is no
noticable impact for the user, as cancellation did properly prevent any
further locations, however this would have resulted in extra
power draw since the system server would not have canceled the
associated provider location request. This would have left
getCurrentLocation() as equivalent to requestSingleUpdate() where it was
supposed to be an all-around improvement that could save power.

Bug: 168666216
Test: manual + presubmits
Change-Id: I5c77db4cc56cce1b984587f65d2bcfb488436bb8
parent 104baf80

location/java/android/location/ILocationManager.aidl

+3 −3
Original line number Original line Diff line number Diff line
@@ -45,9 +45,9 @@ import com.android.internal.location.ProviderProperties; 
interface ILocationManager

interface ILocationManager

{

{

    Location getLastLocation(in LocationRequest request, String packageName, String featureId);

    Location getLastLocation(in LocationRequest request, String packageName, String featureId);

    boolean getCurrentLocation(in LocationRequest request,

    @nullable ICancellationSignal getCurrentLocation(in LocationRequest request,

            in ICancellationSignal cancellationSignal, in ILocationListener listener,

            in ILocationListener listener, String packageName, String featureId,

            String packageName, String featureId, String listenerId);

            String listenerId);





    void requestLocationUpdates(in LocationRequest request, in ILocationListener listener,

    void requestLocationUpdates(in LocationRequest request, in ILocationListener listener,

            in PendingIntent intent, String packageName, String featureId, String listenerId);

            in PendingIntent intent, String packageName, String featureId, String listenerId);

location/java/android/location/LocationManager.java

+13 −29
Original line number Original line Diff line number Diff line
@@ -726,16 +726,15 @@ public class LocationManager { 
            cancellationSignal.throwIfCanceled();

            cancellationSignal.throwIfCanceled();

        }

        }





        ICancellationSignal remoteCancellationSignal = CancellationSignal.createTransport();



        try {

        try {

            if (mService.getCurrentLocation(currentLocationRequest, remoteCancellationSignal,

            ICancellationSignal cancelRemote = mService.getCurrentLocation(

                    transport, mContext.getPackageName(), mContext.getAttributionTag(),

                    currentLocationRequest, transport, mContext.getPackageName(),

                    transport.getListenerId())) {

                    mContext.getAttributionTag(), transport.getListenerId());

            if (cancelRemote != null) {

                transport.register(mContext.getSystemService(AlarmManager.class),

                transport.register(mContext.getSystemService(AlarmManager.class),

                        remoteCancellationSignal);

                        cancellationSignal);

                if (cancellationSignal != null) {

                if (cancellationSignal != null) {

                    cancellationSignal.setOnCancelListener(transport::cancel);

                    cancellationSignal.setRemote(cancelRemote);

                }

                }

            } else {

            } else {

                transport.fail();

                transport.fail();
@@ -2540,7 +2539,7 @@ public class LocationManager { 
    }

    }





    private static class GetCurrentLocationTransport extends ILocationListener.Stub implements

    private static class GetCurrentLocationTransport extends ILocationListener.Stub implements

            AlarmManager.OnAlarmListener {

            AlarmManager.OnAlarmListener, CancellationSignal.OnCancelListener {





        @GuardedBy("this")

        @GuardedBy("this")

        @Nullable

        @Nullable
@@ -2572,7 +2571,7 @@ public class LocationManager { 
        }

        }





        public synchronized void register(AlarmManager alarmManager,

        public synchronized void register(AlarmManager alarmManager,

                ICancellationSignal remoteCancellationSignal) {

                CancellationSignal cancellationSignal) {

            if (mConsumer == null) {

            if (mConsumer == null) {

                return;

                return;

            }

            }
@@ -2585,16 +2584,18 @@ public class LocationManager { 
                    this,

                    this,

                    null);

                    null);





            mRemoteCancellationSignal = remoteCancellationSignal;

            if (cancellationSignal != null) {

                cancellationSignal.setOnCancelListener(this);

            }

        }

        }





        public void cancel() {

        @Override

        public void onCancel() {

            remove();

            remove();

        }

        }





        private Consumer<Location> remove() {

        private Consumer<Location> remove() {

            Consumer<Location> consumer;

            Consumer<Location> consumer;

            ICancellationSignal cancellationSignal;

            synchronized (this) {

            synchronized (this) {

                mExecutor = null;

                mExecutor = null;

                consumer = mConsumer;

                consumer = mConsumer;
@@ -2604,18 +2605,6 @@ public class LocationManager { 
                    mAlarmManager.cancel(this);

                    mAlarmManager.cancel(this);

                    mAlarmManager = null;

                    mAlarmManager = null;

                }

                }



                // ensure only one cancel event will go through

                cancellationSignal = mRemoteCancellationSignal;

                mRemoteCancellationSignal = null;

            }



            if (cancellationSignal != null) {

                try {

                    cancellationSignal.cancel();

                } catch (RemoteException e) {

                    // ignore

                }

            }

            }





            return consumer;

            return consumer;
@@ -2637,11 +2626,6 @@ public class LocationManager { 




        @Override

        @Override

        public void onLocationChanged(Location location) {

        public void onLocationChanged(Location location) {

            synchronized (this) {

                // save ourselves a pointless x-process call to cancel the location request

                mRemoteCancellationSignal = null;

            }



            deliverResult(location);

            deliverResult(location);

        }

        }

services/core/java/com/android/server/location/LocationManagerService.java

+10 −11
Original line number Original line Diff line number Diff line
@@ -2064,10 +2064,12 @@ public class LocationManagerService extends ILocationManager.Stub { 
        }

        }

    }

    }





    @Nullable

    @Override

    @Override

    public boolean getCurrentLocation(LocationRequest locationRequest,

    public ICancellationSignal getCurrentLocation(LocationRequest locationRequest,

            ICancellationSignal remoteCancellationSignal, ILocationListener listener,

            ILocationListener listener, String packageName, String featureId, String listenerId) {

            String packageName, String featureId, String listenerId) {

        ICancellationSignal remoteCancellationSignal = CancellationSignal.createTransport();



        // side effect of validating locationRequest and packageName

        // side effect of validating locationRequest and packageName

        Location lastLocation = getLastLocation(locationRequest, packageName, featureId);

        Location lastLocation = getLastLocation(locationRequest, packageName, featureId);

        if (lastLocation != null) {

        if (lastLocation != null) {
@@ -2077,17 +2079,17 @@ public class LocationManagerService extends ILocationManager.Stub { 
            if (locationAgeMs < MAX_CURRENT_LOCATION_AGE_MS) {

            if (locationAgeMs < MAX_CURRENT_LOCATION_AGE_MS) {

                try {

                try {

                    listener.onLocationChanged(lastLocation);

                    listener.onLocationChanged(lastLocation);

                    return true;

                    return remoteCancellationSignal;

                } catch (RemoteException e) {

                } catch (RemoteException e) {

                    Log.w(TAG, e);

                    Log.w(TAG, e);

                    return false;

                    return null;

                }

                }

            }

            }





            if (!mAppForegroundHelper.isAppForeground(Binder.getCallingUid())) {

            if (!mAppForegroundHelper.isAppForeground(Binder.getCallingUid())) {

                if (locationAgeMs < mSettingsHelper.getBackgroundThrottleIntervalMs()) {

                if (locationAgeMs < mSettingsHelper.getBackgroundThrottleIntervalMs()) {

                    // not allowed to request new locations, so we can't return anything

                    // not allowed to request new locations, so we can't return anything

                    return false;

                    return null;

                }

                }

            }

            }

        }

        }
@@ -2095,11 +2097,8 @@ public class LocationManagerService extends ILocationManager.Stub { 
        requestLocationUpdates(locationRequest, listener, null, packageName, featureId, listenerId);

        requestLocationUpdates(locationRequest, listener, null, packageName, featureId, listenerId);

        CancellationSignal cancellationSignal = CancellationSignal.fromTransport(

        CancellationSignal cancellationSignal = CancellationSignal.fromTransport(

                remoteCancellationSignal);

                remoteCancellationSignal);

        if (cancellationSignal != null) {

        cancellationSignal.setOnCancelListener(() -> removeUpdates(listener, null));

            cancellationSignal.setOnCancelListener(

        return remoteCancellationSignal;

                    () -> removeUpdates(listener, null));

        }

        return true;

    }

    }





    @Override

    @Override