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

Commit f24796c6 authored by Lyn's avatar Lyn
Browse files

Fix status bar app name/icon overlap 

This issue was caused by a missing check for if there is a
waiting entry for the given key in BaseHeadsUpManager.removeEntry

Also add more stack call tracing to HeadsUpManagerLogger to
improve debugging

Fixes: 340334234
Test: manual with test app:
      send default notifs to get notif icons in status bar
      tap add 3 times fast
      during the initial appear animation, tap cancel all
      => see that app name is replaced by app icons
Change-Id: I5d7a59e83cf98d96297d40750ed7d8243b098709
parent b3c6e602

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

+7 −7
Original line number Diff line number Diff line
@@ -159,7 +159,7 @@ public class HeadsUpAppearanceController extends ViewController<HeadsUpStatusBar 
            public void onLayoutChange(View v, int left, int top, int right, int bottom,

                    int oldLeft, int oldTop, int oldRight, int oldBottom) {

                if (shouldBeVisible()) {

                    updateTopEntry();

                    updateTopEntry("onLayoutChange");



                    // trigger scroller to notify the latest panel translation

                    mStackScrollerController.requestLayout();
@@ -220,7 +220,7 @@ public class HeadsUpAppearanceController extends ViewController<HeadsUpStatusBar 


    @Override

    public void onHeadsUpPinned(NotificationEntry entry) {

        updateTopEntry();

        updateTopEntry("onHeadsUpPinned");

        updateHeader(entry);

        updateHeadsUpAndPulsingRoundness(entry);

    }
@@ -231,7 +231,7 @@ public class HeadsUpAppearanceController extends ViewController<HeadsUpStatusBar 
        mPhoneStatusBarTransitions.onHeadsUpStateChanged(isHeadsUp);

    }



    private void updateTopEntry() {

    private void updateTopEntry(String reason) {

        NotificationEntry newEntry = null;

        if (shouldBeVisible()) {

            newEntry = mHeadsUpManager.getTopEntry();
@@ -369,7 +369,7 @@ public class HeadsUpAppearanceController extends ViewController<HeadsUpStatusBar 


    @Override

    public void onHeadsUpUnPinned(NotificationEntry entry) {

        updateTopEntry();

        updateTopEntry("onHeadsUpUnPinned");

        updateHeader(entry);

        updateHeadsUpAndPulsingRoundness(entry);

    }
@@ -387,7 +387,7 @@ public class HeadsUpAppearanceController extends ViewController<HeadsUpStatusBar 
            updateHeadsUpHeaders();

        }

        if (isExpanded() != oldIsExpanded) {

            updateTopEntry();

            updateTopEntry("setAppearFraction");

        }

    }
@@ -461,11 +461,11 @@ public class HeadsUpAppearanceController extends ViewController<HeadsUpStatusBar 
    }



    public void onStateChanged() {

        updateTopEntry();

        updateTopEntry("onStateChanged");

    }



    @Override

    public void onFullyHiddenChanged(boolean isFullyHidden) {

        updateTopEntry();

        updateTopEntry("onFullyHiddenChanged");

    }

}

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

+4 −4
Original line number Diff line number Diff line
@@ -249,7 +249,7 @@ public class HeadsUpManagerPhone extends BaseHeadsUpManager implements 
            for (NotificationEntry entry : mEntriesToRemoveAfterExpand) {

                if (isHeadsUpEntry(entry.getKey())) {

                    // Maybe the heads-up was removed already

                    removeEntry(entry.getKey());

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

                }

            }

        }
@@ -381,7 +381,7 @@ public class HeadsUpManagerPhone extends BaseHeadsUpManager implements 
        for (NotificationEntry entry : mEntriesToRemoveWhenReorderingAllowed) {

            if (isHeadsUpEntry(entry.getKey())) {

                // Maybe the heads-up was removed already

                removeEntry(entry.getKey());

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

            }

        }

        mEntriesToRemoveWhenReorderingAllowed.clear();
@@ -572,7 +572,7 @@ public class HeadsUpManagerPhone extends BaseHeadsUpManager implements 
                } else if (mTrackingHeadsUp) {

                    mEntriesToRemoveAfterExpand.add(entry);

                } else {

                    removeEntry(entry.getKey());

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

                }

            };

        }
