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

Commit 5383ba84 authored by Caitlin Shkuratov's avatar Caitlin Shkuratov
Browse files

[Status Bar] Move icon controller's CommandQueue.Callbacks into object. 

Bug: 265307726
Test: atest StatusBarIconControllerImplTest
Test: manual: (1) Be in silent mode. (2) Make a phone call. (3) Mute the
phone call. -> Verify that *both* silent *and* mute icons appear in the
status bar. Verify that they can each be toggled independently without
affecting each other.

Change-Id: Id3c9a0883412c41943932f8fc887d36690ea8ee0
parent 3d2e83aa

packages/SystemUI/src/com/android/systemui/statusbar/phone/StatusBarIconControllerImpl.java

+21 −13
Original line number Diff line number Diff line
@@ -62,7 +62,7 @@ import javax.inject.Inject; 
 */

@SysUISingleton

public class StatusBarIconControllerImpl implements Tunable,

        ConfigurationListener, Dumpable, CommandQueue.Callbacks, StatusBarIconController, DemoMode {

        ConfigurationListener, Dumpable, StatusBarIconController, DemoMode {



    private static final String TAG = "StatusBarIconController";

    // Use this suffix to prevent external icon slot names from unintentionally overriding our
@@ -93,7 +93,7 @@ public class StatusBarIconControllerImpl implements Tunable, 
        mStatusBarPipelineFlags = statusBarPipelineFlags;



        configurationController.addCallback(this);

        commandQueue.addCallback(this);

        commandQueue.addCallback(mCommandQueueCallbacks);

        tunerService.addTunable(this, ICON_HIDE_LIST);

        demoModeController.addCallback(this);

        dumpManager.registerDumpable(getClass().getSimpleName(), this);
@@ -350,6 +350,8 @@ public class StatusBarIconControllerImpl implements Tunable, 
        }

    }



    // TODO(b/265307726): Determine why we have two [setExternalIcon] methods and why they're

    // different.

    @Override

    public void setExternalIcon(String slot) {

        String slotName = createExternalSlotName(slot);
@@ -359,11 +361,12 @@ public class StatusBarIconControllerImpl implements Tunable, 
        mIconGroups.forEach(l -> l.onIconExternal(viewIndex, height));

    }



    // Override for *both* CommandQueue.Callbacks AND StatusBarIconController.

    // TODO(b/265307726): Pull out the CommandQueue callbacks into a member variable to

    //  differentiate between those callback methods and StatusBarIconController methods.

    @Override

    public void setIcon(String slot, StatusBarIcon icon) {

        setExternalIcon(slot, icon);

    }



    private void setExternalIcon(String slot, StatusBarIcon icon) {

        String slotName = createExternalSlotName(slot);

        if (icon == null) {

            removeAllIconsForSlot(slotName);
@@ -374,6 +377,19 @@ public class StatusBarIconControllerImpl implements Tunable, 
        setIcon(slotName, holder);

    }



    private final CommandQueue.Callbacks mCommandQueueCallbacks = new CommandQueue.Callbacks() {

        @Override

        public void setIcon(String slot, StatusBarIcon icon) {

            // Icons that come from CommandQueue are from external services.

            setExternalIcon(slot, icon);

        }



        @Override

        public void removeIcon(String slot) {

            removeAllIconsForExternalSlot(slot);

        }

    };



    private void setIcon(String slot, @NonNull StatusBarIconHolder holder) {

        boolean isNew = mStatusBarIconList.getIconHolder(slot, holder.getTag()) == null;

        mStatusBarIconList.setIcon(slot, holder);
@@ -417,14 +433,6 @@ public class StatusBarIconControllerImpl implements Tunable, 
        }

    }



    // CommandQueue.Callbacks override

    // TODO(b/265307726): Pull out the CommandQueue callbacks into a member variable to

    //  differentiate between those callback methods and StatusBarIconController methods.

    @Override

    public void removeIcon(String slot) {

        removeAllIconsForExternalSlot(slot);

    }



    /** */

    @Override

    public void removeIcon(String slot, int tag) {

packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/StatusBarIconControllerImplTest.kt

+99 −5
Original line number Diff line number Diff line
@@ -20,13 +20,17 @@ import android.os.UserHandle 
import androidx.test.filters.SmallTest

import com.android.internal.statusbar.StatusBarIcon

import com.android.systemui.SysuiTestCase

import com.android.systemui.statusbar.CommandQueue

import com.android.systemui.statusbar.phone.StatusBarIconController.TAG_PRIMARY

import com.android.systemui.statusbar.phone.StatusBarIconControllerImpl.EXTERNAL_SLOT_SUFFIX

import com.android.systemui.util.mockito.kotlinArgumentCaptor

import com.android.systemui.util.mockito.mock

import com.google.common.truth.Truth.assertThat

import org.junit.Before

import org.junit.Test

import org.mockito.Mock

import org.mockito.Mockito.verify

import org.mockito.MockitoAnnotations



@SmallTest

class StatusBarIconControllerImplTest : SysuiTestCase() {
@@ -34,15 +38,19 @@ class StatusBarIconControllerImplTest : SysuiTestCase() { 
    private lateinit var underTest: StatusBarIconControllerImpl



    private lateinit var iconList: StatusBarIconList

    private lateinit var commandQueueCallbacks: CommandQueue.Callbacks

    private val iconGroup: StatusBarIconController.IconManager = mock()



    @Mock private lateinit var commandQueue: CommandQueue



    @Before

    fun setUp() {

        MockitoAnnotations.initMocks(this)

        iconList = StatusBarIconList(arrayOf())

        underTest =

            StatusBarIconControllerImpl(

                context,

                mock(),

                commandQueue,

                mock(),

                mock(),

                mock(),
@@ -51,6 +59,9 @@ class StatusBarIconControllerImplTest : SysuiTestCase() { 
                mock(),

            )

        underTest.addIconGroup(iconGroup)

        val commandQueueCallbacksCaptor = kotlinArgumentCaptor<CommandQueue.Callbacks>()

        verify(commandQueue).addCallback(commandQueueCallbacksCaptor.capture())

        commandQueueCallbacks = commandQueueCallbacksCaptor.value

    }



    /** Regression test for b/255428281. */
@@ -83,17 +94,45 @@ class StatusBarIconControllerImplTest : SysuiTestCase() { 


    /** Regression test for b/255428281. */

    @Test

    fun internalAndExternalIconWithSameName_externalRemoved_viaRemoveIcon_internalStays() {

    fun internalAndExternalIconWithSameName_externalFromCommandQueue_bothDisplayed() {

        val slotName = "mute"



        // Internal

        underTest.setIcon(slotName, /* resourceId= */ 10, "contentDescription")



        // External

        underTest.setIcon(slotName, createExternalIcon())

        val externalIcon =

            StatusBarIcon(

                "external.package",

                UserHandle.ALL,

                /* iconId= */ 2,

                /* iconLevel= */ 0,

                /* number= */ 0,

                "contentDescription",

            )

        commandQueueCallbacks.setIcon(slotName, externalIcon)



        assertThat(iconList.slots).hasSize(2)

        // Whichever was added last comes first

        assertThat(iconList.slots[0].name).isEqualTo(slotName + EXTERNAL_SLOT_SUFFIX)

        assertThat(iconList.slots[1].name).isEqualTo(slotName)

        assertThat(iconList.slots[0].hasIconsInSlot()).isTrue()

        assertThat(iconList.slots[1].hasIconsInSlot()).isTrue()

    }



        // WHEN the external icon is removed via #removeIcon

        underTest.removeIcon(slotName)

    /** Regression test for b/255428281. */

    @Test

    fun internalAndExternalIconWithSameName_externalRemoved_fromCommandQueue_internalStays() {

        val slotName = "mute"



        // Internal

        underTest.setIcon(slotName, /* resourceId= */ 10, "contentDescription")



        // External

        commandQueueCallbacks.setIcon(slotName, createExternalIcon())



        // WHEN the external icon is removed via CommandQueue.Callbacks#removeIcon

        commandQueueCallbacks.removeIcon(slotName)



        // THEN the external icon is removed but the internal icon remains

        // Note: [StatusBarIconList] never removes slots from its list, it just sets the holder for
@@ -288,6 +327,53 @@ class StatusBarIconControllerImplTest : SysuiTestCase() { 
        assertThat(internalHolder.icon!!.icon.resId).isEqualTo(10)

    }



    /** Regression test for b/255428281. */

    @Test

    fun internalAndExternalIconWithSameName_fromCommandQueue_externalUpdatedIndependently() {

        val slotName = "mute"



        // Internal

        underTest.setIcon(slotName, /* resourceId= */ 10, "contentDescription")



        // External

        val startingExternalIcon =

            StatusBarIcon(

                "external.package",

                UserHandle.ALL,

                /* iconId= */ 20,

                /* iconLevel= */ 0,

                /* number= */ 0,

                "externalDescription",

            )

        commandQueueCallbacks.setIcon(slotName, startingExternalIcon)



        // WHEN the external icon is updated

        val newExternalIcon =

            StatusBarIcon(

                "external.package",

                UserHandle.ALL,

                /* iconId= */ 21,

                /* iconLevel= */ 0,

                /* number= */ 0,

                "newExternalDescription",

            )

        commandQueueCallbacks.setIcon(slotName, newExternalIcon)



        // THEN only the external slot gets the updates

        val externalSlot = iconList.slots[0]

        val externalHolder = externalSlot.getHolderForTag(TAG_PRIMARY)!!

        assertThat(externalSlot.name).isEqualTo(slotName + EXTERNAL_SLOT_SUFFIX)

        assertThat(externalHolder.icon!!.contentDescription).isEqualTo("newExternalDescription")

        assertThat(externalHolder.icon!!.icon.resId).isEqualTo(21)



        // And the internal slot has its own values

        val internalSlot = iconList.slots[1]

        val internalHolder = internalSlot.getHolderForTag(TAG_PRIMARY)!!

        assertThat(internalSlot.name).isEqualTo(slotName)

        assertThat(internalHolder.icon!!.contentDescription).isEqualTo("contentDescription")

        assertThat(internalHolder.icon!!.icon.resId).isEqualTo(10)

    }



    @Test

    fun externalSlot_alreadyEndsWithSuffix_suffixNotAddedTwice() {

        underTest.setIcon("myslot$EXTERNAL_SLOT_SUFFIX", createExternalIcon())
@@ -296,6 +382,14 @@ class StatusBarIconControllerImplTest : SysuiTestCase() { 
        assertThat(iconList.slots[0].name).isEqualTo("myslot$EXTERNAL_SLOT_SUFFIX")

    }



    @Test

    fun externalSlot_fromCommandQueue_alreadyEndsWithSuffix_suffixNotAddedTwice() {

        commandQueueCallbacks.setIcon("myslot$EXTERNAL_SLOT_SUFFIX", createExternalIcon())



        assertThat(iconList.slots).hasSize(1)

        assertThat(iconList.slots[0].name).isEqualTo("myslot$EXTERNAL_SLOT_SUFFIX")

    }



    private fun createExternalIcon(): StatusBarIcon {

        return StatusBarIcon(

            "external.package",