Merge "Notify listeners when syncing group expansion mgr with the pipeline." into udc-qpr-dev (f66b76fb) · Commits · e / os / android_frameworks_base

packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/StackCoordinator.kt

+1 −4

Original line number	Diff line number	Diff line
		@@ -39,10 +39,7 @@ class StackCoordinator @Inject internal constructor(

		override fun attach(pipeline: NotifPipeline) {
		pipeline.addOnAfterRenderListListener(::onAfterRenderList)
		// TODO(b/282865576): This has an issue where it makes changes to some groups without
		// notifying listeners. To be fixed in QPR, but for now let's comment it out to avoid the
		// group expansion bug.
		// groupExpansionManagerImpl.attach(pipeline)
		groupExpansionManagerImpl.attach(pipeline)
		}

		fun onAfterRenderList(entries: List<ListEntry>, controller: NotifStackController) =

packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/render/GroupExpansionManagerImpl.java

+29 −5

Original line number	Diff line number	Diff line
		@@ -67,19 +67,30 @@ public class GroupExpansionManagerImpl implements GroupExpansionManager, Dumpabl
		* Cleanup entries from mExpandedGroups that no longer exist in the pipeline.
		*/
		private final OnBeforeRenderListListener mNotifTracker = (entries) -> {
		if (mExpandedGroups.isEmpty()) {
		return; // nothing to do
		}

		final Set<NotificationEntry> renderingSummaries = new HashSet<>();
		for (ListEntry entry : entries) {
		if (entry instanceof GroupEntry) {
		renderingSummaries.add(entry.getRepresentativeEntry());
		}
		}
		mExpandedGroups.removeIf(expandedGroup -> !renderingSummaries.contains(expandedGroup));

		// Create a copy of mExpandedGroups so we can modify it in a thread-safe way.
		final var currentExpandedGroups = new HashSet<>(mExpandedGroups);
		for (NotificationEntry entry : currentExpandedGroups) {
		setExpanded(entry, renderingSummaries.contains(entry));
		}
		};

		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) {
		@@ -94,11 +105,24 @@ public class GroupExpansionManagerImpl implements GroupExpansionManager, Dumpabl
		@Override
		public void setGroupExpanded(NotificationEntry entry, boolean expanded) {
		final NotificationEntry groupSummary = mGroupMembershipManager.getGroupSummary(entry);
		setExpanded(groupSummary, expanded);
		}

		/**
		* Add or remove {@code entry} to/from {@code mExpandedGroups} and notify listeners if
		* something changed. This assumes that {@code entry} is a group summary.
		* <p>
		* TODO(b/293434635): Currently, in spite of its docs,
		* {@code mGroupMembershipManager.getGroupSummary(entry)} returns null if {@code entry} is
		* already a summary. Instead of needing this helper method to bypass that, we probably want to
		* move this code back to {@code setGroupExpanded} and use that everywhere.
		*/
		private void setExpanded(NotificationEntry entry, boolean expanded) {
		boolean changed;
		if (expanded) {
		changed = mExpandedGroups.add(groupSummary);
		changed = mExpandedGroups.add(entry);
		} else {
		changed = mExpandedGroups.remove(groupSummary);
		changed = mExpandedGroups.remove(entry);
		}

		// Only notify listeners if something changed.

packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/render/GroupExpansionManagerTest.kt

+78 −14

Original line number	Diff line number	Diff line
		@@ -21,11 +21,21 @@ import com.android.systemui.SysuiTestCase
		import com.android.systemui.dump.DumpManager
		import com.android.systemui.flags.FakeFeatureFlags
		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
		import com.android.systemui.statusbar.notification.collection.NotificationEntryBuilder
		import com.android.systemui.statusbar.notification.collection.listbuilder.OnBeforeRenderListListener
		import com.android.systemui.statusbar.notification.collection.render.GroupExpansionManager.OnGroupExpansionChangeListener
		import com.android.systemui.util.mockito.any
		import com.android.systemui.util.mockito.mock
		import com.android.systemui.util.mockito.withArgCaptor
		import org.junit.Assert
		import org.junit.Before
		import org.junit.Test
		import org.mockito.Mockito.never
		import org.mockito.Mockito.verify
		import org.mockito.Mockito.verifyNoMoreInteractions
		import org.mockito.Mockito.`when` as whenever

		@SmallTest
		@@ -36,13 +46,43 @@ class GroupExpansionManagerTest : SysuiTestCase() {
		private val groupMembershipManager: GroupMembershipManager = mock()
		private val featureFlags = FakeFeatureFlags()

		private val entry1 = NotificationEntryBuilder().build()
		private val entry2 = NotificationEntryBuilder().build()
		private val pipeline: NotifPipeline = mock()
		private lateinit var beforeRenderListListener: OnBeforeRenderListListener

		private val summary1 = notificationEntry("foo", 1)
		private val summary2 = notificationEntry("bar", 1)
		private val entries =
		listOf<ListEntry>(
		GroupEntryBuilder()
		.setSummary(summary1)
		.setChildren(
		listOf(
		notificationEntry("foo", 2),
		notificationEntry("foo", 3),
		notificationEntry("foo", 4)
		)
		)
		.build(),
		GroupEntryBuilder()
		.setSummary(summary2)
		.setChildren(
		listOf(
		notificationEntry("bar", 2),
		notificationEntry("bar", 3),
		notificationEntry("bar", 4)
		)
		)
		.build(),
		notificationEntry("baz", 1)
		)

		private fun notificationEntry(pkg: String, id: Int) =
		NotificationEntryBuilder().setPkg(pkg).setId(id).build().apply { row = mock() }

		@Before
		fun setUp() {
		whenever(groupMembershipManager.getGroupSummary(entry1)).thenReturn(entry1)
		whenever(groupMembershipManager.getGroupSummary(entry2)).thenReturn(entry2)
		whenever(groupMembershipManager.getGroupSummary(summary1)).thenReturn(summary1)
		whenever(groupMembershipManager.getGroupSummary(summary2)).thenReturn(summary2)

		gem = GroupExpansionManagerImpl(dumpManager, groupMembershipManager, featureFlags)
		}
		@@ -54,15 +94,15 @@ class GroupExpansionManagerTest : SysuiTestCase() {
		var listenerCalledCount = 0
		gem.registerGroupExpansionChangeListener { _, _ -> listenerCalledCount++ }

		gem.setGroupExpanded(entry1, false)
		gem.setGroupExpanded(summary1, false)
		Assert.assertEquals(0, listenerCalledCount)
		gem.setGroupExpanded(entry1, true)
		gem.setGroupExpanded(summary1, true)
		Assert.assertEquals(1, listenerCalledCount)
		gem.setGroupExpanded(entry2, true)
		gem.setGroupExpanded(summary2, true)
		Assert.assertEquals(2, listenerCalledCount)
		gem.setGroupExpanded(entry1, true)
		gem.setGroupExpanded(summary1, true)
		Assert.assertEquals(2, listenerCalledCount)
		gem.setGroupExpanded(entry2, false)
		gem.setGroupExpanded(summary2, false)
		Assert.assertEquals(3, listenerCalledCount)
		}

		@@ -73,15 +113,39 @@ class GroupExpansionManagerTest : SysuiTestCase() {
		var listenerCalledCount = 0
		gem.registerGroupExpansionChangeListener { _, _ -> listenerCalledCount++ }

		gem.setGroupExpanded(entry1, false)
		gem.setGroupExpanded(summary1, false)
		Assert.assertEquals(1, listenerCalledCount)
		gem.setGroupExpanded(entry1, true)
		gem.setGroupExpanded(summary1, true)
		Assert.assertEquals(2, listenerCalledCount)
		gem.setGroupExpanded(entry2, true)
		gem.setGroupExpanded(summary2, true)
		Assert.assertEquals(3, listenerCalledCount)
		gem.setGroupExpanded(entry1, true)
		gem.setGroupExpanded(summary1, true)
		Assert.assertEquals(4, listenerCalledCount)
		gem.setGroupExpanded(entry2, false)
		gem.setGroupExpanded(summary2, false)
		Assert.assertEquals(5, listenerCalledCount)
		}

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

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

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

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

		// Empty the pipeline list and verify that the group is no longer expanded.
		beforeRenderListListener.onBeforeRenderList(emptyList())
		verify(listener).onGroupExpansionChange(summary1.row, false)
		verifyNoMoreInteractions(listener)
		}
		}