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

Commit 3d7ed28f authored by Soonil Nagarkar's avatar Soonil Nagarkar Committed by Automerger Merge Worker
Browse files

Merge "Fix bugs in AppOp restrictions" into sc-dev am: eb7fc637

Original change: https://googleplex-android-review.googlesource.com/c/platform/frameworks/base/+/14531267

Change-Id: I2e686f9ca34d48685c8c0ec3045d84e226bcac0b
parents 28a297ee eb7fc637
Loading
Loading
Loading
Loading
+60 −20
Original line number Diff line number Diff line
@@ -146,7 +146,8 @@ import java.util.concurrent.CopyOnWriteArrayList;
 * The service class that manages LocationProviders and issues location
 * updates and alerts.
 */
public class LocationManagerService extends ILocationManager.Stub {
public class LocationManagerService extends ILocationManager.Stub implements
        LocationProviderManager.StateChangedListener {

    /**
     * Controls lifecycle of LocationManagerService.
@@ -228,7 +229,7 @@ public class LocationManagerService extends ILocationManager.Stub {

    private static final String ATTRIBUTION_TAG = "LocationService";

    private final Object mLock = new Object();
    final Object mLock = new Object();

    private final Context mContext;
    private final Injector mInjector;
@@ -257,7 +258,7 @@ public class LocationManagerService extends ILocationManager.Stub {
            new CopyOnWriteArrayList<>();

    @GuardedBy("mLock")
    private @Nullable OnProviderLocationTagsChangeListener mOnProviderLocationTagsChangeListener;
    @Nullable OnProviderLocationTagsChangeListener mOnProviderLocationTagsChangeListener;

    LocationManagerService(Context context, Injector injector) {
        mContext = context.createAttributionContext(ATTRIBUTION_TAG);
@@ -331,9 +332,8 @@ public class LocationManagerService extends ILocationManager.Stub {
        synchronized (mProviderManagers) {
            Preconditions.checkState(getLocationProviderManager(manager.getName()) == null);

            manager.startManager();
            manager.setOnProviderLocationTagsChangeListener(
                    mOnProviderLocationTagsChangeListener);
            manager.startManager(this);

            if (realProvider != null) {
                // custom logic wrapping all non-passive providers
                if (manager != mPassiveManager) {
@@ -1356,6 +1356,44 @@ public class LocationManagerService extends ILocationManager.Stub {
        ipw.decreaseIndent();
    }

    @Override
    public void onStateChanged(String provider, AbstractLocationProvider.State oldState,
            AbstractLocationProvider.State newState) {
        if (!Objects.equals(oldState.identity, newState.identity)) {
            refreshAppOpsRestrictions(UserHandle.USER_ALL);
        }

        OnProviderLocationTagsChangeListener listener;
        synchronized (mLock) {
            listener = mOnProviderLocationTagsChangeListener;
        }

        if (listener != null) {
            if (!oldState.extraAttributionTags.equals(newState.extraAttributionTags)
                    || !Objects.equals(oldState.identity, newState.identity)) {
                if (oldState.identity != null) {
                    listener.onLocationTagsChanged(
                            new LocationManagerInternal.LocationTagInfo(
                                    oldState.identity.getUid(),
                                    oldState.identity.getPackageName(),
                                    Collections.emptySet()));
                }
                if (newState.identity != null) {
                    ArraySet<String> attributionTags = new ArraySet<>(
                            newState.extraAttributionTags.size() + 1);
                    attributionTags.addAll(newState.extraAttributionTags);
                    attributionTags.add(newState.identity.getAttributionTag());

                    listener.onLocationTagsChanged(
                            new LocationManagerInternal.LocationTagInfo(
                                    newState.identity.getUid(),
                                    newState.identity.getPackageName(),
                                    attributionTags));
                }
            }
        }
    }

    private void refreshAppOpsRestrictions(int userId) {
        if (userId == UserHandle.USER_ALL) {
            final int[] runningUserIds = mInjector.getUserInfoHelper().getRunningUserIds();
@@ -1367,27 +1405,34 @@ public class LocationManagerService extends ILocationManager.Stub {

        Preconditions.checkArgument(userId >= 0);


        boolean enabled = mInjector.getSettingsHelper().isLocationEnabled(userId);

        String[] allowedPackages = null;
        if (!enabled) {
            ArraySet<String> packages = new ArraySet<>();
            for (LocationProviderManager manager : mProviderManagers) {
            packages.add(manager.getIdentity().getPackageName());
                CallerIdentity identity = manager.getIdentity();
                if (identity != null) {
                    packages.add(identity.getPackageName());
                }
            }
            packages.add(mContext.getPackageName());
            packages.addAll(mInjector.getSettingsHelper().getIgnoreSettingsPackageWhitelist());
        String[] allowedPackages = packages.toArray(new String[0]);

        boolean enabled = mInjector.getSettingsHelper().isLocationEnabled(userId);
            allowedPackages = packages.toArray(new String[0]);
        }

        AppOpsManager appOpsManager = Objects.requireNonNull(
                mContext.getSystemService(AppOpsManager.class));
        appOpsManager.setUserRestrictionForUser(
                AppOpsManager.OP_COARSE_LOCATION,
                enabled,
                !enabled,
                LocationManagerService.this,
                allowedPackages,
                userId);
        appOpsManager.setUserRestrictionForUser(
                AppOpsManager.OP_FINE_LOCATION,
                enabled,
                !enabled,
                LocationManagerService.this,
                allowedPackages,
                userId);
@@ -1467,11 +1512,6 @@ public class LocationManagerService extends ILocationManager.Stub {
                @Nullable OnProviderLocationTagsChangeListener listener) {
            synchronized (mLock) {
                mOnProviderLocationTagsChangeListener = listener;
                final int providerCount = mProviderManagers.size();
                for (int i = 0; i < providerCount; i++) {
                    final LocationProviderManager manager = mProviderManagers.get(i);
                    manager.setOnProviderLocationTagsChangeListener(listener);
                }
            }
        }
    }
+13 −44
Original line number Diff line number Diff line
@@ -55,8 +55,6 @@ import android.location.LastLocationRequest;
import android.location.Location;
import android.location.LocationManager;
import android.location.LocationManagerInternal;
import android.location.LocationManagerInternal.LocationTagInfo;
import android.location.LocationManagerInternal.OnProviderLocationTagsChangeListener;
import android.location.LocationManagerInternal.ProviderEnabledListener;
import android.location.LocationRequest;
import android.location.LocationResult;
@@ -90,7 +88,6 @@ import android.util.TimeUtils;

import com.android.internal.annotations.GuardedBy;
import com.android.internal.util.Preconditions;
import com.android.internal.util.function.pooled.PooledLambda;
import com.android.server.FgThread;
import com.android.server.LocalServices;
import com.android.server.location.LocationPermissions;
@@ -122,7 +119,6 @@ import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.Objects;
import java.util.concurrent.CopyOnWriteArrayList;
import java.util.function.Predicate;
@@ -171,6 +167,11 @@ public class LocationProviderManager extends
    private static final int STATE_STOPPING = 1;
    private static final int STATE_STOPPED = 2;

    public interface StateChangedListener {
        void onStateChanged(String provider, AbstractLocationProvider.State oldState,
                AbstractLocationProvider.State newState);
    }

    protected interface LocationTransport {

        void deliverOnLocationChanged(LocationResult locationResult,
@@ -1315,7 +1316,7 @@ public class LocationProviderManager extends
    private @Nullable OnAlarmListener mDelayedRegister;

    @GuardedBy("mLock")
    private @Nullable OnProviderLocationTagsChangeListener mOnLocationTagsChangeListener;
    private @Nullable StateChangedListener mStateChangedListener;

    public LocationProviderManager(Context context, Injector injector,
            String name, @Nullable PassiveLocationProviderManager passiveManager) {
@@ -1354,10 +1355,11 @@ public class LocationProviderManager extends
        return TAG;
    }

    public void startManager() {
    public void startManager(@Nullable StateChangedListener listener) {
        synchronized (mLock) {
            Preconditions.checkState(mState == STATE_STOPPED);
            mState = STATE_STARTED;
            mStateChangedListener = listener;

            mUserHelper.addListener(mUserChangedListener);
            mSettingsHelper.addOnLocationEnabledChangedListener(mLocationEnabledChangedListener);
@@ -1396,6 +1398,7 @@ public class LocationProviderManager extends

            mEnabled.clear();
            mLastLocations.clear();
            mStateChangedListener = null;
            mState = STATE_STOPPED;
        }
    }
@@ -1476,19 +1479,6 @@ public class LocationProviderManager extends
        }
    }

    /**
     * Registers a listener for the location tags of the provider.
     *
     * @param listener The listener
     */
    public void setOnProviderLocationTagsChangeListener(
            @Nullable OnProviderLocationTagsChangeListener listener) {
        Preconditions.checkArgument(mOnLocationTagsChangeListener == null || listener == null);
        synchronized (mLock) {
            mOnLocationTagsChangeListener = listener;
        }
    }

    public void setMockProvider(@Nullable MockLocationProvider provider) {
        synchronized (mLock) {
            Preconditions.checkState(mState != STATE_STOPPED);
@@ -2292,31 +2282,10 @@ public class LocationProviderManager extends
            updateRegistrations(Registration::onProviderPropertiesChanged);
        }

        if (mOnLocationTagsChangeListener != null) {
            if (!oldState.extraAttributionTags.equals(newState.extraAttributionTags)
                    || !Objects.equals(oldState.identity, newState.identity)) {
                if (oldState.identity != null) {
                    FgThread.getHandler().sendMessage(PooledLambda.obtainMessage(
                            OnProviderLocationTagsChangeListener::onLocationTagsChanged,
                            mOnLocationTagsChangeListener, new LocationTagInfo(
                                    oldState.identity.getUid(), oldState.identity.getPackageName(),
                                    Collections.emptySet())
                    ));
                }
                if (newState.identity != null) {
                    ArraySet<String> attributionTags = new ArraySet<>(
                            newState.extraAttributionTags.size() + 1);
                    attributionTags.addAll(newState.extraAttributionTags);
                    attributionTags.add(newState.identity.getAttributionTag());

                    FgThread.getHandler().sendMessage(PooledLambda.obtainMessage(
                            OnProviderLocationTagsChangeListener::onLocationTagsChanged,
                            mOnLocationTagsChangeListener, new LocationTagInfo(
                                    newState.identity.getUid(), newState.identity.getPackageName(),
                                    attributionTags)
                    ));
                }
            }
        if (mStateChangedListener != null) {
            StateChangedListener listener = mStateChangedListener;
            FgThread.getExecutor().execute(
                    () -> listener.onStateChanged(mName, oldState, newState));
        }
    }

+13 −13
Original line number Diff line number Diff line
@@ -61,8 +61,6 @@ import android.location.ILocationListener;
import android.location.LastLocationRequest;
import android.location.Location;
import android.location.LocationManagerInternal;
import android.location.LocationManagerInternal.LocationTagInfo;
import android.location.LocationManagerInternal.OnProviderLocationTagsChangeListener;
import android.location.LocationManagerInternal.ProviderEnabledListener;
import android.location.LocationRequest;
import android.location.LocationResult;
@@ -133,6 +131,8 @@ public class LocationProviderManagerTest {

    private Random mRandom;

    @Mock
    private LocationProviderManager.StateChangedListener mStateChangedListener;
    @Mock
    private LocationManagerInternal mInternal;
    @Mock
@@ -167,14 +167,14 @@ public class LocationProviderManagerTest {
        mInjector.getUserInfoHelper().startUser(OTHER_USER);

        mPassive = new PassiveLocationProviderManager(mContext, mInjector);
        mPassive.startManager();
        mPassive.startManager(null);
        mPassive.setRealProvider(new PassiveLocationProvider(mContext));

        mProvider = new TestProvider(PROPERTIES, PROVIDER_IDENTITY);
        mProvider.setProviderAllowed(true);

        mManager = new LocationProviderManager(mContext, mInjector, NAME, mPassive);
        mManager.startManager();
        mManager.startManager(mStateChangedListener);
        mManager.setRealProvider(mProvider);
    }

@@ -219,18 +219,18 @@ public class LocationProviderManagerTest {
    }

    @Test
    public void testAttributionTags() {
        OnProviderLocationTagsChangeListener listener = mock(
                OnProviderLocationTagsChangeListener.class);
        mManager.setOnProviderLocationTagsChangeListener(listener);

    public void testStateChangedListener() {
        mProvider.setExtraAttributionTags(Collections.singleton("extra"));

        ArgumentCaptor<LocationTagInfo> captor = ArgumentCaptor.forClass(LocationTagInfo.class);
        verify(listener, times(2)).onLocationTagsChanged(captor.capture());
        ArgumentCaptor<AbstractLocationProvider.State> captorOld = ArgumentCaptor.forClass(
                AbstractLocationProvider.State.class);
        ArgumentCaptor<AbstractLocationProvider.State> captorNew = ArgumentCaptor.forClass(
                AbstractLocationProvider.State.class);
        verify(mStateChangedListener, timeout(TIMEOUT_MS).times(2)).onStateChanged(eq(NAME),
                captorOld.capture(), captorNew.capture());

        assertThat(captor.getAllValues().get(0).getTags()).isEmpty();
        assertThat(captor.getAllValues().get(1).getTags()).containsExactly("extra", "attribution");
        assertThat(captorOld.getAllValues().get(1).extraAttributionTags).isEmpty();
        assertThat(captorNew.getAllValues().get(1).extraAttributionTags).containsExactly("extra");
    }

    @Test