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

Commit 15984367 authored by Lyn's avatar Lyn
Browse files

Fix ArrayIndexOutOfBoundsException after allowing reorder 

Fixes: 378795050

Test: HeadsUpManagerPhoneTest

Test: send delayed HUN, open shade, close shade to allow reorder
      and removeEntry
      => no exception

Test: send delayed HUN, open shade, manually swipe dismiss HUN to
      removeEntry
      => entry removed from mEntriesToRemoveWhenReorderingAllowed,
         no memory leak

Flag: com.android.systemui.notification_avalanche_throttle_hun
Change-Id: I31e74a524dcf406c2c01b4ea524ff757c7726dc1
parent e60636fe

packages/SystemUI/multivalentTests/src/com/android/systemui/statusbar/policy/BaseHeadsUpManagerTest.java

+1 −1
Original line number Diff line number Diff line
@@ -321,7 +321,7 @@ public class BaseHeadsUpManagerTest extends SysuiTestCase { 
                BaseHeadsUpManager.HeadsUpEntry.class);

        headsUpEntry.mEntry = notifEntry;



        hum.onEntryRemoved(headsUpEntry);

        hum.onEntryRemoved(headsUpEntry, "test");



        verify(mLogger, times(1)).logNotificationActuallyRemoved(eq(notifEntry));

    }

packages/SystemUI/multivalentTests/src/com/android/systemui/statusbar/policy/HeadsUpManagerPhoneTest.kt

+19 −1
Original line number Diff line number Diff line
@@ -82,7 +82,6 @@ class HeadsUpManagerPhoneTest(flags: FlagsParameterization) : BaseHeadsUpManager 
    private val mJavaAdapter: JavaAdapter = JavaAdapter(testScope.backgroundScope)



    @Mock private lateinit var mShadeInteractor: ShadeInteractor



    @Mock private lateinit var dumpManager: DumpManager

    private lateinit var mAvalancheController: AvalancheController
@@ -205,6 +204,25 @@ class HeadsUpManagerPhoneTest(flags: FlagsParameterization) : BaseHeadsUpManager 
        assertThat(hmp.mEntriesToRemoveWhenReorderingAllowed.contains(notifEntry)).isTrue()

    }



    class TestAnimationStateHandler : AnimationStateHandler {

        override fun setHeadsUpGoingAwayAnimationsAllowed(allowed: Boolean) {}

    }



    @Test

    @EnableFlags(NotificationThrottleHun.FLAG_NAME)

    fun testReorderingAllowed_clearsListOfEntriesToRemove() {

        whenever(mVSProvider.isReorderingAllowed).thenReturn(true)

        val hmp = createHeadsUpManagerPhone()



        val notifEntry = HeadsUpManagerTestUtil.createEntry(/* id= */ 0, mContext)

        hmp.showNotification(notifEntry)

        assertThat(hmp.mEntriesToRemoveWhenReorderingAllowed.contains(notifEntry)).isTrue()



        hmp.setAnimationStateHandler(TestAnimationStateHandler())

        hmp.mOnReorderingAllowedListener.onReorderingAllowed()

        assertThat(hmp.mEntriesToRemoveWhenReorderingAllowed.isEmpty()).isTrue()

    }



    @Test

    @EnableFlags(NotificationThrottleHun.FLAG_NAME)

    fun testShowNotification_reorderNotAllowed_seenInShadeTrue() {

packages/SystemUI/src/com/android/systemui/statusbar/policy/BaseHeadsUpManager.java

+18 −9
Original line number Diff line number Diff line
@@ -88,7 +88,7 @@ public class BaseHeadsUpManager 
        implements HeadsUpManager, HeadsUpRepository, OnHeadsUpChangedListener {

    private static final String TAG = "BaseHeadsUpManager";

    private static final String SETTING_HEADS_UP_SNOOZE_LENGTH_MS = "heads_up_snooze_length_ms";



    private static final String REASON_REORDER_ALLOWED = "mOnReorderingAllowedListener";

    protected final ListenerSet<OnHeadsUpChangedListener> mListeners = new ListenerSet<>();



    protected final Context mContext;
@@ -633,7 +633,7 @@ public class BaseHeadsUpManager 
            }

            entry.demoteStickyHun();

            mHeadsUpEntryMap.remove(key);

            onEntryRemoved(finalHeadsUpEntry);

            onEntryRemoved(finalHeadsUpEntry, reason);

            // TODO(b/328390331) move accessibility events to the view layer

            entry.sendAccessibilityEvent(AccessibilityEvent.TYPE_WINDOW_CONTENT_CHANGED);

            if (NotificationThrottleHun.isEnabled()) {
@@ -648,8 +648,9 @@ public class BaseHeadsUpManager 
    /**

     * Manager-specific logic that should occur when an entry is removed.

     * @param headsUpEntry entry removed

     * @param reason why onEntryRemoved was called

     */

    protected void onEntryRemoved(HeadsUpEntry headsUpEntry) {

    protected void onEntryRemoved(HeadsUpEntry headsUpEntry, String reason) {

        NotificationEntry entry = headsUpEntry.mEntry;

        entry.setHeadsUp(false);

        setEntryPinned(headsUpEntry, false /* isPinned */, "onEntryRemoved");
@@ -664,10 +665,17 @@ public class BaseHeadsUpManager 
        updateTopHeadsUpFlow();

        updateHeadsUpFlow();

        if (NotificationThrottleHun.isEnabled()) {

            if (headsUpEntry.mEntry != null) {

                if (mEntriesToRemoveWhenReorderingAllowed.contains(headsUpEntry.mEntry)) {

                    mEntriesToRemoveWhenReorderingAllowed.remove(headsUpEntry.mEntry);

            NotificationEntry notifEntry = headsUpEntry.mEntry;

            if (notifEntry == null) {

                return;

            }

            // If reorder was just allowed and we called onEntryRemoved while iterating over

            // mEntriesToRemoveWhenReorderingAllowed, we should not remove from this list (and cause

            // ArrayIndexOutOfBoundsException). We don't need to in this case anyway, because we

            // clear mEntriesToRemoveWhenReorderingAllowed after removing these entries.

            if (!reason.equals(REASON_REORDER_ALLOWED)

                    && mEntriesToRemoveWhenReorderingAllowed.contains(notifEntry)) {

                mEntriesToRemoveWhenReorderingAllowed.remove(notifEntry);

            }

        }

    }
@@ -1135,7 +1143,8 @@ public class BaseHeadsUpManager 
                && Notification.CATEGORY_CALL.equals(n.category));

    }



    private final OnReorderingAllowedListener mOnReorderingAllowedListener = () -> {

    @VisibleForTesting

    public final OnReorderingAllowedListener mOnReorderingAllowedListener = () -> {

        if (NotificationThrottleHun.isEnabled()) {

            mAvalancheController.setEnableAtRuntime(true);

            if (mEntriesToRemoveWhenReorderingAllowed.isEmpty()) {
@@ -1146,7 +1155,7 @@ public class BaseHeadsUpManager 
        for (NotificationEntry entry : mEntriesToRemoveWhenReorderingAllowed) {

            if (entry != null && isHeadsUpEntry(entry.getKey())) {

                // Maybe the heads-up was removed already

                removeEntry(entry.getKey(), "mOnReorderingAllowedListener");

                removeEntry(entry.getKey(), REASON_REORDER_ALLOWED);

            }

        }

        mEntriesToRemoveWhenReorderingAllowed.clear();