Merge "Fix occasional bad placement of Conversation header" into rvc-dev am: c744ad246e

Original change: https://googleplex-android-review.googlesource.com/c/platform/frameworks/base/+/11797119

Change-Id: I1562bf706f7614da400bd5039b87b1bfac22507c

packages/SystemUI/src/com/android/systemui/log/dagger/LogModule.java

@@ -68,7 +68,7 @@ public class LogModule {
     public static LogBuffer provideNotificationSectionLogBuffer(
             LogcatEchoTracker bufferFilter,
             DumpManager dumpManager) {
-        LogBuffer buffer = new LogBuffer("NotifSectionLog", 500, 10, bufferFilter);
+        LogBuffer buffer = new LogBuffer("NotifSectionLog", 1000, 10, bufferFilter);
         buffer.attach(dumpManager);
         return buffer;
     }

packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationSectionsManager.kt

@@ -41,6 +41,7 @@ import com.android.systemui.statusbar.notification.row.StackScrollerDecorView
 import com.android.systemui.statusbar.notification.stack.StackScrollAlgorithm.SectionProvider
 import com.android.systemui.statusbar.policy.ConfigurationController
 import com.android.systemui.util.children
+import com.android.systemui.util.takeUntil
 import com.android.systemui.util.foldToSparseArray
 import javax.inject.Inject
 
@@ -197,7 +198,7 @@ class NotificationSectionsManager @Inject internal constructor(
         else -> null
     }
 
-    private fun logShadeContents() = parent.children.forEachIndexed { i, child ->
+    private fun logShadeChild(i: Int, child: View) {
         when {
             child === incomingHeaderView -> logger.logIncomingHeader(i)
             child === mediaControlsView -> logger.logMediaControls(i)
@@ -216,6 +217,7 @@ class NotificationSectionsManager @Inject internal constructor(
             }
         }
     }
+    private fun logShadeContents() = parent.children.forEachIndexed(::logShadeChild)
 
     private val isUsingMultipleSections: Boolean
         get() = sectionsFeatureManager.getNumberOfBuckets() > 1
@@ -223,6 +225,57 @@ class NotificationSectionsManager @Inject internal constructor(
     @VisibleForTesting
     fun updateSectionBoundaries() = updateSectionBoundaries("test")
 
+    private interface SectionUpdateState<out T : ExpandableView> {
+        val header: T
+        var currentPosition: Int?
+        var targetPosition: Int?
+        fun adjustViewPosition()
+    }
+
+    private fun <T : ExpandableView> expandableViewHeaderState(header: T): SectionUpdateState<T> =
+            object : SectionUpdateState<T> {
+                override val header = header
+                override var currentPosition: Int? = null
+                override var targetPosition: Int? = null
+
+                override fun adjustViewPosition() {
+                    val target = targetPosition
+                    val current = currentPosition
+                    if (target == null) {
+                        if (current != null) {
+                            parent.removeView(header)
+                        }
+                    } else {
+                        if (current == null) {
+                            // If the header is animating away, it will still have a parent, so
+                            // detach it first
+                            // TODO: We should really cancel the active animations here. This will
+                            //  happen automatically when the view's intro animation starts, but
+                            //  it's a fragile link.
+                            header.transientContainer?.removeTransientView(header)
+                            header.transientContainer = null
+                            parent.addView(header, target)
+                        } else {
+                            parent.changeViewPosition(header, target)
+                        }
+                    }
+                }
+    }
+
+    private fun <T : StackScrollerDecorView> decorViewHeaderState(
+        header: T
+    ): SectionUpdateState<T> {
+        val inner = expandableViewHeaderState(header)
+        return object : SectionUpdateState<T> by inner {
+            override fun adjustViewPosition() {
+                inner.adjustViewPosition()
+                if (targetPosition != null && currentPosition == null) {
+                    header.isContentVisible = true
+                }
+            }
+        }
+    }
+
     /**
      * Should be called whenever notifs are added, removed, or updated. Updates section boundary
      * bookkeeping and adds/moves/removes section headers if appropriate.
@@ -238,233 +291,136 @@ class NotificationSectionsManager @Inject internal constructor(
         // Then, once we find the start of a new section, we track that position as the "target" for
         // the section header, adjusted for the case where existing headers are in front of that
         // target, but won't be once they are moved / removed after the pass has completed.
+
         val showHeaders = statusBarStateController.state != StatusBarState.KEYGUARD
         val usingPeopleFiltering = sectionsFeatureManager.isFilteringEnabled()
         val usingMediaControls = sectionsFeatureManager.isMediaControlsEnabled()
 
+        val mediaState = mediaControlsView?.let(::expandableViewHeaderState)
+        val incomingState = incomingHeaderView?.let(::decorViewHeaderState)
+        val peopleState = peopleHeaderView?.let(::decorViewHeaderState)
+        val alertingState = alertingHeaderView?.let(::decorViewHeaderState)
+        val gentleState = silentHeaderView?.let(::decorViewHeaderState)
+
+        fun getSectionState(view: View): SectionUpdateState<ExpandableView>? = when {
+            view === mediaControlsView -> mediaState
+            view === incomingHeaderView -> incomingState
+            view === peopleHeaderView -> peopleState
+            view === alertingHeaderView -> alertingState
+            view === silentHeaderView -> gentleState
+            else -> null
+        }
+
+        val headersOrdered = sequenceOf(
+                mediaState, incomingState, peopleState, alertingState, gentleState
+        ).filterNotNull()
+
         var peopleNotifsPresent = false
-        var currentMediaControlsIdx = -1
-        val mediaControlsTarget = if (usingMediaControls) 0 else -1
-        var currentIncomingHeaderIdx = -1
-        var incomingHeaderTarget = -1
-        var currentPeopleHeaderIdx = -1
-        var peopleHeaderTarget = -1
-        var currentAlertingHeaderIdx = -1
-        var alertingHeaderTarget = -1
-        var currentGentleHeaderIdx = -1
-        var gentleHeaderTarget = -1
-
         var lastNotifIndex = 0
-        var lastIncomingIndex = -1
+        var nextBucket: Int? = null
-        var prev: ExpandableNotificationRow? = null
+        var inIncomingSection = false
 
-        for ((i, child) in parent.children.withIndex()) {
+        // Iterating backwards allows for easier construction of the Incoming section, as opposed
-            when {
+        // to backtracking when a discontinuity in the sections is discovered.
-                // Track the existing positions of the headers
+        // Iterating to -1 in order to support the case where a header is at the very top of the
-                child === incomingHeaderView -> {
+        // shade.
-                    logger.logIncomingHeader(i)
+        for (i in parent.childCount - 1 downTo -1) {
-                    currentIncomingHeaderIdx = i
+            val child: View? = parent.getChildAt(i)
-                }
+            child?.let {
-                child === mediaControlsView -> {
+                logShadeChild(i, child)
-                    logger.logMediaControls(i)
+                // If this child is a header, update the tracked positions
-                    currentMediaControlsIdx = i
+                getSectionState(child)?.let { state ->
-                }
+                    state.currentPosition = i
-                child === peopleHeaderView -> {
+                    // If headers that should appear above this one in the shade already have a
-                    logger.logConversationsHeader(i)
+                    // target index, then we need to decrement them in order to account for this one
-                    currentPeopleHeaderIdx = i
+                    // being either removed, or moved below them.
-                }
+                    headersOrdered.takeUntil { it === state }
-                child === alertingHeaderView -> {
+                            .forEach { it.targetPosition = it.targetPosition?.minus(1) }
-                    logger.logAlertingHeader(i)
-                    currentAlertingHeaderIdx = i
-                }
-                child === silentHeaderView -> {
-                    logger.logSilentHeader(i)
-                    currentGentleHeaderIdx = i
-                }
-                child !is ExpandableNotificationRow -> logger.logOther(i, child.javaClass)
-                else -> {
-                    lastNotifIndex = i
-                    // Is there a section discontinuity? This usually occurs due to HUNs
-                    if (prev?.entry?.bucket?.let { it > child.entry.bucket } == true) {
-                        // Remove existing headers, and move the Incoming header if necessary
-                        incomingHeaderTarget = when {
-                            !showHeaders -> -1
-                            incomingHeaderTarget != -1 -> incomingHeaderTarget
-                            peopleHeaderTarget != -1 -> peopleHeaderTarget
-                            alertingHeaderTarget != -1 -> alertingHeaderTarget
-                            gentleHeaderTarget != -1 -> gentleHeaderTarget
-                            else -> 0
-                        }
-                        peopleHeaderTarget = -1
-                        alertingHeaderTarget = -1
-                        gentleHeaderTarget = -1
-                        // Walk backwards changing all previous notifications to the Incoming
-                        // section
-                        for (j in i - 1 downTo lastIncomingIndex + 1) {
-                            val prevChild = parent.getChildAt(j)
-                            if (prevChild is ExpandableNotificationRow) {
-                                prevChild.entry.bucket = BUCKET_HEADS_UP
-                            }
-                        }
-                        // Track the new bottom of the Incoming section
-                        lastIncomingIndex = i - 1
-                    }
-                    val isHeadsUp = child.isHeadsUp
-                    when (child.entry.bucket) {
-                        BUCKET_FOREGROUND_SERVICE -> logger.logForegroundService(i, isHeadsUp)
-                        BUCKET_PEOPLE -> {
-                            logger.logConversation(i, isHeadsUp)
-                            peopleNotifsPresent = true
-                            if (showHeaders && peopleHeaderTarget == -1) {
-                                peopleHeaderTarget = i
-                                // Offset the target if there are other headers before this that
-                                // will be moved.
-                                if (currentIncomingHeaderIdx != -1 && incomingHeaderTarget == -1) {
-                                    peopleHeaderTarget--
-                                }
-                                if (currentPeopleHeaderIdx != -1) {
-                                    peopleHeaderTarget--
-                                }
-                                if (currentAlertingHeaderIdx != -1) {
-                                    peopleHeaderTarget--
-                                }
-                                if (currentGentleHeaderIdx != -1) {
-                                    peopleHeaderTarget--
-                                }
-                            }
-                        }
-                        BUCKET_ALERTING -> {
-                            logger.logAlerting(i, isHeadsUp)
-                            if (showHeaders && usingPeopleFiltering && alertingHeaderTarget == -1) {
-                                alertingHeaderTarget = i
-                                // Offset the target if there are other headers before this that
-                                // will be moved.
-                                if (currentIncomingHeaderIdx != -1 && incomingHeaderTarget == -1) {
-                                    alertingHeaderTarget--
-                                }
-                                if (currentPeopleHeaderIdx != -1 && peopleHeaderTarget == -1) {
-                                    // People header will be removed
-                                    alertingHeaderTarget--
-                                }
-                                if (currentAlertingHeaderIdx != -1) {
-                                    alertingHeaderTarget--
-                                }
-                                if (currentGentleHeaderIdx != -1) {
-                                    alertingHeaderTarget--
-                                }
-                            }
-                        }
-                        BUCKET_SILENT -> {
-                            logger.logSilent(i, isHeadsUp)
-                            if (showHeaders && gentleHeaderTarget == -1) {
-                                gentleHeaderTarget = i
-                                // Offset the target if there are other headers before this that
-                                // will be moved.
-                                if (currentIncomingHeaderIdx != -1 && incomingHeaderTarget == -1) {
-                                    gentleHeaderTarget--
-                                }
-                                if (currentPeopleHeaderIdx != -1 && peopleHeaderTarget == -1) {
-                                    // People header will be removed
-                                    gentleHeaderTarget--
-                                }
-                                if (currentAlertingHeaderIdx != -1 && alertingHeaderTarget == -1) {
-                                    // Alerting header will be removed
-                                    gentleHeaderTarget--
-                                }
-                                if (currentGentleHeaderIdx != -1) {
-                                    gentleHeaderTarget--
-                                }
-                            }
-                        }
-                    }
-
-                    prev = child
                 }
             }
+
+            val row = child as? ExpandableNotificationRow
+
+            // Is there a section discontinuity? This usually occurs due to HUNs
+            inIncomingSection = inIncomingSection || nextBucket?.let { next ->
+                row?.entry?.bucket?.let { curr -> next < curr }
+            } == true
+
+            if (inIncomingSection) {
+                // Update the bucket to reflect that it's being placed in the Incoming section
+                row?.entry?.bucket = BUCKET_HEADS_UP
+            }
+
+            // Insert a header in front of the next row, if there's a boundary between it and this
+            // row, or if it is the topmost row.
+            val isSectionBoundary = nextBucket != null &&
+                    (child == null || row != null && nextBucket != row.entry.bucket)
+            if (isSectionBoundary && showHeaders) {
+                when (nextBucket) {
+                    BUCKET_HEADS_UP -> incomingState?.targetPosition = i + 1
+                    BUCKET_PEOPLE -> peopleState?.targetPosition = i + 1
+                    BUCKET_ALERTING -> alertingState?.targetPosition = i + 1
+                    BUCKET_SILENT -> gentleState?.targetPosition = i + 1
+                }
+            }
+
+            row ?: continue
+
+            // Check if there are any people notifications
+            peopleNotifsPresent = peopleNotifsPresent || row.entry.bucket == BUCKET_PEOPLE
+
+            if (nextBucket == null) {
+                lastNotifIndex = i
+            }
+            nextBucket = row.entry.bucket
         }
 
-        if (showHeaders && usingPeopleFiltering && peopleHubVisible && peopleHeaderTarget == -1) {
+        if (showHeaders && usingPeopleFiltering && peopleHubVisible) {
-            // Insert the people header even if there are no people visible, in order to show
+            peopleState?.targetPosition = peopleState?.targetPosition
-            // the hub. Put it directly above the next header.
+                    // Insert the people header even if there are no people visible, in order to
-            peopleHeaderTarget = when {
+                    // show the hub. Put it directly above the next header.
-                alertingHeaderTarget != -1 -> alertingHeaderTarget
+                    ?: alertingState?.targetPosition
-                gentleHeaderTarget != -1 -> gentleHeaderTarget
+                    ?: gentleState?.targetPosition
-                else -> lastNotifIndex // Put it at the end of the list.
+                    // Put it at the end of the list.
-            }
+                    ?: lastNotifIndex
+
             // Offset the target to account for the current position of the people header.
-            if (currentPeopleHeaderIdx != -1 && currentPeopleHeaderIdx < peopleHeaderTarget) {
+            peopleState?.targetPosition = peopleState?.currentPosition?.let { current ->
-                peopleHeaderTarget--
+                peopleState?.targetPosition?.let { target ->
+                    if (current < target) target - 1 else target
+                }
             }
         }
 
+        mediaState?.targetPosition = if (usingMediaControls) 0 else null
+
         logger.logStr("New header target positions:")
-        logger.logIncomingHeader(incomingHeaderTarget)
+        logger.logMediaControls(mediaState?.targetPosition ?: -1)
-        logger.logMediaControls(mediaControlsTarget)
+        logger.logIncomingHeader(incomingState?.targetPosition ?: -1)
-        logger.logConversationsHeader(peopleHeaderTarget)
+        logger.logConversationsHeader(peopleState?.targetPosition ?: -1)
-        logger.logAlertingHeader(alertingHeaderTarget)
+        logger.logAlertingHeader(alertingState?.targetPosition ?: -1)
-        logger.logSilentHeader(gentleHeaderTarget)
+        logger.logSilentHeader(gentleState?.targetPosition ?: -1)
 
-        // Add headers in reverse order to preserve indices
+        // Update headers in reverse order to preserve indices, otherwise movements earlier in the
-        silentHeaderView?.let {
+        // list will affect the target indices of the headers later in the list.
-            adjustHeaderVisibilityAndPosition(gentleHeaderTarget, it, currentGentleHeaderIdx)
+        headersOrdered.asIterable().reversed().forEach { it.adjustViewPosition() }
-        }
-        alertingHeaderView?.let {
-            adjustHeaderVisibilityAndPosition(alertingHeaderTarget, it, currentAlertingHeaderIdx)
-        }
-        peopleHeaderView?.let {
-            adjustHeaderVisibilityAndPosition(peopleHeaderTarget, it, currentPeopleHeaderIdx)
-        }
-        incomingHeaderView?.let {
-            adjustHeaderVisibilityAndPosition(incomingHeaderTarget, it, currentIncomingHeaderIdx)
-        }
-        mediaControlsView?.let {
-            adjustViewPosition(mediaControlsTarget, it, currentMediaControlsIdx)
-        }
 
         logger.logStr("Final order:")
         logShadeContents()
         logger.logStr("Section boundary update complete")
 
         // Update headers to reflect state of section contents
-        silentHeaderView?.setAreThereDismissableGentleNotifs(
+        silentHeaderView?.run {
-                parent.hasActiveClearableNotifications(NotificationStackScrollLayout.ROWS_GENTLE)
+            val hasActiveClearableNotifications = this@NotificationSectionsManager.parent
-        )
+                    .hasActiveClearableNotifications(NotificationStackScrollLayout.ROWS_GENTLE)
-        peopleHeaderView?.canSwipe = showHeaders && peopleHubVisible && !peopleNotifsPresent
+            setAreThereDismissableGentleNotifs(hasActiveClearableNotifications)
-        if (peopleHeaderTarget != currentPeopleHeaderIdx) {
-            peopleHeaderView?.resetTranslation()
         }
-    }
+        peopleHeaderView?.run {
-
+            canSwipe = showHeaders && peopleHubVisible && !peopleNotifsPresent
-    private fun adjustHeaderVisibilityAndPosition(
+            peopleState?.targetPosition?.let { targetPosition ->
-        targetPosition: Int,
+                if (targetPosition != peopleState.currentPosition) {
-        header: StackScrollerDecorView,
+                    resetTranslation()
-        currentPosition: Int
+                }
-    ) {
-        adjustViewPosition(targetPosition, header, currentPosition)
-        if (targetPosition != -1 && currentPosition == -1) {
-            header.isContentVisible = true
-        }
-    }
-
-    private fun adjustViewPosition(
-        targetPosition: Int,
-        view: ExpandableView,
-        currentPosition: Int
-    ) {
-        if (targetPosition == -1) {
-            if (currentPosition != -1) {
-                parent.removeView(view)
-            }
-        } else {
-            if (currentPosition == -1) {
-                // If the header is animating away, it will still have a parent, so detach it first
-                // TODO: We should really cancel the active animations here. This will happen
-                // automatically when the view's intro animation starts, but it's a fragile link.
-                view.transientContainer?.removeTransientView(view)
-                view.transientContainer = null
-                parent.addView(view, targetPosition)
-            } else {
-                parent.changeViewPosition(view, targetPosition)
             }
         }
     }

packages/SystemUI/src/com/android/systemui/util/ConvenienceExtensions.kt

@@ -23,3 +23,13 @@ val ViewGroup.children
     get() = sequence {
         for (i in 0 until childCount) yield(getChildAt(i))
     }
+
+/** Inclusive version of [Iterable.takeWhile] */
+fun <T> Sequence<T>.takeUntil(pred: (T) -> Boolean): Sequence<T> = sequence {
+    for (x in this@takeUntil) {
+        yield(x)
+        if (pred(x)) {
+            break
+        }
+    }
+}

packages/SystemUI/src/com/android/systemui/util/SparseArrayUtils.kt

@@ -18,6 +18,22 @@ package com.android.systemui.util
 
 import android.util.SparseArray
 
+/**
+ * Transforms a sequence of Key/Value pairs into a SparseArray.
+ *
+ * See [kotlin.collections.toMap].
+ */
+fun <T> Sequence<Pair<Int, T>>.toSparseArray(size: Int = -1): SparseArray<T> {
+    val sparseArray = when {
+        size < 0 -> SparseArray<T>()
+        else -> SparseArray<T>(size)
+    }
+    for ((i, v) in this) {
+        sparseArray.put(i, v)
+    }
+    return sparseArray
+}
+
 /**
  * Transforms an [Array] into a [SparseArray], by applying each element to [keySelector] in order to
  * generate the index at which it will be placed. If two elements produce the same index, the latter

packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/stack/NotificationSectionsManagerTest.java

@@ -403,11 +403,11 @@ public class NotificationSectionsManagerTest extends SysuiTestCase {
         enablePeopleFiltering();
 
         setupMockStack(
-                PERSON.headsUp(),   // personHeaderTarget = 0
+                PERSON.headsUp(),
-                INCOMING_HEADER,    // currentIncomingHeaderIdx = 1
+                INCOMING_HEADER,
-                ALERTING.headsUp(), // alertingHeaderTarget = 1
+                ALERTING.headsUp(),
-                PEOPLE_HEADER,      // currentPeopleHeaderIdx = 3
+                PEOPLE_HEADER,
-                PERSON              //
+                PERSON
         );
         mSectionsManager.updateSectionBoundaries();
 
@@ -520,6 +520,70 @@ public class NotificationSectionsManagerTest extends SysuiTestCase {
                 ChildType.GENTLE);
     }
 
+    @Test
+    public void testRemoveIncomingHeader() {
+        enablePeopleFiltering();
+        enableMediaControls();
+
+        setupMockStack(
+                MEDIA_CONTROLS,
+                INCOMING_HEADER,
+                PERSON,
+                ALERTING,
+                PEOPLE_HEADER,
+                ALERTING_HEADER,
+                ALERTING,
+                ALERTING,
+                GENTLE_HEADER,
+                GENTLE,
+                GENTLE
+        );
+
+        mSectionsManager.updateSectionBoundaries();
+
+        verifyMockStack(
+                ChildType.MEDIA_CONTROLS,
+                ChildType.PEOPLE_HEADER,
+                ChildType.PERSON,
+                ChildType.ALERTING_HEADER,
+                ChildType.ALERTING,
+                ChildType.ALERTING,
+                ChildType.ALERTING,
+                ChildType.GENTLE_HEADER,
+                ChildType.GENTLE,
+                ChildType.GENTLE
+        );
+    }
+
+    @Test
+    public void testExpandIncomingSection() {
+        enablePeopleFiltering();
+
+        setupMockStack(
+                INCOMING_HEADER,
+                PERSON,
+                ALERTING,
+                PEOPLE_HEADER,
+                ALERTING,
+                PERSON,
+                ALERTING_HEADER,
+                ALERTING
+        );
+
+        mSectionsManager.updateSectionBoundaries();
+
+        verifyMockStack(
+                ChildType.INCOMING_HEADER,
+                ChildType.HEADS_UP,
+                ChildType.HEADS_UP,
+                ChildType.HEADS_UP,
+                ChildType.PEOPLE_HEADER,
+                ChildType.PERSON,
+                ChildType.ALERTING_HEADER,
+                ChildType.ALERTING
+        );
+    }
+
     private void enablePeopleFiltering() {
         when(mSectionsFeatureManager.isFilteringEnabled()).thenReturn(true);
     }
@@ -657,7 +721,13 @@ public class NotificationSectionsManagerTest extends SysuiTestCase {
         final List<View> children = new ArrayList<>();
         when(mNssl.getChildCount()).thenAnswer(invocation -> children.size());
         when(mNssl.getChildAt(anyInt()))
-                .thenAnswer(invocation -> children.get(invocation.getArgument(0)));
+                .thenAnswer(invocation -> {
+                    Integer index = invocation.getArgument(0);
+                    if (index == null || index < 0 || index >= children.size()) {
+                        return null;
+                    }
+                    return children.get(index);
+                });
         when(mNssl.indexOfChild(any()))
                 .thenAnswer(invocation -> children.indexOf(invocation.getArgument(0)));
         doAnswer(invocation -> {