Remove LocationAttributionHelper and minor cleanups

LocationAttributionHelper is not necessary - startOp/finishOp are
reference counted anyways, so the reference counting done within
LocationAttributionHelper is actually unnecessary. Also fix up a couple
deprecated APIs.

Test: presubmits
Change-Id: I287d429a501c92a1ee1802e2daa76c539cea17ab
This commit is contained in:
Soonil Nagarkar
2021-12-08 11:59:09 -08:00
parent 23f1c52c46
commit 98e9d59b70
8 changed files with 25 additions and 310 deletions

View File

@@ -112,7 +112,6 @@ import com.android.server.location.injector.DeviceIdleHelper;
import com.android.server.location.injector.DeviceStationaryHelper;
import com.android.server.location.injector.EmergencyHelper;
import com.android.server.location.injector.Injector;
import com.android.server.location.injector.LocationAttributionHelper;
import com.android.server.location.injector.LocationPermissionsHelper;
import com.android.server.location.injector.LocationPowerSaveModeHelper;
import com.android.server.location.injector.LocationUsageLogger;
@@ -1705,7 +1704,6 @@ public class LocationManagerService extends ILocationManager.Stub implements
private final SystemScreenInteractiveHelper mScreenInteractiveHelper;
private final SystemDeviceStationaryHelper mDeviceStationaryHelper;
private final SystemDeviceIdleHelper mDeviceIdleHelper;
private final LocationAttributionHelper mLocationAttributionHelper;
private final LocationUsageLogger mLocationUsageLogger;
// lazily instantiated since they may not always be used
@@ -1731,7 +1729,6 @@ public class LocationManagerService extends ILocationManager.Stub implements
mScreenInteractiveHelper = new SystemScreenInteractiveHelper(context);
mDeviceStationaryHelper = new SystemDeviceStationaryHelper();
mDeviceIdleHelper = new SystemDeviceIdleHelper(context);
mLocationAttributionHelper = new LocationAttributionHelper(mAppOpsHelper);
mLocationUsageLogger = new LocationUsageLogger();
}
@@ -1807,11 +1804,6 @@ public class LocationManagerService extends ILocationManager.Stub implements
return mDeviceIdleHelper;
}
@Override
public LocationAttributionHelper getLocationAttributionHelper() {
return mLocationAttributionHelper;
}
@Override
public synchronized EmergencyHelper getEmergencyHelper() {
if (mEmergencyCallHelper == null) {

View File

@@ -16,6 +16,8 @@
package com.android.server.location.gnss;
import static android.app.AppOpsManager.OP_MONITOR_HIGH_POWER_LOCATION;
import static com.android.server.location.gnss.GnssManagerService.D;
import static com.android.server.location.gnss.GnssManagerService.TAG;
@@ -32,7 +34,6 @@ import android.util.Log;
import com.android.server.location.gnss.hal.GnssNative;
import com.android.server.location.injector.AppOpsHelper;
import com.android.server.location.injector.Injector;
import com.android.server.location.injector.LocationAttributionHelper;
import com.android.server.location.injector.LocationUsageLogger;
import com.android.server.location.injector.SettingsHelper;
@@ -68,25 +69,23 @@ public final class GnssMeasurementsProvider extends
@Nullable
@Override
protected void onActive() {
mLocationAttributionHelper.reportHighPowerLocationStart(getIdentity());
mAppOpsHelper.startOpNoThrow(OP_MONITOR_HIGH_POWER_LOCATION, getIdentity());
}
@Nullable
@Override
protected void onInactive() {
mLocationAttributionHelper.reportHighPowerLocationStop(getIdentity());
mAppOpsHelper.finishOp(OP_MONITOR_HIGH_POWER_LOCATION, getIdentity());
}
}
private final AppOpsHelper mAppOpsHelper;
private final LocationAttributionHelper mLocationAttributionHelper;
private final LocationUsageLogger mLogger;
private final GnssNative mGnssNative;
public GnssMeasurementsProvider(Injector injector, GnssNative gnssNative) {
super(injector);
mAppOpsHelper = injector.getAppOpsHelper();
mLocationAttributionHelper = injector.getLocationAttributionHelper();
mLogger = injector.getLocationUsageLogger();
mGnssNative = gnssNative;

View File

@@ -58,9 +58,6 @@ public interface Injector {
/** Returns a DeviceIdleHelper. */
DeviceIdleHelper getDeviceIdleHelper();
/** Returns a LocationAttributionHelper. */
LocationAttributionHelper getLocationAttributionHelper();
/** Returns an EmergencyHelper. */
EmergencyHelper getEmergencyHelper();

View File

@@ -1,122 +0,0 @@
/*
* Copyright (C) 2020 The Android Open Source Project
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package com.android.server.location.injector;
import static android.app.AppOpsManager.OP_MONITOR_HIGH_POWER_LOCATION;
import static android.app.AppOpsManager.OP_MONITOR_LOCATION;
import static com.android.server.location.LocationManagerService.D;
import static com.android.server.location.LocationManagerService.TAG;
import android.location.util.identity.CallerIdentity;
import android.util.ArrayMap;
import android.util.Log;
import com.android.internal.annotations.GuardedBy;
import java.util.Map;
/**
* Helps manage appop monitoring for multiple location clients.
*/
public class LocationAttributionHelper {
private final AppOpsHelper mAppOpsHelper;
@GuardedBy("this")
private final Map<CallerIdentity, Integer> mAttributions;
@GuardedBy("this")
private final Map<CallerIdentity, Integer> mHighPowerAttributions;
public LocationAttributionHelper(AppOpsHelper appOpsHelper) {
mAppOpsHelper = appOpsHelper;
mAttributions = new ArrayMap<>();
mHighPowerAttributions = new ArrayMap<>();
}
/**
* Report normal location usage for the given caller in the given bucket, with a unique key.
*/
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);
}
}
/**
* Report normal location usage has stopped for the given caller in the given bucket, with a
* unique key.
*/
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);
}
}
/**
* Report high power location usage for the given caller in the given bucket, with a unique
* key.
*/
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);
}
}
/**
* 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) {
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);
}
}
}

View File

@@ -16,6 +16,8 @@
package com.android.server.location.provider;
import static android.app.AppOpsManager.OP_MONITOR_HIGH_POWER_LOCATION;
import static android.app.AppOpsManager.OP_MONITOR_LOCATION;
import static android.app.compat.CompatChanges.isChangeEnabled;
import static android.location.LocationManager.DELIVER_HISTORICAL_LOCATIONS;
import static android.location.LocationManager.GPS_PROVIDER;
@@ -32,6 +34,7 @@ import static android.os.PowerManager.LOCATION_MODE_FOREGROUND_ONLY;
import static android.os.PowerManager.LOCATION_MODE_GPS_DISABLED_WHEN_SCREEN_OFF;
import static android.os.PowerManager.LOCATION_MODE_THROTTLE_REQUESTS_WHEN_SCREEN_OFF;
import static com.android.internal.util.ConcurrentUtils.DIRECT_EXECUTOR;
import static com.android.server.location.LocationManagerService.D;
import static com.android.server.location.LocationManagerService.TAG;
import static com.android.server.location.LocationPermissions.PERMISSION_COARSE;
@@ -98,7 +101,6 @@ import com.android.server.location.injector.AppForegroundHelper;
import com.android.server.location.injector.AppForegroundHelper.AppForegroundListener;
import com.android.server.location.injector.AppOpsHelper;
import com.android.server.location.injector.Injector;
import com.android.server.location.injector.LocationAttributionHelper;
import com.android.server.location.injector.LocationPermissionsHelper;
import com.android.server.location.injector.LocationPermissionsHelper.LocationPermissionsListener;
import com.android.server.location.injector.LocationPowerSaveModeHelper;
@@ -191,7 +193,7 @@ public class LocationProviderManager extends
private final ILocationListener mListener;
protected LocationListenerTransport(ILocationListener listener) {
LocationListenerTransport(ILocationListener listener) {
mListener = Objects.requireNonNull(listener);
}
@@ -287,7 +289,7 @@ public class LocationProviderManager extends
private final ILocationCallback mCallback;
protected GetCurrentLocationTransport(ILocationCallback callback) {
GetCurrentLocationTransport(ILocationCallback callback) {
mCallback = Objects.requireNonNull(callback);
}
@@ -398,7 +400,7 @@ public class LocationProviderManager extends
EVENT_LOG.logProviderClientActive(mName, getIdentity());
if (!getRequest().isHiddenFromAppOps()) {
mLocationAttributionHelper.reportLocationStart(getIdentity());
mAppOpsHelper.startOpNoThrow(OP_MONITOR_LOCATION, getIdentity());
}
onHighPowerUsageChanged();
@@ -413,7 +415,7 @@ public class LocationProviderManager extends
onHighPowerUsageChanged();
if (!getRequest().isHiddenFromAppOps()) {
mLocationAttributionHelper.reportLocationStop(getIdentity());
mAppOpsHelper.finishOp(OP_MONITOR_LOCATION, getIdentity());
}
onProviderListenerInactive();
@@ -487,11 +489,9 @@ public class LocationProviderManager extends
if (!getRequest().isHiddenFromAppOps()) {
if (mIsUsingHighPower) {
mLocationAttributionHelper.reportHighPowerLocationStart(
getIdentity());
mAppOpsHelper.startOpNoThrow(OP_MONITOR_HIGH_POWER_LOCATION, getIdentity());
} else {
mLocationAttributionHelper.reportHighPowerLocationStop(
getIdentity());
mAppOpsHelper.finishOp(OP_MONITOR_HIGH_POWER_LOCATION, getIdentity());
}
}
}
@@ -994,7 +994,7 @@ public class LocationProviderManager extends
protected final class LocationListenerRegistration extends LocationRegistration implements
IBinder.DeathRecipient {
protected LocationListenerRegistration(LocationRequest request, CallerIdentity identity,
LocationListenerRegistration(LocationRequest request, CallerIdentity identity,
LocationListenerTransport transport, @PermissionLevel int permissionLevel) {
super(request, identity, transport, permissionLevel);
}
@@ -1059,7 +1059,7 @@ public class LocationProviderManager extends
protected final class LocationPendingIntentRegistration extends LocationRegistration implements
PendingIntent.CancelListener {
protected LocationPendingIntentRegistration(LocationRequest request,
LocationPendingIntentRegistration(LocationRequest request,
CallerIdentity identity, LocationPendingIntentTransport transport,
@PermissionLevel int permissionLevel) {
super(request, identity, transport, permissionLevel);
@@ -1068,13 +1068,15 @@ public class LocationProviderManager extends
@GuardedBy("mLock")
@Override
protected void onLocationListenerRegister() {
((PendingIntent) getKey()).registerCancelListener(this);
if (!((PendingIntent) getKey()).addCancelListener(DIRECT_EXECUTOR, this)) {
remove();
}
}
@GuardedBy("mLock")
@Override
protected void onLocationListenerUnregister() {
((PendingIntent) getKey()).unregisterCancelListener(this);
((PendingIntent) getKey()).removeCancelListener(this);
}
@Override
@@ -1117,7 +1119,7 @@ public class LocationProviderManager extends
private long mExpirationRealtimeMs = Long.MAX_VALUE;
protected GetCurrentLocationListenerRegistration(LocationRequest request,
GetCurrentLocationListenerRegistration(LocationRequest request,
CallerIdentity identity, LocationTransport transport, int permissionLevel) {
super(request, identity, transport, permissionLevel);
}
@@ -1326,7 +1328,6 @@ public class LocationProviderManager extends
protected final AppForegroundHelper mAppForegroundHelper;
protected final LocationPowerSaveModeHelper mLocationPowerSaveModeHelper;
protected final ScreenInteractiveHelper mScreenInteractiveHelper;
protected final LocationAttributionHelper mLocationAttributionHelper;
protected final LocationUsageLogger mLocationUsageLogger;
protected final LocationFudger mLocationFudger;
@@ -1394,7 +1395,6 @@ public class LocationProviderManager extends
mAppForegroundHelper = injector.getAppForegroundHelper();
mLocationPowerSaveModeHelper = injector.getLocationPowerSaveModeHelper();
mScreenInteractiveHelper = injector.getScreenInteractiveHelper();
mLocationAttributionHelper = injector.getLocationAttributionHelper();
mLocationUsageLogger = injector.getLocationUsageLogger();
mLocationFudger = new LocationFudger(mSettingsHelper.getCoarseLocationAccuracyM());

View File

@@ -31,7 +31,7 @@ public class FakeAppOpsHelper extends AppOpsHelper {
private static class AppOp {
AppOp() {}
boolean mAllowed = true;
boolean mStarted = false;
int mStarted = 0;
int mNoteCount = 0;
}
@@ -49,7 +49,7 @@ public class FakeAppOpsHelper extends AppOpsHelper {
public boolean isAppOpStarted(int appOp, String packageName) {
AppOp myAppOp = getOp(packageName, appOp);
return myAppOp.mStarted;
return myAppOp.mStarted > 0;
}
public int getAppOpNoteCount(int appOp, String packageName) {
@@ -63,16 +63,15 @@ public class FakeAppOpsHelper extends AppOpsHelper {
if (!myAppOp.mAllowed) {
return false;
}
Preconditions.checkState(!myAppOp.mStarted);
myAppOp.mStarted = true;
myAppOp.mStarted++;
return true;
}
@Override
public void finishOp(int appOp, CallerIdentity callerIdentity) {
AppOp myAppOp = getOp(callerIdentity.getPackageName(), appOp);
Preconditions.checkState(myAppOp.mStarted);
myAppOp.mStarted = false;
Preconditions.checkState(myAppOp.mStarted > 0);
myAppOp.mStarted--;
}
@Override

View File

@@ -1,143 +0,0 @@
/*
* Copyright (C) 2020 The Android Open Source Project
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package com.android.server.location.injector;
import static android.app.AppOpsManager.OP_MONITOR_HIGH_POWER_LOCATION;
import static android.app.AppOpsManager.OP_MONITOR_LOCATION;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyInt;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
import static org.mockito.MockitoAnnotations.initMocks;
import android.location.util.identity.CallerIdentity;
import android.platform.test.annotations.Presubmit;
import androidx.test.filters.SmallTest;
import androidx.test.runner.AndroidJUnit4;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.Mock;
@Presubmit
@SmallTest
@RunWith(AndroidJUnit4.class)
public class LocationAttributionHelperTest {
@Mock private AppOpsHelper mAppOpsHelper;
private LocationAttributionHelper mHelper;
@Before
public void setUp() {
initMocks(this);
when(mAppOpsHelper.startOpNoThrow(anyInt(), any(CallerIdentity.class))).thenReturn(true);
mHelper = new LocationAttributionHelper(mAppOpsHelper);
}
@Test
public void testLocationMonitoring() {
CallerIdentity caller1 = CallerIdentity.forTest(1, 1, "test1", null);
CallerIdentity caller2 = CallerIdentity.forTest(2, 2, "test2", null);
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);
CallerIdentity caller2 = CallerIdentity.forTest(2, 2, "test2", null);
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));
}
}

View File

@@ -33,7 +33,6 @@ public class TestInjector implements Injector {
private final FakeScreenInteractiveHelper mScreenInteractiveHelper;
private final FakeDeviceStationaryHelper mDeviceStationaryHelper;
private final FakeDeviceIdleHelper mDeviceIdleHelper;
private final LocationAttributionHelper mLocationAttributionHelper;
private final FakeEmergencyHelper mEmergencyHelper;
private final LocationUsageLogger mLocationUsageLogger;
@@ -49,7 +48,6 @@ public class TestInjector implements Injector {
mScreenInteractiveHelper = new FakeScreenInteractiveHelper();
mDeviceStationaryHelper = new FakeDeviceStationaryHelper();
mDeviceIdleHelper = new FakeDeviceIdleHelper();
mLocationAttributionHelper = new LocationAttributionHelper(mAppOpsHelper);
mEmergencyHelper = new FakeEmergencyHelper();
mLocationUsageLogger = new LocationUsageLogger();
}
@@ -109,11 +107,6 @@ public class TestInjector implements Injector {
return mDeviceIdleHelper;
}
@Override
public LocationAttributionHelper getLocationAttributionHelper() {
return mLocationAttributionHelper;
}
@Override
public EmergencyHelper getEmergencyHelper() {
return mEmergencyHelper;