DO NOT MERGE: Delete persisted historical app ops on package uninstall

They're removed from the current state, but not the persisted state.

This adds HistoricalRegistry#clearHistoryForPackage which reads the
disk state, strips the corresponding UID/package, and re-writes
to disk.

Bug: 129796626

Test: manual test app with location access
Test: atest AppOpsServiceTest#testPackageRemovedHistoricalOps

Change-Id: I8daa2e3474b400a3789b2eaf178441c6d1578af1

diff --git a/core/java/android/app/AppOpsManager.java b/core/java/android/app/AppOpsManager.java
index 713fd1c..a29b8fe 100644 (file)
--- a/core/java/android/app/AppOpsManager.java
+++ b/core/java/android/app/AppOpsManager.java
@@ -3119,6 +3119,15 @@ public class AppOpsManager {
             return mHistoricalUidOps.get(uid);
         }
 
+        /** @hide */
+        public void clearHistory(int uid, @NonNull String packageName) {
+            HistoricalUidOps historicalUidOps = getOrCreateHistoricalUidOps(uid);
+            historicalUidOps.clearHistory(packageName);
+            if (historicalUidOps.isEmpty()) {
+                mHistoricalUidOps.remove(uid);
+            }
+        }
+
         @Override
         public int describeContents() {
             return 0;
@@ -3396,6 +3405,12 @@ public class AppOpsManager {
             return mHistoricalPackageOps.get(packageName);
         }
 
+        private void clearHistory(@NonNull String packageName) {
+            if (mHistoricalPackageOps != null) {
+                mHistoricalPackageOps.remove(packageName);
+            }
+        }
+
         @Override
         public int describeContents() {
             return 0;

diff --git a/services/core/java/com/android/server/appop/AppOpsService.java b/services/core/java/com/android/server/appop/AppOpsService.java
index d04aa89..db7abf8 100644 (file)
--- a/services/core/java/com/android/server/appop/AppOpsService.java
+++ b/services/core/java/com/android/server/appop/AppOpsService.java
@@ -905,6 +905,8 @@ public class AppOpsService extends IAppOpsService.Stub {
                     }
                 }
             }
+
+            mHistoricalRegistry.clearHistory(uid, packageName);
         }
     }
 

diff --git a/services/core/java/com/android/server/appop/HistoricalRegistry.java b/services/core/java/com/android/server/appop/HistoricalRegistry.java
index d723c7b..69a1c9f 100644 (file)
--- a/services/core/java/com/android/server/appop/HistoricalRegistry.java
+++ b/services/core/java/com/android/server/appop/HistoricalRegistry.java
@@ -472,6 +472,25 @@ final class HistoricalRegistry {
                 DEFAULT_COMPRESSION_STEP);
     }
 