@@ -661,7 +661,7 @@ public class HeadsUpManagerPhone extends BaseHeadsUpManager implements 
                    }

                }

                for (String key : keysToRemove) {

                    removeEntry(key);

                    removeEntry(key, "mStatusBarStateListener");

                }

            }

        }

packages/SystemUI/src/com/android/systemui/statusbar/policy/AvalancheController.kt

+25 −10
Original line number Diff line number Diff line
@@ -79,7 +79,8 @@ constructor( 
            runnable.run()

            return

        }

        val fn = "[$label] => AvalancheController.update [${getKey(entry)}]"

        log { "\n "}

        val fn = "$label => AvalancheController.update ${getKey(entry)}"

        if (entry == null) {

            log { "Entry is NULL, stop update." }

            return
@@ -88,13 +89,13 @@ constructor( 
            debugRunnableLabelMap[runnable] = label

        }

        if (isShowing(entry)) {

            log { "\n$fn => [update showing]" }

            log { "\n$fn => update showing" }

            runnable.run()

        } else if (entry in nextMap) {

            log { "\n$fn => [update next]" }

            log { "\n$fn => update next" }

            nextMap[entry]?.add(runnable)

        } else if (headsUpEntryShowing == null) {

            log { "\n$fn => [showNow]" }

            log { "\n$fn => showNow" }

            showNow(entry, arrayListOf(runnable))

        } else {

            // Clean up invalid state when entry is in list but not map and vice versa
@@ -133,20 +134,22 @@ constructor( 
            runnable.run()

            return

        }

        val fn = "[$label] => AvalancheController.delete " + getKey(entry)

        log { "\n "}

        val fn = "$label => AvalancheController.delete " + getKey(entry)

        if (entry == null) {

            log { "$fn => cannot remove NULL entry" }

            log { "$fn => entry NULL, running runnable" }

            runnable.run()

            return

        }

        if (entry in nextMap) {

            log { "$fn => [remove from next]" }

            log { "$fn => remove from next" }

            if (entry in nextMap) nextMap.remove(entry)

            if (entry in nextList) nextList.remove(entry)

        } else if (entry in debugDropSet) {

            log { "$fn => [remove from dropset]" }

            log { "$fn => remove from dropset" }

            debugDropSet.remove(entry)

        } else if (isShowing(entry)) {

            log { "$fn => [remove showing ${getKey(entry)}]" }

            log { "$fn => remove showing ${getKey(entry)}" }

            previousHunKey = getKey(headsUpEntryShowing)

            // Show the next HUN before removing this one, so that we don't tell listeners

            // onHeadsUpPinnedModeChanged, which causes
@@ -155,7 +158,7 @@ constructor( 
            showNext()

            runnable.run()

        } else {

            log { "$fn => [removing untracked ${getKey(entry)}]" }

            log { "$fn => removing untracked ${getKey(entry)}" }

        }

        logState("after $fn")

    }
@@ -239,6 +242,18 @@ constructor( 
        return keyList

    }



    fun getWaitingEntry(key: String): HeadsUpEntry? {

        if (!NotificationThrottleHun.isEnabled) {

            return null

        }

        for (headsUpEntry in nextMap.keys) {

            if (headsUpEntry.mEntry?.key.equals(key)) {

                return headsUpEntry

            }

        }

        return null

    }



    private fun isShowing(entry: HeadsUpEntry): Boolean {

        return headsUpEntryShowing != null && entry.mEntry?.key == headsUpEntryShowing?.mEntry?.key

    }

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

+39 −27
Original line number Diff line number Diff line
@@ -195,25 +195,28 @@ public abstract class BaseHeadsUpManager implements HeadsUpManager { 
     */

    @Override

    public boolean removeNotification(@NonNull String key, boolean releaseImmediately) {

        mLogger.logRemoveNotification(key, releaseImmediately);

        final boolean isWaiting = mAvalancheController.isWaiting(key);

        mLogger.logRemoveNotification(key, releaseImmediately, isWaiting);



        if (mAvalancheController.isWaiting(key)) {

            removeEntry(key);

            removeEntry(key, "removeNotification (isWaiting)");

            return true;

        }

        HeadsUpEntry headsUpEntry = mHeadsUpEntryMap.get(key);

        if (headsUpEntry == null) {

            return true;

        }

        if (releaseImmediately || canRemoveImmediately(key)) {

            removeEntry(key);

        } else {

            headsUpEntry.removeAsSoonAsPossible();

            return false;

        if (releaseImmediately) {

            removeEntry(key, "removeNotification (releaseImmediately)");

            return true;

        }

        if (canRemoveImmediately(key)) {

            removeEntry(key, "removeNotification (canRemoveImmediately)");

            return true;

        }



        headsUpEntry.removeAsSoonAsPossible();

        return false;

    }



    /**

     * Called when the notification state has been updated.
@@ -245,7 +248,8 @@ public abstract class BaseHeadsUpManager implements HeadsUpManager { 
        if (shouldHeadsUpAgain) {

            headsUpEntry.updateEntry(true /* updatePostTime */, "updateNotification");

            if (headsUpEntry != null) {

                setEntryPinned(headsUpEntry, shouldHeadsUpBecomePinned(headsUpEntry.mEntry));

                setEntryPinned(headsUpEntry, shouldHeadsUpBecomePinned(headsUpEntry.mEntry),

                        "updateNotificationInternal");

            }

        }

    }
@@ -264,10 +268,10 @@ public abstract class BaseHeadsUpManager implements HeadsUpManager { 
        List<String> waitingKeysToRemove = mAvalancheController.getWaitingKeys();



        for (String key : keysToRemove) {

            removeEntry(key);

            removeEntry(key, "releaseAllImmediately (keysToRemove)");

        }

        for (String key : waitingKeysToRemove) {

            removeEntry(key);

            removeEntry(key, "releaseAllImmediately (waitingKeysToRemove)");

        }

    }
@@ -338,8 +342,9 @@ public abstract class BaseHeadsUpManager implements HeadsUpManager { 
    }



    protected void setEntryPinned(

            @NonNull BaseHeadsUpManager.HeadsUpEntry headsUpEntry, boolean isPinned) {

        mLogger.logSetEntryPinned(headsUpEntry.mEntry, isPinned);

            @NonNull BaseHeadsUpManager.HeadsUpEntry headsUpEntry, boolean isPinned,

            String reason) {

        mLogger.logSetEntryPinned(headsUpEntry.mEntry, isPinned, reason);

        NotificationEntry entry = headsUpEntry.mEntry;

        if (!isPinned) {

            headsUpEntry.mWasUnpinned = true;
@@ -376,7 +381,7 @@ public abstract class BaseHeadsUpManager implements HeadsUpManager { 
        entry.setHeadsUp(true);



        final boolean shouldPin = shouldHeadsUpBecomePinned(entry);

        setEntryPinned(headsUpEntry, shouldPin);

        setEntryPinned(headsUpEntry, shouldPin, "onEntryAdded");

        EventLogTags.writeSysuiHeadsUpStatus(entry.getKey(), 1 /* visible */);

        for (OnHeadsUpChangedListener listener : mListeners) {

            listener.onHeadsUpStateChanged(entry, true);
@@ -387,17 +392,24 @@ public abstract class BaseHeadsUpManager implements HeadsUpManager { 
     * Remove a notification from the alerting entries.

     * @param key key of notification to remove

     */

    protected final void removeEntry(@NonNull String key) {

    protected final void removeEntry(@NonNull String key, String reason) {

        HeadsUpEntry headsUpEntry = mHeadsUpEntryMap.get(key);

        mLogger.logRemoveEntryRequest(key);



        boolean isWaiting;

        if (headsUpEntry == null) {

            headsUpEntry = mAvalancheController.getWaitingEntry(key);

            isWaiting = true;

        } else {

            isWaiting = false;

        }

        mLogger.logRemoveEntryRequest(key, reason, isWaiting);

        HeadsUpEntry finalHeadsUpEntry = headsUpEntry;

        Runnable runnable = () -> {

            mLogger.logRemoveEntry(key);

            mLogger.logRemoveEntry(key, reason, isWaiting);



            if (headsUpEntry == null) {

            if (finalHeadsUpEntry == null) {

                return;

            }

            NotificationEntry entry = headsUpEntry.mEntry;

            NotificationEntry entry = finalHeadsUpEntry.mEntry;



            // If the notification is animating, we will remove it at the end of the animation.

            if (entry != null && entry.isExpandAnimationRunning()) {
@@ -405,13 +417,13 @@ public abstract class BaseHeadsUpManager implements HeadsUpManager { 
            }

            entry.demoteStickyHun();

            mHeadsUpEntryMap.remove(key);

            onEntryRemoved(headsUpEntry);

            onEntryRemoved(finalHeadsUpEntry);

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

            entry.sendAccessibilityEvent(AccessibilityEvent.TYPE_WINDOW_CONTENT_CHANGED);

            if (NotificationsHeadsUpRefactor.isEnabled()) {

                headsUpEntry.cancelAutoRemovalCallbacks("removeEntry");

                finalHeadsUpEntry.cancelAutoRemovalCallbacks("removeEntry");

            } else {

                headsUpEntry.reset();

                finalHeadsUpEntry.reset();

            }

        };

        mAvalancheController.delete(headsUpEntry, runnable, "removeEntry");
@@ -424,7 +436,7 @@ public abstract class BaseHeadsUpManager implements HeadsUpManager { 
    protected void onEntryRemoved(HeadsUpEntry headsUpEntry) {

        NotificationEntry entry = headsUpEntry.mEntry;

        entry.setHeadsUp(false);

        setEntryPinned(headsUpEntry, false /* isPinned */);

        setEntryPinned(headsUpEntry, false /* isPinned */, "onEntryRemoved");

        EventLogTags.writeSysuiHeadsUpStatus(entry.getKey(), 0 /* visible */);

        mLogger.logNotificationActuallyRemoved(entry);

        for (OnHeadsUpChangedListener listener : mListeners) {
@@ -584,7 +596,7 @@ public abstract class BaseHeadsUpManager implements HeadsUpManager { 
            Runnable runnable = () -> {

                mLogger.logUnpinEntry(key);



                setEntryPinned(headsUpEntry, false /* isPinned */);

                setEntryPinned(headsUpEntry, false /* isPinned */, "unpinAll");

                // maybe it got un sticky

                headsUpEntry.updateEntry(false /* updatePostTime */, "unpinAll");
@@ -985,7 +997,7 @@ public abstract class BaseHeadsUpManager implements HeadsUpManager { 


        /** Creates a runnable to remove this notification from the alerting entries. */

        protected Runnable createRemoveRunnable(NotificationEntry entry) {

            return () -> removeEntry(entry.getKey());

            return () -> removeEntry(entry.getKey(), "createRemoveRunnable");

        }



        /**

packages/SystemUI/src/com/android/systemui/statusbar/policy/HeadsUpManagerLogger.kt

+14 −8
Original line number Diff line number Diff line
@@ -121,19 +121,23 @@ class HeadsUpManagerLogger @Inject constructor( 
        })

    }



    fun logRemoveEntryRequest(key: String) {

    fun logRemoveEntryRequest(key: String, reason: String, isWaiting: Boolean) {

        buffer.log(TAG, INFO, {

            str1 = logKey(key)

            str2 = reason

            bool1 = isWaiting

        }, {

            "request: remove entry $str1"

            "request: $str2 => remove entry $str1 isWaiting: $isWaiting"

        })

    }



    fun logRemoveEntry(key: String) {

    fun logRemoveEntry(key: String, reason: String, isWaiting: Boolean) {

        buffer.log(TAG, INFO, {

            str1 = logKey(key)

            str2 = reason

            bool1 = isWaiting

        }, {

            "remove entry $str1"

            "$str2 => remove entry $str1 isWaiting: $isWaiting"

        })

    }
@@ -153,12 +157,13 @@ class HeadsUpManagerLogger @Inject constructor( 
        })

    }



    fun logRemoveNotification(key: String, releaseImmediately: Boolean) {

    fun logRemoveNotification(key: String, releaseImmediately: Boolean, isWaiting: Boolean) {

        buffer.log(TAG, INFO, {

            str1 = logKey(key)

            bool1 = releaseImmediately

            bool2 = isWaiting

        }, {

            "remove notification $str1 releaseImmediately: $bool1"

            "remove notification $str1 releaseImmediately: $bool1 isWaiting: $bool2"

        })

    }
@@ -208,12 +213,13 @@ class HeadsUpManagerLogger @Inject constructor( 
        })

    }



    fun logSetEntryPinned(entry: NotificationEntry, isPinned: Boolean) {

    fun logSetEntryPinned(entry: NotificationEntry, isPinned: Boolean, reason: String) {

        buffer.log(TAG, VERBOSE, {

            str1 = entry.logKey

            bool1 = isPinned

            str2 = reason

        }, {

            "set entry pinned $str1 pinned: $bool1"

            "$str2 => set entry pinned $str1 pinned: $bool1"

        })

    }