Reduce needless recoverBuilder calls when updating Notification icons

Right now we repeatedly call recoverBuilder through IconManager.updateIcons() which is surprisingly expensive. We call it to resolve the same exact description set for each icon in a Notification. This CL reuses the description and noticably cuts down on repeated description resolution.

Flag: NONE
Bug: 316554966
Test: newly added unit tests and existing tests
Change-Id: Icc4ed9d1890842fa953d1322bcbc941272539355

6 files changed, 188 insertions, 49 deletions

diff --git a/packages/SystemUI/multivalentTests/src/com/android/systemui/statusbar/notification/NotificationContentDescriptionTest.kt b/packages/SystemUI/multivalentTests/src/com/android/systemui/statusbar/notification/NotificationContentDescriptionTest.kt
new file mode 100644
index 000000000000..f67c70ce783f
--- /dev/null
+++ b/packages/SystemUI/multivalentTests/src/com/android/systemui/statusbar/notification/NotificationContentDescriptionTest.kt
@@ -0,0 +1,97 @@
+/*
+ * Copyright (C) 2023 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
+ */
+
+package com.android.systemui.statusbar.notification
+
+import android.app.Notification
+import androidx.test.ext.junit.runners.AndroidJUnit4
+import androidx.test.filters.SmallTest
+import com.android.systemui.SysuiTestCase
+import com.android.systemui.res.R
+import com.google.common.truth.Truth.assertThat
+import org.junit.Test
+import org.junit.runner.RunWith
+
+@SmallTest
+@RunWith(AndroidJUnit4::class)
+class NotificationContentDescriptionTest : SysuiTestCase() {
+
+    private val TITLE = "this is a title"
+    private val TEXT = "this is text"
+    private val TICKER = "this is a ticker"
+
+    @Test
+    fun notificationWithAllDifferentFields_descriptionIsTitle() {
+        val n = createNotification(TITLE, TEXT, TICKER)
+        val description = contentDescForNotification(context, n)
+        assertThat(description).isEqualTo(createDescriptionText(n, TITLE))
+    }
+
+    @Test
+    fun notificationWithAllDifferentFields_titleMatchesAppName_descriptionIsText() {
+        val n = createNotification(getTestAppName(), TEXT, TICKER)
+        val description = contentDescForNotification(context, n)
+        assertThat(description).isEqualTo(createDescriptionText(n, TEXT))
+    }
+
+    @Test
+    fun notificationWithAllDifferentFields_titleMatchesAppNameNoText_descriptionIsTicker() {
+        val n = createNotification(getTestAppName(), null, TICKER)
+        val description = contentDescForNotification(context, n)
+        assertThat(description).isEqualTo(createDescriptionText(n, TICKER))
+    }
+
+    @Test
+    fun notificationWithAllDifferentFields_titleMatchesAppNameNoTextNoTicker_descriptionEmpty() {
+        val appName = getTestAppName()
+        val n = createNotification(appName, null, null)
+        val description = contentDescForNotification(context, n)
+        assertThat(description).isEqualTo(createDescriptionText(n, ""))
+    }
+
+    @Test
+    fun nullNotification_descriptionIsAppName() {
+        val description = contentDescForNotification(context, null)
+        assertThat(description).isEqualTo(createDescriptionText(null, ""))
+    }
+
+    private fun createNotification(
+        title: String? = null,
+        text: String? = null,
+        ticker: String? = null
+    ): Notification =
+        Notification.Builder(context)
+            .setContentTitle(title)
+            .setContentText(text)
+            .setTicker(ticker)
+            .build()
+
+    private fun getTestAppName(): String {
+        return getAppName(createNotification("", "", ""))
+    }
+
+    private fun getAppName(n: Notification?) =
+        n?.let {
+            val builder = Notification.Builder.recoverBuilder(context, it)
+            builder.loadHeaderAppName()
+        }
+            ?: ""
+
+    private fun createDescriptionText(n: Notification?, desc: String?): String {
+        val appName = getAppName(n)
+        return context.getString(R.string.accessibility_desc_notification_icon, appName, desc)
+    }
+}
diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/StatusBarIconView.java b/packages/SystemUI/src/com/android/systemui/statusbar/StatusBarIconView.java
index 843a454d73a0..984fcad469a8 100644
--- a/packages/SystemUI/src/com/android/systemui/statusbar/StatusBarIconView.java
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/StatusBarIconView.java
@@ -27,7 +27,6 @@ import android.app.ActivityManager;
 import android.app.Notification;
 import android.content.Context;
 import android.content.pm.ActivityInfo;
-import android.content.pm.ApplicationInfo;
 import android.content.res.ColorStateList;
 import android.content.res.Configuration;
 import android.content.res.Resources;
@@ -59,6 +58,7 @@ import com.android.internal.annotations.VisibleForTesting;
 import com.android.internal.statusbar.StatusBarIcon;
 import com.android.internal.util.ContrastColorUtil;
 import com.android.systemui.res.R;
