diff options
5 files changed, 37 insertions, 119 deletions
diff --git a/packages/SystemUI/customization/src/com/android/systemui/shared/clocks/ClockRegistry.kt b/packages/SystemUI/customization/src/com/android/systemui/shared/clocks/ClockRegistry.kt index 0fa4ebd69bf2..dc93400e6581 100644 --- a/packages/SystemUI/customization/src/com/android/systemui/shared/clocks/ClockRegistry.kt +++ b/packages/SystemUI/customization/src/com/android/systemui/shared/clocks/ClockRegistry.kt @@ -51,15 +51,18 @@ import kotlinx.coroutines.withContext private val KEY_TIMESTAMP = "appliedTimestamp" private val KNOWN_PLUGINS = mapOf<String, List<ClockMetadata>>( - "com.android.systemui.falcon.one" to listOf(ClockMetadata("ANALOG_CLOCK_BIGNUM")), - "com.android.systemui.falcon.two" to listOf(ClockMetadata("DIGITAL_CLOCK_CALLIGRAPHY")), - "com.android.systemui.falcon.three" to listOf(ClockMetadata("DIGITAL_CLOCK_FLEX")), - "com.android.systemui.falcon.four" to listOf(ClockMetadata("DIGITAL_CLOCK_GROWTH")), - "com.android.systemui.falcon.five" to listOf(ClockMetadata("DIGITAL_CLOCK_HANDWRITTEN")), - "com.android.systemui.falcon.six" to listOf(ClockMetadata("DIGITAL_CLOCK_INFLATE")), - "com.android.systemui.falcon.seven" to listOf(ClockMetadata("DIGITAL_CLOCK_METRO")), - "com.android.systemui.falcon.eight" to listOf(ClockMetadata("DIGITAL_CLOCK_NUMBEROVERLAP")), - "com.android.systemui.falcon.nine" to listOf(ClockMetadata("DIGITAL_CLOCK_WEATHER")), + "com.android.systemui.clocks.bignum" to listOf(ClockMetadata("ANALOG_CLOCK_BIGNUM")), + "com.android.systemui.clocks.calligraphy" to + listOf(ClockMetadata("DIGITAL_CLOCK_CALLIGRAPHY")), + "com.android.systemui.clocks.flex" to listOf(ClockMetadata("DIGITAL_CLOCK_FLEX")), + "com.android.systemui.clocks.growth" to listOf(ClockMetadata("DIGITAL_CLOCK_GROWTH")), + "com.android.systemui.clocks.handwritten" to + listOf(ClockMetadata("DIGITAL_CLOCK_HANDWRITTEN")), + "com.android.systemui.clocks.inflate" to listOf(ClockMetadata("DIGITAL_CLOCK_INFLATE")), + "com.android.systemui.clocks.metro" to listOf(ClockMetadata("DIGITAL_CLOCK_METRO")), + "com.android.systemui.clocks.numoverlap" to + listOf(ClockMetadata("DIGITAL_CLOCK_NUMBEROVERLAP")), + "com.android.systemui.clocks.weather" to listOf(ClockMetadata("DIGITAL_CLOCK_WEATHER")), ) private fun <TKey, TVal> ConcurrentHashMap<TKey, TVal>.concurrentGetOrPut( diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/StackCoordinator.kt b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/StackCoordinator.kt index 62a0d138fd05..5c2f9a8d28ec 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/StackCoordinator.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/StackCoordinator.kt @@ -39,7 +39,10 @@ class StackCoordinator @Inject internal constructor( override fun attach(pipeline: NotifPipeline) { pipeline.addOnAfterRenderListListener(::onAfterRenderList) - groupExpansionManagerImpl.attach(pipeline) + // 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) } fun onAfterRenderList(entries: List<ListEntry>, controller: NotifStackController) = diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/render/GroupExpansionManagerImpl.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/render/GroupExpansionManagerImpl.java index 5d33804ab6a7..46af03a438f5 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/render/GroupExpansionManagerImpl.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/render/GroupExpansionManagerImpl.java @@ -67,29 +67,18 @@ 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()); } } - - // 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)); - } + mExpandedGroups.removeIf(expandedGroup -> !renderingSummaries.contains(expandedGroup)); }; public void attach(NotifPipeline pipeline) { - if (mFeatureFlags.isEnabled(Flags.NOTIFICATION_GROUP_EXPANSION_CHANGE)) { - mDumpManager.registerDumpable(this); - pipeline.addOnBeforeRenderListListener(mNotifTracker); - } + mDumpManager.registerDumpable(this); + pipeline.addOnBeforeRenderListListener(mNotifTracker); } @Override @@ -105,24 +94,11 @@ 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(entry); + changed = mExpandedGroups.add(groupSummary); } else { - changed = mExpandedGroups.remove(entry); + changed = mExpandedGroups.remove(groupSummary); } // Only notify listeners if something changed. diff --git a/packages/SystemUI/tests/src/com/android/systemui/shared/clocks/ClockRegistryTest.kt b/packages/SystemUI/tests/src/com/android/systemui/shared/clocks/ClockRegistryTest.kt index ecaf13711b52..48665fe0c9b0 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/shared/clocks/ClockRegistryTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/shared/clocks/ClockRegistryTest.kt @@ -340,9 +340,9 @@ class ClockRegistryTest : SysuiTestCase() { @Test fun knownPluginAttached_clockAndListChanged_notLoaded() { val mockPluginLifecycle1 = mock<PluginLifecycleManager<ClockProviderPlugin>>() - whenever(mockPluginLifecycle1.getPackage()).thenReturn("com.android.systemui.falcon.one") + whenever(mockPluginLifecycle1.getPackage()).thenReturn("com.android.systemui.clocks.metro") val mockPluginLifecycle2 = mock<PluginLifecycleManager<ClockProviderPlugin>>() - whenever(mockPluginLifecycle2.getPackage()).thenReturn("com.android.systemui.falcon.two") + whenever(mockPluginLifecycle2.getPackage()).thenReturn("com.android.systemui.clocks.bignum") var changeCallCount = 0 var listChangeCallCount = 0 diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/render/GroupExpansionManagerTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/render/GroupExpansionManagerTest.kt index 38a8f414b0fb..4a94dc819a9e 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/render/GroupExpansionManagerTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/render/GroupExpansionManagerTest.kt @@ -21,21 +21,11 @@ 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 @@ -46,43 +36,13 @@ class GroupExpansionManagerTest : SysuiTestCase() { private val groupMembershipManager: GroupMembershipManager = mock() private val featureFlags = FakeFeatureFlags() - 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() } + private val entry1 = NotificationEntryBuilder().build() + private val entry2 = NotificationEntryBuilder().build() @Before fun setUp() { - whenever(groupMembershipManager.getGroupSummary(summary1)).thenReturn(summary1) - whenever(groupMembershipManager.getGroupSummary(summary2)).thenReturn(summary2) + whenever(groupMembershipManager.getGroupSummary(entry1)).thenReturn(entry1) + whenever(groupMembershipManager.getGroupSummary(entry2)).thenReturn(entry2) gem = GroupExpansionManagerImpl(dumpManager, groupMembershipManager, featureFlags) } @@ -94,15 +54,15 @@ class GroupExpansionManagerTest : SysuiTestCase() { var listenerCalledCount = 0 gem.registerGroupExpansionChangeListener { _, _ -> listenerCalledCount++ } - gem.setGroupExpanded(summary1, false) + gem.setGroupExpanded(entry1, false) Assert.assertEquals(0, listenerCalledCount) - gem.setGroupExpanded(summary1, true) + gem.setGroupExpanded(entry1, true) Assert.assertEquals(1, listenerCalledCount) - gem.setGroupExpanded(summary2, true) + gem.setGroupExpanded(entry2, true) Assert.assertEquals(2, listenerCalledCount) - gem.setGroupExpanded(summary1, true) + gem.setGroupExpanded(entry1, true) Assert.assertEquals(2, listenerCalledCount) - gem.setGroupExpanded(summary2, false) + gem.setGroupExpanded(entry2, false) Assert.assertEquals(3, listenerCalledCount) } @@ -113,39 +73,15 @@ class GroupExpansionManagerTest : SysuiTestCase() { var listenerCalledCount = 0 gem.registerGroupExpansionChangeListener { _, _ -> listenerCalledCount++ } - gem.setGroupExpanded(summary1, false) + gem.setGroupExpanded(entry1, false) Assert.assertEquals(1, listenerCalledCount) - gem.setGroupExpanded(summary1, true) + gem.setGroupExpanded(entry1, true) Assert.assertEquals(2, listenerCalledCount) - gem.setGroupExpanded(summary2, true) + gem.setGroupExpanded(entry2, true) Assert.assertEquals(3, listenerCalledCount) - gem.setGroupExpanded(summary1, true) + gem.setGroupExpanded(entry1, true) Assert.assertEquals(4, listenerCalledCount) - gem.setGroupExpanded(summary2, false) + gem.setGroupExpanded(entry2, 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) - } } |