MediaMetrics: Remove friend access, use STL map

Test: Verify media.metrics dumpsys, atest MediaMetricsTest
Bug: 138583596
Change-Id: I6b085553af3fd1a0c98261d133f251c2db27c93b
diff --git a/media/libmediametrics/MediaMetricsItem.cpp b/media/libmediametrics/MediaMetricsItem.cpp
index 98158d2..485e161 100644
--- a/media/libmediametrics/MediaMetricsItem.cpp
+++ b/media/libmediametrics/MediaMetricsItem.cpp
@@ -63,51 +63,6 @@
     if (DEBUG_ALLOCATIONS) {
         ALOGD("Destroy  mediametrics::Item @ %p", this);
     }
-    clear();
-}
-
-void mediametrics::Item::clear() {
-
-    // clean allocated storage from key
-    mKey.clear();
-
-    // clean attributes
-    // contents of the attributes
-    for (size_t i = 0 ; i < mPropCount; i++ ) {
-        mProps[i].clear();
-    }
-    // the attribute records themselves
-    if (mProps != NULL) {
-        free(mProps);
-        mProps = NULL;
-    }
-    mPropSize = 0;
-    mPropCount = 0;
-
-    return;
-}
-
-// make a deep copy of myself
-mediametrics::Item *mediametrics::Item::dup() {
-    mediametrics::Item *dst = new mediametrics::Item(this->mKey);
-
-    if (dst != NULL) {
-        // key as part of constructor
-        dst->mPid = this->mPid;
-        dst->mUid = this->mUid;
-        dst->mPkgName = this->mPkgName;
-        dst->mPkgVersionCode = this->mPkgVersionCode;
-        dst->mTimestamp = this->mTimestamp;
-
-        // properties aka attributes
-        dst->growProps(this->mPropCount);
-        for(size_t i=0;i<mPropCount;i++) {
-            dst->mProps[i] = this->mProps[i];
-        }
-        dst->mPropCount = this->mPropCount;
-    }
-
-    return dst;
 }
 
 mediametrics::Item &mediametrics::Item::setTimestamp(nsecs_t ts) {
@@ -151,84 +106,12 @@
     return mPkgVersionCode;
 }
 
-
-// find the proper entry in the list
-size_t mediametrics::Item::findPropIndex(const char *name) const
-{
-    size_t i = 0;
-    for (; i < mPropCount; i++) {
-        if (mProps[i].isNamed(name)) break;
-    }
-    return i;
-}
-
-mediametrics::Item::Prop *mediametrics::Item::findProp(const char *name) const {
-    const size_t i = findPropIndex(name);
-    if (i < mPropCount) {
-        return &mProps[i];
-    }
-    return nullptr;
-}
-
-// consider this "find-or-allocate".
-// caller validates type and uses clearPropValue() accordingly
-mediametrics::Item::Prop *mediametrics::Item::allocateProp(const char *name) {
-    const size_t i = findPropIndex(name);
-    if (i < mPropCount) {
-        return &mProps[i]; // already have it, return
-    }
-
-    Prop *prop = allocateProp(); // get a new prop
-    if (prop == nullptr) return nullptr;
-    prop->setName(name);
-    return prop;
-}
-
-mediametrics::Item::Prop *mediametrics::Item::allocateProp() {
-    if (mPropCount == mPropSize && growProps() == false) {
-        ALOGE("%s: failed allocation for new properties", __func__);
-        return nullptr;
-    }
-    return &mProps[mPropCount++];
-}
-
-// used within the summarizers; return whether property existed
-bool mediametrics::Item::removeProp(const char *name) {
-    const size_t i = findPropIndex(name);
-    if (i < mPropCount) {
-        mProps[i].clear();
-        if (i != mPropCount-1) {
-            // in the middle, bring last one down to fill gap
-            mProps[i].swap(mProps[mPropCount-1]);
-        }
-        mPropCount--;
-        return true;
-    }
-    return false;
-}
-
 // remove indicated keys and their values
 // return value is # keys removed
 size_t mediametrics::Item::filter(size_t n, const char *attrs[]) {
     size_t zapped = 0;
     for (size_t i = 0; i < n; ++i) {
-        const char *name = attrs[i];
-        size_t j = findPropIndex(name);
-        if (j >= mPropCount) {
-            // not there
-            continue;
-        } else if (j + 1 == mPropCount) {
-            // last one, shorten
-            zapped++;
-            mProps[j].clear();
-            mPropCount--;
-        } else {
-            // in the middle, bring last one down and shorten
-            zapped++;
-            mProps[j].clear();
-            mProps[j] = mProps[mPropCount-1];
-            mPropCount--;
-        }
+        zapped += mProps.erase(attrs[i]);
     }
     return zapped;
 }
