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

Commit ead153f8 authored by Lyn's avatar Lyn
Browse files

Fix missing grouping after shade closes

Log BaseHeadsUpEntry map

Make logs more readable

Fixes: 367403822
Fixes: 356516285

Test: HeadsUpManagerPhoneTest

Test: send message HUN from two contacts from same app
      open shade, close shade
      => see that HUNs are grouped

Test: send delayed HUN, open shade, close shade before delayed HUN
      times out
      => see no disappear animation (no regression)

Test: adb shell cmd statusbar echo -b NotifHeadsUpLog:verbose
      adb reboot
      adb logcat | grep HeadsUpManager

Flag: com.android.systemui.notification_avalanche_throttle_hun
Change-Id: I88a8f76e63a82fa0473f18b2d5384bcf39209f27
parent de068cc7
Loading
Loading
Loading
Loading
+16 −12
Original line number Diff line number Diff line
@@ -47,6 +47,7 @@ import com.android.systemui.util.kotlin.JavaAdapter
import com.android.systemui.util.mockito.mock
import com.android.systemui.util.settings.GlobalSettings
import com.android.systemui.util.time.SystemClock
import com.google.common.truth.Truth.assertThat
import junit.framework.Assert
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.flow.MutableStateFlow
@@ -247,32 +248,35 @@ class HeadsUpManagerPhoneTest(flags: FlagsParameterization) : BaseHeadsUpManager

    @Test
    @EnableFlags(NotificationThrottleHun.FLAG_NAME)
    fun testShowNotification_reorderNotAllowed_notPulsing_seenInShadeTrue() {
        whenever(mVSProvider.isReorderingAllowed).thenReturn(false)
    fun testShowNotification_removeWhenReorderingAllowedTrue() {
        whenever(mVSProvider.isReorderingAllowed).thenReturn(true)
        val hmp = createHeadsUpManagerPhone()

        val notifEntry = HeadsUpManagerTestUtil.createEntry(/* id= */ 0, mContext)
        val row = mock<ExpandableNotificationRow>()
        whenever(row.showingPulsing()).thenReturn(false)
        notifEntry.row = row
        hmp.showNotification(notifEntry)
        assertThat(hmp.mEntriesToRemoveWhenReorderingAllowed.contains(notifEntry)).isTrue();
    }

    @Test
    @EnableFlags(NotificationThrottleHun.FLAG_NAME)
    fun testShowNotification_reorderNotAllowed_seenInShadeTrue() {
        whenever(mVSProvider.isReorderingAllowed).thenReturn(false)
        val hmp = createHeadsUpManagerPhone()

        val notifEntry = HeadsUpManagerTestUtil.createEntry(/* id= */ 0, mContext)
        hmp.showNotification(notifEntry)
        Assert.assertTrue(notifEntry.isSeenInShade)
        assertThat(notifEntry.isSeenInShade).isTrue();
    }

    @Test
    @EnableFlags(NotificationThrottleHun.FLAG_NAME)
    fun testShowNotification_reorderAllowed_notPulsing_seenInShadeFalse() {
    fun testShowNotification_reorderAllowed_seenInShadeFalse() {
        whenever(mVSProvider.isReorderingAllowed).thenReturn(true)
        val hmp = createHeadsUpManagerPhone()

        val notifEntry = HeadsUpManagerTestUtil.createEntry(/* id= */ 0, mContext)
        val row = mock<ExpandableNotificationRow>()
        whenever(row.showingPulsing()).thenReturn(false)
        notifEntry.row = row

        hmp.showNotification(notifEntry)
        Assert.assertFalse(notifEntry.isSeenInShade)
        assertThat(notifEntry.isSeenInShade).isFalse();
    }

    @Test
+5 −7
Original line number Diff line number Diff line
@@ -103,7 +103,8 @@ public class HeadsUpManagerPhone extends BaseHeadsUpManager implements
    private boolean mTrackingHeadsUp;
    private final HashSet<String> mSwipedOutKeys = new HashSet<>();
    private final HashSet<NotificationEntry> mEntriesToRemoveAfterExpand = new HashSet<>();
    private final ArraySet<NotificationEntry> mEntriesToRemoveWhenReorderingAllowed
    @VisibleForTesting
    public final ArraySet<NotificationEntry> mEntriesToRemoveWhenReorderingAllowed
            = new ArraySet<>();
    private boolean mIsExpanded;
    private int mStatusBarState;
@@ -417,7 +418,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(), "mOnReorderingAllowedListener");
                removeEntry(entry.getKey(), "allowReorder");
            }
        }
        mEntriesToRemoveWhenReorderingAllowed.clear();
