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

Commit b18dd359 authored by Caitlin Shkuratov's avatar Caitlin Shkuratov
Browse files

[Status bar] Clarify the API between TileServices and status bar icons. 

This CL re-names the StatusBarIconController APIs that are only used by
TileServices to specifically have "tile" in them. Then, it's obvious why
the implementation of the method considers those icons external instead
of internal.

This also fixes two  bugs with the tile status bar icons. See first
two linked bug fixes.

Fixes: 273536393
Fixes: 273540833
Bug: 265307726

Test: Add tile service that has status icon -> verify icon is added (see
b/265307726 for demo)
Test: Remove tile service that has status icon -> verify icon is rmeoved
(see b/265307726 for demo)
Test: verify tile status icon is correct size
Test: atest StatusBarIconControllerImplTest

Change-Id: Ieac39be633e85ce5076272f9afc12f953839bedc
parent 5383ba84

packages/SystemUI/src/com/android/systemui/qs/external/TileServices.java

+10 −6
Original line number Original line Diff line number Diff line
@@ -132,9 +132,8 @@ public class TileServices extends IQSService.Stub { 
            mServices.remove(tile);

            mServices.remove(tile);

            mTokenMap.remove(service.getToken());

            mTokenMap.remove(service.getToken());

            mTiles.remove(tile.getComponent());

            mTiles.remove(tile.getComponent());

            final String slot = tile.getComponent().getClassName();

            final String slot = getStatusBarIconSlotName(tile.getComponent());

            // TileServices doesn't know how to add more than 1 icon per slot, so remove all

            mMainHandler.post(() -> mStatusBarIconController.removeIconForTile(slot));

            mMainHandler.post(() -> mStatusBarIconController.removeAllIconsForSlot(slot));

        }

        }

    }

    }
@@ -308,12 +307,11 @@ public class TileServices extends IQSService.Stub { 
                            ? new StatusBarIcon(userHandle, packageName, icon, 0, 0,

                            ? new StatusBarIcon(userHandle, packageName, icon, 0, 0,

                                    contentDescription)

                                    contentDescription)

                            : null;

                            : null;

                    final String slot = getStatusBarIconSlotName(componentName);

                    mMainHandler.post(new Runnable() {

                    mMainHandler.post(new Runnable() {

                        @Override

                        @Override

                        public void run() {

                        public void run() {

                            StatusBarIconController iconController = mStatusBarIconController;

                            mStatusBarIconController.setIconFromTile(slot, statusIcon);

                            iconController.setIcon(componentName.getClassName(), statusIcon);

                            iconController.setExternalIcon(componentName.getClassName());

                        }

                        }

                    });

                    });

                }

                }