+import com.android.systemui.statusbar.notification.NotificationContentDescription;
 import com.android.systemui.statusbar.notification.NotificationDozeHelper;
 import com.android.systemui.statusbar.notification.NotificationUtils;
 import com.android.systemui.statusbar.notification.shared.NotificationIconContainerRefactor;
@@ -350,9 +350,22 @@ public class StatusBarIconView extends AnimatedImageView implements StatusIconDi
     }
 
     public void setNotification(StatusBarNotification notification) {
-        mNotification = notification;
+        CharSequence contentDescription = null;
         if (notification != null) {
-            setContentDescription(notification.getNotification());
+            contentDescription = NotificationContentDescription
+                    .contentDescForNotification(mContext, notification.getNotification());
+        }
+        setNotification(notification, contentDescription);
+    }
+
+    /**
+     * Sets the notification with a pre-set content description.
+     */
+    public void setNotification(@Nullable StatusBarNotification notification,
+            @Nullable CharSequence notificationContentDescription) {
+        mNotification = notification;
+        if (!TextUtils.isEmpty(notificationContentDescription)) {
+            setContentDescription(notificationContentDescription);
         }
         maybeUpdateIconScaleDimens();
     }
@@ -621,15 +634,6 @@ public class StatusBarIconView extends AnimatedImageView implements StatusIconDi
         mNumberBackground.setBounds(w-dw, h-dh, w, h);
     }
 
-    private void setContentDescription(Notification notification) {
-        if (notification != null) {
-            String d = contentDescForNotification(mContext, notification);
-            if (!TextUtils.isEmpty(d)) {
-                setContentDescription(d);
-            }
-        }
-    }
-
     @Override
     public String toString() {
         return "StatusBarIconView("
@@ -647,35 +651,6 @@ public class StatusBarIconView extends AnimatedImageView implements StatusIconDi
         return mSlot;
     }
 
-
-    public static String contentDescForNotification(Context c, Notification n) {
-        String appName = "";
-        try {
-            Notification.Builder builder = Notification.Builder.recoverBuilder(c, n);
-            appName = builder.loadHeaderAppName();
-        } catch (RuntimeException e) {
-            Log.e(TAG, "Unable to recover builder", e);
-            // Trying to get the app name from the app info instead.
-            ApplicationInfo appInfo = n.extras.getParcelable(
-                    Notification.EXTRA_BUILDER_APPLICATION_INFO, ApplicationInfo.class);
-            if (appInfo != null) {
-                appName = String.valueOf(appInfo.loadLabel(c.getPackageManager()));
-            }
-        }
-
-        CharSequence title = n.extras.getCharSequence(Notification.EXTRA_TITLE);
-        CharSequence text = n.extras.getCharSequence(Notification.EXTRA_TEXT);
-        CharSequence ticker = n.tickerText;
-
-        // Some apps just put the app name into the title
-        CharSequence titleOrText = TextUtils.equals(title, appName) ? text : title;
-
-        CharSequence desc = !TextUtils.isEmpty(titleOrText) ? titleOrText
-                : !TextUtils.isEmpty(ticker) ? ticker : "";
-
-        return c.getString(R.string.accessibility_desc_notification_icon, appName, desc);
-    }
-
     /**
      * Set the color that is used to draw decoration like the overflow dot. This will not be applied
      * to the drawable.
diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/NotificationContentDescription.kt b/packages/SystemUI/src/com/android/systemui/statusbar/notification/NotificationContentDescription.kt
new file mode 100644
index 000000000000..e7012ea51caf
--- /dev/null
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/NotificationContentDescription.kt
@@ -0,0 +1,62 @@
+/*
+ * Copyright (C) 2023 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.
+ */
+
+@file:JvmName("NotificationContentDescription")
+
+package com.android.systemui.statusbar.notification
+
+import android.app.Notification
+import android.content.Context
+import android.content.pm.ApplicationInfo
+import android.text.TextUtils
+import android.util.Log
+import androidx.annotation.MainThread
+import com.android.systemui.res.R
+
+/**
+ * Returns accessibility content description for a given notification.
+ *
+ * NOTE: This is a relatively slow call.
+ */
+@MainThread
+fun contentDescForNotification(c: Context, n: Notification?): CharSequence {
+    var appName = ""
+    try {
+        val builder = Notification.Builder.recoverBuilder(c, n)
+        appName = builder.loadHeaderAppName()
+    } catch (e: RuntimeException) {
+        Log.e("ContentDescription", "Unable to recover builder", e)
+        // Trying to get the app name from the app info instead.
+        val appInfo =
+            n?.extras?.getParcelable(
+                Notification.EXTRA_BUILDER_APPLICATION_INFO,
+                ApplicationInfo::class.java
+            )
+        if (appInfo != null) {
+            appName = appInfo.loadLabel(c.packageManager).toString()
+        }
+    }
+    val title = n?.extras?.getCharSequence(Notification.EXTRA_TITLE)
+    val text = n?.extras?.getCharSequence(Notification.EXTRA_TEXT)
+    val ticker = n?.tickerText
+
+    // Some apps just put the app name into the title
+    val titleOrText = if (TextUtils.equals(title, appName)) text else title
+    val desc =
+        if (!TextUtils.isEmpty(titleOrText)) titleOrText
+        else if (!TextUtils.isEmpty(ticker)) ticker else ""
+    return c.getString(R.string.accessibility_desc_notification_icon, appName, desc)
+}
diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/icon/IconBuilder.kt b/packages/SystemUI/src/com/android/systemui/statusbar/notification/icon/IconBuilder.kt
index afc123faba79..319b49972bd2 100644
--- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/icon/IconBuilder.kt
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/icon/IconBuilder.kt
@@ -20,6 +20,7 @@ import android.app.Notification
 import android.content.Context
 import com.android.systemui.statusbar.StatusBarIconView
 import com.android.systemui.statusbar.notification.collection.NotificationEntry
