Fix mistake in Notification Memory Use logging

We didn't correctly aggregate custom views.

Bug: 269228469
Test: wrote new test before fixing it, test failed, now test passes
      deployed to cheetah, used Notify2 to create custom notification,
      verified statsd payload is correct
Change-Id: Ifcd2dc6cb137462335e6753868028febfaff9fe5

packages/SystemUI/src/com/android/systemui/statusbar/notification/logging/NotificationMemoryLogger.kt

@@ -20,6 +20,7 @@ package com.android.systemui.statusbar.notification.logging
 import android.app.StatsManager
 import android.util.Log
 import android.util.StatsEvent
+import androidx.annotation.VisibleForTesting
 import com.android.systemui.dagger.SysUISingleton
 import com.android.systemui.dagger.qualifiers.Background
 import com.android.systemui.dagger.qualifiers.Main
@@ -143,21 +144,26 @@ constructor(
         runBlocking(mainDispatcher) {
             traceSection("NML#getNotifications") { notificationPipeline.allNotifs }
         }
+}
 
-    /** Aggregates memory usage data by package and style, returning sums. */
+/** Aggregates memory usage data by package and style, returning sums. */
-    private fun aggregateMemoryUsageData(
+@VisibleForTesting
+internal fun aggregateMemoryUsageData(
     notificationMemoryUse: List<NotificationMemoryUsage>
-    ): Map<Pair<String, Int>, NotificationMemoryUseAtomBuilder> {
+): Map<Pair<String, Int>, NotificationMemoryLogger.NotificationMemoryUseAtomBuilder> {
     return notificationMemoryUse
         .groupingBy { Pair(it.packageName, it.objectUsage.style) }
         .aggregate {
             _,
-                accumulator: NotificationMemoryUseAtomBuilder?,
+            accumulator: NotificationMemoryLogger.NotificationMemoryUseAtomBuilder?,
             element: NotificationMemoryUsage,
             first ->
             val use =
                 if (first) {
-                        NotificationMemoryUseAtomBuilder(element.uid, element.objectUsage.style)
+                    NotificationMemoryLogger.NotificationMemoryUseAtomBuilder(
+                        element.uid,
+                        element.objectUsage.style
+                    )
                 } else {
                     accumulator!!
                 }
@@ -196,14 +202,12 @@ constructor(
                     use.largeIconViews += it.largeIcon
                     use.systemIconViews += it.systemIcons
                     use.styleViews += it.style
-                        use.customViews += it.style
+                    use.customViews += it.customViews
                     use.softwareBitmaps += it.softwareBitmapsPenalty
                 }
 
             return@aggregate use
         }
-    }
-
-    /** Rounds the passed value to the nearest KB - e.g. 700B rounds to 1KB. */
-    private fun toKb(value: Int): Int = (value.toFloat() / 1024f).roundToInt()
 }
+/** Rounds the passed value to the nearest KB - e.g. 700B rounds to 1KB. */
+private fun toKb(value: Int): Int = (value.toFloat() / 1024f).roundToInt()

packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/logging/NotificationMemoryLoggerTest.kt

@@ -20,6 +20,7 @@ import android.app.Notification
 import android.app.StatsManager
 import android.graphics.Bitmap
 import android.graphics.drawable.Icon
+import android.stats.sysui.NotificationEnums
 import android.testing.AndroidTestingRunner
 import android.util.StatsEvent
 import androidx.test.filters.SmallTest
@@ -31,10 +32,12 @@ import com.android.systemui.util.concurrency.FakeExecutor
 import com.android.systemui.util.mockito.mock
 import com.android.systemui.util.mockito.whenever
 import com.android.systemui.util.time.FakeSystemClock
+import com.google.common.truth.Expect
 import com.google.common.truth.Truth.assertThat
 import java.lang.RuntimeException
 import kotlinx.coroutines.Dispatchers
 import org.junit.Before
+import org.junit.Rule
 import org.junit.Test
 import org.junit.runner.RunWith
 import org.mockito.Mock
