Merge "Migrate puller registration to StatsManagerService"

5 files changed, 204 insertions, 188 deletions

diff --git a/apex/statsd/aidl/android/os/IStatsCompanionService.aidl b/apex/statsd/aidl/android/os/IStatsCompanionService.aidl
index 21b7767e932d..99b9d398e30c 100644
--- a/apex/statsd/aidl/android/os/IStatsCompanionService.aidl
+++ b/apex/statsd/aidl/android/os/IStatsCompanionService.aidl
@@ -67,11 +67,4 @@ interface IStatsCompanionService {
 
     /** Tells StatsCompaionService to grab the uid map snapshot and send it to statsd. */
     oneway void triggerUidSnapshot();
-
-    /** Tells StatsCompanionService to tell statsd to register a puller for the given atom id */
-    oneway void registerPullAtomCallback(int atomTag, long coolDownNs, long timeoutNs,
-            in int[] additiveFields, IPullAtomCallback pullerCallback);
-
-    /** Tells StatsCompanionService to tell statsd to unregister a puller for the given atom id */
-    oneway void unregisterPullAtomCallback(int atomTag);
 }
diff --git a/apex/statsd/aidl/android/os/IStatsManagerService.aidl b/apex/statsd/aidl/android/os/IStatsManagerService.aidl
index dec56345ec2f..4a259f50d2f6 100644
--- a/apex/statsd/aidl/android/os/IStatsManagerService.aidl
+++ b/apex/statsd/aidl/android/os/IStatsManagerService.aidl
@@ -17,6 +17,7 @@
 package android.os;
 
 import android.app.PendingIntent;
+import android.os.IPullAtomCallback;
 
 /**
   * Binder interface to communicate with the Java-based statistics service helper.
@@ -125,4 +126,11 @@ interface IStatsManagerService {
      * Requires Manifest.permission.DUMP and Manifest.permission.PACKAGE_USAGE_STATS.
      */
     void removeConfiguration(in long configId, in String packageName);
-}
\ No newline at end of file
+
+    /** Tell StatsManagerService to register a puller for the given atom tag with statsd. */
+    oneway void registerPullAtomCallback(int atomTag, long coolDownNs, long timeoutNs,
+            in int[] additiveFields, IPullAtomCallback pullerCallback);
+
+    /** Tell StatsManagerService to unregister the pulller for the given atom tag from statsd. */
+    oneway void unregisterPullAtomCallback(int atomTag);
+}
diff --git a/apex/statsd/service/java/com/android/server/stats/StatsCompanionService.java b/apex/statsd/service/java/com/android/server/stats/StatsCompanionService.java
index d57afeeb7157..a9081786866c 100644
--- a/apex/statsd/service/java/com/android/server/stats/StatsCompanionService.java
+++ b/apex/statsd/service/java/com/android/server/stats/StatsCompanionService.java
@@ -75,7 +75,6 @@ import android.os.FileUtils;
 import android.os.Handler;
 import android.os.HandlerThread;
 import android.os.IBinder;
-import android.os.IPullAtomCallback;
 import android.os.IStatsCompanionService;
 import android.os.IStatsd;
 import android.os.IStoraged;
