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

Commit 94a92382 authored by Nicolo' Mazzucato's avatar Nicolo' Mazzucato Committed by Android Build Coastguard Worker
Browse files

Fix shelf icons when on an external display 

The shelf icons were being created with the application context instead
of the display-aware shade context. This caused issues with display
density.

This was not catched from any linter as the way icons are created is very custom (a class that sets fields to a notification entry)

Also, methods to create icons without providing a proper context have been marked as deprecated explicitly: a full cleanup to delete them will foll

Bug: 446842713
Test: IconManagerTest + manually add many notifications while on the external display and observe shelf icon sizing being correct.
Flag: com.android.systemui.shade_window_goes_around
Cherrypick-From: https://googleplex-android-review.googlesource.com/q/commit:9650df1671e712c770b85f865bf6bda01499ca22
Merged-In: Ifcb3c1db7e5bd54e3129d652c2e9eae79595b530
Change-Id: Ifcb3c1db7e5bd54e3129d652c2e9eae79595b530
parent 0ab3fe3f

packages/SystemUI/src/com/android/systemui/statusbar/notification/icon/IconBuilder.kt

+15 −8
Original line number Original line Diff line number Diff line
@@ -27,12 +27,8 @@ import javax.inject.Inject 




/** Testable wrapper around Context. */

/** Testable wrapper around Context. */

class IconBuilder @Inject constructor(@StatusBarMain private val context: Context) {

class IconBuilder @Inject constructor(@StatusBarMain private val context: Context) {

    @JvmOverloads



