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

Commit 1bd60d2d authored by Soonil Nagarkar's avatar Soonil Nagarkar
Browse files

Fix bug in high power location attribution

Bug: 207463764
Test: presubmits
Change-Id: I6b29a03f6858a2cb3eced09402eb9d6da46a0ac0
parent 32e61218
Loading
Loading
Loading
Loading
+2 −6
Original line number Diff line number Diff line
@@ -52,8 +52,6 @@ public final class GnssMeasurementsProvider extends

    private class GnssMeasurementListenerRegistration extends GnssListenerRegistration {

        private static final String GNSS_MEASUREMENTS_BUCKET = "gnss_measurement";

        protected GnssMeasurementListenerRegistration(
                @Nullable GnssMeasurementRequest request,
                CallerIdentity callerIdentity,
@@ -70,15 +68,13 @@ public final class GnssMeasurementsProvider extends
        @Nullable
        @Override
        protected void onActive() {
            mLocationAttributionHelper.reportHighPowerLocationStart(
                    getIdentity(), GNSS_MEASUREMENTS_BUCKET, getKey());
            mLocationAttributionHelper.reportHighPowerLocationStart(getIdentity());
        }

        @Nullable
        @Override
        protected void onInactive() {
            mLocationAttributionHelper.reportHighPowerLocationStop(
                    getIdentity(), GNSS_MEASUREMENTS_BUCKET, getKey());
            mLocationAttributionHelper.reportHighPowerLocationStop(getIdentity());
        }
    }

+36 −63
Original line number Diff line number Diff line
@@ -24,55 +24,23 @@ import static com.android.server.location.LocationManagerService.TAG;

import android.location.util.identity.CallerIdentity;
import android.util.ArrayMap;
import android.util.ArraySet;
import android.util.Log;

import com.android.internal.annotations.GuardedBy;

import java.util.Map;
import java.util.Objects;
import java.util.Set;

/**
 * Helps manage appop monitoring for multiple location clients.
 */
public class LocationAttributionHelper {

    private static class BucketKey {
        private final String mBucket;
        private final Object mKey;

        private BucketKey(String bucket, Object key) {
            mBucket = Objects.requireNonNull(bucket);
            mKey = Objects.requireNonNull(key);
        }

        @Override
        public boolean equals(Object o) {
            if (this == o) {
                return true;
            }
            if (o == null || getClass() != o.getClass()) {
                return false;
            }

            BucketKey that = (BucketKey) o;
            return mBucket.equals(that.mBucket)
                    && mKey.equals(that.mKey);
        }

        @Override
        public int hashCode() {
            return Objects.hash(mBucket, mKey);
        }
    }

    private final AppOpsHelper mAppOpsHelper;

    @GuardedBy("this")
    private final Map<CallerIdentity, Set<BucketKey>> mAttributions;
    private final Map<CallerIdentity, Integer> mAttributions;
    @GuardedBy("this")
    private final Map<CallerIdentity, Set<BucketKey>> mHighPowerAttributions;
    private final Map<CallerIdentity, Integer> mHighPowerAttributions;

    public LocationAttributionHelper(AppOpsHelper appOpsHelper) {
        mAppOpsHelper = appOpsHelper;
@@ -84,15 +52,16 @@ public class LocationAttributionHelper {
    /**
     * Report normal location usage for the given caller in the given bucket, with a unique key.
     */
    public synchronized void reportLocationStart(CallerIdentity identity, String bucket,
            Object key) {
        Set<BucketKey> keySet = mAttributions.computeIfAbsent(identity,
                i -> new ArraySet<>());
        boolean empty = keySet.isEmpty();
        if (keySet.add(new BucketKey(bucket, key)) && empty) {
            if (!mAppOpsHelper.startOpNoThrow(OP_MONITOR_LOCATION, identity)) {
                mAttributions.remove(identity);
    public synchronized void reportLocationStart(CallerIdentity identity) {
        identity = CallerIdentity.forAggregation(identity);

        int count = mAttributions.getOrDefault(identity, 0);
        if (count == 0) {
            if (mAppOpsHelper.startOpNoThrow(OP_MONITOR_LOCATION, identity)) {
                mAttributions.put(identity, 1);
            }
        } else {
            mAttributions.put(identity, count + 1);
        }
    }

@@ -100,13 +69,15 @@ public class LocationAttributionHelper {
     * Report normal location usage has stopped for the given caller in the given bucket, with a
     * unique key.
     */
    public synchronized void reportLocationStop(CallerIdentity identity, String bucket,
            Object key) {
        Set<BucketKey> keySet = mAttributions.get(identity);
        if (keySet != null && keySet.remove(new BucketKey(bucket, key))
                && keySet.isEmpty()) {
    public synchronized void reportLocationStop(CallerIdentity identity) {
        identity = CallerIdentity.forAggregation(identity);

        int count = mAttributions.getOrDefault(identity, 0);
        if (count == 1) {
            mAttributions.remove(identity);
            mAppOpsHelper.finishOp(OP_MONITOR_LOCATION, identity);
        } else if (count > 1) {
            mAttributions.put(identity, count - 1);
        }
    }

@@ -114,19 +85,19 @@ public class LocationAttributionHelper {
     * Report high power location usage for the given caller in the given bucket, with a unique
     * key.
     */
    public synchronized void reportHighPowerLocationStart(CallerIdentity identity, String bucket,
            Object key) {
        Set<BucketKey> keySet = mHighPowerAttributions.computeIfAbsent(identity,
                i -> new ArraySet<>());
        boolean empty = keySet.isEmpty();
        if (keySet.add(new BucketKey(bucket, key)) && empty) {
            if (mAppOpsHelper.startOpNoThrow(OP_MONITOR_HIGH_POWER_LOCATION, identity)) {
    public synchronized void reportHighPowerLocationStart(CallerIdentity identity) {
        identity = CallerIdentity.forAggregation(identity);

        int count = mHighPowerAttributions.getOrDefault(identity, 0);
        if (count == 0) {
            if (D) {
                Log.v(TAG, "starting high power location attribution for " + identity);
            }
            } else {
                mHighPowerAttributions.remove(identity);
            if (mAppOpsHelper.startOpNoThrow(OP_MONITOR_HIGH_POWER_LOCATION, identity)) {
                mHighPowerAttributions.put(identity, 1);
            }
        } else {
            mHighPowerAttributions.put(identity, count + 1);
        }
    }

@@ -134,16 +105,18 @@ public class LocationAttributionHelper {
     * Report high power location usage has stopped for the given caller in the given bucket,
     * with a unique key.
     */
    public synchronized void reportHighPowerLocationStop(CallerIdentity identity, String bucket,
            Object key) {
        Set<BucketKey> keySet = mHighPowerAttributions.get(identity);
        if (keySet != null && keySet.remove(new BucketKey(bucket, key))
                && keySet.isEmpty()) {
    public synchronized void reportHighPowerLocationStop(CallerIdentity identity) {
        identity = CallerIdentity.forAggregation(identity);

        int count = mHighPowerAttributions.getOrDefault(identity, 0);
        if (count == 1) {
            if (D) {
                Log.v(TAG, "stopping high power location attribution for " + identity);
            }
            mHighPowerAttributions.remove(identity);
            mAppOpsHelper.finishOp(OP_MONITOR_HIGH_POWER_LOCATION, identity);
        } else if (count > 1) {
            mHighPowerAttributions.put(identity, count - 1);
        }
    }
}
+4 −4
Original line number Diff line number Diff line
@@ -398,7 +398,7 @@ public class LocationProviderManager extends
            EVENT_LOG.logProviderClientActive(mName, getIdentity());

            if (!getRequest().isHiddenFromAppOps()) {
                mLocationAttributionHelper.reportLocationStart(getIdentity(), getName(), getKey());
                mLocationAttributionHelper.reportLocationStart(getIdentity());
            }
            onHighPowerUsageChanged();

@@ -413,7 +413,7 @@ public class LocationProviderManager extends

            onHighPowerUsageChanged();
            if (!getRequest().isHiddenFromAppOps()) {
                mLocationAttributionHelper.reportLocationStop(getIdentity(), getName(), getKey());
                mLocationAttributionHelper.reportLocationStop(getIdentity());
            }

            onProviderListenerInactive();
@@ -488,10 +488,10 @@ public class LocationProviderManager extends
                if (!getRequest().isHiddenFromAppOps()) {
                    if (mIsUsingHighPower) {
                        mLocationAttributionHelper.reportHighPowerLocationStart(
                                getIdentity(), getName(), getKey());
                                getIdentity());
                    } else {
                        mLocationAttributionHelper.reportHighPowerLocationStop(
                                getIdentity(), getName(), getKey());
                                getIdentity());
                    }
                }
            }
+74 −60
Original line number Diff line number Diff line
@@ -58,72 +58,86 @@ public class LocationAttributionHelperTest {
    @Test
    public void testLocationMonitoring() {
        CallerIdentity caller1 = CallerIdentity.forTest(1, 1, "test1", null);
        Object key1 = new Object();
        Object key2 = new Object();
        CallerIdentity caller2 = CallerIdentity.forTest(2, 2, "test2", null);
        Object key3 = new Object();
        Object key4 = new Object();

        mHelper.reportLocationStart(caller1, "gps", key1);
        verify(mAppOpsHelper).startOpNoThrow(OP_MONITOR_LOCATION, caller1);
        verify(mAppOpsHelper, never()).finishOp(OP_MONITOR_LOCATION, caller1);

        mHelper.reportLocationStart(caller1, "gps", key2);
        verify(mAppOpsHelper).startOpNoThrow(OP_MONITOR_LOCATION, caller1);
        verify(mAppOpsHelper, never()).finishOp(OP_MONITOR_LOCATION, caller1);

        mHelper.reportLocationStart(caller2, "gps", key3);
        verify(mAppOpsHelper).startOpNoThrow(OP_MONITOR_LOCATION, caller2);
        verify(mAppOpsHelper, never()).finishOp(OP_MONITOR_LOCATION, caller2);

        mHelper.reportLocationStart(caller2, "gps", key4);
        verify(mAppOpsHelper).startOpNoThrow(OP_MONITOR_LOCATION, caller2);
        verify(mAppOpsHelper, never()).finishOp(OP_MONITOR_LOCATION, caller2);

        mHelper.reportLocationStop(caller1, "gps", key2);
        verify(mAppOpsHelper, never()).finishOp(OP_MONITOR_LOCATION, caller1);
        mHelper.reportLocationStop(caller1, "gps", key1);
        verify(mAppOpsHelper).finishOp(OP_MONITOR_LOCATION, caller1);

        mHelper.reportLocationStop(caller2, "gps", key3);
        verify(mAppOpsHelper, never()).finishOp(OP_MONITOR_LOCATION, caller2);
        mHelper.reportLocationStop(caller2, "gps", key4);
        verify(mAppOpsHelper).finishOp(OP_MONITOR_LOCATION, caller2);

        mHelper.reportLocationStart(caller1);
        verify(mAppOpsHelper).startOpNoThrow(OP_MONITOR_LOCATION,
                CallerIdentity.forAggregation(caller1));
        verify(mAppOpsHelper, never()).finishOp(OP_MONITOR_LOCATION,
                CallerIdentity.forAggregation(caller1));

        mHelper.reportLocationStart(caller1);
        verify(mAppOpsHelper).startOpNoThrow(OP_MONITOR_LOCATION,
                CallerIdentity.forAggregation(caller1));
        verify(mAppOpsHelper, never()).finishOp(OP_MONITOR_LOCATION,
                CallerIdentity.forAggregation(caller1));

        mHelper.reportLocationStart(caller2);
        verify(mAppOpsHelper).startOpNoThrow(OP_MONITOR_LOCATION,
                CallerIdentity.forAggregation(caller2));
        verify(mAppOpsHelper, never()).finishOp(OP_MONITOR_LOCATION,
                CallerIdentity.forAggregation(caller2));

        mHelper.reportLocationStart(caller2);
        verify(mAppOpsHelper).startOpNoThrow(OP_MONITOR_LOCATION,
                CallerIdentity.forAggregation(caller2));
        verify(mAppOpsHelper, never()).finishOp(OP_MONITOR_LOCATION,
                CallerIdentity.forAggregation(caller2));

        mHelper.reportLocationStop(caller1);
        verify(mAppOpsHelper, never()).finishOp(OP_MONITOR_LOCATION,
                CallerIdentity.forAggregation(caller1));
        mHelper.reportLocationStop(caller1);
        verify(mAppOpsHelper).finishOp(OP_MONITOR_LOCATION, CallerIdentity.forAggregation(caller1));

        mHelper.reportLocationStop(caller2);
        verify(mAppOpsHelper, never()).finishOp(OP_MONITOR_LOCATION,
                CallerIdentity.forAggregation(caller2));
        mHelper.reportLocationStop(caller2);
        verify(mAppOpsHelper).finishOp(OP_MONITOR_LOCATION, CallerIdentity.forAggregation(caller2));
    }

    @Test
    public void testHighPowerLocationMonitoring() {
        CallerIdentity caller1 = CallerIdentity.forTest(1, 1, "test1", null);
        Object key1 = new Object();
        Object key2 = new Object();
        CallerIdentity caller2 = CallerIdentity.forTest(2, 2, "test2", null);
        Object key3 = new Object();
        Object key4 = new Object();

        mHelper.reportHighPowerLocationStart(caller1, "gps", key1);
        verify(mAppOpsHelper).startOpNoThrow(OP_MONITOR_HIGH_POWER_LOCATION, caller1);
        verify(mAppOpsHelper, never()).finishOp(OP_MONITOR_HIGH_POWER_LOCATION, caller1);

        mHelper.reportHighPowerLocationStart(caller1, "gps", key2);
        verify(mAppOpsHelper).startOpNoThrow(OP_MONITOR_HIGH_POWER_LOCATION, caller1);
        verify(mAppOpsHelper, never()).finishOp(OP_MONITOR_HIGH_POWER_LOCATION, caller1);

        mHelper.reportHighPowerLocationStart(caller2, "gps", key3);
        verify(mAppOpsHelper).startOpNoThrow(OP_MONITOR_HIGH_POWER_LOCATION, caller2);
        verify(mAppOpsHelper, never()).finishOp(OP_MONITOR_HIGH_POWER_LOCATION, caller2);

        mHelper.reportHighPowerLocationStart(caller2, "gps", key4);
        verify(mAppOpsHelper).startOpNoThrow(OP_MONITOR_HIGH_POWER_LOCATION, caller2);
        verify(mAppOpsHelper, never()).finishOp(OP_MONITOR_HIGH_POWER_LOCATION, caller2);

        mHelper.reportHighPowerLocationStop(caller1, "gps", key2);
        verify(mAppOpsHelper, never()).finishOp(OP_MONITOR_HIGH_POWER_LOCATION, caller1);
        mHelper.reportHighPowerLocationStop(caller1, "gps", key1);
        verify(mAppOpsHelper).finishOp(OP_MONITOR_HIGH_POWER_LOCATION, caller1);

        mHelper.reportHighPowerLocationStop(caller2, "gps", key3);
        verify(mAppOpsHelper, never()).finishOp(OP_MONITOR_HIGH_POWER_LOCATION, caller2);
        mHelper.reportHighPowerLocationStop(caller2, "gps", key4);
        verify(mAppOpsHelper).finishOp(OP_MONITOR_HIGH_POWER_LOCATION, caller2);

        mHelper.reportHighPowerLocationStart(caller1);
        verify(mAppOpsHelper).startOpNoThrow(OP_MONITOR_HIGH_POWER_LOCATION,
                CallerIdentity.forAggregation(caller1));
        verify(mAppOpsHelper, never()).finishOp(OP_MONITOR_HIGH_POWER_LOCATION,
                CallerIdentity.forAggregation(caller1));

        mHelper.reportHighPowerLocationStart(caller1);
        verify(mAppOpsHelper).startOpNoThrow(OP_MONITOR_HIGH_POWER_LOCATION,
                CallerIdentity.forAggregation(caller1));
        verify(mAppOpsHelper, never()).finishOp(OP_MONITOR_HIGH_POWER_LOCATION,
                CallerIdentity.forAggregation(caller1));

        mHelper.reportHighPowerLocationStart(caller2);
        verify(mAppOpsHelper).startOpNoThrow(OP_MONITOR_HIGH_POWER_LOCATION,
                CallerIdentity.forAggregation(caller2));
        verify(mAppOpsHelper, never()).finishOp(OP_MONITOR_HIGH_POWER_LOCATION,
                CallerIdentity.forAggregation(caller2));

        mHelper.reportHighPowerLocationStart(caller2);
        verify(mAppOpsHelper).startOpNoThrow(OP_MONITOR_HIGH_POWER_LOCATION,
                CallerIdentity.forAggregation(caller2));
        verify(mAppOpsHelper, never()).finishOp(OP_MONITOR_HIGH_POWER_LOCATION,
                CallerIdentity.forAggregation(caller2));

        mHelper.reportHighPowerLocationStop(caller1);
        verify(mAppOpsHelper, never()).finishOp(OP_MONITOR_HIGH_POWER_LOCATION,
                CallerIdentity.forAggregation(caller1));
        mHelper.reportHighPowerLocationStop(caller1);
        verify(mAppOpsHelper).finishOp(OP_MONITOR_HIGH_POWER_LOCATION,
                CallerIdentity.forAggregation(caller1));

        mHelper.reportHighPowerLocationStop(caller2);
        verify(mAppOpsHelper, never()).finishOp(OP_MONITOR_HIGH_POWER_LOCATION,
                CallerIdentity.forAggregation(caller2));
        mHelper.reportHighPowerLocationStop(caller2);
        verify(mAppOpsHelper).finishOp(OP_MONITOR_HIGH_POWER_LOCATION,
                CallerIdentity.forAggregation(caller2));
    }
}
+42 −0
Original line number Diff line number Diff line
@@ -844,6 +844,48 @@ public class LocationProviderManagerTest {
                IDENTITY.getPackageName())).isFalse();
    }

    @Test
    public void testLocationMonitoring_multipleIdentities() {
        CallerIdentity identity1 = CallerIdentity.forTest(CURRENT_USER, 1,
                "mypackage", "attribution", "listener1");
        CallerIdentity identity2 = CallerIdentity.forTest(CURRENT_USER, 1,
                "mypackage", "attribution", "listener2");

        assertThat(mInjector.getAppOpsHelper().isAppOpStarted(OP_MONITOR_LOCATION,
                IDENTITY.getPackageName())).isFalse();
        assertThat(mInjector.getAppOpsHelper().isAppOpStarted(OP_MONITOR_HIGH_POWER_LOCATION,
                IDENTITY.getPackageName())).isFalse();

        ILocationListener listener1 = createMockLocationListener();
        LocationRequest request1 = new LocationRequest.Builder(0).setWorkSource(
                WORK_SOURCE).build();
        mManager.registerLocationRequest(request1, identity1, PERMISSION_FINE, listener1);

        ILocationListener listener2 = createMockLocationListener();
        LocationRequest request2 = new LocationRequest.Builder(0).setWorkSource(
                WORK_SOURCE).build();
        mManager.registerLocationRequest(request2, identity2, PERMISSION_FINE, listener2);

        assertThat(mInjector.getAppOpsHelper().isAppOpStarted(OP_MONITOR_LOCATION,
                "mypackage")).isTrue();
        assertThat(mInjector.getAppOpsHelper().isAppOpStarted(OP_MONITOR_HIGH_POWER_LOCATION,
                "mypackage")).isTrue();

        mManager.unregisterLocationRequest(listener2);

        assertThat(mInjector.getAppOpsHelper().isAppOpStarted(OP_MONITOR_LOCATION,
                "mypackage")).isTrue();
        assertThat(mInjector.getAppOpsHelper().isAppOpStarted(OP_MONITOR_HIGH_POWER_LOCATION,
                "mypackage")).isTrue();

        mManager.unregisterLocationRequest(listener1);

        assertThat(mInjector.getAppOpsHelper().isAppOpStarted(OP_MONITOR_LOCATION,
                "mypackage")).isFalse();
        assertThat(mInjector.getAppOpsHelper().isAppOpStarted(OP_MONITOR_HIGH_POWER_LOCATION,
                "mypackage")).isFalse();
    }

    @Test
    public void testProviderRequest() {
        assertThat(mProvider.getRequest().isActive()).isFalse();