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

Commit 60dc8407 authored by Ioana Alexandru's avatar Ioana Alexandru Committed by Android (Google) Code Review
Browse files

Merge "Revert^2 "Remove NOTIFICATION_GROUP_EXPANSION_CHANGE flag."" into main

parents f2eed99e bcd82187
Loading
Loading
Loading
Loading
+23 −45
Original line number Diff line number Diff line
@@ -16,11 +16,10 @@

package com.android.systemui.statusbar.notification.collection.render

import androidx.test.ext.junit.runners.AndroidJUnit4
import androidx.test.filters.SmallTest
import com.android.systemui.SysuiTestCase
import com.android.systemui.dump.DumpManager
import com.android.systemui.flags.FakeFeatureFlagsClassic
import com.android.systemui.flags.Flags
import com.android.systemui.statusbar.notification.collection.GroupEntryBuilder
import com.android.systemui.statusbar.notification.collection.ListEntry
import com.android.systemui.statusbar.notification.collection.NotifPipeline
@@ -34,18 +33,19 @@ import com.google.common.truth.Truth.assertThat
import org.junit.Assert.assertThrows
import org.junit.Before
import org.junit.Test
import org.junit.runner.RunWith
import org.mockito.Mockito.never
import org.mockito.Mockito.verify
import org.mockito.Mockito.verifyNoMoreInteractions
import org.mockito.Mockito.`when` as whenever

@SmallTest
@RunWith(AndroidJUnit4::class)
class GroupExpansionManagerTest : SysuiTestCase() {
    private lateinit var gem: GroupExpansionManagerImpl
    private lateinit var underTest: GroupExpansionManagerImpl

    private val dumpManager: DumpManager = mock()
    private val groupMembershipManager: GroupMembershipManager = mock()
    private val featureFlags = FakeFeatureFlagsClassic()

    private val pipeline: NotifPipeline = mock()
    private lateinit var beforeRenderListListener: OnBeforeRenderListListener
@@ -85,79 +85,57 @@ class GroupExpansionManagerTest : SysuiTestCase() {
        whenever(groupMembershipManager.getGroupSummary(summary1)).thenReturn(summary1)
        whenever(groupMembershipManager.getGroupSummary(summary2)).thenReturn(summary2)

        gem = GroupExpansionManagerImpl(dumpManager, groupMembershipManager, featureFlags)
        underTest = GroupExpansionManagerImpl(dumpManager, groupMembershipManager)
    }