@@ -373,6 +371,12 @@ public class TileServices extends IQSService.Stub { 
        mCommandQueue.removeCallback(mRequestListeningCallback);

        mCommandQueue.removeCallback(mRequestListeningCallback);

    }

    }





    /** Returns the slot name that should be used when adding or removing status bar icons. */

    private String getStatusBarIconSlotName(ComponentName componentName) {

        return componentName.getClassName();

    }





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

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

        @Override

        @Override

        public void requestTileServiceListeningState(@NonNull ComponentName componentName) {

        public void requestTileServiceListeningState(@NonNull ComponentName componentName) {

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

+9 −27
Original line number Original line Diff line number Diff line
@@ -81,28 +81,22 @@ public interface StatusBarIconController { 
    void refreshIconGroup(IconManager iconManager);

    void refreshIconGroup(IconManager iconManager);





    /**

    /**

     * Adds or updates an icon for a given slot for a **tile service icon**.

     * Adds or updates an icon that comes from an active tile service.

     *

     *

     * TODO(b/265307726): Merge with {@link #setIcon(String, StatusBarIcon)} or make this method

     * If the icon is null, the icon will be removed.

     *   much more clearly distinct from that method.

     */

     */

    void setExternalIcon(String slot);

    void setIconFromTile(String slot, @Nullable StatusBarIcon icon);



    /** Removes an icon that had come from an active tile service. */

    void removeIconForTile(String slot);





    /**

    /**

     * Adds or updates an icon for the given slot for **internal system icons**.

     * Adds or updates an icon for the given slot for **internal system icons**.

     *

     *

     * TODO(b/265307726): Rename to `setInternalIcon`, or merge this appropriately with the

     * TODO(b/265307726): Re-name this to `setInternalIcon`.

     * {@link #setIcon(String, StatusBarIcon)} method.

     */

     */

    void setIcon(String slot, int resourceId, CharSequence contentDescription);

    void setIcon(String slot, int resourceId, CharSequence contentDescription);





    /**

     * Adds or updates an icon for the given slot for an **externally-provided icon**.

     *

     * TODO(b/265307726): Rename to `setExternalIcon` or something similar.

     */

    void setIcon(String slot, StatusBarIcon icon);



    /** */

    /** */

    void setWifiIcon(String slot, WifiIconState state);

    void setWifiIcon(String slot, WifiIconState state);
@@ -152,15 +146,10 @@ public interface StatusBarIconController { 
     */

     */

    void removeIcon(String slot, int tag);

    void removeIcon(String slot, int tag);





    /** */

    void removeAllIconsForSlot(String slot);



    /**

    /**

     * Removes all the icons for the given slot.

     * TODO(b/265307726): Re-name this to `removeAllIconsForInternalSlot`.

     *

     * Only use this for icons that have come from **an external process**.

     */

     */

    void removeAllIconsForExternalSlot(String slot);

    void removeAllIconsForSlot(String slot);





    // TODO: See if we can rename this tunable name.

    // TODO: See if we can rename this tunable name.

    String ICON_HIDE_LIST = "icon_blacklist";

    String ICON_HIDE_LIST = "icon_blacklist";
@@ -618,13 +607,6 @@ public interface StatusBarIconController { 
            mGroup.removeAllViews();

            mGroup.removeAllViews();

        }

        }





        protected void onIconExternal(int viewIndex, int height) {

            ImageView imageView = (ImageView) mGroup.getChildAt(viewIndex);

            imageView.setScaleType(ImageView.ScaleType.FIT_CENTER);

            imageView.setAdjustViewBounds(true);

            setHeightAndCenter(imageView, height);

        }



        protected void onDensityOrFontScaleChanged() {

        protected void onDensityOrFontScaleChanged() {

            for (int i = 0; i < mGroup.getChildCount(); i++) {

            for (int i = 0; i < mGroup.getChildCount(); i++) {

                View child = mGroup.getChildAt(i);

                View child = mGroup.getChildAt(i);

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

+20 −29
Original line number Original line Diff line number Diff line
@@ -32,7 +32,6 @@ import androidx.annotation.VisibleForTesting; 




import com.android.internal.statusbar.StatusBarIcon;

import com.android.internal.statusbar.StatusBarIcon;

import com.android.systemui.Dumpable;

import com.android.systemui.Dumpable;

import com.android.systemui.R;

import com.android.systemui.dagger.SysUISingleton;

import com.android.systemui.dagger.SysUISingleton;

import com.android.systemui.demomode.DemoMode;

import com.android.systemui.demomode.DemoMode;

import com.android.systemui.demomode.DemoModeController;

import com.android.systemui.demomode.DemoModeController;
@@ -350,45 +349,38 @@ public class StatusBarIconControllerImpl implements Tunable, 
        }

        }

    }

    }





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

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

    // different.

    @Override

    public void setExternalIcon(String slot) {

        String slotName = createExternalSlotName(slot);

        int viewIndex = mStatusBarIconList.getViewIndex(slotName, 0);

        int height = mContext.getResources().getDimensionPixelSize(

                R.dimen.status_bar_icon_drawing_size);

        mIconGroups.forEach(l -> l.onIconExternal(viewIndex, height));

    }



        @Override

        @Override

        public void setIcon(String slot, StatusBarIcon icon) {

        public void setIcon(String slot, StatusBarIcon icon) {

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

            setExternalIcon(slot, icon);

            setExternalIcon(slot, icon);

        }

        }





    private void setExternalIcon(String slot, StatusBarIcon icon) {

        @Override

        String slotName = createExternalSlotName(slot);

        public void removeIcon(String slot) {

        if (icon == null) {

            removeAllIconsForExternalSlot(slot);

            removeAllIconsForSlot(slotName);

            return;

        }



        StatusBarIconHolder holder = StatusBarIconHolder.fromIcon(icon);

        setIcon(slotName, holder);

        }

        }

    };





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

    @Override

    @Override

        public void setIcon(String slot, StatusBarIcon icon) {

    public void setIconFromTile(String slot, StatusBarIcon icon) {

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

        setExternalIcon(slot, icon);

        setExternalIcon(slot, icon);

    }

    }





    @Override

    @Override

        public void removeIcon(String slot) {

    public void removeIconForTile(String slot) {

        removeAllIconsForExternalSlot(slot);

        removeAllIconsForExternalSlot(slot);

    }

    }

    };



    private void setExternalIcon(String slot, StatusBarIcon icon) {

        if (icon == null) {

            removeAllIconsForExternalSlot(slot);

            return;

        }

        String slotName = createExternalSlotName(slot);

        StatusBarIconHolder holder = StatusBarIconHolder.fromIcon(icon);

        setIcon(slotName, holder);

    }





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

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

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

        boolean isNew = mStatusBarIconList.getIconHolder(slot, holder.getTag()) == null;
@@ -452,8 +444,7 @@ public class StatusBarIconControllerImpl implements Tunable, 
        mIconGroups.forEach(l -> l.onRemoveIcon(viewIndex));

        mIconGroups.forEach(l -> l.onRemoveIcon(viewIndex));

    }

    }





    @Override

    private void removeAllIconsForExternalSlot(String slotName) {

    public void removeAllIconsForExternalSlot(String slotName) {

        removeAllIconsForSlot(createExternalSlotName(slotName));

        removeAllIconsForSlot(createExternalSlotName(slotName));

    }

    }

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

