Merge changes from topic "tej-native-feedback" into rvc-dev

* changes:
  Minor followups from api feedback
  Native API feedback for puller API
diff --git a/libstats/pull/Android.bp b/libstats/pull/Android.bp
index 1a9cb92..0fb8f1b 100644
--- a/libstats/pull/Android.bp
+++ b/libstats/pull/Android.bp
@@ -65,3 +65,25 @@
         "//frameworks/base/apex/statsd/tests/libstatspull",
     ],
 }
+
+// Note: These unit tests only test PullAtomMetadata.
+// For full E2E tests of libstatspull, use LibStatsPullTests
+cc_test {
+    name: "libstatspull_test",
+    srcs: [
+        "tests/pull_atom_metadata_test.cpp",
+    ],
+    shared_libs: [
+        "libstatspull",
+        "libstatssocket",
+    ],
+    test_suites: ["general-tests"],
+    cflags: [
+        "-Wall",
+        "-Werror",
+        "-Wno-missing-field-initializers",
+        "-Wno-unused-variable",
+        "-Wno-unused-function",
+        "-Wno-unused-parameter",
+    ],
+}
\ No newline at end of file
diff --git a/libstats/pull/TEST_MAPPING b/libstats/pull/TEST_MAPPING
new file mode 100644
index 0000000..76f4f02
--- /dev/null
+++ b/libstats/pull/TEST_MAPPING
@@ -0,0 +1,7 @@
+{
+  "presubmit" : [
+    {
+      "name" : "libstatspull_test"
+    }
+  ]
+}
\ No newline at end of file
diff --git a/libstats/pull/include/stats_pull_atom_callback.h b/libstats/pull/include/stats_pull_atom_callback.h
index 0b0df2b..c976c68 100644
--- a/libstats/pull/include/stats_pull_atom_callback.h
+++ b/libstats/pull/include/stats_pull_atom_callback.h
@@ -45,17 +45,27 @@
 void AStatsManager_PullAtomMetadata_release(AStatsManager_PullAtomMetadata* metadata);
 
 /**
- * Set the cool down time of the pull in nanoseconds. If two successive pulls are issued
+ * Set the cool down time of the pull in milliseconds. If two successive pulls are issued
  * within the cool down, a cached version of the first will be used for the second.
  */
-void AStatsManager_PullAtomMetadata_setCoolDownNs(AStatsManager_PullAtomMetadata* metadata,
-                                                  int64_t cool_down_ns);
+void AStatsManager_PullAtomMetadata_setCoolDownMillis(AStatsManager_PullAtomMetadata* metadata,
+                                                      int64_t cool_down_millis);
 
 /**
- * Set the maximum time the pull can take in nanoseconds.
+ * Get the cool down time of the pull in milliseconds.
  */
-void AStatsManager_PullAtomMetadata_setTimeoutNs(AStatsManager_PullAtomMetadata* metadata,
-                                                 int64_t timeout_ns);
+int64_t AStatsManager_PullAtomMetadata_getCoolDownMillis(AStatsManager_PullAtomMetadata* metadata);
+
+/**
+ * Set the maximum time the pull can take in milliseconds.
+ */
+void AStatsManager_PullAtomMetadata_setTimeoutMillis(AStatsManager_PullAtomMetadata* metadata,
+                                                     int64_t timeout_millis);
+
+/**
+ * Get the maximum time the pull can take in milliseconds.
+ */
+int64_t AStatsManager_PullAtomMetadata_getTimeoutMillis(AStatsManager_PullAtomMetadata* metadata);
 
 /**
  * Set the additive fields of this pulled atom.
@@ -65,7 +75,25 @@
  * will be combined when the non-additive fields are the same.
  */
 void AStatsManager_PullAtomMetadata_setAdditiveFields(AStatsManager_PullAtomMetadata* metadata,
-                                                      int* additive_fields, int num_fields);
+                                                      int32_t* additive_fields, int32_t num_fields);
+
+/**
+ * Get the number of additive fields for this pulled atom. This is intended to be called before
+ * AStatsManager_PullAtomMetadata_getAdditiveFields to determine the size of the array.
+ */
+int32_t AStatsManager_PullAtomMetadata_getNumAdditiveFields(
+        AStatsManager_PullAtomMetadata* metadata);
+
+/**
+ * Get the additive fields of this pulled atom.
+ *
+ * \param fields an output parameter containing the additive fields for this PullAtomMetadata.
+ *               Fields is an array and it is assumed that it is at least as large as the number of
+ *               additive fields, which can be obtained by calling
+ *               AStatsManager_PullAtomMetadata_getNumAdditiveFields.
+ */
+void AStatsManager_PullAtomMetadata_getAdditiveFields(AStatsManager_PullAtomMetadata* metadata,
+                                                      int32_t* fields);
 
 /**
  * Return codes for the result of a pull.
@@ -108,7 +136,7 @@
 typedef AStatsManager_PullAtomCallbackReturn (*AStatsManager_PullAtomCallback)(
         int32_t atom_tag, AStatsEventList* data, void* cookie);
 /**
- * Registers a callback for an atom when that atom is to be pulled. The stats service will
+ * Sets a callback for an atom when that atom is to be pulled. The stats service will
  * invoke the callback when the stats service determines that this atom needs to be
  * pulled.
  *
@@ -122,19 +150,18 @@
  * \param cookie            A pointer that will be passed back to the callback.
  *                          It has no meaning to statsd.
  */