    @Test
    fun testNotifyOnlyOnChange_enabled() {
        featureFlags.set(Flags.NOTIFICATION_GROUP_EXPANSION_CHANGE, true)

    fun notifyOnlyOnChange() {
        var listenerCalledCount = 0
        gem.registerGroupExpansionChangeListener { _, _ -> listenerCalledCount++ }
        underTest.registerGroupExpansionChangeListener { _, _ -> listenerCalledCount++ }

        gem.setGroupExpanded(summary1, false)
        underTest.setGroupExpanded(summary1, false)
        assertThat(listenerCalledCount).isEqualTo(0)
        gem.setGroupExpanded(summary1, true)
        underTest.setGroupExpanded(summary1, true)
        assertThat(listenerCalledCount).isEqualTo(1)
        gem.setGroupExpanded(summary2, true)
        assertThat(listenerCalledCount).isEqualTo(2)
        gem.setGroupExpanded(summary1, true)
        underTest.setGroupExpanded(summary2, true)
        assertThat(listenerCalledCount).isEqualTo(2)
        gem.setGroupExpanded(summary2, false)
        assertThat(listenerCalledCount).isEqualTo(3)
    }

    @Test
    fun testNotifyOnlyOnChange_disabled() {
        featureFlags.set(Flags.NOTIFICATION_GROUP_EXPANSION_CHANGE, false)

        var listenerCalledCount = 0
        gem.registerGroupExpansionChangeListener { _, _ -> listenerCalledCount++ }

        gem.setGroupExpanded(summary1, false)
        assertThat(listenerCalledCount).isEqualTo(1)
        gem.setGroupExpanded(summary1, true)
        underTest.setGroupExpanded(summary1, true)
        assertThat(listenerCalledCount).isEqualTo(2)
        gem.setGroupExpanded(summary2, true)
        underTest.setGroupExpanded(summary2, false)
        assertThat(listenerCalledCount).isEqualTo(3)
        gem.setGroupExpanded(summary1, true)
        assertThat(listenerCalledCount).isEqualTo(4)
        gem.setGroupExpanded(summary2, false)
        assertThat(listenerCalledCount).isEqualTo(5)
    }

    @Test
    fun testExpandUnattachedEntry() {
        featureFlags.set(Flags.NOTIFICATION_GROUP_EXPANSION_CHANGE, true)

    fun expandUnattachedEntry() {
        // First, expand the entry when it is attached.
        gem.setGroupExpanded(summary1, true)
        assertThat(gem.isGroupExpanded(summary1)).isTrue()
        underTest.setGroupExpanded(summary1, true)
        assertThat(underTest.isGroupExpanded(summary1)).isTrue()

        // Un-attach it, and un-expand it.
        NotificationEntryBuilder.setNewParent(summary1, null)
        gem.setGroupExpanded(summary1, false)
        underTest.setGroupExpanded(summary1, false)

        // Expanding again should throw.
        assertThrows(IllegalArgumentException::class.java) { gem.setGroupExpanded(summary1, true) }
        assertThrows(IllegalArgumentException::class.java) {
            underTest.setGroupExpanded(summary1, true)
        }
    }

    @Test
    fun testSyncWithPipeline() {
        featureFlags.set(Flags.NOTIFICATION_GROUP_EXPANSION_CHANGE, true)
        gem.attach(pipeline)
    fun syncWithPipeline() {
        underTest.attach(pipeline)
        beforeRenderListListener = withArgCaptor {
            verify(pipeline).addOnBeforeRenderListListener(capture())
        }

        val listener: OnGroupExpansionChangeListener = mock()
        gem.registerGroupExpansionChangeListener(listener)
        underTest.registerGroupExpansionChangeListener(listener)

        beforeRenderListListener.onBeforeRenderList(entries)
        verify(listener, never()).onGroupExpansionChange(any(), any())

        // Expand one of the groups.
        gem.setGroupExpanded(summary1, true)
        underTest.setGroupExpanded(summary1, true)
        verify(listener).onGroupExpansionChange(summary1.row, true)

        // Empty the pipeline list and verify that the group is no longer expanded.
+22 −71
Original line number Diff line number Diff line
@@ -16,67 +16,35 @@

package com.android.systemui.statusbar.notification.collection.render

import androidx.test.ext.junit.runners.AndroidJUnit4
import androidx.test.filters.SmallTest
import com.android.systemui.SysuiTestCase
import com.android.systemui.flags.FakeFeatureFlagsClassic
import com.android.systemui.flags.Flags
import com.android.systemui.statusbar.notification.collection.GroupEntry
import com.android.systemui.statusbar.notification.collection.GroupEntryBuilder
import com.android.systemui.statusbar.notification.collection.NotificationEntryBuilder
import com.google.common.truth.Truth.assertThat
import org.junit.Before
import org.junit.Test
import org.junit.runner.RunWith

@SmallTest
@RunWith(AndroidJUnit4::class)
class GroupMembershipManagerTest : SysuiTestCase() {
    private lateinit var gmm: GroupMembershipManagerImpl

    private val featureFlags = FakeFeatureFlagsClassic()

    @Before
    fun setUp() {
        gmm = GroupMembershipManagerImpl(featureFlags)
    }
    private var underTest = GroupMembershipManagerImpl()

    @Test
    fun testIsChildInGroup_topLevel() {
        featureFlags.set(Flags.NOTIFICATION_GROUP_EXPANSION_CHANGE, false)
    fun isChildInGroup_topLevel() {
        val topLevelEntry = NotificationEntryBuilder().setParent(GroupEntry.ROOT_ENTRY).build()
        assertThat(gmm.isChildInGroup(topLevelEntry)).isFalse()
    }

    @Test
    fun testIsChildInGroup_noParent_old() {
        featureFlags.set(Flags.NOTIFICATION_GROUP_EXPANSION_CHANGE, false)
        val noParentEntry = NotificationEntryBuilder().setParent(null).build()
        assertThat(gmm.isChildInGroup(noParentEntry)).isTrue()
        assertThat(underTest.isChildInGroup(topLevelEntry)).isFalse()
    }

    @Test
    fun testIsChildInGroup_noParent_new() {
        featureFlags.set(Flags.NOTIFICATION_GROUP_EXPANSION_CHANGE, true)
    fun isChildInGroup_noParent() {
        val noParentEntry = NotificationEntryBuilder().setParent(null).build()
        assertThat(gmm.isChildInGroup(noParentEntry)).isFalse()
    }
    @Test
    fun testIsChildInGroup_summary_old() {
        featureFlags.set(Flags.NOTIFICATION_GROUP_EXPANSION_CHANGE, false)

        val groupKey = "group"
        val summary =
            NotificationEntryBuilder()
                .setGroup(mContext, groupKey)
                .setGroupSummary(mContext, true)
                .build()
        GroupEntryBuilder().setKey(groupKey).setSummary(summary).build()

        assertThat(gmm.isChildInGroup(summary)).isTrue()
        assertThat(underTest.isChildInGroup(noParentEntry)).isFalse()
    }

    @Test
    fun testIsChildInGroup_summary_new() {
        featureFlags.set(Flags.NOTIFICATION_GROUP_EXPANSION_CHANGE, true)

    fun isChildInGroup_summary() {
        val groupKey = "group"
        val summary =
            NotificationEntryBuilder()
@@ -85,27 +53,17 @@ class GroupMembershipManagerTest : SysuiTestCase() {
                .build()
        GroupEntryBuilder().setKey(groupKey).setSummary(summary).build()

        assertThat(gmm.isChildInGroup(summary)).isFalse()
        assertThat(underTest.isChildInGroup(summary)).isFalse()
    }

    @Test
    fun testIsChildInGroup_child() {
        featureFlags.set(Flags.NOTIFICATION_GROUP_EXPANSION_CHANGE, false)
        val childEntry = NotificationEntryBuilder().build()
        assertThat(gmm.isChildInGroup(childEntry)).isTrue()
    }

    @Test
    fun testIsGroupSummary_topLevelEntry() {
        featureFlags.set(Flags.NOTIFICATION_GROUP_EXPANSION_CHANGE, true)
    fun isGroupSummary_topLevelEntry() {
        val entry = NotificationEntryBuilder().setParent(GroupEntry.ROOT_ENTRY).build()
        assertThat(gmm.isGroupSummary(entry)).isFalse()
        assertThat(underTest.isGroupSummary(entry)).isFalse()
    }

    @Test
    fun testIsGroupSummary_summary() {
        featureFlags.set(Flags.NOTIFICATION_GROUP_EXPANSION_CHANGE, true)

    fun isGroupSummary_summary() {
        val groupKey = "group"
        val summary =
            NotificationEntryBuilder()
@@ -114,13 +72,11 @@ class GroupMembershipManagerTest : SysuiTestCase() {
                .build()
        GroupEntryBuilder().setKey(groupKey).setSummary(summary).build()

        assertThat(gmm.isGroupSummary(summary)).isTrue()
        assertThat(underTest.isGroupSummary(summary)).isTrue()
    }

    @Test
    fun testIsGroupSummary_child() {
        featureFlags.set(Flags.NOTIFICATION_GROUP_EXPANSION_CHANGE, true)

    fun isGroupSummary_child() {
        val groupKey = "group"
        val summary =
            NotificationEntryBuilder()
@@ -130,20 +86,17 @@ class GroupMembershipManagerTest : SysuiTestCase() {
        val entry = NotificationEntryBuilder().setGroup(mContext, groupKey).build()
        GroupEntryBuilder().setKey(groupKey).setSummary(summary).addChild(entry).build()

        assertThat(gmm.isGroupSummary(entry)).isFalse()
        assertThat(underTest.isGroupSummary(entry)).isFalse()
    }

    @Test
    fun testGetGroupSummary_topLevelEntry() {
        featureFlags.set(Flags.NOTIFICATION_GROUP_EXPANSION_CHANGE, true)
    fun getGroupSummary_topLevelEntry() {
        val entry = NotificationEntryBuilder().setParent(GroupEntry.ROOT_ENTRY).build()
        assertThat(gmm.getGroupSummary(entry)).isNull()
        assertThat(underTest.getGroupSummary(entry)).isNull()
    }

    @Test
    fun testGetGroupSummary_summary() {
        featureFlags.set(Flags.NOTIFICATION_GROUP_EXPANSION_CHANGE, true)

    fun getGroupSummary_summary() {
        val groupKey = "group"
        val summary =
            NotificationEntryBuilder()
@@ -152,13 +105,11 @@ class GroupMembershipManagerTest : SysuiTestCase() {
                .build()
        GroupEntryBuilder().setKey(groupKey).setSummary(summary).build()

        assertThat(gmm.getGroupSummary(summary)).isEqualTo(summary)
        assertThat(underTest.getGroupSummary(summary)).isEqualTo(summary)
    }

    @Test
    fun testGetGroupSummary_child() {
        featureFlags.set(Flags.NOTIFICATION_GROUP_EXPANSION_CHANGE, true)

    fun getGroupSummary_child() {
        val groupKey = "group"
        val summary =
            NotificationEntryBuilder()
@@ -168,6 +119,6 @@ class GroupMembershipManagerTest : SysuiTestCase() {
        val entry = NotificationEntryBuilder().setGroup(mContext, groupKey).build()
        GroupEntryBuilder().setKey(groupKey).setSummary(summary).addChild(entry).build()

        assertThat(gmm.getGroupSummary(entry)).isEqualTo(summary)
        assertThat(underTest.getGroupSummary(entry)).isEqualTo(summary)
    }
}
+0 −6
Original line number Diff line number Diff line
@@ -102,12 +102,6 @@ object Flags {
            default = true
        )

    /** Only notify group expansion listeners when a change happens. */
    // TODO(b/292213543): Tracking Bug
    @JvmField
    val NOTIFICATION_GROUP_EXPANSION_CHANGE =
            releasedFlag("notification_group_expansion_change")

    // TODO(b/301955929)
    @JvmField
    val NOTIF_LS_BACKGROUND_THREAD =
+5 −13
Original line number Diff line number Diff line
@@ -21,8 +21,6 @@ import androidx.annotation.NonNull;
import com.android.systemui.Dumpable;
import com.android.systemui.dagger.SysUISingleton;
import com.android.systemui.dump.DumpManager;
import com.android.systemui.flags.FeatureFlags;
import com.android.systemui.flags.Flags;
import com.android.systemui.statusbar.notification.collection.GroupEntry;
import com.android.systemui.statusbar.notification.collection.ListEntry;
import com.android.systemui.statusbar.notification.collection.NotifPipeline;
@@ -53,14 +51,11 @@ public class GroupExpansionManagerImpl implements GroupExpansionManager, Dumpabl
     */
    private final Set<NotificationEntry> mExpandedGroups = new HashSet<>();

    private final FeatureFlags mFeatureFlags;

    @Inject
    public GroupExpansionManagerImpl(DumpManager dumpManager,
            GroupMembershipManager groupMembershipManager, FeatureFlags featureFlags) {
            GroupMembershipManager groupMembershipManager) {
        mDumpManager = dumpManager;
        mGroupMembershipManager = groupMembershipManager;
        mFeatureFlags = featureFlags;
    }

    /**
@@ -86,11 +81,9 @@ public class GroupExpansionManagerImpl implements GroupExpansionManager, Dumpabl
    };

    public void attach(NotifPipeline pipeline) {
        if (mFeatureFlags.isEnabled(Flags.NOTIFICATION_GROUP_EXPANSION_CHANGE)) {
        mDumpManager.registerDumpable(this);
        pipeline.addOnBeforeRenderListListener(mNotifTracker);
    }
    }

    @Override
    public void registerGroupExpansionChangeListener(OnGroupExpansionChangeListener listener) {
@@ -105,8 +98,7 @@ public class GroupExpansionManagerImpl implements GroupExpansionManager, Dumpabl
    @Override
    public void setGroupExpanded(NotificationEntry entry, boolean expanded) {
        NotificationEntry groupSummary = mGroupMembershipManager.getGroupSummary(entry);
        if (mFeatureFlags.isEnabled(Flags.NOTIFICATION_GROUP_EXPANSION_CHANGE)
                && entry.getParent() == null) {
        if (entry.getParent() == null) {
            if (expanded) {
                throw new IllegalArgumentException("Cannot expand group that is not attached");
            } else {
@@ -124,7 +116,7 @@ public class GroupExpansionManagerImpl implements GroupExpansionManager, Dumpabl
        }

        // Only notify listeners if something changed.
        if (!mFeatureFlags.isEnabled(Flags.NOTIFICATION_GROUP_EXPANSION_CHANGE) || changed) {
        if (changed) {
            sendOnGroupExpandedChange(entry, expanded);
        }
    }
+8 −22
Original line number Diff line number Diff line
@@ -22,8 +22,6 @@ import androidx.annotation.NonNull;
import androidx.annotation.Nullable;

import com.android.systemui.dagger.SysUISingleton;
import com.android.systemui.flags.FeatureFlagsClassic;
import com.android.systemui.flags.Flags;
import com.android.systemui.statusbar.notification.collection.GroupEntry;
import com.android.systemui.statusbar.notification.collection.ListEntry;
import com.android.systemui.statusbar.notification.collection.NotificationEntry;
@@ -38,25 +36,17 @@ import javax.inject.Inject;
 */
@SysUISingleton
public class GroupMembershipManagerImpl implements GroupMembershipManager {
    FeatureFlagsClassic mFeatureFlags;

    @Inject
    public GroupMembershipManagerImpl(FeatureFlagsClassic featureFlags) {
        mFeatureFlags = featureFlags;
    }
    public GroupMembershipManagerImpl() {}

    @Override
    public boolean isGroupSummary(@NonNull NotificationEntry entry) {
        if (mFeatureFlags.isEnabled(Flags.NOTIFICATION_GROUP_EXPANSION_CHANGE)) {
        if (entry.getParent() == null) {
            // The entry is not attached, so it doesn't count.
            return false;
        }
        // If entry is a summary, its parent is a GroupEntry with summary = entry.
        return entry.getParent().getSummary() == entry;
        } else {
            return getGroupSummary(entry) == entry;
        }
    }

    @Nullable
@@ -70,12 +60,8 @@ public class GroupMembershipManagerImpl implements GroupMembershipManager {

    @Override
    public boolean isChildInGroup(@NonNull NotificationEntry entry) {
        if (mFeatureFlags.isEnabled(Flags.NOTIFICATION_GROUP_EXPANSION_CHANGE)) {
        // An entry is a child if it's not a summary or top level entry, but it is attached.
        return !isGroupSummary(entry) && !isTopLevelEntry(entry) && entry.getParent() != null;
        } else {
            return !isTopLevelEntry(entry);
        }
    }

    @Override
Loading