+20 −20
Original line number Original line Diff line number Diff line
@@ -66,7 +66,7 @@ class StatusBarIconControllerImplTest : SysuiTestCase() { 




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

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

    @Test

    @Test

    fun internalAndExternalIconWithSameName_bothDisplayed() {

    fun internalAndExternalIconWithSameName_externalFromTile_bothDisplayed() {

        val slotName = "mute"

        val slotName = "mute"





        // Internal

        // Internal
@@ -82,7 +82,7 @@ class StatusBarIconControllerImplTest : SysuiTestCase() { 
                /* number= */ 0,

                /* number= */ 0,

                "contentDescription",

                "contentDescription",

            )

            )

        underTest.setIcon(slotName, externalIcon)

        underTest.setIconFromTile(slotName, externalIcon)





        assertThat(iconList.slots).hasSize(2)

        assertThat(iconList.slots).hasSize(2)

        // Whichever was added last comes first

        // Whichever was added last comes first
@@ -148,17 +148,17 @@ class StatusBarIconControllerImplTest : SysuiTestCase() { 




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

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

    @Test

    @Test

    fun internalAndExternalIconWithSameName_externalRemoved_viaRemoveAll_internalStays() {

    fun internalAndExternalIconWithSameName_externalRemoved_fromTileRemove_internalStays() {

        val slotName = "mute"

        val slotName = "mute"





        // Internal

        // Internal

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

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





        // External

        // External

        underTest.setIcon(slotName, createExternalIcon())

        underTest.setIconFromTile(slotName, createExternalIcon())





        // WHEN the external icon is removed via #removeAllIconsForExternalSlot

        // WHEN the external icon is removed via #removeIconForTile

        underTest.removeAllIconsForExternalSlot(slotName)

        underTest.removeIconForTile(slotName)





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

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

        assertThat(iconList.slots).hasSize(2)

        assertThat(iconList.slots).hasSize(2)
@@ -172,17 +172,17 @@ class StatusBarIconControllerImplTest : SysuiTestCase() { 




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

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

    @Test

    @Test

    fun internalAndExternalIconWithSameName_externalRemoved_viaSetNull_internalStays() {

    fun internalAndExternalIconWithSameName_externalRemoved_fromTileSetNull_internalStays() {

        val slotName = "mute"

        val slotName = "mute"





        // Internal

        // Internal

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

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





        // External

        // External

        underTest.setIcon(slotName, createExternalIcon())

        underTest.setIconFromTile(slotName, createExternalIcon())





        // WHEN the external icon is removed via a #setIcon(null)

        // WHEN the external icon is removed via a #setIconFromTile(null)

        underTest.setIcon(slotName, /* icon= */ null)

        underTest.setIconFromTile(slotName, /* icon= */ null)





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

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

        assertThat(iconList.slots).hasSize(2)

        assertThat(iconList.slots).hasSize(2)
@@ -203,12 +203,12 @@ class StatusBarIconControllerImplTest : SysuiTestCase() { 
        underTest.setIcon(slotName, /* resourceId= */ 10, "contentDescription")

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





        // External

        // External

        underTest.setIcon(slotName, createExternalIcon())

        underTest.setIconFromTile(slotName, createExternalIcon())





        // WHEN the internal icon is removed via #removeIcon

        // WHEN the internal icon is removed via #removeIcon

        underTest.removeIcon(slotName, /* tag= */ 0)

        underTest.removeIcon(slotName, /* tag= */ 0)





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

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

        assertThat(iconList.slots).hasSize(2)

        assertThat(iconList.slots).hasSize(2)

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

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

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

        assertThat(iconList.slots[1].name).isEqualTo(slotName)
@@ -227,12 +227,12 @@ class StatusBarIconControllerImplTest : SysuiTestCase() { 
        underTest.setIcon(slotName, /* resourceId= */ 10, "contentDescription")

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





        // External

        // External

        underTest.setIcon(slotName, createExternalIcon())

        underTest.setIconFromTile(slotName, createExternalIcon())





        // WHEN the internal icon is removed via #removeAllIconsForSlot

        // WHEN the internal icon is removed via #removeAllIconsForSlot

        underTest.removeAllIconsForSlot(slotName)

        underTest.removeAllIconsForSlot(slotName)





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

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

        assertThat(iconList.slots).hasSize(2)

        assertThat(iconList.slots).hasSize(2)

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

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

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

        assertThat(iconList.slots[1].name).isEqualTo(slotName)
@@ -260,7 +260,7 @@ class StatusBarIconControllerImplTest : SysuiTestCase() { 
                /* number= */ 0,

                /* number= */ 0,

                "externalDescription",

                "externalDescription",

            )

            )

        underTest.setIcon(slotName, startingExternalIcon)

        underTest.setIconFromTile(slotName, startingExternalIcon)





        // WHEN the internal icon is updated

        // WHEN the internal icon is updated

        underTest.setIcon(slotName, /* resourceId= */ 11, "newContentDescription")

        underTest.setIcon(slotName, /* resourceId= */ 11, "newContentDescription")
@@ -282,7 +282,7 @@ class StatusBarIconControllerImplTest : SysuiTestCase() { 




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

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

    @Test

    @Test

    fun internalAndExternalIconWithSameName_externalUpdatedIndependently() {

    fun internalAndExternalIconWithSameName_fromTile_externalUpdatedIndependently() {

        val slotName = "mute"

        val slotName = "mute"





        // Internal

        // Internal
@@ -298,7 +298,7 @@ class StatusBarIconControllerImplTest : SysuiTestCase() { 
                /* number= */ 0,

                /* number= */ 0,

                "externalDescription",

                "externalDescription",

            )

            )

        underTest.setIcon(slotName, startingExternalIcon)

        underTest.setIconFromTile(slotName, startingExternalIcon)





        // WHEN the external icon is updated

        // WHEN the external icon is updated

        val newExternalIcon =

        val newExternalIcon =
@@ -310,7 +310,7 @@ class StatusBarIconControllerImplTest : SysuiTestCase() { 
                /* number= */ 0,

                /* number= */ 0,

                "newExternalDescription",

                "newExternalDescription",

            )

            )

        underTest.setIcon(slotName, newExternalIcon)

        underTest.setIconFromTile(slotName, newExternalIcon)





        // THEN only the external slot gets the updates

        // THEN only the external slot gets the updates

        val externalSlot = iconList.slots[0]

        val externalSlot = iconList.slots[0]
@@ -375,8 +375,8 @@ class StatusBarIconControllerImplTest : SysuiTestCase() { 
    }

    }





    @Test

    @Test

    fun externalSlot_alreadyEndsWithSuffix_suffixNotAddedTwice() {

    fun externalSlot_fromTile_alreadyEndsWithSuffix_suffixNotAddedTwice() {

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

        underTest.setIconFromTile("myslot$EXTERNAL_SLOT_SUFFIX", createExternalIcon())





        assertThat(iconList.slots).hasSize(1)

        assertThat(iconList.slots).hasSize(1)

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

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

packages/SystemUI/tests/utils/src/com/android/systemui/utils/leaks/FakeStatusBarIconController.java

+3 −7
Original line number Original line Diff line number Diff line
@@ -47,17 +47,17 @@ public class FakeStatusBarIconController extends BaseLeakChecker<IconManager> 
    }

    }





    @Override

    @Override

    public void setExternalIcon(String slot) {

    public void setIconFromTile(String slot, StatusBarIcon icon) {





    }

    }





    @Override

    @Override

    public void setIcon(String slot, int resourceId, CharSequence contentDescription) {

    public void removeIconForTile(String slot) {





    }

    }





    @Override

    @Override

    public void setIcon(String slot, StatusBarIcon icon) {

    public void setIcon(String slot, int resourceId, CharSequence contentDescription) {





    }

    }
@@ -97,10 +97,6 @@ public class FakeStatusBarIconController extends BaseLeakChecker<IconManager> 
    public void removeAllIconsForSlot(String slot) {

    public void removeAllIconsForSlot(String slot) {

    }

    }





    @Override

    public void removeAllIconsForExternalSlot(String slot) {

    }



    @Override

    @Override

    public void setIconAccessibilityLiveRegion(String slot, int mode) {

    public void setIconAccessibilityLiveRegion(String slot, int mode) {

    }

    }