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

Commit fcfb9c8e authored by Kweku Adams's avatar Kweku Adams
Browse files

Simplify app removal code.

All the trackers care about is userId. Forcing uid pass-in complicates
things.

Also do some code cleanup.

Bug: N/A
Test: atest CountQuotaTrackerTest
Change-Id: Ie731f70e774aff178e5e04170089995a947bed52
parent 770e6072
Loading
Loading
Loading
Loading
+1 −3
Original line number Diff line number Diff line
@@ -27,7 +27,6 @@ import android.content.Context;
import android.os.Handler;
import android.os.Looper;
import android.os.Message;
import android.os.UserHandle;
import android.util.ArrayMap;
import android.util.LongArrayQueue;
import android.util.Slog;
@@ -294,12 +293,11 @@ public class CountQuotaTracker extends QuotaTracker {

    @Override
    @GuardedBy("mLock")
    void handleRemovedAppLocked(String packageName, int uid) {
    void handleRemovedAppLocked(final int userId, @NonNull String packageName) {
        if (packageName == null) {
            Slog.wtf(TAG, "Told app removed but given null package name.");
            return;
        }
        final int userId = UserHandle.getUserId(uid);

        mEventTimes.delete(userId, packageName);
        mExecutionStatsCache.delete(userId, packageName);
+12 −21
Original line number Diff line number Diff line
@@ -19,8 +19,6 @@ package com.android.server.utils.quota;
import android.annotation.NonNull;
import android.annotation.Nullable;
import android.content.Context;
import android.content.pm.PackageManager;
import android.util.Slog;

import com.android.internal.annotations.GuardedBy;
import com.android.internal.annotations.VisibleForTesting;
@@ -57,11 +55,9 @@ public class MultiRateLimiter {
    private final Object mLock = new Object();
    @GuardedBy("mLock")
    private final CountQuotaTracker[] mQuotaTrackers;
    private final PackageManager mPackageManager;

    private MultiRateLimiter(List<CountQuotaTracker> quotaTrackers, PackageManager packageManager) {
    private MultiRateLimiter(List<CountQuotaTracker> quotaTrackers) {
        mQuotaTrackers = quotaTrackers.toArray(EMPTY_TRACKER_ARRAY);
        mPackageManager = packageManager;
    }

    /** Record that an event happened and count it towards the given quota. */
@@ -105,17 +101,11 @@ public class MultiRateLimiter {

    @GuardedBy("mLock")
    private void clearLocked(int userId, @NonNull String packageName) {
        try {
            int uid = mPackageManager.getApplicationInfoAsUser(packageName, 0, userId).uid;
        for (CountQuotaTracker quotaTracker : mQuotaTrackers) {
            // This method behaves as if the package has been removed from the device, which
            // isn't the case here, but it does similar clean-up to what we are aiming for here,
            // so it works for this use case.
                quotaTracker.onAppRemovedLocked(packageName, uid);
            }
        } catch (PackageManager.NameNotFoundException e) {
            Slog.e(TAG, "clear(userId, packageName) called with unrecognized arguments, no "
                    + "action taken");
            quotaTracker.onAppRemovedLocked(userId, packageName);
        }
    }

@@ -126,7 +116,8 @@ public class MultiRateLimiter {
        private final Context mContext;
        private final Categorizer mCategorizer;
        private final Category mCategory;
        @Nullable private final QuotaTracker.Injector mInjector;
        @Nullable
        private final QuotaTracker.Injector mInjector;

        /**
         * Creates a new builder and allows to inject an object that can be used
@@ -182,7 +173,7 @@ public class MultiRateLimiter {
         * limit.
         */
        public MultiRateLimiter build() {
            return new MultiRateLimiter(mQuotaTrackers, mContext.getPackageManager());
            return new MultiRateLimiter(mQuotaTrackers);
        }
    }

+4 −5
Original line number Diff line number Diff line
@@ -132,7 +132,7 @@ abstract class QuotaTracker {
                case Intent.ACTION_PACKAGE_FULLY_REMOVED:
                    final int uid = intent.getIntExtra(Intent.EXTRA_UID, -1);
                    synchronized (mLock) {
                        onAppRemovedLocked(getPackageName(intent), uid);
                        onAppRemovedLocked(UserHandle.getUserId(uid), getPackageName(intent));
                    }
                    break;
                case Intent.ACTION_USER_REMOVED:
@@ -358,21 +358,20 @@ abstract class QuotaTracker {
    }

    @GuardedBy("mLock")
    abstract void handleRemovedAppLocked(String packageName, int uid);
    abstract void handleRemovedAppLocked(int userId, @NonNull String packageName);

    @GuardedBy("mLock")
    void onAppRemovedLocked(String packageName, int uid) {
    void onAppRemovedLocked(final int userId, @NonNull String packageName) {
        if (packageName == null) {
            Slog.wtf(TAG, "Told app removed but given null package name.");
            return;
        }
        final int userId = UserHandle.getUserId(uid);

        mInQuotaAlarmListener.removeAlarmsLocked(userId, packageName);

        mFreeQuota.delete(userId, packageName);

        handleRemovedAppLocked(packageName, uid);
        handleRemovedAppLocked(userId, packageName);
    }

    @GuardedBy("mLock")
+7 −55
Original line number Diff line number Diff line
@@ -16,29 +16,15 @@

package com.android.server.utils.quota;

import static com.android.dx.mockito.inline.extended.ExtendedMockito.doReturn;
import static com.android.dx.mockito.inline.extended.ExtendedMockito.mockitoSession;

import static com.google.common.truth.Truth.assertThat;

import static org.mockito.Mockito.when;

import android.content.ContextWrapper;
import android.content.pm.ApplicationInfo;
import android.content.pm.PackageManager;
import android.os.UserHandle;
import android.testing.TestableContext;

import androidx.test.InstrumentationRegistry;
import androidx.test.filters.SmallTest;

import org.junit.After;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
import org.mockito.Mock;
import org.mockito.MockitoSession;
import org.mockito.quality.Strictness;

import java.time.Duration;

@@ -46,20 +32,15 @@ import java.time.Duration;
public class MultiRateLimiterTest {

    private static final int USER_ID = 1;
    private static final int UID_1 = 10_001;
    private static final String PACKAGE_NAME_1 = "com.android.package.one";
    private static final String PACKAGE_NAME_2 = "com.android.package.two";
    private static final String TAG = "tag";

    @Rule
    public final TestableContext mTestableContext =
    public final TestableContext mContext =
            new TestableContext(InstrumentationRegistry.getContext(), null);

    private final InjectorForTest mInjector = new InjectorForTest();
    private MockitoSession mMockingSession;
    private ContextWrapper mContext;

    @Mock private PackageManager mPackageManager;

    private static class InjectorForTest extends QuotaTracker.Injector {
        Duration mElapsedTime = Duration.ZERO;
@@ -75,35 +56,6 @@ public class MultiRateLimiterTest {
        }
    }

    @Before
    public void setup() throws Exception {
        mMockingSession = mockitoSession()
                .initMocks(this)
                .strictness(Strictness.LENIENT)
                .mockStatic(UserHandle.class)
                .startMocking();
        doReturn(USER_ID).when(() -> UserHandle.getUserId(UID_1));

        ApplicationInfo applicationInfo = new ApplicationInfo();
        applicationInfo.uid = UID_1;
        when(mPackageManager.getApplicationInfoAsUser(PACKAGE_NAME_1, 0, USER_ID))
                .thenReturn(applicationInfo);

        mContext = new ContextWrapper(mTestableContext) {
            @Override
            public PackageManager getPackageManager() {
                return mPackageManager;
            }
        };
    }

    @After
    public void tearDown() {
        if (mMockingSession != null) {
            mMockingSession.finishMocking();
        }
    }

    @Test
    public void testSingleRateLimit_belowLimit_isWithinQuota() {
        MultiRateLimiter multiRateLimiter = new MultiRateLimiter.Builder(mContext, mInjector)