-void AStatsManager_registerPullAtomCallback(int32_t atom_tag,
-                                            AStatsManager_PullAtomCallback callback,
-                                            AStatsManager_PullAtomMetadata* metadata, void* cookie);
+void AStatsManager_setPullAtomCallback(int32_t atom_tag, AStatsManager_PullAtomMetadata* metadata,
+                                       AStatsManager_PullAtomCallback callback, void* cookie);
 
 /**
- * Unregisters a callback for an atom when that atom is to be pulled. Note that any ongoing
+ * Clears a callback for an atom when that atom is to be pulled. Note that any ongoing
  * pulls will still occur.
  *
  * Requires the REGISTER_STATS_PULL_ATOM permission.
  *
  * \param atomTag           The tag of the atom of which to unregister
  */
-void AStatsManager_unregisterPullAtomCallback(int32_t atom_tag);
+void AStatsManager_clearPullAtomCallback(int32_t atom_tag);
 
 #ifdef __cplusplus
 }
diff --git a/libstats/pull/libstatspull.map.txt b/libstats/pull/libstatspull.map.txt
index dc3fd8b..e0e851a 100644
--- a/libstats/pull/libstatspull.map.txt
+++ b/libstats/pull/libstatspull.map.txt
@@ -2,12 +2,16 @@
     global:
         AStatsManager_PullAtomMetadata_obtain; # apex # introduced=30
         AStatsManager_PullAtomMetadata_release; # apex # introduced=30
-        AStatsManager_PullAtomMetadata_setCoolDownNs; # apex # introduced=30
-        AStatsManager_PullAtomMetadata_setTimeoutNs; # apex # introduced=30
+        AStatsManager_PullAtomMetadata_setCoolDownMillis; # apex # introduced=30
+        AStatsManager_PullAtomMetadata_getCoolDownMillis; # apex # introduced=30
+        AStatsManager_PullAtomMetadata_setTimeoutMillis; # apex # introduced=30
+        AStatsManager_PullAtomMetadata_getTimeoutMillis; # apex # introduced=30
         AStatsManager_PullAtomMetadata_setAdditiveFields; # apex # introduced=30
+        AStatsManager_PullAtomMetadata_getNumAdditiveFields; # apex # introduced=30
+        AStatsManager_PullAtomMetadata_getAdditiveFields; # apex # introduced=30
         AStatsEventList_addStatsEvent; # apex # introduced=30
-        AStatsManager_registerPullAtomCallback; # apex # introduced=30
-        AStatsManager_unregisterPullAtomCallback; # apex # introduced=30
+        AStatsManager_setPullAtomCallback; # apex # introduced=30
+        AStatsManager_clearPullAtomCallback; # apex # introduced=30
     local:
         *;
 };
diff --git a/libstats/pull/stats_pull_atom_callback.cpp b/libstats/pull/stats_pull_atom_callback.cpp
index 27e9d29..2d184bd 100644
--- a/libstats/pull/stats_pull_atom_callback.cpp
+++ b/libstats/pull/stats_pull_atom_callback.cpp
@@ -46,19 +46,19 @@
     return event;
 }
 
