Remove references to Discrete Register from AppOpsService.

Also record DiscreteOp when AppOp is started, later update it with proper
duration when it is finished. Previously we only recorded it when it is
finished.

Test: In separate CL
Change-Id: Ieada561de58a6fe34911bba21f770dcb2fb87f12

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

@@ -848,10 +848,7 @@ public class AppOpsService extends IAppOpsService.Stub {
                     proxyAttributionTag, uidState, flags);
 
             mHistoricalRegistry.incrementOpAccessedCount(parent.op, parent.uid, parent.packageName,
-                    tag, uidState, flags);
-
-            mHistoricalRegistry.mDiscreteRegistry.recordDiscreteAccess(parent.uid,
-                    parent.packageName, parent.op, tag, flags, uidState, accessTime, -1);
+                    tag, uidState, flags, accessTime);
         }
 
         /**
@@ -955,9 +952,10 @@ public class AppOpsService extends IAppOpsService.Stub {
                 mInProgressEvents = new ArrayMap<>(1);
             }
 
+            long startTime = System.currentTimeMillis();
             InProgressStartOpEvent event = mInProgressEvents.get(clientId);
             if (event == null) {
-                event = mInProgressStartOpEventPool.acquire(System.currentTimeMillis(),
+                event = mInProgressStartOpEventPool.acquire(startTime,
                         SystemClock.elapsedRealtime(), clientId,
                         PooledLambda.obtainRunnable(AppOpsService::onClientDeath, this, clientId),
                         proxyUid, proxyPackageName, proxyAttributionTag, uidState, flags);
@@ -971,7 +969,7 @@ public class AppOpsService extends IAppOpsService.Stub {
             event.numUnfinishedStarts++;
 
             mHistoricalRegistry.incrementOpAccessedCount(parent.op, parent.uid, parent.packageName,
-                    tag, uidState, flags);
+                    tag, uidState, flags, startTime);
         }
 
         /**
@@ -1017,11 +1015,7 @@ public class AppOpsService extends IAppOpsService.Stub {
 
                 mHistoricalRegistry.increaseOpAccessDuration(parent.op, parent.uid,
                         parent.packageName, tag, event.getUidState(),
-                        event.getFlags(), finishedEvent.getDuration());
-
-                mHistoricalRegistry.mDiscreteRegistry.recordDiscreteAccess(parent.uid,
-                        parent.packageName, parent.op, tag, event.getFlags(), event.getUidState(),
-                        event.getStartTime(), accessDurationMillis);
+                        event.getFlags(), finishedEvent.getNoteTime(), finishedEvent.getDuration());
 
                 mInProgressStartOpEventPool.release(event);
 
@@ -4769,7 +4763,7 @@ public class AppOpsService extends IAppOpsService.Stub {
                 mFile.failWrite(stream);
             }
         }
-        mHistoricalRegistry.mDiscreteRegistry.writeAndClearAccessHistory();
+        mHistoricalRegistry.writeAndClearDiscreteHistory();
     }
 
     static class Shell extends ShellCommand {
@@ -6125,8 +6119,7 @@ public class AppOpsService extends IAppOpsService.Stub {
         mContext.enforceCallingOrSelfPermission(android.Manifest.permission.MANAGE_APPOPS,
                 "clearHistory");
         // Must not hold the appops lock
-        mHistoricalRegistry.clearHistory();
-        mHistoricalRegistry.mDiscreteRegistry.clearHistory();
+        mHistoricalRegistry.clearAllHistory();
     }
 
     @Override

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

@@ -495,6 +495,12 @@ final class DiscreteRegistry {
             int nAttributedOps = attributedOps.size();
             for (int i = nAttributedOps - 1; i >= 0; i--) {
                 DiscreteOpEvent previousOp = attributedOps.get(i);
+                if (i == nAttributedOps - 1 && previousOp.mNoteTime == accessTime
+                        && accessDuration > -1) {
+                    // existing event with updated duration
+                    attributedOps.remove(i);
+                    break;
+                }
                 if (previousOp.mNoteTime < accessTime) {
                     break;
                 }

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

@@ -138,7 +138,7 @@ final class HistoricalRegistry {
     private static final String PARAMETER_ASSIGNMENT = "=";
     private static final String PROPERTY_PERMISSIONS_HUB_ENABLED = "permissions_hub_enabled";
 
-    volatile @NonNull DiscreteRegistry mDiscreteRegistry;
+    private volatile @NonNull DiscreteRegistry mDiscreteRegistry;
 
     @GuardedBy("mLock")
     private @NonNull LinkedList<HistoricalOps> mPendingWrites = new LinkedList<>();
@@ -477,7 +477,8 @@ final class HistoricalRegistry {
     }
 
     void incrementOpAccessedCount(int op, int uid, @NonNull String packageName,
-            @Nullable String attributionTag, @UidState int uidState, @OpFlags int flags) {
+            @Nullable String attributionTag, @UidState int uidState, @OpFlags int flags,
+            long accessTime) {
         synchronized (mInMemoryLock) {
             if (mMode == AppOpsManager.HISTORICAL_MODE_ENABLED_ACTIVE) {
                 if (!isPersistenceInitializedMLocked()) {
@@ -487,6 +488,9 @@ final class HistoricalRegistry {
                 getUpdatedPendingHistoricalOpsMLocked(
                         System.currentTimeMillis()).increaseAccessCount(op, uid, packageName,
                         attributionTag, uidState, flags, 1);
+
+                mDiscreteRegistry.recordDiscreteAccess(uid, packageName, op, attributionTag,
+                        flags, uidState, accessTime, -1);
             }
         }
     }
@@ -508,7 +512,7 @@ final class HistoricalRegistry {
 
     void increaseOpAccessDuration(int op, int uid, @NonNull String packageName,
             @Nullable String attributionTag, @UidState int uidState, @OpFlags int flags,
-            long increment) {
+            long eventStartTime, long increment) {
         synchronized (mInMemoryLock) {
             if (mMode == AppOpsManager.HISTORICAL_MODE_ENABLED_ACTIVE) {
                 if (!isPersistenceInitializedMLocked()) {
@@ -518,6 +522,8 @@ final class HistoricalRegistry {
                 getUpdatedPendingHistoricalOpsMLocked(
                         System.currentTimeMillis()).increaseAccessDuration(op, uid, packageName,
                         attributionTag, uidState, flags, increment);
+                mDiscreteRegistry.recordDiscreteAccess(uid, packageName, op, attributionTag,
+                        flags, uidState, increment, eventStartTime);
             }
         }
     }
@@ -562,7 +568,7 @@ final class HistoricalRegistry {
                     return;
                 }
                 final List<HistoricalOps> history = mPersistence.readHistoryDLocked();
-                clearHistory();
+                clearHistoricalRegistry();
                 if (history != null) {
                     final int historySize = history.size();
                     for (int i = 0; i < historySize; i++) {
@@ -631,7 +637,16 @@ final class HistoricalRegistry {
         }
     }
 
-    void clearHistory() {
+    void writeAndClearDiscreteHistory() {
+        mDiscreteRegistry.writeAndClearAccessHistory();
+    }
+
+    void clearAllHistory() {
+        clearHistoricalRegistry();
+        mDiscreteRegistry.clearHistory();
+    }
+
+    void clearHistoricalRegistry() {
         synchronized (mOnDiskLock) {
             synchronized (mInMemoryLock) {
                 if (!isPersistenceInitializedMLocked()) {