@@ -617,11 +618,8 @@ public class HeadsUpManagerPhone extends BaseHeadsUpManager implements
            super.setEntry(entry, removeRunnable);

            if (NotificationThrottleHun.isEnabled()) {
                if (!mVisualStabilityProvider.isReorderingAllowed()
                        // We don't want to allow reordering while pulsing, but headsup need to
                        // time out anyway
                        && !entry.showingPulsing()) {
                mEntriesToRemoveWhenReorderingAllowed.add(entry);
                if (!mVisualStabilityProvider.isReorderingAllowed()) {
                    entry.setSeenInShade(true);
                }
            }
+48 −23
Original line number Diff line number Diff line
@@ -46,6 +46,8 @@ constructor(

    private val tag = "AvalancheController"
    private val debug = Compile.IS_DEBUG && Log.isLoggable(tag, Log.DEBUG)
    var baseEntryMapStr : () -> String = { "baseEntryMapStr not initialized" }

    var enableAtRuntime = true
        set(value) {
            if (!value) {
@@ -116,32 +118,43 @@ constructor(
        val key = getKey(entry)

        if (runnable == null) {
            headsUpManagerLogger.logAvalancheUpdate(caller, isEnabled, key, "Runnable NULL, stop")
            headsUpManagerLogger.logAvalancheUpdate(
                caller, isEnabled, key,
                "Runnable NULL, stop. ${getStateStr()}"
            )
            return
        }
        if (!isEnabled) {
            headsUpManagerLogger.logAvalancheUpdate(caller, isEnabled, key,
                    "NOT ENABLED, run runnable")
            headsUpManagerLogger.logAvalancheUpdate(
                caller, isEnabled, key,
                "NOT ENABLED, run runnable. ${getStateStr()}"
            )
            runnable.run()
            return
        }
        if (entry == null) {
            headsUpManagerLogger.logAvalancheUpdate(caller, isEnabled, key, "Entry NULL, stop")
            headsUpManagerLogger.logAvalancheUpdate(
                caller, isEnabled, key,
                "Entry NULL, stop. ${getStateStr()}"
            )
            return
        }
        if (debug) {
            debugRunnableLabelMap[runnable] = caller
        }
        var outcome = ""
        var stateAfter = ""
        if (isShowing(entry)) {
            outcome = "update showing"
            runnable.run()
            stateAfter = "update showing"

        } else if (entry in nextMap) {
            outcome = "update next"
            nextMap[entry]?.add(runnable)
            stateAfter = "update next"

        } else if (headsUpEntryShowing == null) {
            outcome = "show now"
            showNow(entry, arrayListOf(runnable))
            stateAfter = "show now"

        } else {
            // Clean up invalid state when entry is in list but not map and vice versa
            if (entry in nextMap) nextMap.remove(entry)
@@ -162,8 +175,8 @@ constructor(
                )
            }
        }
        outcome += getStateStr()
        headsUpManagerLogger.logAvalancheUpdate(caller, isEnabled, key, outcome)
        stateAfter += getStateStr()
        headsUpManagerLogger.logAvalancheUpdate(caller, isEnabled = true, key, stateAfter)
    }

    @VisibleForTesting
@@ -181,32 +194,40 @@ constructor(
        val key = getKey(entry)

        if (runnable == null) {
            headsUpManagerLogger.logAvalancheDelete(caller, isEnabled, key, "Runnable NULL, stop")
            headsUpManagerLogger.logAvalancheDelete(
                caller, isEnabled, key,
                "Runnable NULL, stop. ${getStateStr()}"
            )
            return
        }
        if (!isEnabled) {
            headsUpManagerLogger.logAvalancheDelete(caller, isEnabled, key,
                    "NOT ENABLED, run runnable")
            runnable.run()
            headsUpManagerLogger.logAvalancheDelete(
                caller, isEnabled = false, key,
                "NOT ENABLED, run runnable. ${getStateStr()}"
            )
            return
        }
        if (entry == null) {
            headsUpManagerLogger.logAvalancheDelete(caller, isEnabled, key,
                    "Entry NULL, run runnable")
            runnable.run()
            headsUpManagerLogger.logAvalancheDelete(
                caller, isEnabled = true, key,
                "Entry NULL, run runnable. ${getStateStr()}"
            )
            return
        }
        var outcome = ""
        var stateAfter: String
        if (entry in nextMap) {
            outcome = "remove from next"
            if (entry in nextMap) nextMap.remove(entry)
            if (entry in nextList) nextList.remove(entry)
            uiEventLogger.log(ThrottleEvent.AVALANCHE_THROTTLING_HUN_REMOVED)
            stateAfter = "remove from next. ${getStateStr()}"

        } else if (entry in debugDropSet) {
            outcome = "remove from dropset"
            debugDropSet.remove(entry)
            stateAfter = "remove from dropset. ${getStateStr()}"

        } else if (isShowing(entry)) {
            outcome = "remove showing"
            previousHunKey = getKey(headsUpEntryShowing)
            // Show the next HUN before removing this one, so that we don't tell listeners
            // onHeadsUpPinnedModeChanged, which causes
@@ -214,11 +235,13 @@ constructor(
            // HUN is animating out, resulting in a flicker.
            showNext()
            runnable.run()
            stateAfter = "remove showing. ${getStateStr()}"

        } else {
            outcome = "run runnable for untracked shown"
            runnable.run()
            stateAfter = "run runnable for untracked shown HUN. ${getStateStr()}"
        }
        headsUpManagerLogger.logAvalancheDelete(caller, isEnabled(), getKey(entry), outcome)
        headsUpManagerLogger.logAvalancheDelete(caller, isEnabled(), getKey(entry), stateAfter)
    }

    /**
@@ -400,12 +423,14 @@ constructor(
    }

    private fun getStateStr(): String {
        return "\navalanche state:" +
        return "\nAvalancheController:" +
                "\n\tshowing: [${getKey(headsUpEntryShowing)}]" +
                "\n\tprevious: [$previousHunKey]" +
                "\n\tnext list: $nextListStr" +
                "\n\tnext map: $nextMapStr" +
                "\n\tdropped: $dropSetStr"
                "\n\tdropped: $dropSetStr" +
                "\nBHUM.mHeadsUpEntryMap: " +
                baseEntryMapStr()
    }

    private val dropSetStr: String
+14 −1
Original line number Diff line number Diff line
@@ -116,6 +116,7 @@ public abstract class BaseHeadsUpManager implements HeadsUpManager {
        mAccessibilityMgr = accessibilityManagerWrapper;
        mUiEventLogger = uiEventLogger;
        mAvalancheController = avalancheController;
        mAvalancheController.setBaseEntryMapStr(this::getEntryMapStr);
        Resources resources = context.getResources();
        mMinimumDisplayTime = NotificationThrottleHun.isEnabled()
                ? 500 : resources.getInteger(R.integer.heads_up_notification_minimum_time);
@@ -589,6 +590,18 @@ public abstract class BaseHeadsUpManager implements HeadsUpManager {
        dumpInternal(pw, args);
    }

    private String getEntryMapStr() {
        if (mHeadsUpEntryMap.isEmpty()) {
            return "EMPTY";
        }
        StringBuilder entryMapStr = new StringBuilder();
        for (HeadsUpEntry entry: mHeadsUpEntryMap.values()) {
            entryMapStr.append("\n\t").append(
                    entry.mEntry == null ? "null" : entry.mEntry.getKey());
        }
        return entryMapStr.toString();
    }

    protected void dumpInternal(@NonNull PrintWriter pw, @NonNull String[] args) {
        pw.print("  mTouchAcceptanceDelay="); pw.println(mTouchAcceptanceDelay);
        pw.print("  mSnoozeLengthMs="); pw.println(mSnoozeLengthMs);
@@ -992,7 +1005,6 @@ public abstract class BaseHeadsUpManager implements HeadsUpManager {
         * Clear any pending removal runnables.
         */
        public void cancelAutoRemovalCallbacks(@Nullable String reason) {
            mLogger.logAutoRemoveCancelRequest(this.mEntry, reason);
            Runnable runnable = () -> {
                final boolean removed = cancelAutoRemovalCallbackInternal();

@@ -1001,6 +1013,7 @@ public abstract class BaseHeadsUpManager implements HeadsUpManager {
                }
            };
            if (mEntry != null && isHeadsUpEntry(mEntry.getKey())) {
                mLogger.logAutoRemoveCancelRequest(this.mEntry, reason);
                mAvalancheController.update(this, runnable, reason + " cancelAutoRemovalCallbacks");
            } else {
                // Just removed
+14 −14
Original line number Diff line number Diff line
@@ -52,7 +52,7 @@ constructor(@NotificationHeadsUpLog private val buffer: LogBuffer) {
        caller: String,
        isEnabled: Boolean,
        notifEntryKey: String,
        outcome: String
        stateAfter: String
    ) {
        buffer.log(
            TAG,
@@ -60,7 +60,7 @@ constructor(@NotificationHeadsUpLog private val buffer: LogBuffer) {
            {
                str1 = caller
                str2 = notifEntryKey
                str3 = outcome
                str3 = stateAfter
                bool1 = isEnabled
            },
            { "$str1\n\t=> AC[isEnabled:$bool1] update: $str2\n\t=> $str3" }
@@ -71,7 +71,7 @@ constructor(@NotificationHeadsUpLog private val buffer: LogBuffer) {
        caller: String,
        isEnabled: Boolean,
        notifEntryKey: String,
        outcome: String
        stateAfter: String
    ) {
        buffer.log(
            TAG,
@@ -79,7 +79,7 @@ constructor(@NotificationHeadsUpLog private val buffer: LogBuffer) {
            {
                str1 = caller
                str2 = notifEntryKey
                str3 = outcome
                str3 = stateAfter
                bool1 = isEnabled
            },
            { "$str1\n\t=> AC[isEnabled:$bool1] delete: $str2\n\t=> $str3" }
@@ -136,7 +136,7 @@ constructor(@NotificationHeadsUpLog private val buffer: LogBuffer) {
                str1 = entry.logKey
                str2 = reason ?: "unknown"
            },
            { "request: cancel auto remove of $str1 reason: $str2" }
            { "$str2 => request: cancelAutoRemovalCallbacks: $str1" }
        )
    }

@@ -148,7 +148,7 @@ constructor(@NotificationHeadsUpLog private val buffer: LogBuffer) {
                str1 = entry.logKey
                str2 = reason ?: "unknown"
            },
            { "cancel auto remove of $str1 reason: $str2" }
            { "$str2 => cancel auto remove: $str1" }
        )
    }

@@ -161,7 +161,7 @@ constructor(@NotificationHeadsUpLog private val buffer: LogBuffer) {
                str2 = reason
                bool1 = isWaiting
            },
            { "request: $str2 => remove entry $str1 isWaiting: $isWaiting" }
            { "request: $str2 => removeEntry: $str1 isWaiting: $isWaiting" }
        )
    }

@@ -174,7 +174,7 @@ constructor(@NotificationHeadsUpLog private val buffer: LogBuffer) {
                str2 = reason
                bool1 = isWaiting
            },
            { "$str2 => remove entry $str1 isWaiting: $isWaiting" }
            { "$str2 => removeEntry: $str1 isWaiting: $isWaiting" }
        )
    }

@@ -216,12 +216,12 @@ constructor(@NotificationHeadsUpLog private val buffer: LogBuffer) {
                str1 = logKey(key)
                str2 = reason
            },
            { "remove notification $str1 when headsUpEntry is null, reason: $str2" }
            { "remove notif $str1 when headsUpEntry is null, reason: $str2" }
        )
    }

    fun logNotificationActuallyRemoved(entry: NotificationEntry) {
        buffer.log(TAG, INFO, { str1 = entry.logKey }, { "notification removed $str1 " })
        buffer.log(TAG, INFO, { str1 = entry.logKey }, { "removed: $str1 " })
    }

    fun logUpdateNotificationRequest(key: String, alert: Boolean, hasEntry: Boolean) {
@@ -233,7 +233,7 @@ constructor(@NotificationHeadsUpLog private val buffer: LogBuffer) {
                bool1 = alert
                bool2 = hasEntry
            },
            { "request: update notification $str1 alert: $bool1 hasEntry: $bool2" }
            { "request: update notif $str1 alert: $bool1 hasEntry: $bool2" }
        )
    }

@@ -246,7 +246,7 @@ constructor(@NotificationHeadsUpLog private val buffer: LogBuffer) {
                bool1 = alert
                bool2 = hasEntry
            },
            { "update notification $str1 alert: $bool1 hasEntry: $bool2" }
            { "update notif $str1 alert: $bool1 hasEntry: $bool2" }
        )
    }

@@ -281,7 +281,7 @@ constructor(@NotificationHeadsUpLog private val buffer: LogBuffer) {
                bool1 = isPinned
                str2 = reason
            },
            { "$str2 => set entry pinned $str1 pinned: $bool1" }
            { "$str2 => setEntryPinned[$bool1]: $str1" }
        )
    }

@@ -290,7 +290,7 @@ constructor(@NotificationHeadsUpLog private val buffer: LogBuffer) {
            TAG,
            INFO,
            { bool1 = hasPinnedNotification },
            { "has pinned notification changed to $bool1" }
            { "hasPinnedNotification[$bool1]" }
        )
    }