Merge "Only add packageOps if they don't already exist in systemReady" into udc-dev

services/core/java/com/android/server/appop/AppOpsService.java

@@ -157,10 +157,10 @@ import com.android.server.LockGuard;
 import com.android.server.SystemServerInitThreadPool;
 import com.android.server.SystemServiceManager;
 import com.android.server.pm.PackageList;
+import com.android.server.pm.PackageManagerLocal;
 import com.android.server.pm.UserManagerInternal;
 import com.android.server.pm.pkg.AndroidPackage;
 import com.android.server.pm.pkg.PackageState;
-import com.android.server.pm.pkg.PackageStateInternal;
 import com.android.server.pm.pkg.component.ParsedAttribution;
 import com.android.server.policy.AppOpsPolicy;
 
@@ -367,6 +367,9 @@ public class AppOpsService extends IAppOpsService.Stub {
     /** Package Manager internal. Access via {@link #getPackageManagerInternal()} */
     private @Nullable PackageManagerInternal mPackageManagerInternal;
 
+    /** Package Manager local. Access via {@link #getPackageManagerLocal()} */
+    private @Nullable PackageManagerLocal mPackageManagerLocal;
+
     /** User Manager internal. Access via {@link #getUserManagerInternal()} */
     private @Nullable UserManagerInternal mUserManagerInternal;
 
@@ -1189,42 +1192,64 @@ public class AppOpsService extends IAppOpsService.Stub {
     /**
      * Initialize uid state objects for state contained in the checking service.
      */
-    private void initializeUidStates() {
+    @VisibleForTesting
+    void initializeUidStates() {
         UserManagerInternal umi = getUserManagerInternal();
-        int[] userIds = umi.getUserIds();
         synchronized (this) {
-            for (int i = 0; i < userIds.length; i++) {
+            int[] userIds = umi.getUserIds();
-                int userId = userIds[i];
+            try (PackageManagerLocal.UnfilteredSnapshot snapshot =
-                initializeUserUidStatesLocked(userId);
+                         getPackageManagerLocal().withUnfilteredSnapshot()) {
+                Map<String, PackageState> packageStates = snapshot.getPackageStates();
+                for (int i = 0; i < userIds.length; i++) {
+                    int userId = userIds[i];
+                    initializeUserUidStatesLocked(userId, packageStates);
+                }
             }
         }
     }
 
     private void initializeUserUidStates(int userId) {
         synchronized (this) {
-            initializeUserUidStatesLocked(userId);
+            try (PackageManagerLocal.UnfilteredSnapshot snapshot =
+                    getPackageManagerLocal().withUnfilteredSnapshot()) {
+                initializeUserUidStatesLocked(userId, snapshot.getPackageStates());
+            }
         }
     }
 
-    private void initializeUserUidStatesLocked(int userId) {
+    private void initializeUserUidStatesLocked(int userId, Map<String,
-        ArrayMap<String, ? extends PackageStateInternal> packageStates =
+            PackageState> packageStates) {
-                getPackageManagerInternal().getPackageStates();
+        for (Map.Entry<String, PackageState> entry : packageStates.entrySet()) {
-        for (int j = 0; j < packageStates.size(); j++) {
+            int appId = entry.getValue().getAppId();
-            PackageStateInternal packageState = packageStates.valueAt(j);
+            String packageName = entry.getKey();
-            int uid = UserHandle.getUid(userId, packageState.getAppId());
-            UidState uidState = getUidStateLocked(uid, true);
-            String packageName = packageStates.keyAt(j);
-            Ops ops = new Ops(packageName, uidState);
-            uidState.pkgOps.put(packageName, ops);
 
-            SparseIntArray packageModes =
+            initializePackageUidStateLocked(userId, appId, packageName);
-                    mAppOpsCheckingService.getNonDefaultPackageModes(packageName, userId);
+        }
-            for (int k = 0; k < packageModes.size(); k++) {
+    }
-                int code = packageModes.get(k);
+
+    /*
+      Be careful not to clear any existing data; only want to add objects that don't already exist.
+     */
+    private void initializePackageUidStateLocked(int userId, int appId, String packageName) {
+        int uid = UserHandle.getUid(userId, appId);
+        UidState uidState = getUidStateLocked(uid, true);
+        Ops ops = uidState.pkgOps.get(packageName);
+        if (ops == null) {
+            ops = new Ops(packageName, uidState);
+            uidState.pkgOps.put(packageName, ops);
+        }
+
+        SparseIntArray packageModes =
+                mAppOpsCheckingService.getNonDefaultPackageModes(packageName, userId);
+        for (int k = 0; k < packageModes.size(); k++) {
+            int code = packageModes.keyAt(k);
+
+            if (ops.indexOfKey(code) < 0) {
                 ops.put(code, new Op(uidState, packageName, code, uid));
             }
-            uidState.evalForegroundOps();
         }
+
+        uidState.evalForegroundOps();
     }
 
     /**
@@ -3648,6 +3673,20 @@ public class AppOpsService extends IAppOpsService.Stub {
         return mPackageManagerInternal;
     }
 
+    /**
+     * @return {@link PackageManagerLocal}
+     */
+    private @NonNull PackageManagerLocal getPackageManagerLocal() {
+        if (mPackageManagerLocal == null) {
+            mPackageManagerLocal = LocalManagerRegistry.getManager(PackageManagerLocal.class);
+        }
+        if (mPackageManagerLocal == null) {
+            throw new IllegalStateException("PackageManagerLocal not loaded");
+        }
+
+        return mPackageManagerLocal;
+    }
+
     /**
      * @return {@link UserManagerInternal}
      */

services/tests/mockingservicestests/src/com/android/server/appop/AppOpsServiceTest.java

@@ -19,9 +19,11 @@ import static android.app.AppOpsManager.MODE_ALLOWED;
 import static android.app.AppOpsManager.MODE_ERRORED;
 import static android.app.AppOpsManager.OP_COARSE_LOCATION;
 import static android.app.AppOpsManager.OP_FLAGS_ALL;
+import static android.app.AppOpsManager.OP_FLAG_SELF;
 import static android.app.AppOpsManager.OP_READ_SMS;
 import static android.app.AppOpsManager.OP_WIFI_SCAN;
 import static android.app.AppOpsManager.OP_WRITE_SMS;
+import static android.os.UserHandle.getUserId;
 
 import static com.android.dx.mockito.inline.extended.ExtendedMockito.doNothing;
 import static com.android.dx.mockito.inline.extended.ExtendedMockito.doReturn;
@@ -33,12 +35,15 @@ import static com.android.dx.mockito.inline.extended.ExtendedMockito.when;
 import static com.google.common.truth.Truth.assertThat;
 import static com.google.common.truth.Truth.assertWithMessage;
 
+import static org.junit.Assert.assertNotEquals;
+import static org.junit.Assert.assertNotNull;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.anyInt;
 import static org.mockito.ArgumentMatchers.anyString;
 import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.ArgumentMatchers.nullable;
 
+import android.app.AppOpsManager;
 import android.app.AppOpsManager.OpEntry;
 import android.app.AppOpsManager.PackageOps;
 import android.content.ContentResolver;
@@ -48,14 +53,19 @@ import android.os.Handler;
 import android.os.HandlerThread;
 import android.os.Process;
 import android.provider.Settings;
+import android.util.ArrayMap;
 
 import androidx.test.InstrumentationRegistry;
 import androidx.test.filters.SmallTest;
 import androidx.test.runner.AndroidJUnit4;
 
 import com.android.dx.mockito.inline.extended.StaticMockitoSession;
+import com.android.server.LocalManagerRegistry;
 import com.android.server.LocalServices;
+import com.android.server.pm.PackageManagerLocal;
+import com.android.server.pm.UserManagerInternal;
 import com.android.server.pm.pkg.AndroidPackage;
+import com.android.server.pm.pkg.PackageState;
 import com.android.server.pm.pkg.PackageStateInternal;
 
 import org.junit.After;
@@ -67,6 +77,7 @@ import org.mockito.quality.Strictness;
 import java.io.File;
 import java.util.Collections;
 import java.util.List;
+import java.util.Map;
 
 /**
  * Unit tests for AppOpsService. Covers functionality that is difficult to test using CTS tests
@@ -133,6 +144,7 @@ public class AppOpsServiceTest {
         mMockingSession = mockitoSession()
                 .strictness(Strictness.LENIENT)
                 .spyStatic(LocalServices.class)
+                .spyStatic(LocalManagerRegistry.class)
                 .spyStatic(Settings.Global.class)
                 .startMocking();
 
@@ -152,6 +164,23 @@ public class AppOpsServiceTest {
         doReturn(mockPackageManagerInternal).when(
                 () -> LocalServices.getService(PackageManagerInternal.class));
 
+        PackageManagerLocal mockPackageManagerLocal = mock(PackageManagerLocal.class);
+        PackageManagerLocal.UnfilteredSnapshot mockUnfilteredSnapshot =
+                mock(PackageManagerLocal.UnfilteredSnapshot.class);
+        PackageState mockMyPS = mock(PackageState.class);
+        ArrayMap<String, PackageState> packageStates = new ArrayMap<>();
+        packageStates.put(sMyPackageName, mockMyPS);
+        when(mockMyPS.getAppId()).thenReturn(mMyUid);
+        when(mockUnfilteredSnapshot.getPackageStates()).thenReturn(packageStates);
+        when(mockPackageManagerLocal.withUnfilteredSnapshot()).thenReturn(mockUnfilteredSnapshot);
+        doReturn(mockPackageManagerLocal).when(
+                () -> LocalManagerRegistry.getManager(PackageManagerLocal.class));
+
+        UserManagerInternal mockUserManagerInternal = mock(UserManagerInternal.class);
+        when(mockUserManagerInternal.getUserIds()).thenReturn(new int[] {getUserId(mMyUid)});
+        doReturn(mockUserManagerInternal).when(
+                () -> LocalServices.getService(UserManagerInternal.class));
+
         // Mock behavior to use specific Settings.Global.APPOP_HISTORY_PARAMETERS
         doReturn(null).when(() -> Settings.Global.getString(any(ContentResolver.class),
                 eq(Settings.Global.APPOP_HISTORY_PARAMETERS)));
@@ -337,6 +366,25 @@ public class AppOpsServiceTest {
         assertThat(getLoggedOps()).isNull();
     }
 
+    @Test
+    public void testUidStateInitializationDoesntClearState() throws InterruptedException {
+        mAppOpsService.setMode(OP_READ_SMS, mMyUid, sMyPackageName, MODE_ALLOWED);
+        mAppOpsService.noteOperation(OP_READ_SMS, mMyUid, sMyPackageName, null, false, null, false);
+        mAppOpsService.initializeUidStates();
+        List<PackageOps> ops = mAppOpsService.getOpsForPackage(mMyUid, sMyPackageName,
+                new int[]{OP_READ_SMS});
+        assertNotNull(ops);
+        for (int i = 0; i < ops.size(); i++) {
+            List<OpEntry> opEntries = ops.get(i).getOps();
+            for (int j = 0; j < opEntries.size(); j++) {
+                Map<String, AppOpsManager.AttributedOpEntry> attributedOpEntries = opEntries.get(
+                        j).getAttributedOpEntries();
+                assertNotEquals(-1, attributedOpEntries.get(null)
+                        .getLastAccessTime(OP_FLAG_SELF));
+            }
+        }
+    }
+
     private List<PackageOps> getLoggedOps() {
         return mAppOpsService.getOpsForPackage(mMyUid, sMyPackageName, null /* all ops */);
     }