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

Commit 9ec01856 authored by Soonil Nagarkar's avatar Soonil Nagarkar
Browse files

Fix bug with provider enabled state across profiles

Pull all user logic out into a utility class (which can be mocked for
unit testing!).

Provider useability was not properly being updated for all current user
profiles, only for the current user. This fix ensures that all profiles
see the correct useable status for the provider.

Test: atest CtsLocationManagerFineTestCases
Change-Id: I799b16613d6a98e52eb2c561b6605d2c872fbf23
parent 9672b8e2
Loading
Loading
Loading
Loading
+85 −102
Original line number Diff line number Diff line
@@ -69,7 +69,6 @@ import android.os.Process;
import android.os.RemoteException;
import android.os.SystemClock;
import android.os.UserHandle;
import android.os.UserManager;
import android.os.WorkSource;
import android.os.WorkSource.WorkChain;
import android.stats.location.LocationStatsEnums;
@@ -84,7 +83,6 @@ import com.android.internal.annotations.GuardedBy;
import com.android.internal.content.PackageMonitor;
import com.android.internal.location.ProviderProperties;
import com.android.internal.location.ProviderRequest;
import com.android.internal.util.ArrayUtils;
import com.android.internal.util.DumpUtils;
import com.android.internal.util.IndentingPrintWriter;
import com.android.internal.util.Preconditions;
@@ -105,6 +103,7 @@ import com.android.server.location.LocationUsageLogger;
import com.android.server.location.MockProvider;
import com.android.server.location.MockableLocationProvider;
import com.android.server.location.PassiveProvider;
import com.android.server.location.UserInfoStore;
import com.android.server.pm.permission.PermissionManagerServiceInternal;