+import com.android.systemui.statusbar.notification.contentDescForNotification
 import javax.inject.Inject
 
 /**
@@ -36,6 +37,6 @@ class IconBuilder @Inject constructor(
     }
 
     fun getIconContentDescription(n: Notification): CharSequence {
-        return StatusBarIconView.contentDescForNotification(context, n)
+        return contentDescForNotification(context, n)
     }
 }
diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/icon/IconManager.kt b/packages/SystemUI/src/com/android/systemui/statusbar/notification/icon/IconManager.kt
index 9e8654a66bde..61f9be54c795 100644
--- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/icon/IconManager.kt
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/icon/IconManager.kt
@@ -26,15 +26,15 @@ import android.os.Bundle
 import android.util.Log
 import android.view.View
 import android.widget.ImageView
+import com.android.app.tracing.traceSection
 import com.android.internal.statusbar.StatusBarIcon
-import com.android.systemui.res.R
 import com.android.systemui.dagger.SysUISingleton
+import com.android.systemui.res.R
 import com.android.systemui.statusbar.StatusBarIconView
 import com.android.systemui.statusbar.notification.InflationException
 import com.android.systemui.statusbar.notification.collection.NotificationEntry
 import com.android.systemui.statusbar.notification.collection.notifcollection.CommonNotifCollection
 import com.android.systemui.statusbar.notification.collection.notifcollection.NotifCollectionListener
-import com.android.app.tracing.traceSection
 import javax.inject.Inject
 
 /**
@@ -153,24 +153,27 @@ class IconManager @Inject constructor(
         entry.icons.peopleAvatarDescriptor = null
 
         val (normalIconDescriptor, sensitiveIconDescriptor) = getIconDescriptors(entry)
+        val notificationContentDescription = entry.sbn.notification?.let {
+            iconBuilder.getIconContentDescription(it)
+        }
 
         entry.icons.statusBarIcon?.let {
-            it.notification = entry.sbn
+            it.setNotification(entry.sbn, notificationContentDescription)
             setIcon(entry, normalIconDescriptor, it)
         }
 
         entry.icons.shelfIcon?.let {
-            it.notification = entry.sbn
+            it.setNotification(entry.sbn, notificationContentDescription)
             setIcon(entry, normalIconDescriptor, it)
         }
 
         entry.icons.aodIcon?.let {
-            it.notification = entry.sbn
+            it.setNotification(entry.sbn, notificationContentDescription)
             setIcon(entry, sensitiveIconDescriptor, it)
         }
 
         entry.icons.centeredIcon?.let {
-            it.notification = entry.sbn
+            it.setNotification(entry.sbn, notificationContentDescription)
             setIcon(entry, sensitiveIconDescriptor, it)
         }
     }
diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/StatusBarIconViewTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/StatusBarIconViewTest.java
index a6180ec8ea0e..f178046b665a 100644
--- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/StatusBarIconViewTest.java
+++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/StatusBarIconViewTest.java
@@ -54,6 +54,7 @@ import com.android.internal.statusbar.StatusBarIcon;
 import com.android.internal.util.ContrastColorUtil;
 import com.android.systemui.res.R;
 import com.android.systemui.SysuiTestCase;
+import com.android.systemui.statusbar.notification.NotificationContentDescription;
 
 import org.junit.Before;
 import org.junit.Rule;
@@ -183,7 +184,7 @@ public class StatusBarIconViewTest extends SysuiTestCase {
                 .build();
         // should be ApplicationInfo
         n.extras.putParcelable(Notification.EXTRA_BUILDER_APPLICATION_INFO, new Bundle());
-        StatusBarIconView.contentDescForNotification(mContext, n);
+        NotificationContentDescription.contentDescForNotification(mContext, n);
 
         // no crash, good
     }