Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit acd96548 authored by Jeff DeCew's avatar Jeff DeCew
Browse files

Refactor AppIconProvider to fix bugs 

* Instead of taking a context, the provider takes a UserHandle
* The UserHandle is used to internally look up whether the app icon should show the work profile badge
* The skeleton codepath suppresses the work profile badging
Fixes: 416215382

* Renamed internal non-skeleton members to 'standard' for clarity
Fixes: 406484337

* Adds the ability for individual callers of getOrFetchAppIcon to request a different Drawable instance, which resolves an issue where the mutations of the drawable caused by expansion of the compose bundle header would resize the app icon in other notification rows.
* Cache the BitmapInfo across all drawable instances to avoid increased memory cost
Fixes: 420908272

* Validate that single layer app icons appear
Fixes: 422748092

Flag: com.android.systemui.notification_bundle_ui
Flag: android.app.notifications_redesign_app_icons
Test: presubmit
Change-Id: I6d840310b72b30b8265ff4b3852d7fe1e969081e
parent a8efffb2

packages/SystemUI/multivalentTests/src/com/android/systemui/statusbar/notification/row/NotificationInfoTest.kt

+2 −14
Original line number Diff line number Diff line
@@ -125,10 +125,7 @@ class NotificationInfoTest : SysuiTestCase() { 


        // Inflate the layout

        val inflater = LayoutInflater.from(mContext)

        val layoutId =

            if (Flags.notificationsRedesignTemplates()) R.layout.notification_2025_info

            else R.layout.notification_info

        underTest = inflater.inflate(layoutId, null) as NotificationInfo

        underTest = inflater.inflate(R.layout.notification_info, null) as NotificationInfo



        underTest.setGutsParent(mock<NotificationGuts>())
@@ -231,16 +228,7 @@ class NotificationInfoTest : SysuiTestCase() { 
    @EnableFlags(Flags.FLAG_NOTIFICATIONS_REDESIGN_TEMPLATES)

    fun testBindNotification_SetsPackageIcon_flagOn() {

        val iconDrawable = mock<Drawable>()

        whenever(mockIconStyleProvider.shouldShowWorkProfileBadge(anyOrNull(), anyOrNull()))

            .thenReturn(false)

        whenever(

                mockAppIconProvider.getOrFetchAppIcon(

                    anyOrNull(),

                    anyOrNull(),

                    anyBoolean(),

                    anyBoolean(),

                )

            )

        whenever(mockAppIconProvider.getOrFetchAppIcon(anyOrNull(), anyOrNull(), anyOrNull()))

            .thenReturn(iconDrawable)

        bindNotification()

        val iconView = underTest.findViewById<ImageView>(R.id.pkg_icon)

packages/SystemUI/multivalentTests/src/com/android/systemui/statusbar/notification/row/domain/interactor/BundleInteractorTest.kt

+16 −26
Original line number Diff line number Diff line
@@ -18,10 +18,10 @@ 


package com.android.systemui.statusbar.notification.row.domain.interactor



import android.content.Context

import android.graphics.Color

import android.graphics.drawable.ColorDrawable

import android.graphics.drawable.Drawable

import android.os.UserHandle

import android.platform.test.annotations.EnableFlags

import androidx.compose.material3.ExperimentalMaterial3ExpressiveApi

import androidx.compose.material3.MotionScheme
@@ -55,7 +55,6 @@ import org.mockito.Mockito.times 
import org.mockito.Mockito.verify

import org.mockito.junit.MockitoJUnit

import org.mockito.junit.MockitoRule

import org.mockito.kotlin.anyOrNull

import org.mockito.kotlin.mock

import org.mockito.kotlin.whenever

import platform.test.motion.compose.runMonotonicClockTest
@@ -101,9 +100,8 @@ class BundleInteractorTest : SysuiTestCase() { 
            whenever(

                    kosmos.mockAppIconProvider.getOrFetchAppIcon(

                        any<String>(),

                        any<Context>(),

                        eq(false),

                        eq(false),

                        any<UserHandle>(),

                        any<String>(),

                    )

                )

                .thenReturn(drawable1)
@@ -119,7 +117,7 @@ class BundleInteractorTest : SysuiTestCase() { 


            // Assert

            verify(kosmos.mockAppIconProvider, times(4))

                .getOrFetchAppIcon(any<String>(), any<Context>(), eq(false), eq(false))

                .getOrFetchAppIcon(any<String>(), any<UserHandle>(), any<String>())



            assertThat(result).hasSize(3)

            assertThat(result).containsExactly(drawable1, drawable2, drawable3).inOrder()
@@ -144,18 +142,16 @@ class BundleInteractorTest : SysuiTestCase() { 
            whenever(

                    kosmos.mockAppIconProvider.getOrFetchAppIcon(

                        eq("app1"),

                        anyOrNull<Context>(),

                        eq(false),

                        eq(false),

                        any<UserHandle>(),

                        any<String>(),

                    )

                )

                .thenReturn(drawable1)

            whenever(

                    kosmos.mockAppIconProvider.getOrFetchAppIcon(

                        eq("app2"),

                        anyOrNull<Context>(),

                        eq(false),

                        eq(false),

                        any<UserHandle>(),

                        any<String>(),

                    )

                )

                .thenReturn(drawable2)
@@ -170,9 +166,9 @@ class BundleInteractorTest : SysuiTestCase() { 
            // Assert

            assertThat(result).containsExactly(drawable1, drawable2).inOrder()

            verify(kosmos.mockAppIconProvider)

                .getOrFetchAppIcon(eq("app1"), anyOrNull<Context>(), eq(false), eq(false))

                .getOrFetchAppIcon(eq("app1"), any<UserHandle>(), any<String>())

            verify(kosmos.mockAppIconProvider)

                .getOrFetchAppIcon(eq("app2"), anyOrNull<Context>(), eq(false), eq(false))

                .getOrFetchAppIcon(eq("app2"), any<UserHandle>(), any<String>())

        }



    @Test
@@ -201,9 +197,8 @@ class BundleInteractorTest : SysuiTestCase() { 
            whenever(

                    kosmos.mockAppIconProvider.getOrFetchAppIcon(

                        eq("new_app"),

                        anyOrNull<Context>(),

                        eq(false),

                        eq(false),

                        any<UserHandle>(),

                        any<String>(),

                    )

                )

                .thenReturn(drawable3)
@@ -218,16 +213,11 @@ class BundleInteractorTest : SysuiTestCase() { 
            // Assert

            assertThat(result).containsExactly(drawable3)

            verify(kosmos.mockAppIconProvider, times(0))

                .getOrFetchAppIcon(eq("old_app"), anyOrNull<Context>(), eq(false), eq(false))

                .getOrFetchAppIcon(eq("old_app"), any<UserHandle>(), any<String>())

            verify(kosmos.mockAppIconProvider, times(0))

                .getOrFetchAppIcon(

                    eq("at_collapse_app"),

                    anyOrNull<Context>(),

                    eq(false),

                    eq(false),

                )

                .getOrFetchAppIcon(eq("at_collapse_app"), any<UserHandle>(), any<String>())

            verify(kosmos.mockAppIconProvider)

                .getOrFetchAppIcon(eq("new_app"), anyOrNull<Context>(), eq(false), eq(false))

                .getOrFetchAppIcon(eq("new_app"), any<UserHandle>(), any<String>())

        }



    @Test
@@ -253,7 +243,7 @@ class BundleInteractorTest : SysuiTestCase() { 
            // Assert

            assertThat(result).isEmpty()

            verify(kosmos.mockAppIconProvider, times(0))

                .getOrFetchAppIcon(anyString(), anyOrNull<Context>(), eq(false), eq(false))

                .getOrFetchAppIcon(anyString(), any<UserHandle>(), any<String>())

        }



    @Test

packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotifCollectionCache.kt

+31 −2
Original line number Diff line number Diff line
@@ -157,9 +157,38 @@ class NotifCollectionCache<V>( 
     * purge((c));    // deletes a from the cache and marks b for deletion, etc.

     * ```

     */

    fun purge(wantedKeys: Collection<String>) {

    fun purge(wantedKeys: Collection<String>) = purgeUnless { it in wantedKeys }



    /**

     * Clear entries whose keys do NOT match [predicate]. This can be called from any thread.

     *

     * If a key matches the [predicate], it will be preserved, otherwise it may be purged depending

     * on internal retainCount.

     *

     * If retainCount > 0, a given key will need to match [predicate] in ([retainCount] + 1)

     * consecutive [purge] calls made more than [purgeTimeoutMillis] apart in order to be cleared.

     * This count will be reset for any given key 1) if [getOrFetch] is called for the key or 2) if

     * the key matches [predicate] in a subsequent [purge] call. We prioritize keeping the entry if

     * possible, so if [purge] is called simultaneously with [getOrFetch] on different threads for

     * example, we will try to keep it in the cache, although it is not guaranteed. If avoiding

     * cache misses is a concern, consider increasing the [retainCount] or [purgeTimeoutMillis].

     *

     * For example, say [retainCount] = 1 and [purgeTimeoutMillis] = 1000 and we start with entries

     * (a, b, c) in the cache:

     * ```kotlin

     * purgeUnless { it in (a, c) } // marks b for deletion

     * Thread.sleep(500)

     * purgeUnless { it in (a, c) } // does nothing as it was called earlier than the min 1s

     * Thread.sleep(500)

     * purgeUnless { it in (b, c) } // b is no longer marked for deletion, but now a is

     * Thread.sleep(1000);

     * purgeUnless { it == c }      // deletes a from the cache and marks b for deletion, etc.

     * ```

     */

    fun purgeUnless(predicate: (String) -> Boolean) {

        for ((key, entry) in cache) {

            if (key in wantedKeys) {

            val keep = predicate(key)

            if (keep) {

                entry.resetLives()

            } else if (entry.tryPurge()) {

                cache.remove(key)

packages/SystemUI/src/com/android/systemui/statusbar/notification/promoted/PromotedNotificationContentExtractor.kt

+1 −3
Original line number Diff line number Diff line
@@ -324,9 +324,7 @@ constructor( 
    private fun StatusBarNotification.skeletonAppIcon(packageContext: Context): NotifIcon.AppIcon? {

        if (!android.app.Flags.notificationsRedesignAppIcons()) return null

        if (!notificationIconStyleProvider.shouldShowAppIcon(this, packageContext)) return null

        return NotifIcon.AppIcon(

            appIconProvider.getOrFetchSkeletonAppIcon(packageName, packageContext)

        )

        return NotifIcon.AppIcon(appIconProvider.getOrFetchSkeletonAppIcon(packageName, user))

    }



    private fun Notification.title(): CharSequence? = getCharSequenceExtraUnlessEmpty(EXTRA_TITLE)

packages/SystemUI/src/com/android/systemui/statusbar/notification/row/NotificationInfo.java

+2 −6
Original line number Diff line number Diff line
@@ -17,7 +17,6 @@ 
package com.android.systemui.statusbar.notification.row;



import static android.app.Flags.notificationsRedesignTemplates;

import static android.app.Flags.notificationsRedesignThemedAppIcons;

import static android.app.Notification.EXTRA_BUILDER_APPLICATION_INFO;

import static android.app.NotificationChannel.SYSTEM_RESERVED_IDS;

import static android.app.NotificationManager.IMPORTANCE_DEFAULT;
@@ -340,11 +339,8 @@ public class NotificationInfo extends LinearLayout implements NotificationGuts.G 
                try {

                    mAppName = String.valueOf(mPm.getApplicationLabel(info));

                    // The app icon is likely already in the cache, so let's use it

                    boolean withWorkProfileBadge =

                            mIconStyleProvider.shouldShowWorkProfileBadge(mSbn, getContext());

                    mPkgIcon = mAppIconProvider.getOrFetchAppIcon(info.packageName, getContext(),

                            withWorkProfileBadge,

                            /* themed = */ notificationsRedesignThemedAppIcons());

                    mPkgIcon = mAppIconProvider.getOrFetchAppIcon(info.packageName,

                            mSbn.getUser(), /* instanceKey= */ "LEGACY");

                } catch (Exception ignored) {

                }

            }