+    void clearHistory(int uid, String packageName) {
+        synchronized (mOnDiskLock) {
+            synchronized (mInMemoryLock) {
+                if (mMode != AppOpsManager.HISTORICAL_MODE_ENABLED_ACTIVE) {
+                    return;
+                }
+
+                for (int index = 0; index < mPendingWrites.size(); index++) {
+                    mPendingWrites.get(index).clearHistory(uid, packageName);
+                }
+
+                getUpdatedPendingHistoricalOpsMLocked(System.currentTimeMillis())
+                        .clearHistory(uid, packageName);
+
+                mPersistence.clearHistoryDLocked(uid, packageName);
+            }
+        }
+    }
+
     void clearHistory() {
         synchronized (mOnDiskLock) {
             clearHistoryOnDiskLocked();
@@ -628,6 +647,22 @@ final class HistoricalRegistry {
             return new File(baseDir, Long.toString(globalBeginMillis) + HISTORY_FILE_SUFFIX);
         }
 
+        void clearHistoryDLocked(int uid, String packageName) {
+            List<HistoricalOps> historicalOps = readHistoryDLocked();
+
+            if (historicalOps == null) {
+                return;
+            }
+
+            for (int index = 0; index < historicalOps.size(); index++) {
+                historicalOps.get(index).clearHistory(uid, packageName);
+            }
+
+            clearHistoryDLocked();
+
+            persistHistoricalOpsDLocked(historicalOps);
+        }
+
         void clearHistoryDLocked() {
             mHistoricalAppOpsDir.delete();
         }

diff --git a/services/tests/servicestests/AndroidManifest.xml b/services/tests/servicestests/AndroidManifest.xml
index 01f2f6b..25bd4ec 100644 (file)
--- a/services/tests/servicestests/AndroidManifest.xml
+++ b/services/tests/servicestests/AndroidManifest.xml
@@ -69,6 +69,7 @@
     <uses-permission android:name="android.permission.READ_DEVICE_CONFIG" />
     <uses-permission android:name="android.permission.WRITE_DEVICE_CONFIG" />
     <uses-permission android:name="android.permission.HARDWARE_TEST"/>
+    <uses-permission android:name="android.permission.MANAGE_APPOPS"/>
 
     <!-- Uses API introduced in O (26) -->
     <uses-sdk android:minSdkVersion="1"

diff --git a/services/tests/servicestests/src/com/android/server/appop/AppOpsServiceTest.java b/services/tests/servicestests/src/com/android/server/appop/AppOpsServiceTest.java
index c42a718..d901179 100644 (file)
--- a/services/tests/servicestests/src/com/android/server/appop/AppOpsServiceTest.java
+++ b/services/tests/servicestests/src/com/android/server/appop/AppOpsServiceTest.java
@@ -29,25 +29,28 @@ import static com.google.common.truth.Truth.assertThat;
 import static com.google.common.truth.Truth.assertWithMessage;
 
 import android.app.ActivityManager;
+import android.app.AppOpsManager;
 import android.app.AppOpsManager.OpEntry;
 import android.app.AppOpsManager.PackageOps;
 import android.content.Context;
 import android.os.Handler;
 import android.os.HandlerThread;
 import android.os.Process;
+import android.os.RemoteCallback;
 
 import androidx.test.InstrumentationRegistry;
 import androidx.test.filters.SmallTest;
 import androidx.test.runner.AndroidJUnit4;
 
-import com.android.server.appop.AppOpsService;
-
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 
 import java.io.File;
 import java.util.List;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicReference;
 
 /**
  * Unit tests for AppOpsService. Covers functionality that is difficult to test using CTS tests
@@ -216,6 +219,45 @@ public class AppOpsServiceTest {
     }
 
     @Test
+    public void testPackageRemovedHistoricalOps() throws InterruptedException {
+        mAppOpsService.setMode(OP_READ_SMS, mMyUid, mMyPackageName, MODE_ALLOWED);
+        mAppOpsService.noteOperation(OP_READ_SMS, mMyUid, mMyPackageName);
+
+        AppOpsManager.HistoricalOps historicalOps = new AppOpsManager.HistoricalOps(0, 15000);
+        historicalOps.increaseAccessCount(OP_READ_SMS, mMyUid, mMyPackageName,
+                AppOpsManager.UID_STATE_PERSISTENT, 0, 1);
+
+        mAppOpsService.addHistoricalOps(historicalOps);
+
+        AtomicReference<AppOpsManager.HistoricalOps> resultOpsRef = new AtomicReference<>();
+        AtomicReference<CountDownLatch> latchRef = new AtomicReference<>(new CountDownLatch(1));
+        RemoteCallback callback = new RemoteCallback(result -> {
+            resultOpsRef.set(result.getParcelable(AppOpsManager.KEY_HISTORICAL_OPS));
+            latchRef.get().countDown();
+        });
+
+        // First, do a fetch to ensure it's written
+        mAppOpsService.getHistoricalOps(mMyUid, mMyPackageName, null, 0, Long.MAX_VALUE, 0,
+                callback);
+
+        latchRef.get().await(5, TimeUnit.SECONDS);
+        assertThat(latchRef.get().getCount()).isEqualTo(0);
+        assertThat(resultOpsRef.get().isEmpty()).isFalse();
+
+        // Then, check it's deleted on removal
+        mAppOpsService.packageRemoved(mMyUid, mMyPackageName);
+
+        latchRef.set(new CountDownLatch(1));
+
+        mAppOpsService.getHistoricalOps(mMyUid, mMyPackageName, null, 0, Long.MAX_VALUE, 0,
+                callback);
+
+        latchRef.get().await(5, TimeUnit.SECONDS);
+        assertThat(latchRef.get().getCount()).isEqualTo(0);
+        assertThat(resultOpsRef.get().isEmpty()).isTrue();
+    }
+
+    @Test
     public void testUidRemoved() {
         mAppOpsService.setMode(OP_READ_SMS, mMyUid, mMyPackageName, MODE_ALLOWED);
         mAppOpsService.noteOperation(OP_READ_SMS, mMyUid, mMyPackageName);