Show correct last notification time for grouped notifications

The time value in the current group notifications is not synced with its child notifications. That causes sub-optimal user experience for time critical contents. We decided to solve this problem with a small group coordinator(GroupTimeCoordinator): - if all notifications before now, we show the closest time value by now aka maximum value. - if all notifications after now, we show the closest time value by now aka minimum value. - if there are notifications before and after now, we show the closest the future time by now. Because, any future event is more important than even the most recent past event. Test: atest GroupWhenCoordinatorTest Bug: 181790059 Change-Id: I1bbbffb8d5c7a056f19b743b2984f86611978610
2023-02-27 19:22:02 +00:00
parent 8247263184
commit 27c4e7be4b
8 changed files with 426 additions and 25 deletions
--- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/GroupWhenCoordinator.kt
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/GroupWhenCoordinator.kt
@@ -0,0 +1,108 @@
+/*
+ * Copyright (C) 2023 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.android.systemui.statusbar.notification.collection.coordinator
+
+import android.util.ArrayMap
+import com.android.systemui.dagger.qualifiers.Main
+import com.android.systemui.statusbar.notification.collection.GroupEntry
+import com.android.systemui.statusbar.notification.collection.ListEntry
+import com.android.systemui.statusbar.notification.collection.NotifPipeline
+import com.android.systemui.statusbar.notification.collection.coordinator.dagger.CoordinatorScope
+import com.android.systemui.statusbar.notification.collection.listbuilder.pluggable.Invalidator
+import com.android.systemui.statusbar.notification.collection.render.NotifGroupController
+import com.android.systemui.util.concurrency.DelayableExecutor
+import com.android.systemui.util.time.SystemClock
+import javax.inject.Inject
+import kotlin.math.max
+import kotlin.math.min
+
+/** A small coordinator which finds, stores, and applies the closest notification time. */
+@CoordinatorScope
+class GroupWhenCoordinator
+@Inject
+constructor(
+    @Main private val delayableExecutor: DelayableExecutor,
+    private val systemClock: SystemClock
+) : Coordinator {
+
+    private val invalidator = object : Invalidator("GroupWhenCoordinator") {}
+    private val notificationGroupTimes = ArrayMap<GroupEntry, Long>()
+    private var cancelInvalidateListRunnable: Runnable? = null
+
+    private val invalidateListRunnable: Runnable = Runnable {
+        invalidator.invalidateList("future notification invalidation")
+    }
+
+    override fun attach(pipeline: NotifPipeline) {
+        pipeline.addOnBeforeFinalizeFilterListener(::onBeforeFinalizeFilterListener)
+        pipeline.addOnAfterRenderGroupListener(::onAfterRenderGroupListener)
+        pipeline.addPreRenderInvalidator(invalidator)
+    }
+
+    private fun onBeforeFinalizeFilterListener(entries: List<ListEntry>) {
+        cancelListInvalidation()
+        notificationGroupTimes.clear()
+
+        val now = systemClock.currentTimeMillis()
+        var closestFutureTime = Long.MAX_VALUE
+        entries.asSequence().filterIsInstance<GroupEntry>().forEach { groupEntry ->
+            val whenMillis = calculateGroupNotificationTime(groupEntry, now)
+            notificationGroupTimes[groupEntry] = whenMillis
+            if (whenMillis > now) {
+                closestFutureTime = min(closestFutureTime, whenMillis)
+            }
+        }
+
+        if (closestFutureTime != Long.MAX_VALUE) {
+            cancelInvalidateListRunnable =
+                delayableExecutor.executeDelayed(invalidateListRunnable, closestFutureTime - now)
+        }
+    }
+
+    private fun cancelListInvalidation() {
+        cancelInvalidateListRunnable?.run()
+        cancelInvalidateListRunnable = null
+    }
+
+    private fun onAfterRenderGroupListener(group: GroupEntry, controller: NotifGroupController) {
+        notificationGroupTimes[group]?.let(controller::setNotificationGroupWhen)
+    }
+
+    private fun calculateGroupNotificationTime(
+        groupEntry: GroupEntry,
+        currentTimeMillis: Long
+    ): Long {
+        var pastTime = Long.MIN_VALUE
+        var futureTime = Long.MAX_VALUE
+        groupEntry.children
+            .asSequence()
+            .mapNotNull { child -> child.sbn.notification.`when`.takeIf { it > 0 } }
+            .forEach { time ->
+                val isInThePast = currentTimeMillis - time > 0
+                if (isInThePast) {
+                    pastTime = max(pastTime, time)
+                } else {
+                    futureTime = min(futureTime, time)
+                }
+            }
+
+        if (pastTime == Long.MIN_VALUE && futureTime == Long.MAX_VALUE) {
+            return checkNotNull(groupEntry.summary).creationTime
+        }
+
+        return if (futureTime != Long.MAX_VALUE) futureTime else pastTime
+    }
+}
--- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/NotifCoordinators.kt
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/NotifCoordinators.kt
@@ -31,31 +31,32 @@ interface NotifCoordinators : Coordinator, PipelineDumpable