@@ -263,71 +262,6 @@ public class StatsCompanionService extends IStatsCompanionService.Stub {
 
     private StatsManagerService mStatsManagerService;
 
-    private static final class PullerKey {
-        private final int mUid;
-        private final int mAtomTag;
-
-        PullerKey(int uid, int atom) {
-            mUid = uid;
-            mAtomTag = atom;
-        }
-
-        public int getUid() {
-            return mUid;
-        }
-
-        public int getAtom() {
-            return mAtomTag;
-        }
-
-        @Override
-        public int hashCode() {
-            return Objects.hash(mUid, mAtomTag);
-        }
-
-        @Override
-        public boolean equals(Object obj) {
-            if (obj instanceof PullerKey) {
-                PullerKey other = (PullerKey) obj;
-                return this.mUid == other.getUid() && this.mAtomTag == other.getAtom();
-            }
-            return false;
-        }
-    }
-
-    private static final class PullerValue {
-        private final long mCoolDownNs;
-        private final long mTimeoutNs;
-        private int[] mAdditiveFields;
-        private IPullAtomCallback mCallback;
-
-        PullerValue(long coolDownNs, long timeoutNs, int[] additiveFields,
-                IPullAtomCallback callback) {
-            mCoolDownNs = coolDownNs;
-            mTimeoutNs = timeoutNs;
-            mAdditiveFields = additiveFields;
-            mCallback = callback;
-        }
-
-        public long getCoolDownNs() {
-            return mCoolDownNs;
-        }
-
-        public long getTimeoutNs() {
-            return mTimeoutNs;
-        }
-
-        public int[] getAdditiveFields() {
-            return mAdditiveFields;
-        }
-
-        public IPullAtomCallback getCallback() {
-            return mCallback;
-        }
-    }
-
-    private final HashMap<PullerKey, PullerValue> mPullers = new HashMap<>();
-
     private final KernelWakelockReader mKernelWakelockReader = new KernelWakelockReader();
     private final KernelWakelockStats mTmpWakelockStats = new KernelWakelockStats();
     private WifiManager mWifiManager = null;
@@ -2634,57 +2568,6 @@ public class StatsCompanionService extends IStatsCompanionService.Stub {
         }
     }
 
-    @Override
-    public void registerPullAtomCallback(int atomTag, long coolDownNs, long timeoutNs,
-            int[] additiveFields, IPullAtomCallback pullerCallback) {
-        synchronized (sStatsdLock) {
-            // Always cache the puller in SCS.
-            // If statsd is down, we will register it when it comes back up.
-            int callingUid = Binder.getCallingUid();
-            final long token = Binder.clearCallingIdentity();
-            PullerKey key = new PullerKey(callingUid, atomTag);
-            PullerValue val = new PullerValue(
-                    coolDownNs, timeoutNs, additiveFields, pullerCallback);
-            mPullers.put(key, val);
-
-            if (sStatsd == null) {
-                Slog.w(TAG, "Could not access statsd for registering puller for atom " + atomTag);
-                return;
-            }
-            try {
-                sStatsd.registerPullAtomCallback(
-                        callingUid, atomTag, coolDownNs, timeoutNs, additiveFields, pullerCallback);
-            } catch (RemoteException e) {
-                Slog.e(TAG, "Failed to access statsd to register puller for atom " + atomTag);
-            } finally {
-                Binder.restoreCallingIdentity(token);
-            }
-        }
-    }
-
-    @Override
-    public void unregisterPullAtomCallback(int atomTag) {
-        synchronized (sStatsdLock) {
-            // Always remove the puller in SCS.
-            // If statsd is down, we will not register it when it comes back up.
-            int callingUid = Binder.getCallingUid();
-            final long token = Binder.clearCallingIdentity();
-            PullerKey key = new PullerKey(callingUid, atomTag);
-            mPullers.remove(key);
-
-            if (sStatsd == null) {
-                Slog.w(TAG, "Could not access statsd for registering puller for atom " + atomTag);
-                return;
-            }
-            try {
-                sStatsd.unregisterPullAtomCallback(callingUid, atomTag);
-            } catch (RemoteException e) {
-                Slog.e(TAG, "Failed to access statsd to register puller for atom " + atomTag);
-            } finally {
-                Binder.restoreCallingIdentity(token);
-            }
-        }
-    }
 
     // Statsd related code
 
@@ -2763,8 +2646,6 @@ public class StatsCompanionService extends IStatsCompanionService.Stub {
                     // Pull the latest state of UID->app name, version mapping when
                     // statsd starts.
                     informAllUidsLocked(mContext);
-                    // Register all pullers. If SCS has just started, this should be empty.
-                    registerAllPullersLocked();
                 } finally {
                     restoreCallingIdentity(token);
                 }