-static const int64_t DEFAULT_COOL_DOWN_NS = 1000000000LL;  // 1 second.
-static const int64_t DEFAULT_TIMEOUT_NS = 10000000000LL;   // 10 seconds.
+static const int64_t DEFAULT_COOL_DOWN_MILLIS = 1000LL;  // 1 second.
+static const int64_t DEFAULT_TIMEOUT_MILLIS = 10000LL;   // 10 seconds.
 
 struct AStatsManager_PullAtomMetadata {
-    int64_t cool_down_ns;
-    int64_t timeout_ns;
+    int64_t cool_down_millis;
+    int64_t timeout_millis;
     std::vector<int32_t> additive_fields;
 };
 
 AStatsManager_PullAtomMetadata* AStatsManager_PullAtomMetadata_obtain() {
     AStatsManager_PullAtomMetadata* metadata = new AStatsManager_PullAtomMetadata();
-    metadata->cool_down_ns = DEFAULT_COOL_DOWN_NS;
-    metadata->timeout_ns = DEFAULT_TIMEOUT_NS;
+    metadata->cool_down_millis = DEFAULT_COOL_DOWN_MILLIS;
+    metadata->timeout_millis = DEFAULT_TIMEOUT_MILLIS;
     metadata->additive_fields = std::vector<int32_t>();
     return metadata;
 }
@@ -67,30 +67,49 @@
     delete metadata;
 }
 
-void AStatsManager_PullAtomMetadata_setCoolDownNs(AStatsManager_PullAtomMetadata* metadata,
-                                                  int64_t cool_down_ns) {
-    metadata->cool_down_ns = cool_down_ns;
+void AStatsManager_PullAtomMetadata_setCoolDownMillis(AStatsManager_PullAtomMetadata* metadata,
+                                                      int64_t cool_down_millis) {
+    metadata->cool_down_millis = cool_down_millis;
 }
 
-void AStatsManager_PullAtomMetadata_setTimeoutNs(AStatsManager_PullAtomMetadata* metadata,
-                                                 int64_t timeout_ns) {
-    metadata->timeout_ns = timeout_ns;
+int64_t AStatsManager_PullAtomMetadata_getCoolDownMillis(AStatsManager_PullAtomMetadata* metadata) {
+    return metadata->cool_down_millis;
+}
+
+void AStatsManager_PullAtomMetadata_setTimeoutMillis(AStatsManager_PullAtomMetadata* metadata,
+                                                     int64_t timeout_millis) {
+    metadata->timeout_millis = timeout_millis;
+}
+
+int64_t AStatsManager_PullAtomMetadata_getTimeoutMillis(AStatsManager_PullAtomMetadata* metadata) {
+    return metadata->timeout_millis;
 }
 
 void AStatsManager_PullAtomMetadata_setAdditiveFields(AStatsManager_PullAtomMetadata* metadata,
-                                                      int* additive_fields, int num_fields) {
+                                                      int32_t* additive_fields,
+                                                      int32_t num_fields) {
     metadata->additive_fields.assign(additive_fields, additive_fields + num_fields);
 }
 
+int32_t AStatsManager_PullAtomMetadata_getNumAdditiveFields(
+        AStatsManager_PullAtomMetadata* metadata) {
+    return metadata->additive_fields.size();
+}
+
+void AStatsManager_PullAtomMetadata_getAdditiveFields(AStatsManager_PullAtomMetadata* metadata,
+                                                      int32_t* fields) {
+    std::copy(metadata->additive_fields.begin(), metadata->additive_fields.end(), fields);
+}
+
 class StatsPullAtomCallbackInternal : public BnPullAtomCallback {
   public:
     StatsPullAtomCallbackInternal(const AStatsManager_PullAtomCallback callback, void* cookie,
-                                  const int64_t coolDownNs, const int64_t timeoutNs,
+                                  const int64_t coolDownMillis, const int64_t timeoutMillis,
                                   const std::vector<int32_t> additiveFields)
         : mCallback(callback),
           mCookie(cookie),
-          mCoolDownNs(coolDownNs),
-          mTimeoutNs(timeoutNs),
+          mCoolDownMillis(coolDownMillis),
+          mTimeoutMillis(timeoutMillis),
           mAdditiveFields(additiveFields) {}
 
     Status onPullAtom(int32_t atomTag,
@@ -119,15 +138,15 @@
         return Status::ok();
     }
 
-    const int64_t& getCoolDownNs() const { return mCoolDownNs; }
-    const int64_t& getTimeoutNs() const { return mTimeoutNs; }
+    int64_t getCoolDownMillis() const { return mCoolDownMillis; }
+    int64_t getTimeoutMillis() const { return mTimeoutMillis; }
     const std::vector<int32_t>& getAdditiveFields() const { return mAdditiveFields; }
 
   private:
     const AStatsManager_PullAtomCallback mCallback;
     void* mCookie;
-    const int64_t mCoolDownNs;
-    const int64_t mTimeoutNs;
+    const int64_t mCoolDownMillis;
+    const int64_t mTimeoutMillis;
     const std::vector<int32_t> mAdditiveFields;
 };
 
@@ -156,8 +175,8 @@
         pullersCopy = mPullers;
     }
     for (const auto& it : pullersCopy) {
-        statsService->registerNativePullAtomCallback(it.first, it.second->getCoolDownNs(),
-                                                     it.second->getTimeoutNs(),
+        statsService->registerNativePullAtomCallback(it.first, it.second->getCoolDownMillis(),
+                                                     it.second->getTimeoutMillis(),
                                                      it.second->getAdditiveFields(), it.second);
     }
 }
@@ -186,8 +205,8 @@
         return;
     }
 
