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 8460d67975434..1781588b0ba2f 100644 --- a/services/core/java/com/android/server/location/gnss/GnssMeasurementsProvider.java +++ b/services/core/java/com/android/server/location/gnss/GnssMeasurementsProvider.java @@ -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()); } } diff --git a/services/core/java/com/android/server/location/injector/LocationAttributionHelper.java b/services/core/java/com/android/server/location/injector/LocationAttributionHelper.java index 5cb360be819ad..483875230ac40 100644 --- a/services/core/java/com/android/server/location/injector/LocationAttributionHelper.java +++ b/services/core/java/com/android/server/location/injector/LocationAttributionHelper.java @@ -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> mAttributions; + private final Map mAttributions; @GuardedBy("this") - private final Map> mHighPowerAttributions; + private final Map 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 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 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 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)) { - if (D) { - Log.v(TAG, "starting high power location attribution for " + identity); - } - } else { - mHighPowerAttributions.remove(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); } + 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 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); } } } diff --git a/services/core/java/com/android/server/location/provider/LocationProviderManager.java b/services/core/java/com/android/server/location/provider/LocationProviderManager.java index f7c4d03bfdf30..155b61891d129 100644 --- a/services/core/java/com/android/server/location/provider/LocationProviderManager.java +++ b/services/core/java/com/android/server/location/provider/LocationProviderManager.java @@ -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()); } } } diff --git a/services/tests/mockingservicestests/src/com/android/server/location/injector/LocationAttributionHelperTest.java b/services/tests/mockingservicestests/src/com/android/server/location/injector/LocationAttributionHelperTest.java index e2e7f5dd7ba28..94dcdf92d9d4d 100644 --- a/services/tests/mockingservicestests/src/com/android/server/location/injector/LocationAttributionHelperTest.java +++ b/services/tests/mockingservicestests/src/com/android/server/location/injector/LocationAttributionHelperTest.java @@ -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); + verify(mAppOpsHelper).startOpNoThrow(OP_MONITOR_LOCATION, + CallerIdentity.forAggregation(caller1)); + verify(mAppOpsHelper, never()).finishOp(OP_MONITOR_LOCATION, + CallerIdentity.forAggregation(caller1)); - mHelper.reportLocationStart(caller1, "gps", key2); - verify(mAppOpsHelper).startOpNoThrow(OP_MONITOR_LOCATION, caller1); - verify(mAppOpsHelper, never()).finishOp(OP_MONITOR_LOCATION, 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, "gps", key3); - verify(mAppOpsHelper).startOpNoThrow(OP_MONITOR_LOCATION, caller2); - verify(mAppOpsHelper, never()).finishOp(OP_MONITOR_LOCATION, caller2); + 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, "gps", key4); - verify(mAppOpsHelper).startOpNoThrow(OP_MONITOR_LOCATION, caller2); - verify(mAppOpsHelper, never()).finishOp(OP_MONITOR_LOCATION, 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, "gps", key2); - verify(mAppOpsHelper, never()).finishOp(OP_MONITOR_LOCATION, caller1); - mHelper.reportLocationStop(caller1, "gps", key1); - verify(mAppOpsHelper).finishOp(OP_MONITOR_LOCATION, caller1); + 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, "gps", key3); - verify(mAppOpsHelper, never()).finishOp(OP_MONITOR_LOCATION, caller2); - mHelper.reportLocationStop(caller2, "gps", key4); - verify(mAppOpsHelper).finishOp(OP_MONITOR_LOCATION, caller2); + 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); + 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, "gps", key2); - verify(mAppOpsHelper).startOpNoThrow(OP_MONITOR_HIGH_POWER_LOCATION, caller1); - verify(mAppOpsHelper, never()).finishOp(OP_MONITOR_HIGH_POWER_LOCATION, 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, "gps", key3); - verify(mAppOpsHelper).startOpNoThrow(OP_MONITOR_HIGH_POWER_LOCATION, caller2); - verify(mAppOpsHelper, never()).finishOp(OP_MONITOR_HIGH_POWER_LOCATION, 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.reportHighPowerLocationStart(caller2, "gps", key4); - verify(mAppOpsHelper).startOpNoThrow(OP_MONITOR_HIGH_POWER_LOCATION, caller2); - verify(mAppOpsHelper, never()).finishOp(OP_MONITOR_HIGH_POWER_LOCATION, 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, "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(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, "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.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)); } } diff --git a/services/tests/mockingservicestests/src/com/android/server/location/provider/LocationProviderManagerTest.java b/services/tests/mockingservicestests/src/com/android/server/location/provider/LocationProviderManagerTest.java index d0b2edadc7141..890a5495ef164 100644 --- a/services/tests/mockingservicestests/src/com/android/server/location/provider/LocationProviderManagerTest.java +++ b/services/tests/mockingservicestests/src/com/android/server/location/provider/LocationProviderManagerTest.java @@ -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();