@@ -2776,17 +2657,6 @@ public class StatsCompanionService extends IStatsCompanionService.Stub {
         }
     }
 
-    @GuardedBy("sStatsdLock")
-    private void registerAllPullersLocked() throws RemoteException {
-        // TODO: pass in one call, using a file descriptor (similar to uidmap).
-        for (Map.Entry<PullerKey, PullerValue> entry : mPullers.entrySet()) {
-            PullerKey key = entry.getKey();
-            PullerValue val = entry.getValue();
-            sStatsd.registerPullAtomCallback(key.getUid(), key.getAtom(), val.getCoolDownNs(),
-                    val.getTimeoutNs(), val.getAdditiveFields(), val.getCallback());
-        }
-    }
-
     private class StatsdDeathRecipient implements IBinder.DeathRecipient {
         @Override
         public void binderDied() {
diff --git a/apex/statsd/service/java/com/android/server/stats/StatsManagerService.java b/apex/statsd/service/java/com/android/server/stats/StatsManagerService.java
index b27d0f7699fc..04d8b006f51d 100644
--- a/apex/statsd/service/java/com/android/server/stats/StatsManagerService.java
+++ b/apex/statsd/service/java/com/android/server/stats/StatsManagerService.java
@@ -24,6 +24,7 @@ import android.app.AppOpsManager;
 import android.app.PendingIntent;
 import android.content.Context;
 import android.os.Binder;
+import android.os.IPullAtomCallback;
 import android.os.IStatsManagerService;
 import android.os.IStatsd;
 import android.os.Process;
@@ -60,8 +61,7 @@ public class StatsManagerService extends IStatsManagerService.Stub {
     @GuardedBy("mLock")
     private ArrayMap<ConfigKey, PendingIntentRef> mDataFetchPirMap = new ArrayMap<>();
     @GuardedBy("mLock")
-    private ArrayMap<Integer, PendingIntentRef> mActiveConfigsPirMap =
-            new ArrayMap<>();
+    private ArrayMap<Integer, PendingIntentRef> mActiveConfigsPirMap = new ArrayMap<>();
     @GuardedBy("mLock")
     private ArrayMap<ConfigKey, ArrayMap<Long, PendingIntentRef>> mBroadcastSubscriberPirMap =
             new ArrayMap<>();
@@ -72,8 +72,8 @@ public class StatsManagerService extends IStatsManagerService.Stub {
     }
 
     private static class ConfigKey {
-        private int mUid;
-        private long mConfigId;
+        private final int mUid;
+        private final long mConfigId;
 
         ConfigKey(int uid, long configId) {
             mUid = uid;
@@ -103,6 +103,126 @@ public class StatsManagerService extends IStatsManagerService.Stub {
         }
     }
 
+    private static class PullerKey {
+        private final int mUid;
+        private final int mAtomTag;
+
+        PullerKey(int uid, int atom) {
+            mUid = uid;
+            mAtomTag = atom;
+        }
+
+        public int getUid() {
+            return mUid;
+        }
+
+        public int getAtom() {
+            return mAtomTag;
+        }
+
+        @Override
+        public int hashCode() {
+            return Objects.hash(mUid, mAtomTag);
+        }
+
+        @Override
+        public boolean equals(Object obj) {
+            if (obj instanceof PullerKey) {
+                PullerKey other = (PullerKey) obj;
+                return this.mUid == other.getUid() && this.mAtomTag == other.getAtom();
+            }
+            return false;
+        }
+    }
+
+    private static class PullerValue {
+        private final long mCoolDownNs;
+        private final long mTimeoutNs;
+        private final int[] mAdditiveFields;
+        private final IPullAtomCallback mCallback;
+
+        PullerValue(long coolDownNs, long timeoutNs, int[] additiveFields,
+                IPullAtomCallback callback) {
+            mCoolDownNs = coolDownNs;
+            mTimeoutNs = timeoutNs;
+            mAdditiveFields = additiveFields;
+            mCallback = callback;
+        }
+
+        public long getCoolDownNs() {
+            return mCoolDownNs;
+        }
+
+        public long getTimeoutNs() {
+            return mTimeoutNs;
+        }
+
+        public int[] getAdditiveFields() {
+            return mAdditiveFields;
+        }
+
+        public IPullAtomCallback getCallback() {
+            return mCallback;
+        }
+    }
+
+    private final ArrayMap<PullerKey, PullerValue> mPullers = new ArrayMap<>();
+
+    @Override
+    public void registerPullAtomCallback(int atomTag, long coolDownNs, long timeoutNs,
+            int[] additiveFields, IPullAtomCallback pullerCallback) {
+        int callingUid = Binder.getCallingUid();
+        final long token = Binder.clearCallingIdentity();
+        PullerKey key = new PullerKey(callingUid, atomTag);
+        PullerValue val = new PullerValue(coolDownNs, timeoutNs, additiveFields, pullerCallback);
+
+        // Always cache the puller in StatsManagerService. If statsd is down, we will register the
+        // puller when statsd comes back up.
+        synchronized (mLock) {
+            mPullers.put(key, val);
+        }
+
+        IStatsd statsd = getStatsdNonblocking();
+        if (statsd == null) {
+            return;
+        }
+
+        try {
+            statsd.registerPullAtomCallback(
+                    callingUid, atomTag, coolDownNs, timeoutNs, additiveFields, pullerCallback);
+        } catch (RemoteException e) {
+            Slog.e(TAG, "Failed to access statsd to register puller for atom " + atomTag);
+        } finally {
+            Binder.restoreCallingIdentity(token);
+        }
+    }
+
+    @Override
+    public void unregisterPullAtomCallback(int atomTag) {
+        int callingUid = Binder.getCallingUid();
+        final long token = Binder.clearCallingIdentity();
+        PullerKey key = new PullerKey(callingUid, atomTag);
+
+        // Always remove the puller from StatsManagerService even if statsd is down. When statsd
+        // comes back up, we will not re-register the removed puller.
+        synchronized (mLock) {
+            mPullers.remove(key);
+        }
+
+        IStatsd statsd = getStatsdNonblocking();
+        if (statsd == null) {
+            return;
+        }
+
+        try {
+            statsd.unregisterPullAtomCallback(callingUid, atomTag);
+        } catch (RemoteException e) {
+            Slog.e(TAG, "Failed to access statsd to unregister puller for atom " + atomTag);
+        } finally {
+            Binder.restoreCallingIdentity(token);
+        }
+    }
+
     @Override
     public void setDataFetchOperation(long configId, PendingIntent pendingIntent,
             String packageName) {
@@ -441,46 +561,85 @@ public class StatsManagerService extends IStatsManagerService.Stub {
         if (statsd == null) {
             return;
         }
-        // Since we do not want to make an IPC with the a lock held, we first create local deep
-        // copies of the data with the lock held before iterating through the maps.
+
+        final long token = Binder.clearCallingIdentity();
+        try {
+            registerAllPullers(statsd);
+            registerAllDataFetchOperations(statsd);
+            registerAllActiveConfigsChangedOperations(statsd);
+            registerAllBroadcastSubscribers(statsd);
+        } catch (RemoteException e) {
+            Slog.e(TAG, "StatsManager failed to (re-)register data with statsd");
+        } finally {
+            Binder.restoreCallingIdentity(token);
+        }
+    }
+
+    // Pre-condition: the Binder calling identity has already been cleared
+    private void registerAllPullers(IStatsd statsd) throws RemoteException {
+        // Since we do not want to make an IPC with the lock held, we first create a copy of the
+        // data with the lock held before iterating through the map.
+        ArrayMap<PullerKey, PullerValue> pullersCopy;
+        synchronized (mLock) {
+            pullersCopy = new ArrayMap<>(mPullers);
+        }
+
+        for (Map.Entry<PullerKey, PullerValue> entry : pullersCopy.entrySet()) {
+            PullerKey key = entry.getKey();
+            PullerValue value = entry.getValue();
+            statsd.registerPullAtomCallback(key.getUid(), key.getAtom(), value.getCoolDownNs(),
+                    value.getTimeoutNs(), value.getAdditiveFields(), value.getCallback());
+        }
+    }
+
+    // Pre-condition: the Binder calling identity has already been cleared
+    private void registerAllDataFetchOperations(IStatsd statsd) throws RemoteException {
+        // Since we do not want to make an IPC with the lock held, we first create a copy of the
+        // data with the lock held before iterating through the map.
         ArrayMap<ConfigKey, PendingIntentRef> dataFetchCopy;
-        ArrayMap<Integer, PendingIntentRef> activeConfigsChangedCopy;
-        ArrayMap<ConfigKey, ArrayMap<Long, PendingIntentRef>> broadcastSubscriberCopy;
         synchronized (mLock) {
             dataFetchCopy = new ArrayMap<>(mDataFetchPirMap);
-            activeConfigsChangedCopy = new ArrayMap<>(mActiveConfigsPirMap);
-            broadcastSubscriberCopy = new ArrayMap<>();
-            for (Map.Entry<ConfigKey, ArrayMap<Long, PendingIntentRef>> entry
-                    : mBroadcastSubscriberPirMap.entrySet()) {
-                broadcastSubscriberCopy.put(entry.getKey(), new ArrayMap<>(entry.getValue()));
-            }
         }
+
         for (Map.Entry<ConfigKey, PendingIntentRef> entry : dataFetchCopy.entrySet()) {
             ConfigKey key = entry.getKey();
-            try {
-                statsd.setDataFetchOperation(key.getConfigId(), entry.getValue(), key.getUid());
-            } catch (RemoteException e) {
-                Slog.e(TAG, "Failed to setDataFetchOperation from pirMap");
-            }
+            statsd.setDataFetchOperation(key.getConfigId(), entry.getValue(), key.getUid());
+        }
+    }
+
+    // Pre-condition: the Binder calling identity has already been cleared
+    private void registerAllActiveConfigsChangedOperations(IStatsd statsd) throws RemoteException {
+        // Since we do not want to make an IPC with the lock held, we first create a copy of the
+        // data with the lock held before iterating through the map.
+        ArrayMap<Integer, PendingIntentRef> activeConfigsChangedCopy;
+        synchronized (mLock) {
+            activeConfigsChangedCopy = new ArrayMap<>(mActiveConfigsPirMap);
+        }
+
+        for (Map.Entry<Integer, PendingIntentRef> entry : activeConfigsChangedCopy.entrySet()) {
+            statsd.setActiveConfigsChangedOperation(entry.getValue(), entry.getKey());
         }
-        for (Map.Entry<Integer, PendingIntentRef> entry
-                : activeConfigsChangedCopy.entrySet()) {
-            try {
-                statsd.setActiveConfigsChangedOperation(entry.getValue(), entry.getKey());
-            } catch (RemoteException e) {
-                Slog.e(TAG, "Failed to setActiveConfigsChangedOperation from pirMap");
+    }
+
+    // Pre-condition: the Binder calling identity has already been cleared
+    private void registerAllBroadcastSubscribers(IStatsd statsd) throws RemoteException {
+        // Since we do not want to make an IPC with the lock held, we first create a deep copy of
+        // the data with the lock held before iterating through the map.
+        ArrayMap<ConfigKey, ArrayMap<Long, PendingIntentRef>> broadcastSubscriberCopy =
+                new ArrayMap<>();
+        synchronized (mLock) {
+            for (Map.Entry<ConfigKey, ArrayMap<Long, PendingIntentRef>> entry :
+                    mBroadcastSubscriberPirMap.entrySet()) {
+                broadcastSubscriberCopy.put(entry.getKey(), new ArrayMap(entry.getValue()));
             }
         }
-        for (Map.Entry<ConfigKey, ArrayMap<Long, PendingIntentRef>> entry
-                : broadcastSubscriberCopy.entrySet()) {
+
+        for (Map.Entry<ConfigKey, ArrayMap<Long, PendingIntentRef>> entry :
+                mBroadcastSubscriberPirMap.entrySet()) {
+            ConfigKey configKey = entry.getKey();
             for (Map.Entry<Long, PendingIntentRef> subscriberEntry : entry.getValue().entrySet()) {
-                ConfigKey configKey = entry.getKey();
-                try {
-                    statsd.setBroadcastSubscriber(configKey.getConfigId(), subscriberEntry.getKey(),
-                            subscriberEntry.getValue(), configKey.getUid());
-                } catch (RemoteException e) {
-                    Slog.e(TAG, "Failed to setBroadcastSubscriber from pirMap");
-                }
+                statsd.setBroadcastSubscriber(configKey.getConfigId(), subscriberEntry.getKey(),
+                        subscriberEntry.getValue(), configKey.getUid());
             }
         }
     }
diff --git a/core/java/android/app/StatsManager.java b/core/java/android/app/StatsManager.java
index 84263749232d..dde6dda8e448 100644
--- a/core/java/android/app/StatsManager.java
+++ b/core/java/android/app/StatsManager.java
@@ -26,7 +26,6 @@ import android.content.Context;
 import android.os.IBinder;
 import android.os.IPullAtomCallback;
 import android.os.IPullAtomResultReceiver;
-import android.os.IStatsCompanionService;
 import android.os.IStatsManagerService;
 import android.os.IStatsPullerCallback;
 import android.os.IStatsd;
@@ -61,9 +60,6 @@ public final class StatsManager {
     private IStatsd mService;
 
     @GuardedBy("sLock")
-    private IStatsCompanionService mStatsCompanion;
-
-    @GuardedBy("sLock")
     private IStatsManagerService mStatsManagerService;
 
     /**
@@ -538,7 +534,7 @@ public final class StatsManager {
         }
         synchronized (sLock) {
             try {
-                IStatsCompanionService service = getIStatsCompanionServiceLocked();
+                IStatsManagerService service = getIStatsManagerServiceLocked();
                 PullAtomCallbackInternal rec =
                     new PullAtomCallbackInternal(atomTag, callback, executor);
                 service.registerPullAtomCallback(atomTag, coolDownNs, timeoutNs, additiveFields,
@@ -560,7 +556,7 @@ public final class StatsManager {
     public void unregisterPullAtomCallback(int atomTag) {
         synchronized (sLock) {
             try {
-                IStatsCompanionService service = getIStatsCompanionServiceLocked();
+                IStatsManagerService service = getIStatsManagerServiceLocked();
                 service.unregisterPullAtomCallback(atomTag);
             } catch (RemoteException e) {
                 throw new RuntimeException("Unable to unregister pull atom callback");
@@ -746,16 +742,6 @@ public final class StatsManager {
     }
 
     @GuardedBy("sLock")
-    private IStatsCompanionService getIStatsCompanionServiceLocked() {
-        if (mStatsCompanion != null) {
-            return mStatsCompanion;
-        }
-        mStatsCompanion = IStatsCompanionService.Stub.asInterface(
-                ServiceManager.getService("statscompanion"));
-        return mStatsCompanion;
-    }
-
-    @GuardedBy("sLock")
     private IStatsManagerService getIStatsManagerServiceLocked() {
         if (mStatsManagerService != null) {
             return mStatsManagerService;