import java.io.ByteArrayOutputStream;
@@ -194,6 +193,7 @@ public class LocationManagerService extends ILocationManager.Stub {
    private final Object mLock = new Object();
    private final Context mContext;
    private final Handler mHandler;
    private final UserInfoStore mUserInfoStore;
    private final LocationSettingsStore mSettingsStore;
    private final LocationUsageLogger mLocationUsageLogger;

@@ -203,7 +203,6 @@ public class LocationManagerService extends ILocationManager.Stub {
    private PackageManager mPackageManager;
    private PowerManager mPowerManager;
    private ActivityManager mActivityManager;
    private UserManager mUserManager;

    private GeofenceManager mGeofenceManager;
    private LocationFudger mLocationFudger;
@@ -237,10 +236,6 @@ public class LocationManagerService extends ILocationManager.Stub {
    private final HashMap<String, Location> mLastLocationCoarseInterval =
            new HashMap<>();

    // current active user on the device
    private int mCurrentUserId;
    private int[] mCurrentUserProfiles;

    @GuardedBy("mLock")
    @PowerManager.LocationPowerSaveMode
    private int mBatterySaverMode;
@@ -248,12 +243,10 @@ public class LocationManagerService extends ILocationManager.Stub {
    private LocationManagerService(Context context) {
        mContext = context;
        mHandler = FgThread.getHandler();
        mUserInfoStore = new UserInfoStore(mContext);
        mSettingsStore = new LocationSettingsStore(mContext, mHandler);
        mLocationUsageLogger = new LocationUsageLogger();

        mCurrentUserId = UserHandle.USER_NULL;
        mCurrentUserProfiles = new int[]{UserHandle.USER_NULL};

        // set up passive provider -  we do this early because it has no dependencies on system
        // services or external code that isn't ready yet, and because this allows the variable to
        // be final. other more complex providers are initialized later, when system services are
@@ -277,6 +270,7 @@ public class LocationManagerService extends ILocationManager.Stub {
    }

    private void onSystemReady() {
        mUserInfoStore.onSystemReady();
        mSettingsStore.onSystemReady();

        synchronized (mLock) {
@@ -284,7 +278,6 @@ public class LocationManagerService extends ILocationManager.Stub {
            mAppOps = mContext.getSystemService(AppOpsManager.class);
            mPowerManager = mContext.getSystemService(PowerManager.class);
            mActivityManager = mContext.getSystemService(ActivityManager.class);
            mUserManager = mContext.getSystemService(UserManager.class);

            mLocationFudger = new LocationFudger(mContext, mHandler);
            mGeofenceManager = new GeofenceManager(mContext, mSettingsStore);
@@ -372,10 +365,13 @@ public class LocationManagerService extends ILocationManager.Stub {
                }
            }.register(mContext, mHandler.getLooper(), true);

            mUserInfoStore.addListener((oldUserId, newUserId) -> {
                synchronized (mLock) {
                    onUserChangedLocked(oldUserId, newUserId);
                }
            });

            IntentFilter intentFilter = new IntentFilter();
            intentFilter.addAction(Intent.ACTION_USER_SWITCHED);
            intentFilter.addAction(Intent.ACTION_MANAGED_PROFILE_ADDED);
            intentFilter.addAction(Intent.ACTION_MANAGED_PROFILE_REMOVED);
            intentFilter.addAction(Intent.ACTION_SCREEN_OFF);
            intentFilter.addAction(Intent.ACTION_SCREEN_ON);

@@ -388,14 +384,6 @@ public class LocationManagerService extends ILocationManager.Stub {
                    }
                    synchronized (mLock) {
                        switch (action) {
                            case Intent.ACTION_USER_SWITCHED:
                                onUserChangedLocked(
                                        intent.getIntExtra(Intent.EXTRA_USER_HANDLE, 0));
                                break;
                            case Intent.ACTION_MANAGED_PROFILE_ADDED:
                            case Intent.ACTION_MANAGED_PROFILE_REMOVED:
                                onUserProfilesChangedLocked();
                                break;
                            case Intent.ACTION_SCREEN_ON:
                            case Intent.ACTION_SCREEN_OFF:
                                onScreenStateChangedLocked();
@@ -405,11 +393,10 @@ public class LocationManagerService extends ILocationManager.Stub {
                }
            }, UserHandle.ALL, intentFilter, null, mHandler);

            // switching the user from null to system here performs the bulk of the initialization
            // switching the user from null to current here performs the bulk of the initialization
            // work. the user being changed will cause a reload of all user specific settings, which
            // causes initialization, and propagates changes until a steady state is reached
            mCurrentUserId = UserHandle.USER_NULL;
            onUserChangedLocked(ActivityManager.getCurrentUser());
            onUserChangedLocked(UserHandle.USER_NULL, mUserInfoStore.getCurrentUserId());
        }
    }

@@ -550,16 +537,6 @@ public class LocationManagerService extends ILocationManager.Stub {
        }
    }

    @GuardedBy("mLock")
    private void onUserProfilesChangedLocked() {
        mCurrentUserProfiles = mUserManager.getProfileIdsWithDisabled(mCurrentUserId);
    }

    @GuardedBy("mLock")
    private boolean isCurrentProfileLocked(int userId) {
        return ArrayUtils.contains(mCurrentUserProfiles, userId);
    }

    @GuardedBy("mLock")
    private void ensureFallbackFusedProviderPresentLocked(String[] pkgs) {
        PackageManager pm = mContext.getPackageManager();
@@ -568,7 +545,7 @@ public class LocationManagerService extends ILocationManager.Stub {

        List<ResolveInfo> rInfos = pm.queryIntentServicesAsUser(
                new Intent(FUSED_LOCATION_SERVICE_ACTION),
                PackageManager.GET_META_DATA, mCurrentUserId);
                PackageManager.GET_META_DATA, mUserInfoStore.getCurrentUserId());
        for (ResolveInfo rInfo : rInfos) {
            String packageName = rInfo.serviceInfo.packageName;

@@ -752,27 +729,18 @@ public class LocationManagerService extends ILocationManager.Stub {
    }

    @GuardedBy("mLock")
    private void onUserChangedLocked(int userId) {
        if (mCurrentUserId == userId) {
            return;
        }

    private void onUserChangedLocked(int oldUserId, int newUserId) {
        if (D) {
            Log.d(TAG, "foreground user is changing to " + userId);
            Log.d(TAG, "foreground user is changing to " + newUserId);
        }

        int oldUserId = mCurrentUserId;
        mCurrentUserId = userId;
        onUserProfilesChangedLocked();

        // let providers know the current user has changed
        for (LocationProviderManager manager : mProviderManagers) {
            // update LOCATION_PROVIDERS_ALLOWED for best effort backwards compatibility
            mSettingsStore.setLocationProviderAllowed(manager.getName(),
                    manager.isUseable(mCurrentUserId), mCurrentUserId);
                    manager.isUseable(newUserId), newUserId);

            manager.onUseableChangedLocked(oldUserId);
            manager.onUseableChangedLocked(mCurrentUserId);
            manager.onUseableChangedLocked(newUserId);
        }
    }

@@ -786,8 +754,12 @@ public class LocationManagerService extends ILocationManager.Stub {
        // acquiring mLock makes operations on mProvider atomic, but is otherwise unnecessary
        protected final MockableLocationProvider mProvider;

        // useable state for parent user ids, no entry implies false. location state is only kept
        // for parent user ids, the location state for a profile user id is assumed to be the same
        // as for the parent. if querying this structure, ensure that the user id being used is a
        // parent id or the results may be incorrect.
        @GuardedBy("mLock")
        private final SparseArray<Boolean> mUseable;  // combined state for each user id
        private final SparseArray<Boolean> mUseable;

        private LocationProviderManager(String name) {
            mName = name;
@@ -795,6 +767,9 @@ public class LocationManagerService extends ILocationManager.Stub {

            // initialize last since this lets our reference escape
            mProvider = new MockableLocationProvider(mContext, mLock, this);

            // we can assume all users start with unuseable location state since the initial state
            // of all providers is disabled. no need to initialize mUseable further.
        }

        public String getName() {
@@ -868,29 +843,6 @@ public class LocationManagerService extends ILocationManager.Stub {
            mProvider.sendExtraCommand(uid, pid, command, extras);
        }

        public void dump(FileDescriptor fd, IndentingPrintWriter pw, String[] args) {
            synchronized (mLock) {
                pw.print(mName + " provider");
                if (mProvider.isMock()) {
                    pw.print(" [mock]");
                }
                pw.println(":");

                pw.increaseIndent();

                pw.println("useable=" + isUseable(mCurrentUserId));
                if (!isUseable(mCurrentUserId)) {
                    pw.println("enabled=" + mProvider.getState().enabled);
                }

                pw.println("properties=" + mProvider.getState().properties);
            }

            mProvider.dump(fd, pw, args);

            pw.decreaseIndent();
        }

        @GuardedBy("mLock")
        @Override
        public void onReportLocation(Location location) {
@@ -927,18 +879,20 @@ public class LocationManagerService extends ILocationManager.Stub {
                // it would be more correct to call this for all users, but we know this can
                // only affect the current user since providers are disabled for non-current
                // users
                onUseableChangedLocked(mCurrentUserId);
                onUseableChangedLocked(mUserInfoStore.getCurrentUserId());
            }
        }

        @GuardedBy("mLock")
        public boolean isUseable() {
            return isUseable(mCurrentUserId);
            return isUseable(mUserInfoStore.getCurrentUserId());
        }

        @GuardedBy("mLock")
        public boolean isUseable(int userId) {
            synchronized (mLock) {
                // normalize user id to always refer to parent since profile state is always the
                // same as parent state
                userId = mUserInfoStore.getParentUserId(userId);

                return mUseable.get(userId, Boolean.FALSE);
            }
        }
@@ -950,15 +904,20 @@ public class LocationManagerService extends ILocationManager.Stub {
                return;
            }

            // normalize user id to always refer to parent since profile state is always the same
            // as parent state
            userId = mUserInfoStore.getParentUserId(userId);

            // if any property that contributes to "useability" here changes state, it MUST result
            // in a direct or indrect call to onUseableChangedLocked. this allows the provider to
            // guarantee that it will always eventually reach the correct state.
            boolean useable = isCurrentProfileLocked(userId)
            boolean useable = (userId == mUserInfoStore.getCurrentUserId())
                    && mSettingsStore.isLocationEnabled(userId) && mProvider.getState().enabled;

            if (useable == isUseable(userId)) {
                return;
            }

            mUseable.put(userId, useable);

            if (D) {
@@ -986,6 +945,30 @@ public class LocationManagerService extends ILocationManager.Stub {

            updateProviderUseableLocked(this);
        }

        public void dump(FileDescriptor fd, IndentingPrintWriter pw, String[] args) {
            synchronized (mLock) {
                pw.print(mName + " provider");
                if (mProvider.isMock()) {
                    pw.print(" [mock]");
                }
                pw.println(":");

                pw.increaseIndent();

                boolean useable = isUseable();
                pw.println("useable=" + useable);
                if (!useable) {
                    pw.println("enabled=" + mProvider.getState().enabled);
                }

                pw.println("properties=" + mProvider.getState().properties);
            }

            mProvider.dump(fd, pw, args);

            pw.decreaseIndent();
        }
    }

    class PassiveLocationProviderManager extends LocationProviderManager {
@@ -1626,7 +1609,7 @@ public class LocationManagerService extends ILocationManager.Stub {
        ArrayList<UpdateRecord> records = mRecordsByProvider.get(manager.getName());
        if (records != null) {
            for (UpdateRecord record : records) {
                if (!isCurrentProfileLocked(
                if (!mUserInfoStore.isCurrentUserOrProfile(
                        UserHandle.getUserId(record.mReceiver.mCallerIdentity.mUid))) {
                    continue;
                }
@@ -1691,7 +1674,7 @@ public class LocationManagerService extends ILocationManager.Stub {
            // initialize the low power mode to true and set to false if any of the records requires
            providerRequest.setLowPowerMode(true);
            for (UpdateRecord record : records) {
                if (!isCurrentProfileLocked(
                if (!mUserInfoStore.isCurrentUserOrProfile(
                        UserHandle.getUserId(record.mReceiver.mCallerIdentity.mUid))) {
                    continue;
                }
@@ -1750,7 +1733,7 @@ public class LocationManagerService extends ILocationManager.Stub {
                // TODO: overflow
                long thresholdInterval = (providerRequest.getInterval() + 1000) * 3 / 2;
                for (UpdateRecord record : records) {
                    if (isCurrentProfileLocked(
                    if (mUserInfoStore.isCurrentUserOrProfile(
                            UserHandle.getUserId(record.mReceiver.mCallerIdentity.mUid))) {
                        LocationRequest locationRequest = record.mRequest;

@@ -2243,8 +2226,8 @@ public class LocationManagerService extends ILocationManager.Stub {
                if (manager == null) return null;

                // only the current user or location providers may get location this way
                if (!isCurrentProfileLocked(UserHandle.getUserId(uid)) && !isProviderPackage(
                        packageName)) {
                if (!mUserInfoStore.isCurrentUserOrProfile(UserHandle.getUserId(uid))
                        && !isProviderPackage(packageName)) {
                    return null;
                }

@@ -2773,12 +2756,11 @@ public class LocationManagerService extends ILocationManager.Stub {
            }

            int receiverUserId = UserHandle.getUserId(receiver.mCallerIdentity.mUid);
            if (!isCurrentProfileLocked(receiverUserId)
            if (!mUserInfoStore.isCurrentUserOrProfile(receiverUserId)
                    && !isProviderPackage(receiver.mCallerIdentity.mPackageName)) {
                if (D) {
                    Log.d(TAG, "skipping loc update for background user " + receiverUserId +
                            " (current user: " + mCurrentUserId + ", app: " +
                            receiver.mCallerIdentity.mPackageName + ")");
                            " (app: " + receiver.mCallerIdentity.mPackageName + ")");
                }
                continue;
            }
@@ -3028,9 +3010,17 @@ public class LocationManagerService extends ILocationManager.Stub {
                    + TimeUtils.logTimeOfDay(System.currentTimeMillis()));
            ipw.println(", Current Elapsed Time: "
                    + TimeUtils.formatDuration(SystemClock.elapsedRealtime()));
            ipw.println("Current user: " + mCurrentUserId + " " + Arrays.toString(
                    mCurrentUserProfiles));
            ipw.println("Location Mode: " + isLocationEnabledForUser(mCurrentUserId));

            ipw.println("User Info:");
            ipw.increaseIndent();
            mUserInfoStore.dump(fd, ipw, args);
            ipw.decreaseIndent();

            ipw.println("Location Settings:");
            ipw.increaseIndent();
            mSettingsStore.dump(fd, ipw, args);
            ipw.decreaseIndent();

            ipw.println("Battery Saver Location Mode: "
                    + locationPowerSaveModeToString(mBatterySaverMode));

@@ -3096,12 +3086,6 @@ public class LocationManagerService extends ILocationManager.Stub {
                mLocationFudger.dump(fd, ipw, args);
                ipw.decreaseIndent();
            }
        }

        ipw.println("Location Settings:");
        ipw.increaseIndent();
        mSettingsStore.dump(fd, ipw, args);
        ipw.decreaseIndent();

            ipw.println("Location Providers:");
            ipw.increaseIndent();
@@ -3110,7 +3094,6 @@ public class LocationManagerService extends ILocationManager.Stub {
            }
            ipw.decreaseIndent();

        synchronized (mLock) {
            if (mGnssManagerService != null) {
                ipw.println("GNSS:");
                ipw.increaseIndent();
+226 −0
Original line number Diff line number Diff line
/*
 * 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;

import android.annotation.Nullable;
import android.annotation.UserIdInt;
import android.app.ActivityManager;
import android.content.BroadcastReceiver;
import android.content.Context;
import android.content.Intent;
import android.content.IntentFilter;
import android.content.pm.UserInfo;
import android.os.Build;
import android.os.UserHandle;
import android.os.UserManager;

import com.android.internal.annotations.GuardedBy;
import com.android.internal.util.ArrayUtils;
import com.android.internal.util.Preconditions;
import com.android.server.FgThread;

import java.io.FileDescriptor;
import java.io.PrintWriter;
import java.util.Arrays;
import java.util.concurrent.CopyOnWriteArrayList;

/**
 * Provides accessors and listeners for all user info.
 */
public class UserInfoStore {

    /**
     * Listener for current user changes.
     */
    public interface UserChangedListener {
        /**
         * Called when the current user changes.
         */
        void onUserChanged(@UserIdInt int oldUserId, @UserIdInt int newUserId);
    }

    private final Context mContext;
    private final CopyOnWriteArrayList<UserChangedListener> mListeners;

    @GuardedBy("this")
    @Nullable
    private UserManager mUserManager;

    @GuardedBy("this")
    @UserIdInt
    private int mCurrentUserId;

    @GuardedBy("this")
    @UserIdInt
    private int mCachedParentUserId;
    @GuardedBy("this")
    private int[] mCachedProfileUserIds;

    public UserInfoStore(Context context) {
        mContext = context;
        mListeners = new CopyOnWriteArrayList<>();

        mCurrentUserId = UserHandle.USER_NULL;
        mCachedParentUserId = UserHandle.USER_NULL;
        mCachedProfileUserIds = new int[]{UserHandle.USER_NULL};
    }

    /** Called when system is ready. */
    public synchronized void onSystemReady() {
        if (mUserManager != null) {
            return;
        }

        mUserManager = mContext.getSystemService(UserManager.class);

        IntentFilter intentFilter = new IntentFilter();
        intentFilter.addAction(Intent.ACTION_USER_SWITCHED);
        intentFilter.addAction(Intent.ACTION_MANAGED_PROFILE_ADDED);
        intentFilter.addAction(Intent.ACTION_MANAGED_PROFILE_REMOVED);

        mContext.registerReceiverAsUser(new BroadcastReceiver() {
            @Override
            public void onReceive(Context context, Intent intent) {
                String action = intent.getAction();
                if (action == null) {
                    return;
                }
                switch (action) {
                    case Intent.ACTION_USER_SWITCHED:
                        int userId = intent.getIntExtra(Intent.EXTRA_USER_HANDLE,
                                UserHandle.USER_NULL);
                        if (userId != UserHandle.USER_NULL) {
                            onUserChanged(userId);
                        }
                        break;
                    case Intent.ACTION_MANAGED_PROFILE_ADDED:
                    case Intent.ACTION_MANAGED_PROFILE_REMOVED:
                        onUserProfilesChanged();
                        break;
                }
            }
        }, UserHandle.ALL, intentFilter, null, FgThread.getHandler());

        mCurrentUserId = ActivityManager.getCurrentUser();
    }

    /**
     * Adds a listener for user changed events.
     */
    public void addListener(UserChangedListener listener) {
        mListeners.add(listener);
    }

    /**
     * Removes a listener for user changed events.
     */
    public void removeListener(UserChangedListener listener) {
        mListeners.remove(listener);
    }

    private void onUserChanged(@UserIdInt int newUserId) {
        int oldUserId;
        synchronized (this) {
            if (newUserId == mCurrentUserId) {
                return;
            }

            oldUserId = mCurrentUserId;
            mCurrentUserId = newUserId;
        }

        for (UserChangedListener listener : mListeners) {
            listener.onUserChanged(oldUserId, newUserId);
        }
    }

    private synchronized void onUserProfilesChanged() {
        // this intent is only sent to the current user
        if (mCachedParentUserId == mCurrentUserId) {
            mCachedParentUserId = UserHandle.USER_NULL;
            mCachedProfileUserIds = null;
        }
    }

    /**
     * Returns the user id of the current user.
     */
    @UserIdInt
    public synchronized int getCurrentUserId() {
        return mCurrentUserId;
    }

    /**
     * Returns true if the given user id is either the current user or a profile of the current
     * user.
     */
    public synchronized boolean isCurrentUserOrProfile(@UserIdInt int userId) {
        return userId == mCurrentUserId || ArrayUtils.contains(
                getProfileUserIdsForParentUser(mCurrentUserId), userId);
    }

    /**
     * Returns the parent user id of the given user id, or the user id itself if the user id either
     * is a parent or has no profiles.
     */
    @UserIdInt
    public synchronized int getParentUserId(@UserIdInt int userId) {
        int parentUserId;
        if (userId == mCachedParentUserId || ArrayUtils.contains(mCachedProfileUserIds, userId)) {
            parentUserId = mCachedParentUserId;
        } else {
            Preconditions.checkState(mUserManager != null);

            UserInfo userInfo = mUserManager.getProfileParent(userId);
            if (userInfo != null) {
                parentUserId = userInfo.id;
            } else {
                // getProfileParent() returns null if the userId is already the parent...
                parentUserId = userId;
            }

            // force profiles into cache
            getProfileUserIdsForParentUser(parentUserId);
        }

        return parentUserId;
    }

    @GuardedBy("this")
    private int[] getProfileUserIdsForParentUser(@UserIdInt int parentUserId) {
        Preconditions.checkState(mUserManager != null);

        if (Build.IS_DEBUGGABLE) {
            Preconditions.checkArgument(mUserManager.getProfileParent(parentUserId) == null);
        }

        if (parentUserId != mCachedParentUserId) {
            mCachedParentUserId = parentUserId;
            mCachedProfileUserIds = mUserManager.getProfileIdsWithDisabled(parentUserId);
        }

        return mCachedProfileUserIds;
    }

    /**
     * Dump info for debugging.
     */
    public synchronized void dump(FileDescriptor fd, PrintWriter pw, String[] args) {
        pw.println("Current User: " + mCurrentUserId + " " + Arrays.toString(
                getProfileUserIdsForParentUser(mCurrentUserId)));
    }
}
+177 −0

File added.

Preview size limit exceeded, changes collapsed.