@CoordinatorScope
 class NotifCoordinatorsImpl @Inject constructor(
-    notifPipelineFlags: NotifPipelineFlags,
-    dataStoreCoordinator: DataStoreCoordinator,
-    hideLocallyDismissedNotifsCoordinator: HideLocallyDismissedNotifsCoordinator,
-    hideNotifsForOtherUsersCoordinator: HideNotifsForOtherUsersCoordinator,
-    keyguardCoordinator: KeyguardCoordinator,
-    rankingCoordinator: RankingCoordinator,
-    appOpsCoordinator: AppOpsCoordinator,
-    deviceProvisionedCoordinator: DeviceProvisionedCoordinator,
-    bubbleCoordinator: BubbleCoordinator,
-    headsUpCoordinator: HeadsUpCoordinator,
-    gutsCoordinator: GutsCoordinator,
-    conversationCoordinator: ConversationCoordinator,
-    debugModeCoordinator: DebugModeCoordinator,
-    groupCountCoordinator: GroupCountCoordinator,
-    mediaCoordinator: MediaCoordinator,
-    preparationCoordinator: PreparationCoordinator,
-    remoteInputCoordinator: RemoteInputCoordinator,
-    rowAppearanceCoordinator: RowAppearanceCoordinator,
-    stackCoordinator: StackCoordinator,
-    shadeEventCoordinator: ShadeEventCoordinator,
-    smartspaceDedupingCoordinator: SmartspaceDedupingCoordinator,
-    viewConfigCoordinator: ViewConfigCoordinator,
-    visualStabilityCoordinator: VisualStabilityCoordinator,
-    sensitiveContentCoordinator: SensitiveContentCoordinator,
-    dismissibilityCoordinator: DismissibilityCoordinator
+        notifPipelineFlags: NotifPipelineFlags,
+        dataStoreCoordinator: DataStoreCoordinator,
+        hideLocallyDismissedNotifsCoordinator: HideLocallyDismissedNotifsCoordinator,
+        hideNotifsForOtherUsersCoordinator: HideNotifsForOtherUsersCoordinator,
+        keyguardCoordinator: KeyguardCoordinator,
+        rankingCoordinator: RankingCoordinator,
+        appOpsCoordinator: AppOpsCoordinator,
+        deviceProvisionedCoordinator: DeviceProvisionedCoordinator,
+        bubbleCoordinator: BubbleCoordinator,
+        headsUpCoordinator: HeadsUpCoordinator,
+        gutsCoordinator: GutsCoordinator,
+        conversationCoordinator: ConversationCoordinator,
+        debugModeCoordinator: DebugModeCoordinator,
+        groupCountCoordinator: GroupCountCoordinator,
+        groupWhenCoordinator: GroupWhenCoordinator,
+        mediaCoordinator: MediaCoordinator,
+        preparationCoordinator: PreparationCoordinator,
+        remoteInputCoordinator: RemoteInputCoordinator,
+        rowAppearanceCoordinator: RowAppearanceCoordinator,
+        stackCoordinator: StackCoordinator,
+        shadeEventCoordinator: ShadeEventCoordinator,
+        smartspaceDedupingCoordinator: SmartspaceDedupingCoordinator,
+        viewConfigCoordinator: ViewConfigCoordinator,
+        visualStabilityCoordinator: VisualStabilityCoordinator,
+        sensitiveContentCoordinator: SensitiveContentCoordinator,
+        dismissibilityCoordinator: DismissibilityCoordinator
 ) : NotifCoordinators {

    private val mCoordinators: MutableList<Coordinator> = ArrayList()
@@ -82,6 +83,7 @@ class NotifCoordinatorsImpl @Inject constructor(
        mCoordinators.add(debugModeCoordinator)
        mCoordinators.add(conversationCoordinator)
        mCoordinators.add(groupCountCoordinator)
+        mCoordinators.add(groupWhenCoordinator)
        mCoordinators.add(mediaCoordinator)
        mCoordinators.add(rowAppearanceCoordinator)
        mCoordinators.add(stackCoordinator)
--- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/render/NotifGroupController.kt
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/render/NotifGroupController.kt
@@ -20,4 +20,7 @@ package com.android.systemui.statusbar.notification.collection.render
 interface NotifGroupController {
    /** Set the number of children that this group would have if not for the 8-child max */
    fun setUntruncatedChildCount(untruncatedChildCount: Int)
+
+    /** Set the when value of notification group that reflects most important closest notification time */
+    fun setNotificationGroupWhen(whenMillis: Long)
 }
--- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/ExpandableNotificationRow.java
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/ExpandableNotificationRow.java
@@ -852,6 +852,19 @@ public class ExpandableNotificationRow extends ActivatableNotificationView
        mChildrenContainer.setUntruncatedChildCount(childCount);
    }

+    /**
+     * @see NotificationChildrenContainer#setNotificationGroupWhen(long)
+     */
+    public void setNotificationGroupWhen(long whenMillis) {
+        if (mIsSummaryWithChildren) {
+            mChildrenContainer.setNotificationGroupWhen(whenMillis);
+        } else {
+            Log.w(TAG, "setNotificationGroupWhen( whenMillis: " + whenMillis + ")"
+                    + " mIsSummaryWithChildren: false"
+                    + " mChildrenContainer has not been inflated yet.");
+        }
+    }
+
    /**
     * Called after children have been attached to set the expansion states
     */
--- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/ExpandableNotificationRowController.java
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/ExpandableNotificationRowController.java
@@ -348,6 +348,15 @@ public class ExpandableNotificationRowController implements NotifViewController
        }
    }