@@ -238,49 +121,17 @@
 size_t mediametrics::Item::filterNot(size_t n, const char *attrs[]) {
     std::set<std::string> check(attrs, attrs + n);
     size_t zapped = 0;
-    for (size_t j = 0; j < mPropCount;) {
-        if (check.find(mProps[j].getName()) != check.end()) {
-            ++j;
-            continue;
-        }
-        if (j + 1 == mPropCount) {
-            // last one, shorten
-            zapped++;
-            mProps[j].clear();
-            mPropCount--;
-            break;
+    for (auto it = mProps.begin(); it != mProps.end();) {
+        if (check.find(it->first) != check.end()) {
+            ++it;
         } else {
-            // in the middle, bring last one down and shorten
-            zapped++;
-            mProps[j].clear();
-            mProps[j] = mProps[mPropCount-1];
-            mPropCount--;
+           it = mProps.erase(it);
+           ++zapped;
         }
     }
     return zapped;
 }
 
-bool mediametrics::Item::growProps(int increment)
-{
-    if (increment <= 0) {
-        increment = kGrowProps;
-    }
-    int nsize = mPropSize + increment;
-    Prop *ni = (Prop *)realloc(mProps, sizeof(Prop) * nsize);
-
-    if (ni != NULL) {
-        for (int i = mPropSize; i < nsize; i++) {
-            new (&ni[i]) Prop(); // placement new
-        }
-        mProps = ni;
-        mPropSize = nsize;
-        return true;
-    } else {
-        ALOGW("mediametrics::Item::growProps fails");
-        return false;
-    }
-}
-
 // Parcel / serialize things for binder calls
 //
 
@@ -315,10 +166,11 @@
     if (count < 0) return BAD_VALUE;
     mPkgVersionCode = version;
     mTimestamp = timestamp;
-    for (int i = 0; i < count ; i++) {
-        Prop *prop = allocateProp();
-        status_t status = prop->readFromParcel(data);
+    for (int i = 0; i < count; i++) {
+        Prop prop;
+        status_t status = prop.readFromParcel(data);
         if (status != NO_ERROR) return status;
+        mProps[prop.getName()] = std::move(prop);
     }
     return NO_ERROR;
 }
@@ -349,9 +201,9 @@
         ?: data->writeInt64(mTimestamp);
     if (status != NO_ERROR) return status;
 
-    data->writeInt32((int32_t)mPropCount);
-    for (size_t i = 0 ; i < mPropCount; ++i) {
-        status = mProps[i].writeToParcel(data);
+    data->writeInt32((int32_t)mProps.size());
+    for (auto &prop : *this) {
+        status = prop.writeToParcel(data);
         if (status != NO_ERROR) return status;
     }
     return NO_ERROR;
@@ -376,10 +228,10 @@
 
     snprintf(buffer, sizeof(buffer), "[%d:%s:%d:%d:%lld:%s:%zu:",
             version, mKey.c_str(), mPid, mUid, (long long)mTimestamp,
-            mPkgName.c_str(), mPropCount);
+            mPkgName.c_str(), mProps.size());
     result.append(buffer);
-    for (size_t i = 0 ; i < mPropCount; ++i) {
-        mProps[i].toString(buffer, sizeof(buffer));
+    for (auto &prop : *this) {
+        prop.toString(buffer, sizeof(buffer));
         result.append(buffer);
     }
     result.append("]");
@@ -390,7 +242,7 @@
 // calls the appropriate daemon
 bool mediametrics::Item::selfrecord() {
     ALOGD_IF(DEBUG_API, "%s: delivering %s", __func__, this->toString().c_str());
-    sp<IMediaMetricsService> svc = getInstance();
+    sp<IMediaMetricsService> svc = getService();
     if (svc != NULL) {
         status_t status = svc->submit(this);
         if (status != NO_ERROR) {
@@ -460,7 +312,7 @@
     */
 
     ALOGD_IF(DEBUG_API, "%s: delivering %zu bytes", __func__, size);
-    sp<IMediaMetricsService> svc = getInstance();
+    sp<IMediaMetricsService> svc = getService();
     if (svc != nullptr) {
         const status_t status = svc->submitBuffer(buffer, size);
         if (status != NO_ERROR) {
@@ -473,7 +325,7 @@
 }
 
 //static
-sp<IMediaMetricsService> BaseItem::getInstance() {
+sp<IMediaMetricsService> BaseItem::getService() {
     static const char *servicename = "media.metrics";
     static const bool enabled = isEnabled(); // singleton initialized
 
@@ -512,47 +364,6 @@
 }
 
 
-// merge the info from 'incoming' into this record.
-// we finish with a union of this+incoming and special handling for collisions
-bool mediametrics::Item::merge(mediametrics::Item *incoming) {
-
-    // if I don't have key or session id, take them from incoming
-    // 'this' should never be missing both of them...
-    if (mKey.empty()) {
-        mKey = incoming->mKey;
-    }
-
-    // for each attribute from 'incoming', resolve appropriately
-    int nattr = incoming->mPropCount;
-    for (int i = 0 ; i < nattr; i++ ) {
-        Prop *iprop = &incoming->mProps[i];
-        const char *p = iprop->mName;
-        size_t len = strlen(p);
-
-        // should ignore a zero length name...
-        if (len == 0) {
-            continue;
-        }
-
-        Prop *oprop = findProp(iprop->mName);
-
-        if (oprop == NULL) {
-            // no oprop, so we insert the new one
-            oprop = allocateProp(p);
-            if (oprop != NULL) {
-                *oprop = *iprop;
-            } else {
-                ALOGW("dropped property '%s'", iprop->mName);
-            }
-        } else {
-            *oprop = *iprop;
-        }
-    }
-
-    // not sure when we'd return false...
-    return true;
-}
-
 namespace {
 
 template <typename T>
@@ -647,14 +458,14 @@
     uint32_t size = header_size
         + sizeof(uint32_t) // # properties
         ;
-    for (size_t i = 0 ; i < mPropCount; ++i) {
-        const size_t propSize = mProps[i].getByteStringSize();
+    for (auto &prop : *this) {
+        const size_t propSize = prop.getByteStringSize();
         if (propSize > UINT16_MAX) {
-            ALOGW("%s: prop %zu size %zu too large", __func__, i, propSize);
+            ALOGW("%s: prop %s size %zu too large", __func__, prop.getName(), propSize);
             return INVALID_OPERATION;
         }
         if (__builtin_add_overflow(size, propSize, &size)) {
-            ALOGW("%s: item size overflow at property %zu", __func__, i);
+            ALOGW("%s: item size overflow at property %s", __func__, prop.getName());
             return INVALID_OPERATION;
         }
     }
@@ -674,16 +485,16 @@
             || insert((int32_t)mPid, &filling, buildmax) != NO_ERROR
             || insert((int32_t)mUid, &filling, buildmax) != NO_ERROR
             || insert((int64_t)mTimestamp, &filling, buildmax) != NO_ERROR
-            || insert((uint32_t)mPropCount, &filling, buildmax) != NO_ERROR) {
+            || insert((uint32_t)mProps.size(), &filling, buildmax) != NO_ERROR) {
         ALOGE("%s:could not write header", __func__);  // shouldn't happen
         free(build);
         return INVALID_OPERATION;
     }
-    for (size_t i = 0 ; i < mPropCount; ++i) {
-        if (mProps[i].writeToByteString(&filling, buildmax) != NO_ERROR) {
+    for (auto &prop : *this) {
+        if (prop.writeToByteString(&filling, buildmax) != NO_ERROR) {
             free(build);
             // shouldn't happen
-            ALOGE("%s:could not write prop %zu of %zu", __func__, i, mPropCount);
+            ALOGE("%s:could not write prop %s", __func__, prop.getName());
             return INVALID_OPERATION;
         }
     }
@@ -750,11 +561,12 @@
     mUid = uid;
     mTimestamp = timestamp;
     for (size_t i = 0; i < propCount; ++i) {
-        Prop *prop = allocateProp();
-        if (prop->readFromByteString(&read, readend) != NO_ERROR) {
+        Prop prop;
+        if (prop.readFromByteString(&read, readend) != NO_ERROR) {
             ALOGW("%s: cannot read prop %zu", __func__, i);
             return INVALID_OPERATION;
         }
+        mProps[prop.getName()] = std::move(prop);
     }
     return NO_ERROR;
 }
@@ -762,29 +574,29 @@
 status_t mediametrics::Item::Prop::writeToParcel(Parcel *data) const
 {
    switch (mType) {
-   case kTypeInt32:
-       return data->writeCString(mName)
+   case mediametrics::kTypeInt32:
+       return data->writeCString(mName.c_str())
                ?: data->writeInt32(mType)
                ?: data->writeInt32(u.int32Value);
-   case kTypeInt64:
-       return data->writeCString(mName)
+   case mediametrics::kTypeInt64:
+       return data->writeCString(mName.c_str())
                ?: data->writeInt32(mType)
                ?: data->writeInt64(u.int64Value);
-   case kTypeDouble:
-       return data->writeCString(mName)
+   case mediametrics::kTypeDouble:
+       return data->writeCString(mName.c_str())
                ?: data->writeInt32(mType)
                ?: data->writeDouble(u.doubleValue);
-   case kTypeRate:
-       return data->writeCString(mName)
+   case mediametrics::kTypeRate:
+       return data->writeCString(mName.c_str())
                ?: data->writeInt32(mType)
                ?: data->writeInt64(u.rate.first)
                ?: data->writeInt64(u.rate.second);
-   case kTypeCString:
-       return data->writeCString(mName)
+   case mediametrics::kTypeCString:
+       return data->writeCString(mName.c_str())
                ?: data->writeInt32(mType)
                ?: data->writeCString(u.CStringValue);
    default:
-       ALOGE("%s: found bad type: %d, name %s", __func__, mType, mName);
+       ALOGE("%s: found bad type: %d, name %s", __func__, mType, mName.c_str());
        return BAD_VALUE;
    }
 }
@@ -797,22 +609,22 @@
     status_t status = data.readInt32(&type);
     if (status != NO_ERROR) return status;
     switch (type) {
-    case kTypeInt32:
+    case mediametrics::kTypeInt32:
         status = data.readInt32(&u.int32Value);
         break;
-    case kTypeInt64:
+    case mediametrics::kTypeInt64:
         status = data.readInt64(&u.int64Value);
         break;
-    case kTypeDouble:
+    case mediametrics::kTypeDouble:
         status = data.readDouble(&u.doubleValue);
         break;
-    case kTypeCString: {
+    case mediametrics::kTypeCString: {
         const char *s = data.readCString();
         if (s == nullptr) return BAD_VALUE;
         set(s);
         break;
         }
-    case kTypeRate: {
+    case mediametrics::kTypeRate: {
         std::pair<int64_t, int64_t> rate;
         status = data.readInt64(&rate.first)
                 ?: data.readInt64(&rate.second);
@@ -827,7 +639,7 @@
     }
     if (status == NO_ERROR) {
         setName(key);
-        mType = (Type)type;
+        mType = (mediametrics::Type)type;
     }
     return status;
 }
@@ -835,25 +647,25 @@
 void mediametrics::Item::Prop::toString(char *buffer, size_t length) const
 {
     switch (mType) {
-    case kTypeInt32:
-        snprintf(buffer, length, "%s=%d:", mName, u.int32Value);
+    case mediametrics::kTypeInt32:
+        snprintf(buffer, length, "%s=%d:", mName.c_str(), u.int32Value);
         break;
-    case mediametrics::Item::kTypeInt64:
-        snprintf(buffer, length, "%s=%lld:", mName, (long long)u.int64Value);
+    case mediametrics::kTypeInt64:
+        snprintf(buffer, length, "%s=%lld:", mName.c_str(), (long long)u.int64Value);
         break;
-    case mediametrics::Item::kTypeDouble:
-        snprintf(buffer, length, "%s=%e:", mName, u.doubleValue);
+    case mediametrics::kTypeDouble:
+        snprintf(buffer, length, "%s=%e:", mName.c_str(), u.doubleValue);
         break;
-    case mediametrics::Item::kTypeRate:
+    case mediametrics::kTypeRate:
         snprintf(buffer, length, "%s=%lld/%lld:",
-                mName, (long long)u.rate.first, (long long)u.rate.second);
+                mName.c_str(), (long long)u.rate.first, (long long)u.rate.second);
         break;
-    case mediametrics::Item::kTypeCString:
+    case mediametrics::kTypeCString:
         // TODO sanitize string for ':' '='
-        snprintf(buffer, length, "%s=%s:", mName, u.CStringValue);
+        snprintf(buffer, length, "%s=%s:", mName.c_str(), u.CStringValue);
         break;
     default:
-        ALOGE("%s: bad item type: %d for %s", __func__, mType, mName);
+        ALOGE("%s: bad item type: %d for %s", __func__, mType, mName.c_str());
         if (length > 0) buffer[0] = 0;
         break;
     }
@@ -864,27 +676,27 @@
     const size_t header =
         sizeof(uint16_t)      // length
         + sizeof(uint8_t)     // type
-        + strlen(mName) + 1;  // mName + 0 termination
+        + mName.size() + 1;  // mName + 0 termination
     size_t payload = 0;
     switch (mType) {
-    case mediametrics::Item::kTypeInt32:
+    case mediametrics::kTypeInt32:
         payload = sizeof(u.int32Value);
         break;
-    case mediametrics::Item::kTypeInt64:
+    case mediametrics::kTypeInt64:
         payload = sizeof(u.int64Value);
         break;
-    case mediametrics::Item::kTypeDouble:
+    case mediametrics::kTypeDouble:
         payload = sizeof(u.doubleValue);
         break;
-    case mediametrics::Item::kTypeRate:
+    case mediametrics::kTypeRate:
         payload = sizeof(u.rate.first) + sizeof(u.rate.second);
         break;
-    case mediametrics::Item::kTypeCString:
+    case mediametrics::kTypeCString:
         payload = strlen(u.CStringValue) + 1;
         break;
     default:
         ALOGE("%s: found bad prop type: %d, name %s",
-                __func__, mType, mName); // no payload computed
+                __func__, mType, mName.c_str()); // no payload computed
         break;
     }
     return header + payload;
@@ -898,7 +710,7 @@
     const size_t len = 2 + 1 + strlen(name) + 1 + sizeof(value);
     if (len > UINT16_MAX) return BAD_VALUE;
     return insert((uint16_t)len, bufferpptr, bufferptrmax)
-            ?: insert((uint8_t)kTypeInt32, bufferpptr, bufferptrmax)
+            ?: insert((uint8_t)mediametrics::kTypeInt32, bufferpptr, bufferptrmax)
             ?: insert(name, bufferpptr, bufferptrmax)
             ?: insert(value, bufferpptr, bufferptrmax);
 }
@@ -909,7 +721,7 @@
     const size_t len = 2 + 1 + strlen(name) + 1 + sizeof(value);
     if (len > UINT16_MAX) return BAD_VALUE;
     return insert((uint16_t)len, bufferpptr, bufferptrmax)
-            ?: insert((uint8_t)kTypeInt64, bufferpptr, bufferptrmax)
+            ?: insert((uint8_t)mediametrics::kTypeInt64, bufferpptr, bufferptrmax)
             ?: insert(name, bufferpptr, bufferptrmax)
             ?: insert(value, bufferpptr, bufferptrmax);
 }
@@ -920,7 +732,7 @@
     const size_t len = 2 + 1 + strlen(name) + 1 + sizeof(value);
     if (len > UINT16_MAX) return BAD_VALUE;
     return insert((uint16_t)len, bufferpptr, bufferptrmax)
-            ?: insert((uint8_t)kTypeDouble, bufferpptr, bufferptrmax)
+            ?: insert((uint8_t)mediametrics::kTypeDouble, bufferpptr, bufferptrmax)
             ?: insert(name, bufferpptr, bufferptrmax)
             ?: insert(value, bufferpptr, bufferptrmax);
 }
@@ -931,7 +743,7 @@
     const size_t len = 2 + 1 + strlen(name) + 1 + 8 + 8;
     if (len > UINT16_MAX) return BAD_VALUE;
     return insert((uint16_t)len, bufferpptr, bufferptrmax)
-            ?: insert((uint8_t)kTypeRate, bufferpptr, bufferptrmax)
+            ?: insert((uint8_t)mediametrics::kTypeRate, bufferpptr, bufferptrmax)
             ?: insert(name, bufferpptr, bufferptrmax)
             ?: insert(value.first, bufferpptr, bufferptrmax)
             ?: insert(value.second, bufferpptr, bufferptrmax);
@@ -949,7 +761,7 @@
     const size_t len = 2 + 1 + strlen(name) + 1 + strlen(value) + 1;
     if (len > UINT16_MAX) return BAD_VALUE;
     return insert((uint16_t)len, bufferpptr, bufferptrmax)
-            ?: insert((uint8_t)kTypeCString, bufferpptr, bufferptrmax)
+            ?: insert((uint8_t)mediametrics::kTypeCString, bufferpptr, bufferptrmax)
             ?: insert(name, bufferpptr, bufferptrmax)
             ?: insert(value, bufferpptr, bufferptrmax);
 }
@@ -961,7 +773,7 @@
     const size_t len = 2 + 1 + strlen(name) + 1;
     if (len > UINT16_MAX) return BAD_VALUE;
     return insert((uint16_t)len, bufferpptr, bufferptrmax)
-            ?: insert((uint8_t)kTypeCString, bufferpptr, bufferptrmax)
+            ?: insert((uint8_t)mediametrics::kTypeCString, bufferpptr, bufferptrmax)
             ?: insert(name, bufferpptr, bufferptrmax);
 }
 
@@ -970,21 +782,22 @@
         char **bufferpptr, char *bufferptrmax) const
 {
     switch (mType) {
-    case kTypeInt32:
-        return BaseItem::writeToByteString(mName, u.int32Value, bufferpptr, bufferptrmax);
-    case kTypeInt64:
-        return BaseItem::writeToByteString(mName, u.int64Value, bufferpptr, bufferptrmax);
-    case kTypeDouble:
-        return BaseItem::writeToByteString(mName, u.doubleValue, bufferpptr, bufferptrmax);
-    case kTypeRate:
-        return BaseItem::writeToByteString(mName, u.rate, bufferpptr, bufferptrmax);
-    case kTypeCString:
-        return BaseItem::writeToByteString(mName, u.CStringValue, bufferpptr, bufferptrmax);
-    case kTypeNone:
-        return BaseItem::writeToByteString(mName, none_t{}, bufferpptr, bufferptrmax);
+    case mediametrics::kTypeInt32:
+        return BaseItem::writeToByteString(mName.c_str(), u.int32Value, bufferpptr, bufferptrmax);
+    case mediametrics::kTypeInt64:
+        return BaseItem::writeToByteString(mName.c_str(), u.int64Value, bufferpptr, bufferptrmax);
+    case mediametrics::kTypeDouble:
+        return BaseItem::writeToByteString(mName.c_str(), u.doubleValue, bufferpptr, bufferptrmax);
+    case mediametrics::kTypeRate:
+        return BaseItem::writeToByteString(mName.c_str(), u.rate, bufferpptr, bufferptrmax);
+    case mediametrics::kTypeCString:
+        return BaseItem::writeToByteString(
+                mName.c_str(), u.CStringValue, bufferpptr, bufferptrmax);
+    case mediametrics::kTypeNone:
+        return BaseItem::writeToByteString(mName.c_str(), none_t{}, bufferpptr, bufferptrmax);
     default:
         ALOGE("%s: found bad prop type: %d, name %s",
-                __func__, mType, mName);  // no payload sent
+                __func__, mType, mName.c_str());  // no payload sent
         return BAD_VALUE;
     }
 }
@@ -999,35 +812,32 @@
             ?: extract(&type, bufferpptr, bufferptrmax)
             ?: extract(&name, bufferpptr, bufferptrmax);
     if (status != NO_ERROR) return status;
-    if (mName != nullptr) {
-        free(mName);
-    }
     mName = name;
-    if (mType == kTypeCString) {
+    if (mType == mediametrics::kTypeCString) {
         free(u.CStringValue);
         u.CStringValue = nullptr;
     }
-    mType = (Type)type;
+    mType = (mediametrics::Type)type;
     switch (mType) {
-    case kTypeInt32:
+    case mediametrics::kTypeInt32:
         return extract(&u.int32Value, bufferpptr, bufferptrmax);
-    case kTypeInt64:
+    case mediametrics::kTypeInt64:
         return extract(&u.int64Value, bufferpptr, bufferptrmax);
-    case kTypeDouble:
+    case mediametrics::kTypeDouble:
         return extract(&u.doubleValue, bufferpptr, bufferptrmax);
-    case kTypeRate:
+    case mediametrics::kTypeRate:
         return extract(&u.rate.first, bufferpptr, bufferptrmax)
                 ?: extract(&u.rate.second, bufferpptr, bufferptrmax);
-    case kTypeCString:
+    case mediametrics::kTypeCString:
         status = extract(&u.CStringValue, bufferpptr, bufferptrmax);
-        if (status != NO_ERROR) mType = kTypeNone;
+        if (status != NO_ERROR) mType = mediametrics::kTypeNone;
         return status;
-    case kTypeNone:
+    case mediametrics::kTypeNone:
         return NO_ERROR;
     default:
-        mType = kTypeNone;
+        mType = mediametrics::kTypeNone;
         ALOGE("%s: found bad prop type: %d, name %s",
-                __func__, mType, mName);  // no payload sent
+                __func__, mType, mName.c_str());  // no payload sent
         return BAD_VALUE;
     }
 }
diff --git a/media/libmediametrics/include/MediaMetricsItem.h b/media/libmediametrics/include/MediaMetricsItem.h
index 5f33650..f69de7a 100644
--- a/media/libmediametrics/include/MediaMetricsItem.h
+++ b/media/libmediametrics/include/MediaMetricsItem.h
@@ -14,12 +14,13 @@
  * limitations under the License.
  */
 
-#ifndef ANDROID_MEDIA_MEDIAANALYTICSITEM_H
-#define ANDROID_MEDIA_MEDIAANALYTICSITEM_H
+#ifndef ANDROID_MEDIA_MEDIAMETRICSITEM_H
+#define ANDROID_MEDIA_MEDIAMETRICSITEM_H
 
 #include "MediaMetrics.h"
 
 #include <algorithm>
+#include <map>
 #include <string>
 #include <sys/types.h>
 
@@ -106,8 +107,9 @@
     friend class MediaMetricsDeathNotifier; // for dropInstance
     // enabled 1, disabled 0
 public:
-    // are we collecting analytics data
+    // are we collecting metrics data
     static bool isEnabled();
+    static sp<IMediaMetricsService> getService();
 
 protected:
     static constexpr const char * const EnabledProperty = "media.metrics.enabled";
@@ -116,7 +118,7 @@
 
     // let's reuse a binder connection
     static sp<IMediaMetricsService> sMediaMetricsService;
-    static sp<IMediaMetricsService> getInstance();
+
     static void dropInstance();
     static bool submitBuffer(const char *buffer, size_t len);
 
@@ -133,7 +135,7 @@
             const char *name, char * const &value, char **bufferpptr, char *bufferptrmax);
     static status_t writeToByteString(
             const char *name, const char * const &value, char **bufferpptr, char *bufferptrmax);
-    struct none_t {}; // for kTypeNone
+    struct none_t {}; // for mediametrics::kTypeNone
     static status_t writeToByteString(
             const char *name, const none_t &, char **bufferpptr, char *bufferptrmax);
 
@@ -329,7 +331,7 @@
 };
 
 /**
- * MediaMetrics LogItem is a stack allocated media analytics item used for
+ * MediaMetrics LogItem is a stack allocated mediametrics item used for
  * fast logging.  It falls over to a malloc if needed.
  *
  * This is templated with a buffer size to allocate on the stack.
@@ -370,23 +372,8 @@
  * The Item is designed for the service as it has getters.
  */
 class Item : public mediametrics::BaseItem {
-    friend class MediaMetricsJNI;           // TODO: remove this access
-
 public:
 
-     // TODO: remove this duplicate definition when frameworks base is updated.
-            enum Type {
-                kTypeNone = 0,
-                kTypeInt32 = 1,
-                kTypeInt64 = 2,
-                kTypeDouble = 3,
-                kTypeCString = 4,
-                kTypeRate = 5,
-            };
-
-    static constexpr const char * const kKeyNone = "none";
-    static constexpr const char * const kKeyAny = "any";
-
         enum {
             PROTO_V0 = 0,
             PROTO_FIRST = PROTO_V0,
@@ -400,21 +387,14 @@
         : mKey(key) { }
     Item() = default;
 
-    Item(const Item&) = delete;
-    Item &operator=(const Item&) = delete;
-
     bool operator==(const Item& other) const {
-        if (mPropCount != other.mPropCount
-            || mPid != other.mPid
+        if (mPid != other.mPid
             || mUid != other.mUid
             || mPkgName != other.mPkgName
             || mPkgVersionCode != other.mPkgVersionCode
             || mKey != other.mKey
-            || mTimestamp != other.mTimestamp) return false;
-         for (size_t i = 0; i < mPropCount; ++i) {
-             Prop *p = other.findProp(mProps[i].getName());
-             if (p == nullptr || mProps[i] != *p) return false;
-         }
+            || mTimestamp != other.mTimestamp
+            || mProps != other.mProps) return false;
          return true;
     }
     bool operator!=(const Item& other) const {
@@ -435,9 +415,17 @@
         // access functions for the class
         ~Item();
 
-        // reset all contents, discarding any extra data
-        void clear();
-        Item *dup();
+    void clear() {
+        mPid = -1;
+        mUid = -1;
+        mPkgName.clear();
+        mPkgVersionCode = 0;
+        mTimestamp = 0;
+        mKey.clear();
+        mProps.clear();
+    }
+
+    Item *dup() const { return new Item(*this); }
 
     Item &setKey(const char *key) {
         mKey = key;
@@ -446,11 +434,11 @@
     const std::string& getKey() const { return mKey; }
 
     // # of properties in the record
-    size_t count() const { return mPropCount; }
+    size_t count() const { return mProps.size(); }
 
     template<typename S, typename T>
     Item &set(S key, T value) {
-        allocateProp(key)->set(value);
+        findOrAllocateProp(key).set(value);
         return *this;
     }
 
@@ -475,7 +463,7 @@
     // type-mismatch counts as "wasn't there".
     template<typename S, typename T>
     Item &add(S key, T value) {
-        allocateProp(key)->add(value);
+        findOrAllocateProp(key).add(value);
         return *this;
     }
 
@@ -497,7 +485,7 @@
     // NULL parameter value suppresses storage of value.
     template<typename S, typename T>
     bool get(S key, T *value) const {
-        Prop *prop = findProp(key);
+        const Prop *prop = findProp(key);
         return prop != nullptr && prop->get(value);
     }
 
@@ -582,21 +570,11 @@
         const char *toCString();
         const char *toCString(int version);
 
-    protected:
-
-        // merge fields from arg into this
-        // with rules for first/last/add, etc
-        // XXX: document semantics and how they are indicated
-        // caller continues to own 'incoming'
-        bool merge(Item *incoming);
-
 private:
     // handle Parcel version 0
     int32_t writeToParcel0(Parcel *) const;
     int32_t readFromParcel0(const Parcel&);
 
-
-
     // checks equality even with nullptr.
     static bool stringEquals(const char *a, const char *b) {
         if (a == nullptr) {
@@ -607,38 +585,32 @@
     }
 
 public:
-
     class Prop {
-    friend class MediaMetricsJNI;           // TODO: remove this access
     public:
         Prop() = default;
         Prop(const Prop& other) {
            *this = other;
         }
         Prop& operator=(const Prop& other) {
-            if (other.mName != nullptr) {
-                mName = strdup(other.mName);
-            } else {
-                mName = nullptr;
-            }
+            mName = other.mName;
             mType = other.mType;
             switch (mType) {
-            case kTypeInt32:
+            case mediametrics::kTypeInt32:
                 u.int32Value = other.u.int32Value;
                 break;
-            case kTypeInt64:
+            case mediametrics::kTypeInt64:
                 u.int64Value = other.u.int64Value;
                 break;
-            case kTypeDouble:
+            case mediametrics::kTypeDouble:
                 u.doubleValue = other.u.doubleValue;
                 break;
-            case kTypeCString:
+            case mediametrics::kTypeCString:
                 u.CStringValue = strdup(other.u.CStringValue);
                 break;
-            case kTypeRate:
+            case mediametrics::kTypeRate:
                 u.rate = other.u.rate;
                 break;
-            case kTypeNone:
+            case mediametrics::kTypeNone:
                 break;
             default:
                 // abort?
@@ -646,21 +618,53 @@
             }
             return *this;
         }
+        Prop(Prop&& other) {
+            *this = std::move(other);
+        }
+        Prop& operator=(Prop&& other) {
+            mName = std::move(other.mName);
+            mType = other.mType;
+            switch (mType) {
+            case mediametrics::kTypeInt32:
+                u.int32Value = other.u.int32Value;
+                break;
+            case mediametrics::kTypeInt64:
+                u.int64Value = other.u.int64Value;
+                break;
+            case mediametrics::kTypeDouble:
+                u.doubleValue = other.u.doubleValue;
+                break;
+            case mediametrics::kTypeCString:
+                u.CStringValue = other.u.CStringValue;
+                break;
+            case mediametrics::kTypeRate:
+                u.rate = other.u.rate;
+                break;
+            case mediametrics::kTypeNone:
+                break;
+            default:
+                // abort?
+                break;
+            }
+            other.mType = mediametrics::kTypeNone;
+            return *this;
+        }
+
         bool operator==(const Prop& other) const {
-            if (!stringEquals(mName, other.mName)
+            if (mName != other.mName
                     || mType != other.mType) return false;
             switch (mType) {
-            case kTypeInt32:
+            case mediametrics::kTypeInt32:
                 return u.int32Value == other.u.int32Value;
-            case kTypeInt64:
+            case mediametrics::kTypeInt64:
                 return u.int64Value == other.u.int64Value;
-            case kTypeDouble:
+            case mediametrics::kTypeDouble:
                 return u.doubleValue == other.u.doubleValue;
-            case kTypeCString:
+            case mediametrics::kTypeCString:
                 return stringEquals(u.CStringValue, other.u.CStringValue);
-            case kTypeRate:
+            case mediametrics::kTypeRate:
                 return u.rate == other.u.rate;
-            case kTypeNone:
+            case mediametrics::kTypeNone:
             default:
                 return true;
             }
@@ -670,24 +674,23 @@
         }
 
         void clear() {
-            free(mName);
-            mName = nullptr;
+            mName.clear();
             clearValue();
         }
         void clearValue() {
-            if (mType == kTypeCString) {
+            if (mType == mediametrics::kTypeCString) {
                 free(u.CStringValue);
                 u.CStringValue = nullptr;
             }
-            mType = kTypeNone;
+            mType = mediametrics::kTypeNone;
         }
 
-        Type getType() const {
+        mediametrics::Type getType() const {
             return mType;
         }
 
         const char *getName() const {
-            return mName;
+            return mName.c_str();
         }
 
         void swap(Prop& other) {
@@ -697,33 +700,28 @@
         }
 
         void setName(const char *name) {
-            free(mName);
-            if (name != nullptr) {
-                mName = strdup(name);
-            } else {
-                mName = nullptr;
-            }
+            mName = name;
         }
 
         bool isNamed(const char *name) const {
-            return stringEquals(name, mName);
+            return mName == name;
         }
 
         template <typename T> void visit(T f) const {
             switch (mType) {
-            case Item::kTypeInt32:
+            case mediametrics::kTypeInt32:
                 f(u.int32Value);
                 return;
-            case Item::kTypeInt64:
+            case mediametrics::kTypeInt64:
                 f(u.int64Value);
                 return;
-            case Item::kTypeDouble:
+            case mediametrics::kTypeDouble:
                 f(u.doubleValue);
                 return;
-            case Item::kTypeRate:
+            case mediametrics::kTypeRate:
                 f(u.rate);
                 return;
-            case Item::kTypeCString:
+            case mediametrics::kTypeCString:
                 f(u.CStringValue);
                 return;
             default:
@@ -734,37 +732,37 @@
         template <typename T> bool get(T *value) const = delete;
         template <>
         bool get(int32_t *value) const {
-           if (mType != kTypeInt32) return false;
+           if (mType != mediametrics::kTypeInt32) return false;
            if (value != nullptr) *value = u.int32Value;
            return true;
         }
         template <>
         bool get(int64_t *value) const {
-           if (mType != kTypeInt64) return false;
+           if (mType != mediametrics::kTypeInt64) return false;
            if (value != nullptr) *value = u.int64Value;
            return true;
         }
         template <>
         bool get(double *value) const {
-           if (mType != kTypeDouble) return false;
+           if (mType != mediametrics::kTypeDouble) return false;
            if (value != nullptr) *value = u.doubleValue;
            return true;
         }
         template <>
         bool get(const char** value) const {
-            if (mType != kTypeCString) return false;
+            if (mType != mediametrics::kTypeCString) return false;
             if (value != nullptr) *value = u.CStringValue;
             return true;
         }
         template <>
         bool get(std::string* value) const {
-            if (mType != kTypeCString) return false;
+            if (mType != mediametrics::kTypeCString) return false;
             if (value != nullptr) *value = u.CStringValue;
             return true;
         }
         template <>
         bool get(std::pair<int64_t, int64_t> *value) const {
-           if (mType != kTypeRate) return false;
+           if (mType != mediametrics::kTypeRate) return false;
            if (value != nullptr) {
                *value = u.rate;
            }
@@ -774,25 +772,25 @@
         template <typename T> void set(const T& value) = delete;
         template <>
         void set(const int32_t& value) {
-            mType = kTypeInt32;
+            mType = mediametrics::kTypeInt32;
             u.int32Value = value;
         }
         template <>
         void set(const int64_t& value) {
-            mType = kTypeInt64;
+            mType = mediametrics::kTypeInt64;
             u.int64Value = value;
         }
         template <>
         void set(const double& value) {
-            mType = kTypeDouble;
+            mType = mediametrics::kTypeDouble;
             u.doubleValue = value;
         }
         template <>
         void set(const char* const& value) {
-            if (mType == kTypeCString) {
+            if (mType == mediametrics::kTypeCString) {
                 free(u.CStringValue);
             } else {
-                mType = kTypeCString;
+                mType = mediametrics::kTypeCString;
             }
             if (value == nullptr) {
                 u.CStringValue = nullptr;
@@ -808,45 +806,45 @@
         }
         template <>
         void set(const std::pair<int64_t, int64_t> &value) {
-            mType = kTypeRate;
+            mType = mediametrics::kTypeRate;
             u.rate = {value.first, value.second};
         }
 
         template <typename T> void add(const T& value) = delete;
         template <>
         void add(const int32_t& value) {
-            if (mType == kTypeInt32) {
+            if (mType == mediametrics::kTypeInt32) {
                 u.int32Value += value;
             } else {
-                mType = kTypeInt32;
+                mType = mediametrics::kTypeInt32;
                 u.int32Value = value;
             }
         }
         template <>
         void add(const int64_t& value) {
-            if (mType == kTypeInt64) {
+            if (mType == mediametrics::kTypeInt64) {
                 u.int64Value += value;
             } else {
-                mType = kTypeInt64;
+                mType = mediametrics::kTypeInt64;
                 u.int64Value = value;
             }
         }
         template <>
         void add(const double& value) {
-            if (mType == kTypeDouble) {
+            if (mType == mediametrics::kTypeDouble) {
                 u.doubleValue += value;
             } else {
-                mType = kTypeDouble;
+                mType = mediametrics::kTypeDouble;
                 u.doubleValue = value;
             }
         }
         template <>
         void add(const std::pair<int64_t, int64_t>& value) {
-            if (mType == kTypeRate) {
+            if (mType == mediametrics::kTypeRate) {
                 u.rate.first += value.first;
                 u.rate.second += value.second;
             } else {
-                mType = kTypeRate;
+                mType = mediametrics::kTypeRate;
                 u.rate = value;
             }
         }
@@ -858,10 +856,10 @@
         status_t writeToByteString(char **bufferpptr, char *bufferptrmax) const;
         status_t readFromByteString(const char **bufferpptr, const char *bufferptrmax);
 
-    // TODO: make private (and consider converting to std::variant)
-    // private:
-        char *mName = nullptr;
-        Type mType = kTypeNone;
+    // TODO: consider converting to std::variant
+    private:
+        std::string mName;
+        mediametrics::Type mType = mediametrics::kTypeNone;
         union u__ {
             u__() { zero(); }
             u__(u__ &&other) {
@@ -882,62 +880,58 @@
         } u;
     };
 
+    // Iteration of props within item
     class iterator {
     public:
-       iterator(size_t pos, const Item &_item)
-           : i(std::min(pos, _item.count()))
-           , item(_item) { }
-       iterator &operator++() {
-           i = std::min(i + 1, item.count());
-           return *this;
-       }
-       bool operator!=(iterator &other) const {
-           return i != other.i;
-       }
-       Prop &operator*() const {
-           return item.mProps[i];
-       }
+        iterator(const std::map<std::string, Prop>::const_iterator &_it) : it(_it) { }
+        iterator &operator++() {
+            ++it;
+            return *this;
+        }
+        bool operator!=(iterator &other) const {
+            return it != other.it;
+        }
+        const Prop &operator*() const {
+            return it->second;
+        }
 
     private:
-      size_t i;
-      const Item &item;
+        std::map<std::string, Prop>::const_iterator it;
     };
 
     iterator begin() const {
-        return iterator(0, *this);
+        return iterator(mProps.cbegin());
     }
+
     iterator end() const {
-        return iterator(SIZE_MAX, *this);
+        return iterator(mProps.cend());
     }
 
 private:
 
-    // TODO: make prop management class
-    size_t findPropIndex(const char *name) const;
-    Prop *findProp(const char *name) const;
-    Prop *allocateProp();
+    const Prop *findProp(const char *key) const {
+        auto it = mProps.find(key);
+        return it != mProps.end() ? &it->second : nullptr;
+    }
 
-        enum {
-            kGrowProps = 10
-        };
-        bool growProps(int increment = kGrowProps);
-        Prop *allocateProp(const char *name);
-        bool removeProp(const char *name);
-    Prop *allocateProp(const std::string& name) { return allocateProp(name.c_str()); }
-
-        size_t mPropCount = 0;
-        size_t mPropSize = 0;
-        Prop *mProps = nullptr;
+    Prop &findOrAllocateProp(const char *key) {
+        auto it = mProps.find(key);
+        if (it != mProps.end()) return it->second;
+        Prop &prop = mProps[key];
+        prop.setName(key);
+        return prop;
+    }
 
     pid_t         mPid = -1;
     uid_t         mUid = -1;
     std::string   mPkgName;
     int64_t       mPkgVersionCode = 0;
-    std::string   mKey{kKeyNone};
+    std::string   mKey;
     nsecs_t       mTimestamp = 0;
+    std::map<std::string, Prop> mProps;
 };
 
 } // namespace mediametrics
 } // namespace android
 
-#endif
+#endif // ANDROID_MEDIA_MEDIAMETRICSITEM_H