    fun createIconView(

    fun createIconView(entry: NotificationEntry, context: Context): StatusBarIconView {

        entry: NotificationEntry,

        // TODO b/362720336: remove all usages of this without a provided context.

        context: Context = this.context,

    ): StatusBarIconView {

        return StatusBarIconView(

        return StatusBarIconView(

            context,

            context,

            "${entry.sbn.packageName}/0x${Integer.toHexString(entry.sbn.id)}",

            "${entry.sbn.packageName}/0x${Integer.toHexString(entry.sbn.id)}",
@@ -40,11 +36,22 @@ class IconBuilder @Inject constructor(@StatusBarMain private val context: Contex 
        )

        )

    }

    }





    @JvmOverloads

    fun createIconView(entry: BundleEntry, context: Context): StatusBarIconView {

    fun createIconView(entry: BundleEntry, context: Context = this.context): StatusBarIconView {

        return StatusBarIconView(context, entry.key, null, entry)

        return StatusBarIconView(context, entry.key, null, entry)

    }

    }





    // TODO b/362720336: remove all usages of this without a provided context.

    @Deprecated("Use createIconView(entry, context), providing the correct context.")

    fun createIconView(entry: NotificationEntry): StatusBarIconView {

        return createIconView(entry, context)

    }



    // TODO b/362720336: remove all usages of this without a provided context.

    @Deprecated("Use createIconView(entry, context), providing the correct context.")

    fun createIconView(entry: BundleEntry): StatusBarIconView {

        return createIconView(entry, context)

    }



    fun getIconContentDescription(n: Notification): CharSequence {

    fun getIconContentDescription(n: Notification): CharSequence {

        return contentDescForNotification(context, n)

        return contentDescForNotification(context, n)

    }

    }

packages/SystemUI/src/com/android/systemui/statusbar/notification/icon/IconManager.kt

+15 −2
Original line number Original line Diff line number Diff line
@@ -35,6 +35,8 @@ import com.android.systemui.dagger.qualifiers.Application 
import com.android.systemui.dagger.qualifiers.Background

import com.android.systemui.dagger.qualifiers.Background

import com.android.systemui.dagger.qualifiers.Main

import com.android.systemui.dagger.qualifiers.Main

import com.android.systemui.res.R

import com.android.systemui.res.R

import com.android.systemui.shade.ShadeDisplayAware

import com.android.systemui.shade.shared.flag.ShadeWindowGoesAround

import com.android.systemui.statusbar.StatusBarIconView

import com.android.systemui.statusbar.StatusBarIconView

import com.android.systemui.statusbar.core.StatusBarConnectedDisplays

import com.android.systemui.statusbar.core.StatusBarConnectedDisplays

import com.android.systemui.statusbar.notification.InflationException

import com.android.systemui.statusbar.notification.InflationException
@@ -70,6 +72,7 @@ constructor( 
    @Application private val applicationCoroutineScope: CoroutineScope,

    @Application private val applicationCoroutineScope: CoroutineScope,

    @Background private val bgCoroutineContext: CoroutineContext,

    @Background private val bgCoroutineContext: CoroutineContext,

    @Main private val mainCoroutineContext: CoroutineContext,

    @Main private val mainCoroutineContext: CoroutineContext,

    @ShadeDisplayAware private val shadeContext: Context,

) : ConversationIconManager {

) : ConversationIconManager {





    /**

    /**
@@ -171,7 +174,12 @@ constructor( 
            }

            }





            // Construct the shelf icon view.

            // Construct the shelf icon view.

            val shelfIcon = iconBuilder.createIconView(entry)

            val shelfIcon =

                if (ShadeWindowGoesAround.isEnabled) {

                    iconBuilder.createIconView(entry, shadeContext)

                } else {

                    iconBuilder.createIconView(entry)

                }

            shelfIcon.scaleType = ImageView.ScaleType.CENTER_INSIDE

            shelfIcon.scaleType = ImageView.ScaleType.CENTER_INSIDE

            shelfIcon.visibility = View.INVISIBLE

            shelfIcon.visibility = View.INVISIBLE
@@ -291,7 +299,12 @@ constructor( 
            }

            }





            // Construct the shelf icon view.

            // Construct the shelf icon view.

            val shelfIcon = iconBuilder.createIconView(entry)

            val shelfIcon =

                if (ShadeWindowGoesAround.isEnabled) {

                    iconBuilder.createIconView(entry, context)

                } else {

                    iconBuilder.createIconView(entry)

                }

            shelfIcon.scaleType = ImageView.ScaleType.CENTER_INSIDE

            shelfIcon.scaleType = ImageView.ScaleType.CENTER_INSIDE

            shelfIcon.visibility = View.INVISIBLE

            shelfIcon.visibility = View.INVISIBLE

packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/icon/IconManagerTest.kt

+52 −0
Original line number Original line Diff line number Diff line
@@ -35,7 +35,10 @@ import androidx.test.ext.junit.runners.AndroidJUnit4 
import androidx.test.filters.SmallTest

import androidx.test.filters.SmallTest

import com.android.systemui.SysuiTestCase

import com.android.systemui.SysuiTestCase

import com.android.systemui.controls.controller.AuxiliaryPersistenceWrapperTest.Companion.any

import com.android.systemui.controls.controller.AuxiliaryPersistenceWrapperTest.Companion.any

import com.android.systemui.shade.shared.flag.ShadeWindowGoesAround

import com.android.systemui.statusbar.core.StatusBarConnectedDisplays

import com.android.systemui.statusbar.core.StatusBarConnectedDisplays

import com.android.systemui.statusbar.notification.collection.BundleEntry

import com.android.systemui.statusbar.notification.collection.BundleSpec

import com.android.systemui.statusbar.notification.collection.NotificationEntry

import com.android.systemui.statusbar.notification.collection.NotificationEntry

import com.android.systemui.statusbar.notification.collection.NotificationEntryBuilder

import com.android.systemui.statusbar.notification.collection.NotificationEntryBuilder

import com.android.systemui.statusbar.notification.collection.notifcollection.CommonNotifCollection

import com.android.systemui.statusbar.notification.collection.notifcollection.CommonNotifCollection
@@ -49,6 +52,7 @@ import org.junit.Test 
import org.junit.runner.RunWith

import org.junit.runner.RunWith

import org.mockito.Mock

import org.mockito.Mock

import org.mockito.Mockito.anyInt

import org.mockito.Mockito.anyInt

import org.mockito.Mockito.spy

import org.mockito.Mockito.`when`

import org.mockito.Mockito.`when`

import org.mockito.MockitoAnnotations

import org.mockito.MockitoAnnotations
@@ -63,6 +67,7 @@ class IconManagerTest : SysuiTestCase() { 




    private var id = 0

    private var id = 0

    private val context = InstrumentationRegistry.getTargetContext()

    private val context = InstrumentationRegistry.getTargetContext()

    private val shadeContext = spy(context)





    @Mock private lateinit var shortcut: ShortcutInfo

    @Mock private lateinit var shortcut: ShortcutInfo

    @Mock private lateinit var shortcutIc: Icon

    @Mock private lateinit var shortcutIc: Icon
@@ -104,6 +109,7 @@ class IconManagerTest : SysuiTestCase() { 
                testScope,

                testScope,

                bgContext,

                bgContext,

                mainContext,

                mainContext,

                shadeContext,

            )

            )

    }

    }
@@ -279,6 +285,52 @@ class IconManagerTest : SysuiTestCase() { 
        assertThat(entry?.icons?.shelfIcon?.sourceIcon).isEqualTo(shortcutIc)

        assertThat(entry?.icons?.shelfIcon?.sourceIcon).isEqualTo(shortcutIc)

    }

    }





    @Test

    @EnableFlags(ShadeWindowGoesAround.FLAG_NAME)

    fun createIcons_forEntry_shelfIconCreatedWithShadeContext() {

        val entry =

            notificationEntry(hasShortcut = true, hasMessageSenderIcon = true, hasLargeIcon = true)

        entry?.let { iconManager.createIcons(it) }



        assertThat(entry?.icons?.shelfIcon?.context).isEqualTo(shadeContext)

        assertThat(entry?.icons?.shelfIcon?.context).isNotEqualTo(context)

    }



    @Test

    @DisableFlags(ShadeWindowGoesAround.FLAG_NAME)

    fun createIcons_forEntry_shelfIconCreatedWithoutShadeContext() {

        val entry =

            notificationEntry(hasShortcut = true, hasMessageSenderIcon = true, hasLargeIcon = true)

        entry?.let { iconManager.createIcons(it) }



        assertThat(entry?.icons?.shelfIcon?.context).isNotEqualTo(shadeContext)

        assertThat(entry?.icons?.shelfIcon?.context).isEqualTo(context)

    }



    @Test

    @EnableFlags(ShadeWindowGoesAround.FLAG_NAME)

    fun createIcons_forBundleEntry_withContextProvided_createsShelfIconWithProvidedContext() {

        val entry = BundleEntry(BundleSpec.NEWS)

        val newContext = spy(context)

        iconManager.createIcons(newContext, entry)



        assertThat(entry.icons.shelfIcon?.context).isEqualTo(newContext)

        assertThat(entry.icons.shelfIcon?.context).isNotEqualTo(shadeContext)

        assertThat(entry.icons.shelfIcon?.context).isNotEqualTo(context)

    }



    @Test

    @DisableFlags(ShadeWindowGoesAround.FLAG_NAME)

    fun createIcons_forBundleEntry_shelfIconCreatedWithoutShadeContext() {

        val entry = BundleEntry(BundleSpec.NEWS)

        val newContext = spy(context)

        iconManager.createIcons(newContext, entry)



        assertThat(entry.icons.shelfIcon?.context).isNotEqualTo(newContext)

        assertThat(entry.icons.shelfIcon?.context).isNotEqualTo(shadeContext)

        assertThat(entry.icons.shelfIcon?.context).isEqualTo(context)

    }



    private fun notificationEntry(

    private fun notificationEntry(

        hasShortcut: Boolean,

        hasShortcut: Boolean,

        hasMessageSenderIcon: Boolean,

        hasMessageSenderIcon: Boolean,

packages/SystemUI/tests/utils/src/com/android/systemui/statusbar/notification/icon/IconManagerKosmos.kt

+2 −0
Original line number Original line Diff line number Diff line
@@ -16,6 +16,7 @@ 




package com.android.systemui.statusbar.notification.icon

package com.android.systemui.statusbar.notification.icon





import android.content.mockedContext

import android.content.pm.launcherApps

import android.content.pm.launcherApps

import com.android.systemui.kosmos.Kosmos

import com.android.systemui.kosmos.Kosmos

import com.android.systemui.kosmos.applicationCoroutineScope

import com.android.systemui.kosmos.applicationCoroutineScope
@@ -32,5 +33,6 @@ val Kosmos.iconManager by 
            applicationCoroutineScope,

            applicationCoroutineScope,

            backgroundCoroutineContext,

            backgroundCoroutineContext,

            mainCoroutineContext,

            mainCoroutineContext,

            mockedContext,

        )

        )

    }
 
    }

packages/SystemUI/tests/utils/src/com/android/systemui/statusbar/notification/row/ExpandableNotificationRowBuilder.kt

+1 −0
Original line number Original line Diff line number Diff line
@@ -178,6 +178,7 @@ class ExpandableNotificationRowBuilder( 
                mTestScope,

                mTestScope,

                mBgCoroutineContext,

                mBgCoroutineContext,

                mMainCoroutineContext,

                mMainCoroutineContext,

                context,

            )

            )





        mSmartReplyConstants =

        mSmartReplyConstants =