@@ -45,6 +48,8 @@ import org.mockito.MockitoAnnotations
 @RunWith(AndroidTestingRunner::class)
 class NotificationMemoryLoggerTest : SysuiTestCase() {
 
+    @Rule @JvmField val expect = Expect.create()
+
     private val bgExecutor = FakeExecutor(FakeSystemClock())
     private val immediate = Dispatchers.Main.immediate
 
@@ -132,6 +137,123 @@ class NotificationMemoryLoggerTest : SysuiTestCase() {
             .isEqualTo(StatsManager.PULL_SKIP)
     }
 
+    @Test
+    fun aggregateMemoryUsageData_returnsCorrectlyAggregatedSamePackageData() {
+        val usage = getPresetMemoryUsages()
+        val aggregateUsage = aggregateMemoryUsageData(usage)
+
+        assertThat(aggregateUsage).hasSize(3)
+        assertThat(aggregateUsage)
+            .containsKey(Pair("package 1", NotificationEnums.STYLE_BIG_PICTURE))
+
+        // Aggregated fields
+        val aggregatedData =
+            aggregateUsage[Pair("package 1", NotificationEnums.STYLE_BIG_PICTURE)]!!
+        val presetUsage1 = usage[0]
+        val presetUsage2 = usage[1]
+        assertAggregatedData(
+            aggregatedData,
+            2,
+            2,
+            smallIconObject =
+                presetUsage1.objectUsage.smallIcon + presetUsage2.objectUsage.smallIcon,
+            smallIconBitmapCount = 2,
+            largeIconObject =
+                presetUsage1.objectUsage.largeIcon + presetUsage2.objectUsage.largeIcon,
+            largeIconBitmapCount = 2,
+            bigPictureObject =
+                presetUsage1.objectUsage.bigPicture + presetUsage2.objectUsage.bigPicture,
+            bigPictureBitmapCount = 2,
+            extras = presetUsage1.objectUsage.extras + presetUsage2.objectUsage.extras,
+            extenders = presetUsage1.objectUsage.extender + presetUsage2.objectUsage.extender,
+            // Only totals need to be summarized.
+            smallIconViews =
+                presetUsage1.viewUsage[0].smallIcon + presetUsage2.viewUsage[0].smallIcon,
+            largeIconViews =
+                presetUsage1.viewUsage[0].largeIcon + presetUsage2.viewUsage[0].largeIcon,
+            systemIconViews =
+                presetUsage1.viewUsage[0].systemIcons + presetUsage2.viewUsage[0].systemIcons,
+            styleViews = presetUsage1.viewUsage[0].style + presetUsage2.viewUsage[0].style,
+            customViews =
+                presetUsage1.viewUsage[0].customViews + presetUsage2.viewUsage[0].customViews,
+            softwareBitmaps =
+                presetUsage1.viewUsage[0].softwareBitmapsPenalty +
+                    presetUsage2.viewUsage[0].softwareBitmapsPenalty,
+            seenCount = 0
+        )
+    }
+
+    @Test
+    fun aggregateMemoryUsageData_correctlySeparatesDifferentStyles() {
+        val usage = getPresetMemoryUsages()
+        val aggregateUsage = aggregateMemoryUsageData(usage)
+
+        assertThat(aggregateUsage).hasSize(3)
+        assertThat(aggregateUsage)
+            .containsKey(Pair("package 1", NotificationEnums.STYLE_BIG_PICTURE))
+        assertThat(aggregateUsage).containsKey(Pair("package 1", NotificationEnums.STYLE_BIG_TEXT))
+
+        // Different style should be separate
+        val separateStyleData =
+            aggregateUsage[Pair("package 1", NotificationEnums.STYLE_BIG_TEXT)]!!
+        val presetUsage = usage[2]
+        assertAggregatedData(
+            separateStyleData,
+            1,
+            1,
+            presetUsage.objectUsage.smallIcon,
+            1,
+            presetUsage.objectUsage.largeIcon,
+            1,
+            presetUsage.objectUsage.bigPicture,
+            1,
+            presetUsage.objectUsage.extras,
+            presetUsage.objectUsage.extender,
+            presetUsage.viewUsage[0].smallIcon,
+            presetUsage.viewUsage[0].largeIcon,
+            presetUsage.viewUsage[0].systemIcons,
+            presetUsage.viewUsage[0].style,
+            presetUsage.viewUsage[0].customViews,
+            presetUsage.viewUsage[0].softwareBitmapsPenalty,
+            0
+        )
+    }
+
+    @Test
+    fun aggregateMemoryUsageData_correctlySeparatesDifferentProcess() {
+        val usage = getPresetMemoryUsages()
+        val aggregateUsage = aggregateMemoryUsageData(usage)
+
+        assertThat(aggregateUsage).hasSize(3)
+        assertThat(aggregateUsage)
+            .containsKey(Pair("package 2", NotificationEnums.STYLE_BIG_PICTURE))
+
+        // Different UID/package should also be separate
+        val separatePackageData =
+            aggregateUsage[Pair("package 2", NotificationEnums.STYLE_BIG_PICTURE)]!!
+        val presetUsage = usage[3]
+        assertAggregatedData(
+            separatePackageData,
+            1,
+            1,
+            presetUsage.objectUsage.smallIcon,
+            1,
+            presetUsage.objectUsage.largeIcon,
+            1,
+            presetUsage.objectUsage.bigPicture,
+            1,
+            presetUsage.objectUsage.extras,
+            presetUsage.objectUsage.extender,
+            presetUsage.viewUsage[0].smallIcon,
+            presetUsage.viewUsage[0].largeIcon,
+            presetUsage.viewUsage[0].systemIcons,
+            presetUsage.viewUsage[0].style,
+            presetUsage.viewUsage[0].customViews,
+            presetUsage.viewUsage[0].softwareBitmapsPenalty,
+            0
+        )
+    }
+
     private fun createLoggerWithNotifications(
         notifications: List<Notification>
     ): NotificationMemoryLogger {
@@ -143,4 +265,182 @@ class NotificationMemoryLoggerTest : SysuiTestCase() {
         whenever(pipeline.allNotifs).thenReturn(notifications)
         return NotificationMemoryLogger(pipeline, statsManager, immediate, bgExecutor)
     }
+
+    /**
+     * Short hand for making sure the passed NotificationMemoryUseAtomBuilder object contains
+     * expected values.
+     */
+    private fun assertAggregatedData(
+        value: NotificationMemoryLogger.NotificationMemoryUseAtomBuilder,
+        count: Int,
+        countWithInflatedViews: Int,
+        smallIconObject: Int,
+        smallIconBitmapCount: Int,
+        largeIconObject: Int,
+        largeIconBitmapCount: Int,
+        bigPictureObject: Int,
+        bigPictureBitmapCount: Int,
+        extras: Int,
+        extenders: Int,
+        smallIconViews: Int,
+        largeIconViews: Int,
+        systemIconViews: Int,
+        styleViews: Int,
+        customViews: Int,
+        softwareBitmaps: Int,
+        seenCount: Int
+    ) {
+        expect.withMessage("count").that(value.count).isEqualTo(count)
+        expect
+            .withMessage("countWithInflatedViews")
+            .that(value.countWithInflatedViews)
+            .isEqualTo(countWithInflatedViews)
+        expect.withMessage("smallIconObject").that(value.smallIconObject).isEqualTo(smallIconObject)
+        expect
+            .withMessage("smallIconBitmapCount")
+            .that(value.smallIconBitmapCount)
+            .isEqualTo(smallIconBitmapCount)
+        expect.withMessage("largeIconObject").that(value.largeIconObject).isEqualTo(largeIconObject)
+        expect
+            .withMessage("largeIconBitmapCount")
+            .that(value.largeIconBitmapCount)
+            .isEqualTo(largeIconBitmapCount)
+        expect
+            .withMessage("bigPictureObject")
+            .that(value.bigPictureObject)
+            .isEqualTo(bigPictureObject)
+        expect
+            .withMessage("bigPictureBitmapCount")
+            .that(value.bigPictureBitmapCount)
+            .isEqualTo(bigPictureBitmapCount)
+        expect.withMessage("extras").that(value.extras).isEqualTo(extras)
+        expect.withMessage("extenders").that(value.extenders).isEqualTo(extenders)
+        expect.withMessage("smallIconViews").that(value.smallIconViews).isEqualTo(smallIconViews)
+        expect.withMessage("largeIconViews").that(value.largeIconViews).isEqualTo(largeIconViews)
+        expect.withMessage("systemIconViews").that(value.systemIconViews).isEqualTo(systemIconViews)
+        expect.withMessage("styleViews").that(value.styleViews).isEqualTo(styleViews)
+        expect.withMessage("customViews").that(value.customViews).isEqualTo(customViews)
+        expect.withMessage("softwareBitmaps").that(value.softwareBitmaps).isEqualTo(softwareBitmaps)
+        expect.withMessage("seenCount").that(value.seenCount).isEqualTo(seenCount)
+    }
+
+    /** Generates a static set of [NotificationMemoryUsage] objects. */
+    private fun getPresetMemoryUsages() =
+        listOf(
+            // A pair of notifications that have to be aggregated, same UID and style
+            NotificationMemoryUsage(
+                "package 1",
+                384,
+                "key1",
+                Notification.Builder(context).setStyle(Notification.BigPictureStyle()).build(),
+                NotificationObjectUsage(
+                    23,
+                    45,
+                    67,
+                    NotificationEnums.STYLE_BIG_PICTURE,
+                    12,
+                    483,
+                    4382,
+                    true
+                ),
+                listOf(
+                    NotificationViewUsage(ViewType.TOTAL, 493, 584, 4833, 584, 4888, 5843),
+                    NotificationViewUsage(
+                        ViewType.PRIVATE_CONTRACTED_VIEW,
+                        100,
+                        250,
+                        300,
+                        594,
+                        6000,
+                        5843
+                    )
+                )
+            ),
+            NotificationMemoryUsage(
+                "package 1",
+                384,
+                "key2",
+                Notification.Builder(context).setStyle(Notification.BigPictureStyle()).build(),
+                NotificationObjectUsage(
+                    77,
+                    54,
+                    34,
+                    NotificationEnums.STYLE_BIG_PICTURE,
+                    77,
+                    432,
+                    2342,
+                    true
+                ),
+                listOf(
+                    NotificationViewUsage(ViewType.TOTAL, 3245, 1234, 7653, 543, 765, 7655),
+                    NotificationViewUsage(
+                        ViewType.PRIVATE_CONTRACTED_VIEW,
+                        160,
+                        350,
+                        300,
+                        5544,
+                        66500,
+                        5433
+                    )
+                )
+            ),
+            // Different style is different aggregation
+            NotificationMemoryUsage(
+                "package 1",
+                384,
+                "key2",
+                Notification.Builder(context).setStyle(Notification.BigTextStyle()).build(),
+                NotificationObjectUsage(
+                    77,
+                    54,
+                    34,
+                    NotificationEnums.STYLE_BIG_TEXT,
+                    77,
+                    432,
+                    2342,
+                    true
+                ),
+                listOf(
+                    NotificationViewUsage(ViewType.TOTAL, 3245, 1234, 7653, 543, 765, 7655),
+                    NotificationViewUsage(
+                        ViewType.PRIVATE_CONTRACTED_VIEW,
+                        160,
+                        350,
+                        300,
+                        5544,
+                        66500,
+                        5433
+                    )
+                )
+            ),
+            // Different package is also different aggregation
+            NotificationMemoryUsage(
+                "package 2",
+                684,
+                "key2",
+                Notification.Builder(context).setStyle(Notification.BigPictureStyle()).build(),
+                NotificationObjectUsage(
+                    32,
+                    654,
+                    234,
+                    NotificationEnums.STYLE_BIG_PICTURE,
+                    211,
+                    776,
+                    435,
+                    true
+                ),
+                listOf(
+                    NotificationViewUsage(ViewType.TOTAL, 4355, 6543, 4322, 5435, 6546, 65485),
+                    NotificationViewUsage(
+                        ViewType.PRIVATE_CONTRACTED_VIEW,
+                        6546,
+                        7657,
+                        4353,
+                        6546,
+                        76575,
+                        54654
+                    )
+                )
+            )
+        )
 }