-    statsService->registerNativePullAtomCallback(atomTag, cb->getCoolDownNs(), cb->getTimeoutNs(),
-                                                 cb->getAdditiveFields(), cb);
+    statsService->registerNativePullAtomCallback(
+            atomTag, cb->getCoolDownMillis(), cb->getTimeoutMillis(), cb->getAdditiveFields(), cb);
 }
 
 void unregisterStatsPullAtomCallbackBlocking(int32_t atomTag) {
@@ -200,12 +219,11 @@
     statsService->unregisterNativePullAtomCallback(atomTag);
 }
 
-void AStatsManager_registerPullAtomCallback(int32_t atom_tag,
-                                            AStatsManager_PullAtomCallback callback,
-                                            AStatsManager_PullAtomMetadata* metadata,
-                                            void* cookie) {
-    int64_t coolDownNs = metadata == nullptr ? DEFAULT_COOL_DOWN_NS : metadata->cool_down_ns;
-    int64_t timeoutNs = metadata == nullptr ? DEFAULT_TIMEOUT_NS : metadata->timeout_ns;
+void AStatsManager_setPullAtomCallback(int32_t atom_tag, AStatsManager_PullAtomMetadata* metadata,
+                                       AStatsManager_PullAtomCallback callback, void* cookie) {
+    int64_t coolDownMillis =
+            metadata == nullptr ? DEFAULT_COOL_DOWN_MILLIS : metadata->cool_down_millis;
+    int64_t timeoutMillis = metadata == nullptr ? DEFAULT_TIMEOUT_MILLIS : metadata->timeout_millis;
 
     std::vector<int32_t> additiveFields;
     if (metadata != nullptr) {
@@ -213,8 +231,8 @@
     }
 
     std::shared_ptr<StatsPullAtomCallbackInternal> callbackBinder =
-            SharedRefBase::make<StatsPullAtomCallbackInternal>(callback, cookie, coolDownNs,
-                                                               timeoutNs, additiveFields);
+            SharedRefBase::make<StatsPullAtomCallbackInternal>(callback, cookie, coolDownMillis,
+                                                               timeoutMillis, additiveFields);
 
     {
         std::lock_guard<std::mutex> lg(pullAtomMutex);
@@ -226,7 +244,7 @@
     registerThread.detach();
 }
 
-void AStatsManager_unregisterPullAtomCallback(int32_t atom_tag) {
+void AStatsManager_clearPullAtomCallback(int32_t atom_tag) {
     {
         std::lock_guard<std::mutex> lg(pullAtomMutex);
         // Always remove the puller from our map.
diff --git a/libstats/pull/tests/pull_atom_metadata_test.cpp b/libstats/pull/tests/pull_atom_metadata_test.cpp
new file mode 100644
index 0000000..cf19303
--- /dev/null
+++ b/libstats/pull/tests/pull_atom_metadata_test.cpp
@@ -0,0 +1,92 @@
+/*
+ * Copyright (C) 2020, The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include <gtest/gtest.h>
+
+#include <stats_pull_atom_callback.h>
+
+namespace {
+
+static const int64_t DEFAULT_COOL_DOWN_MILLIS = 1000LL;  // 1 second.
+static const int64_t DEFAULT_TIMEOUT_MILLIS = 10000LL;   // 10 seconds.
+
+}  // anonymous namespace
+
+TEST(AStatsManager_PullAtomMetadataTest, TestEmpty) {
+    AStatsManager_PullAtomMetadata* metadata = AStatsManager_PullAtomMetadata_obtain();
+    EXPECT_EQ(AStatsManager_PullAtomMetadata_getCoolDownMillis(metadata), DEFAULT_COOL_DOWN_MILLIS);
+    EXPECT_EQ(AStatsManager_PullAtomMetadata_getTimeoutMillis(metadata), DEFAULT_TIMEOUT_MILLIS);
+    EXPECT_EQ(AStatsManager_PullAtomMetadata_getNumAdditiveFields(metadata), 0);
+    AStatsManager_PullAtomMetadata_release(metadata);
+}
+
+TEST(AStatsManager_PullAtomMetadataTest, TestSetTimeoutMillis) {
+    int64_t timeoutMillis = 500;
+    AStatsManager_PullAtomMetadata* metadata = AStatsManager_PullAtomMetadata_obtain();
+    AStatsManager_PullAtomMetadata_setTimeoutMillis(metadata, timeoutMillis);
+    EXPECT_EQ(AStatsManager_PullAtomMetadata_getCoolDownMillis(metadata), DEFAULT_COOL_DOWN_MILLIS);
+    EXPECT_EQ(AStatsManager_PullAtomMetadata_getTimeoutMillis(metadata), timeoutMillis);
+    EXPECT_EQ(AStatsManager_PullAtomMetadata_getNumAdditiveFields(metadata), 0);
+    AStatsManager_PullAtomMetadata_release(metadata);
+}
+
+TEST(AStatsManager_PullAtomMetadataTest, TestSetCoolDownMillis) {
+    int64_t coolDownMillis = 10000;
+    AStatsManager_PullAtomMetadata* metadata = AStatsManager_PullAtomMetadata_obtain();
+    AStatsManager_PullAtomMetadata_setCoolDownMillis(metadata, coolDownMillis);
+    EXPECT_EQ(AStatsManager_PullAtomMetadata_getCoolDownMillis(metadata), coolDownMillis);
+    EXPECT_EQ(AStatsManager_PullAtomMetadata_getTimeoutMillis(metadata), DEFAULT_TIMEOUT_MILLIS);
+    EXPECT_EQ(AStatsManager_PullAtomMetadata_getNumAdditiveFields(metadata), 0);
+    AStatsManager_PullAtomMetadata_release(metadata);
+}
+
+TEST(AStatsManager_PullAtomMetadataTest, TestSetAdditiveFields) {
+    const int numFields = 3;
+    int inputFields[numFields] = {2, 4, 6};
+    AStatsManager_PullAtomMetadata* metadata = AStatsManager_PullAtomMetadata_obtain();
+    AStatsManager_PullAtomMetadata_setAdditiveFields(metadata, inputFields, numFields);
+    EXPECT_EQ(AStatsManager_PullAtomMetadata_getCoolDownMillis(metadata), DEFAULT_COOL_DOWN_MILLIS);
+    EXPECT_EQ(AStatsManager_PullAtomMetadata_getTimeoutMillis(metadata), DEFAULT_TIMEOUT_MILLIS);
+    EXPECT_EQ(AStatsManager_PullAtomMetadata_getNumAdditiveFields(metadata), numFields);
+    int outputFields[numFields];
+    AStatsManager_PullAtomMetadata_getAdditiveFields(metadata, outputFields);
+    for (int i = 0; i < numFields; i++) {
+        EXPECT_EQ(inputFields[i], outputFields[i]);
+    }
+    AStatsManager_PullAtomMetadata_release(metadata);
+}
+
+TEST(AStatsManager_PullAtomMetadataTest, TestSetAllElements) {
+    int64_t timeoutMillis = 500;
+    int64_t coolDownMillis = 10000;
+    const int numFields = 3;
+    int inputFields[numFields] = {2, 4, 6};
+
+    AStatsManager_PullAtomMetadata* metadata = AStatsManager_PullAtomMetadata_obtain();
+    AStatsManager_PullAtomMetadata_setTimeoutMillis(metadata, timeoutMillis);
+    AStatsManager_PullAtomMetadata_setCoolDownMillis(metadata, coolDownMillis);
+    AStatsManager_PullAtomMetadata_setAdditiveFields(metadata, inputFields, numFields);
+
+    EXPECT_EQ(AStatsManager_PullAtomMetadata_getCoolDownMillis(metadata), coolDownMillis);
+    EXPECT_EQ(AStatsManager_PullAtomMetadata_getTimeoutMillis(metadata), timeoutMillis);
+    EXPECT_EQ(AStatsManager_PullAtomMetadata_getNumAdditiveFields(metadata), numFields);
+    int outputFields[numFields];
+    AStatsManager_PullAtomMetadata_getAdditiveFields(metadata, outputFields);
+    for (int i = 0; i < numFields; i++) {
+        EXPECT_EQ(inputFields[i], outputFields[i]);
+    }
+    AStatsManager_PullAtomMetadata_release(metadata);
+}