+    @Override
+    public void setNotificationGroupWhen(long whenMillis) {
+        if (mView.isSummaryWithChildren()) {
+            mView.setNotificationGroupWhen(whenMillis);
+        } else {
+            Log.w(TAG, "Called setNotificationTime(" + whenMillis + ") on a leaf row");
+        }
+    }
+
    @Override
    public void setSystemExpanded(boolean systemExpanded) {
        mView.setSystemExpanded(systemExpanded);
--- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/wrapper/NotificationHeaderViewWrapper.java
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/wrapper/NotificationHeaderViewWrapper.java
@@ -27,6 +27,7 @@ import android.view.View;
 import android.view.ViewGroup;
 import android.view.animation.Interpolator;
 import android.view.animation.PathInterpolator;
+import android.widget.DateTimeView;
 import android.widget.ImageButton;
 import android.widget.ImageView;
 import android.widget.TextView;
@@ -344,6 +345,21 @@ public class NotificationHeaderViewWrapper extends NotificationViewWrapper imple
        mTransformationHelper.setVisible(visible);
    }

+    /***
+     * Set Notification when value
+     * @param whenMillis
+     */
+    public void setNotificationWhen(long whenMillis) {
+        if (mNotificationHeader == null) {
+            return;
+        }
+
+        final View timeView = mNotificationHeader.findViewById(com.android.internal.R.id.time);
+
+        if (timeView instanceof DateTimeView) {
+            ((DateTimeView) timeView).setTime(whenMillis);
+        }
+    }
    protected void addTransformedViews(View... views) {
        for (View view : views) {
            if (view != null) {
--- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationChildrenContainer.java
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationChildrenContainer.java
@@ -295,6 +295,19 @@ public class NotificationChildrenContainer extends ViewGroup
        updateGroupOverflow();
    }

+    /**
+     * Set the notification time in the group so that the view can show the latest event in the UI
+     * appropriately.
+     */
+    public void setNotificationGroupWhen(long whenMillis) {
+        if (mNotificationHeaderWrapper != null) {
+            mNotificationHeaderWrapper.setNotificationWhen(whenMillis);
+        }
+        if (mNotificationHeaderWrapperLowPriority != null) {
+            mNotificationHeaderWrapperLowPriority.setNotificationWhen(whenMillis);
+        }
+    }
+
    /**
     * Add a child notification to this view.
     *
--- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coordinator/GroupWhenCoordinatorTest.kt
+++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coordinator/GroupWhenCoordinatorTest.kt
@@ -0,0 +1,237 @@
+/*
+ * Copyright (C) 2023 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.android.systemui.statusbar.notification.collection.coordinator
+
+import android.app.Notification
+import android.testing.AndroidTestingRunner
+import android.testing.TestableLooper
+import androidx.test.filters.SmallTest
+import com.android.systemui.SysuiTestCase
+import com.android.systemui.statusbar.SbnBuilder
+import com.android.systemui.statusbar.notification.collection.GroupEntryBuilder
+import com.android.systemui.statusbar.notification.collection.NotifPipeline
+import com.android.systemui.statusbar.notification.collection.NotificationEntry
+import com.android.systemui.statusbar.notification.collection.NotificationEntryBuilder
+import com.android.systemui.statusbar.notification.collection.listbuilder.OnAfterRenderGroupListener
+import com.android.systemui.statusbar.notification.collection.listbuilder.OnBeforeFinalizeFilterListener
+import com.android.systemui.statusbar.notification.collection.render.NotifGroupController
+import com.android.systemui.util.concurrency.DelayableExecutor
+import com.android.systemui.util.mockito.any
+import com.android.systemui.util.mockito.eq
+import com.android.systemui.util.mockito.mock
+import com.android.systemui.util.mockito.whenever
+import com.android.systemui.util.mockito.withArgCaptor
+import com.android.systemui.util.time.SystemClock
+import org.junit.Before
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.mockito.InjectMocks
+import org.mockito.Mock
+import org.mockito.Mockito.never
+import org.mockito.Mockito.verify
+import org.mockito.MockitoAnnotations.initMocks
+
+@SmallTest
+@RunWith(AndroidTestingRunner::class)
+@TestableLooper.RunWithLooper
+class GroupWhenCoordinatorTest : SysuiTestCase() {
+
+    private lateinit var beforeFinalizeFilterListener: OnBeforeFinalizeFilterListener
+    private lateinit var afterRenderGroupListener: OnAfterRenderGroupListener
+
+    @Mock private lateinit var pipeline: NotifPipeline
+
+    @Mock private lateinit var delayableExecutor: DelayableExecutor
+
+    @Mock private lateinit var groupController: NotifGroupController
+
+    @Mock private lateinit var systemClock: SystemClock
+
+    @InjectMocks private lateinit var coordinator: GroupWhenCoordinator
+
+    @Before
+    fun setUp() {
+        initMocks(this)
+        whenever(systemClock.currentTimeMillis()).thenReturn(NOW)
+        coordinator.attach(pipeline)
+
+        beforeFinalizeFilterListener = withArgCaptor {
+            verify(pipeline).addOnBeforeFinalizeFilterListener(capture())
+        }
+        afterRenderGroupListener = withArgCaptor {
+            verify(pipeline).addOnAfterRenderGroupListener(capture())
+        }
+    }
+
+    @Test
+    fun setNotificationGroupWhen_setClosestTimeByNow_whenAllNotificationsAreBeforeNow() {
+        // GIVEN
+        val summaryEntry = buildNotificationEntry(0, NOW)
+        val childEntry1 = buildNotificationEntry(1, NOW - 10L)
+        val childEntry2 = buildNotificationEntry(2, NOW - 100L)
+        val groupEntry =
+            GroupEntryBuilder()
+                .setSummary(summaryEntry)
+                .setChildren(listOf(childEntry1, childEntry2))
+                .build()
+        // WHEN
+        beforeFinalizeFilterListener.onBeforeFinalizeFilter(listOf(groupEntry))
+        afterRenderGroupListener.onAfterRenderGroup(groupEntry, groupController)
+
+        // THEN
+        verify(groupController).setNotificationGroupWhen(eq(NOW - 10L))
+    }
+
+    @Test
+    fun setNotificationGroupWhen_setClosestTimeByNow_whenAllNotificationsAreAfterNow() {
+        // GIVEN
+        val summaryEntry = buildNotificationEntry(0, NOW)
+        val childEntry1 = buildNotificationEntry(1, NOW + 10L)
+        val childEntry2 = buildNotificationEntry(2, NOW + 100L)
+
+        val groupEntry =
+            GroupEntryBuilder()
+                .setSummary(summaryEntry)
+                .setChildren(listOf(childEntry1, childEntry2))
+                .build()
+
+        // WHEN
+        beforeFinalizeFilterListener.onBeforeFinalizeFilter(listOf(groupEntry))
+        afterRenderGroupListener.onAfterRenderGroup(groupEntry, groupController)
+
+        // THEN
+        verify(groupController).setNotificationGroupWhen(eq(NOW + 10L))
+    }
+
+    @Test
+    fun setNotificationGroupWhen_setClosestFutureTimeByNow_whenThereAreBothBeforeAndAfterNow() {
+        // GIVEN
+        val summaryEntry = buildNotificationEntry(0, NOW)
+        val childEntry1 = buildNotificationEntry(1, NOW + 100L)
+        val childEntry2 = buildNotificationEntry(2, NOW + 10L)
+        val childEntry3 = buildNotificationEntry(3, NOW - 100L)
+        val childEntry4 = buildNotificationEntry(4, NOW - 9L)
+
+        val groupEntry =
+            GroupEntryBuilder()
+                .setSummary(summaryEntry)
+                .setChildren(listOf(childEntry1, childEntry2, childEntry3, childEntry4))
+                .build()
+
+        // WHEN
+        beforeFinalizeFilterListener.onBeforeFinalizeFilter(listOf(groupEntry))
+        afterRenderGroupListener.onAfterRenderGroup(groupEntry, groupController)
+
+        // THEN
+        verify(groupController).setNotificationGroupWhen(eq(NOW + 10L))
+    }
+
+    @Test
+    fun setNotificationGroupWhen_filterInvalidNotificationTimes() {
+        // GIVEN
+        val summaryEntry = buildNotificationEntry(0, NOW)
+        val childEntry1 = buildNotificationEntry(1, NOW + 100L)
+        val childEntry2 = buildNotificationEntry(2, -20000L)
+        val childEntry3 = buildNotificationEntry(4, 0)
+
+        val groupEntry =
+            GroupEntryBuilder()
+                .setSummary(summaryEntry)
+                .setChildren(listOf(childEntry1, childEntry2, childEntry3))
+                .build()
+
+        // WHEN
+        beforeFinalizeFilterListener.onBeforeFinalizeFilter(listOf(groupEntry))
+        afterRenderGroupListener.onAfterRenderGroup(groupEntry, groupController)
+
+        // THEN
+        verify(groupController).setNotificationGroupWhen(eq(NOW + 100))
+    }
+
+    @Test
+    fun setNotificationGroupWhen_setSummaryTimeWhenAllNotificationTimesAreInvalid() {
+        // GIVEN
+        val summaryEntry = buildNotificationEntry(0, NOW)
+        val childEntry1 = buildNotificationEntry(1, 0)
+        val childEntry2 = buildNotificationEntry(2, -1)
+
+        val groupEntry =
+            GroupEntryBuilder()
+                .setSummary(summaryEntry)
+                .setChildren(listOf(childEntry1, childEntry2))
+                .build()
+
+        // WHEN
+        beforeFinalizeFilterListener.onBeforeFinalizeFilter(listOf(groupEntry))
+        afterRenderGroupListener.onAfterRenderGroup(groupEntry, groupController)
+
+        // THEN
+        verify(groupController, never()).setNotificationGroupWhen(NOW)
+    }
+
+    @Test
+    fun setNotificationGroupWhen_schedulePipelineInvalidationWhenAnyNotificationIsInTheFuture() {
+        // GIVEN
+        val summaryEntry = buildNotificationEntry(0, NOW)
+        val childEntry1 = buildNotificationEntry(1, NOW + 1000L)
+        val childEntry2 = buildNotificationEntry(2, NOW + 2000L)
+        val childEntry3 = buildNotificationEntry(3, NOW - 100L)
+
+        val groupEntry =
+            GroupEntryBuilder()
+                .setSummary(summaryEntry)
+                .setChildren(listOf(childEntry1, childEntry2, childEntry3))
+                .build()
+
+        // WHEN
+        beforeFinalizeFilterListener.onBeforeFinalizeFilter(listOf(groupEntry))
+        afterRenderGroupListener.onAfterRenderGroup(groupEntry, groupController)
+
+        // THEN
+        verify(delayableExecutor).executeDelayed(any(), eq(1000))
+    }
+
+    @Test
+    fun setNotificationGroupWhen_cancelPrevPipelineInvalidation() {
+        // GIVEN
+        val summaryEntry = buildNotificationEntry(0, NOW)
+        val childEntry1 = buildNotificationEntry(1, NOW + 1L)
+        val prevInvalidation = mock<Runnable>()
+        whenever(delayableExecutor.executeDelayed(any(), any())).thenReturn(prevInvalidation)
+
+        val groupEntry =
+            GroupEntryBuilder().setSummary(summaryEntry).setChildren(listOf(childEntry1)).build()
+
+        // WHEN
+        beforeFinalizeFilterListener.onBeforeFinalizeFilter(listOf(groupEntry))
+        afterRenderGroupListener.onAfterRenderGroup(groupEntry, groupController)
+
+        beforeFinalizeFilterListener.onBeforeFinalizeFilter(listOf(groupEntry))
+
+        // THEN
+        verify(prevInvalidation).run()
+    }
+
+    private fun buildNotificationEntry(id: Int, timeMillis: Long): NotificationEntry {
+        val notification = Notification.Builder(mContext).setWhen(timeMillis).build()
+        val sbn = SbnBuilder().setNotification(notification).build()
+        return NotificationEntryBuilder().setId(id).setSbn(sbn).build()
+    }
+
+    private companion object {
+        private const val NOW = 1000L
+    }
+}