Merge "Add a BindEventManager to fix ConversationNotifications.kt"

2022-01-22 12:30:52 +00:00
parent f995a92f4a 8519a9cb82
commit 68d19ef260
7 changed files with 135 additions and 69 deletions
--- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/ConversationNotifications.kt
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/ConversationNotifications.kt
@@ -23,7 +23,6 @@ import android.graphics.drawable.AnimatedImageDrawable
 import android.os.Handler
 import android.service.notification.NotificationListenerService.Ranking
 import android.service.notification.NotificationListenerService.RankingMap
-import com.android.internal.statusbar.NotificationVisibility
 import com.android.internal.widget.ConversationLayout
 import com.android.internal.widget.MessagingImageMessage
 import com.android.internal.widget.MessagingLayout
@@ -31,6 +30,7 @@ import com.android.systemui.dagger.SysUISingleton
 import com.android.systemui.dagger.qualifiers.Main
 import com.android.systemui.plugins.statusbar.StatusBarStateController
 import com.android.systemui.statusbar.notification.collection.NotificationEntry
+import com.android.systemui.statusbar.notification.collection.inflation.BindEventManager
 import com.android.systemui.statusbar.notification.collection.legacy.NotificationGroupManagerLegacy
 import com.android.systemui.statusbar.notification.collection.notifcollection.CommonNotifCollection
 import com.android.systemui.statusbar.notification.collection.notifcollection.NotifCollectionListener
@@ -72,7 +72,8 @@ class ConversationNotificationProcessor @Inject constructor(
 */
@SysUISingleton
 class AnimatedImageNotificationManager @Inject constructor(
-    private val notificationEntryManager: NotificationEntryManager,
+    private val notifCollection: CommonNotifCollection,
+    private val bindEventManager: BindEventManager,
    private val headsUpManager: HeadsUpManager,
    private val statusBarStateController: StatusBarStateController
 ) {
@@ -83,33 +84,23 @@ class AnimatedImageNotificationManager @Inject constructor(
    fun bind() {
        headsUpManager.addListener(object : OnHeadsUpChangedListener {
            override fun onHeadsUpStateChanged(entry: NotificationEntry, isHeadsUp: Boolean) {
-                entry.row?.let { row ->
-                    updateAnimatedImageDrawables(row, animating = isHeadsUp || isStatusBarExpanded)
-                }
+                updateAnimatedImageDrawables(entry)
            }
        })
        statusBarStateController.addCallback(object : StatusBarStateController.StateListener {
            override fun onExpandedChanged(isExpanded: Boolean) {
                isStatusBarExpanded = isExpanded
-                notificationEntryManager.activeNotificationsForCurrentUser.forEach { entry ->
-                    entry.row?.let { row ->
-                        updateAnimatedImageDrawables(row, animating = isExpanded || row.isHeadsUp)
-                    }
-                }
+                notifCollection.allNotifs.forEach(::updateAnimatedImageDrawables)
            }
        })
-        notificationEntryManager.addNotificationEntryListener(object : NotificationEntryListener {
-            override fun onEntryInflated(entry: NotificationEntry) {
-                entry.row?.let { row ->
-                    updateAnimatedImageDrawables(
-                            row,
-                            animating = isStatusBarExpanded || row.isHeadsUp)
-                }
-            }
-            override fun onEntryReinflated(entry: NotificationEntry) = onEntryInflated(entry)
-        })
+        bindEventManager.addListener(::updateAnimatedImageDrawables)
    }

+    private fun updateAnimatedImageDrawables(entry: NotificationEntry) =
+        entry.row?.let { row ->
+            updateAnimatedImageDrawables(row, animating = row.isHeadsUp || isStatusBarExpanded)
+        }
+
    private fun updateAnimatedImageDrawables(row: ExpandableNotificationRow, animating: Boolean) =
            (row.layouts?.asSequence() ?: emptySequence())
                    .flatMap { layout -> layout.allViews.asSequence() }
@@ -138,7 +129,7 @@ class AnimatedImageNotificationManager @Inject constructor(
 */
@SysUISingleton
 class ConversationNotificationManager @Inject constructor(
-    private val notificationEntryManager: NotificationEntryManager,
+    private val bindEventManager: BindEventManager,
    private val notificationGroupManager: NotificationGroupManagerLegacy,
    private val context: Context,
    private val notifCollection: CommonNotifCollection,
@@ -151,35 +142,12 @@ class ConversationNotificationManager @Inject constructor(

    private var notifPanelCollapsed = true

-    private val entryManagerListener = object : NotificationEntryListener {
-        override fun onNotificationRankingUpdated(rankingMap: RankingMap) =
-                updateNotificationRanking(rankingMap)
-        override fun onEntryInflated(entry: NotificationEntry) =
-                onEntryViewBound(entry)
-        override fun onEntryReinflated(entry: NotificationEntry) = onEntryInflated(entry)
-        override fun onEntryRemoved(
-            entry: NotificationEntry,
-            visibility: NotificationVisibility?,
-            removedByUser: Boolean,
-            reason: Int
-        ) = removeTrackedEntry(entry)
-    }
-
-    private val notifCollectionListener = object : NotifCollectionListener {
-        override fun onRankingUpdate(ranking: RankingMap) =
-                updateNotificationRanking(ranking)
-
-        override fun onEntryRemoved(entry: NotificationEntry, reason: Int) {
-            removeTrackedEntry(entry)
-        }
-    }
-
    private fun updateNotificationRanking(rankingMap: RankingMap) {
        fun getLayouts(view: NotificationContentView) =
                sequenceOf(view.contractedChild, view.expandedChild, view.headsUpChild)
        val ranking = Ranking()
        val activeConversationEntries = states.keys.asSequence()
-                .mapNotNull { notificationEntryManager.getActiveNotificationUnfiltered(it) }
+                .mapNotNull { notifCollection.getEntry(it) }
        for (entry in activeConversationEntries) {
            if (rankingMap.getRanking(entry.sbn.key, ranking) && ranking.isConversation) {
                val important = ranking.channel.isImportantConversation
@@ -204,7 +172,7 @@ class ConversationNotificationManager @Inject constructor(
                                layout.setIsImportantConversation(important, false)
                            }
                        }
-                if (changed) {
+                if (changed && !featureFlags.isNewPipelineEnabled()) {
                    notificationGroupManager.updateIsolation(entry)
                }
            }
@@ -233,11 +201,14 @@ class ConversationNotificationManager @Inject constructor(
    }

    init {
-        if (featureFlags.isNewPipelineEnabled()) {
-            notifCollection.addCollectionListener(notifCollectionListener)
-        } else {
-            notificationEntryManager.addNotificationEntryListener(entryManagerListener)
-        }
+        notifCollection.addCollectionListener(object : NotifCollectionListener {
+            override fun onRankingUpdate(ranking: RankingMap) =
+                updateNotificationRanking(ranking)
+
+            override fun onEntryRemoved(entry: NotificationEntry, reason: Int) =
+                removeTrackedEntry(entry)
+        })
+        bindEventManager.addListener(::onEntryViewBound)
    }

    private fun ConversationState.shouldIncrementUnread(newBuilder: Notification.Builder) =
@@ -265,11 +236,10 @@ class ConversationNotificationManager @Inject constructor(
        val expanded = states
                .asSequence()
                .mapNotNull { (key, _) ->
-                    notificationEntryManager.getActiveNotificationUnfiltered(key)
-                            ?.let { entry ->
-                                if (entry.row?.isExpanded == true) key to entry
-                                else null
-                            }
+                    notifCollection.getEntry(key)?.let { entry ->
+                        if (entry.row?.isExpanded == true) key to entry
+                        else null
+                    }
                }
                .toMap()
        states.replaceAll { key, state ->
--- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/PreparationCoordinator.java
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/PreparationCoordinator.java
@@ -31,13 +31,13 @@ import androidx.annotation.Nullable;

 import com.android.internal.annotations.VisibleForTesting;
 import com.android.internal.statusbar.IStatusBarService;
-import com.android.systemui.statusbar.notification.ConversationNotificationManager;
 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.NotificationEntry;
 import com.android.systemui.statusbar.notification.collection.ShadeListBuilder;
 import com.android.systemui.statusbar.notification.collection.coordinator.dagger.CoordinatorScope;
+import com.android.systemui.statusbar.notification.collection.inflation.BindEventManagerImpl;
 import com.android.systemui.statusbar.notification.collection.inflation.NotifInflater;
 import com.android.systemui.statusbar.notification.collection.inflation.NotifUiAdjustment;
 import com.android.systemui.statusbar.notification.collection.inflation.NotifUiAdjustmentProvider;
@@ -99,7 +99,7 @@ public class PreparationCoordinator implements Coordinator {

    /** How long we can delay a group while waiting for all children to inflate */
    private final long mMaxGroupInflationDelay;
-    private final ConversationNotificationManager mConversationManager;
+    private final BindEventManagerImpl mBindEventManager;

    @Inject
    public PreparationCoordinator(
@@ -109,7 +109,7 @@ public class PreparationCoordinator implements Coordinator {
            NotifViewBarn viewBarn,
            NotifUiAdjustmentProvider adjustmentProvider,
            IStatusBarService service,
-            ConversationNotificationManager conversationManager) {
+            BindEventManagerImpl bindEventManager) {
        this(
                logger,
                notifInflater,
@@ -117,7 +117,7 @@ public class PreparationCoordinator implements Coordinator {
                viewBarn,
                adjustmentProvider,
                service,
-                conversationManager,
+                bindEventManager,
                CHILD_BIND_CUTOFF,
                MAX_GROUP_INFLATION_DELAY);
    }
@@ -130,7 +130,7 @@ public class PreparationCoordinator implements Coordinator {
            NotifViewBarn viewBarn,
            NotifUiAdjustmentProvider adjustmentProvider,
            IStatusBarService service,
-            ConversationNotificationManager conversationManager,
+            BindEventManagerImpl bindEventManager,
            int childBindCutoff,
            long maxGroupInflationDelay) {
        mLogger = logger;
@@ -141,7 +141,7 @@ public class PreparationCoordinator implements Coordinator {
        mStatusBarService = service;
        mChildBindCutoff = childBindCutoff;
        mMaxGroupInflationDelay = maxGroupInflationDelay;
-        mConversationManager = conversationManager;
+        mBindEventManager = bindEventManager;
    }

    @Override
@@ -369,9 +369,7 @@ public class PreparationCoordinator implements Coordinator {
        mInflatingNotifs.remove(entry);
        mViewBarn.registerViewForEntry(entry, controller);
        mInflationStates.put(entry, STATE_INFLATED);
-        // NOTE: under the new pipeline there's no way to register for an inflation callback,
-        // so this one method is called by the PreparationCoordinator directly.
-        mConversationManager.onEntryViewBound(entry);
+        mBindEventManager.notifyViewBound(entry);
        mNotifInflatingFilter.invalidateList();
    }

--- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/inflation/BindEventManager.kt
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/inflation/BindEventManager.kt
@@ -0,0 +1,45 @@
+/*
+ * Copyright (C) 2022 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.inflation
+
+import com.android.systemui.statusbar.notification.collection.NotificationEntry
+import com.android.systemui.util.ListenerSet
+
+/**
+ * Helper class that allows distributing bind events regardless of the pipeline.
+ *
+ * NOTE: This class isn't ideal; this exposes the concept of view inflation as something that can be
+ * globally registered for. This is built as it is to provide compatibility with patterns developed
+ * for the legacy pipeline. Ideally we'd have functionality that needs to know this information be
+ * handled by events that go through the ViewController itself.
+ */
+open class BindEventManager {
+    protected val listeners = ListenerSet<Listener>()
+
+    /** Register a listener */
+    fun addListener(listener: Listener) =
+        listeners.addIfAbsent(listener)
+
+    /** Deregister a listener */
+    fun removeListener(listener: Listener) =
+        listeners.remove(listener)
+
+    /** Listener interface for view bind events */
+    fun interface Listener {
+        fun onViewBound(entry: NotificationEntry)
+    }
+}
--- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/inflation/BindEventManagerImpl.kt
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/inflation/BindEventManagerImpl.kt
@@ -0,0 +1,42 @@
+/*
+ * Copyright (C) 2022 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.inflation
+
+import com.android.systemui.dagger.SysUISingleton
+import com.android.systemui.statusbar.notification.NotificationEntryListener
+import com.android.systemui.statusbar.notification.NotificationEntryManager
+import com.android.systemui.statusbar.notification.collection.NotificationEntry
+import com.android.systemui.statusbar.notification.collection.inflation.BindEventManager.Listener
+import javax.inject.Inject
+
+/**
+ * Helper class that allows distributing bind events regardless of the pipeline.
+ */
+@SysUISingleton
+class BindEventManagerImpl @Inject constructor() : BindEventManager() {
+    /** Emit the [Listener.onViewBound] event to all registered listeners. */
+    fun notifyViewBound(entry: NotificationEntry) =
+        listeners.forEach { listener -> listener.onViewBound(entry) }
+
+    /** Initialize this for the legacy pipeline. */
+    fun attachToLegacyPipeline(notificationEntryManager: NotificationEntryManager) {
+        notificationEntryManager.addNotificationEntryListener(object : NotificationEntryListener {
+            override fun onEntryInflated(entry: NotificationEntry) = notifyViewBound(entry)
+            override fun onEntryReinflated(entry: NotificationEntry) = notifyViewBound(entry)
+        })
+    }
+}
--- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/dagger/NotificationsModule.java
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/dagger/NotificationsModule.java
@@ -50,6 +50,8 @@ import com.android.systemui.statusbar.notification.collection.NotifPipeline;
 import com.android.systemui.statusbar.notification.collection.coordinator.ShadeEventCoordinator;
 import com.android.systemui.statusbar.notification.collection.coordinator.VisualStabilityCoordinator;
 import com.android.systemui.statusbar.notification.collection.coordinator.dagger.CoordinatorsModule;
+import com.android.systemui.statusbar.notification.collection.inflation.BindEventManager;
+import com.android.systemui.statusbar.notification.collection.inflation.BindEventManagerImpl;
 import com.android.systemui.statusbar.notification.collection.inflation.NotifInflater;
 import com.android.systemui.statusbar.notification.collection.inflation.NotificationRowBinder;
 import com.android.systemui.statusbar.notification.collection.inflation.OnUserInteractionCallbackImpl;
@@ -356,6 +358,10 @@ public interface NotificationsModule {
    @Binds
    NotifInflater bindNotifInflater(NotifInflaterImpl notifInflaterImpl);

+    /** */
+    @Binds
+    BindEventManager bindBindEventManagerImpl(BindEventManagerImpl bindEventManagerImpl);
+
    /** */
    @Binds
    NotifLiveDataStore bindNotifLiveDataStore(NotifLiveDataStoreImpl notifLiveDataStoreImpl);
--- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/init/NotificationsControllerImpl.kt
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/init/NotificationsControllerImpl.kt
@@ -32,6 +32,7 @@ import com.android.systemui.statusbar.notification.collection.NotifLiveDataStore
 import com.android.systemui.statusbar.notification.collection.NotifPipeline
 import com.android.systemui.statusbar.notification.collection.NotificationRankingManager
 import com.android.systemui.statusbar.notification.collection.TargetSdkResolver
+import com.android.systemui.statusbar.notification.collection.inflation.BindEventManagerImpl
 import com.android.systemui.statusbar.notification.collection.inflation.NotificationRowBinderImpl
 import com.android.systemui.statusbar.notification.collection.init.NotifPipelineInitializer
 import com.android.systemui.statusbar.notification.collection.legacy.NotificationGroupManagerLegacy
@@ -76,6 +77,7 @@ class NotificationsControllerImpl @Inject constructor(
    private val notifBindPipelineInitializer: NotifBindPipelineInitializer,
    private val deviceProvisionedController: DeviceProvisionedController,
    private val notificationRowBinder: NotificationRowBinderImpl,
+    private val bindEventManagerImpl: BindEventManagerImpl,
    private val remoteInputUriController: RemoteInputUriController,
    private val groupManagerLegacy: Lazy<NotificationGroupManagerLegacy>,
    private val groupAlertTransferHelper: NotificationGroupAlertTransferHelper,
@@ -131,6 +133,7 @@ class NotificationsControllerImpl @Inject constructor(
            targetSdkResolver.initialize(entryManager)
            remoteInputUriController.attach(entryManager)
            groupAlertTransferHelper.bind(entryManager, groupManagerLegacy.get())
+            bindEventManagerImpl.attachToLegacyPipeline(entryManager)
            headsUpManager.addListener(groupManagerLegacy.get())
            headsUpManager.addListener(groupAlertTransferHelper)
            headsUpController.attach(entryManager, headsUpManager)
--- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coordinator/PreparationCoordinatorTest.java
+++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coordinator/PreparationCoordinatorTest.java
@@ -26,6 +26,7 @@ import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyNoMoreInteractions;
 import static org.mockito.Mockito.when;

 import static java.util.Objects.requireNonNull;
@@ -40,7 +41,6 @@ import androidx.test.filters.SmallTest;
 import com.android.internal.statusbar.IStatusBarService;
 import com.android.systemui.SysuiTestCase;
 import com.android.systemui.statusbar.RankingBuilder;
-import com.android.systemui.statusbar.notification.ConversationNotificationManager;
 import com.android.systemui.statusbar.notification.SectionClassifier;
 import com.android.systemui.statusbar.notification.collection.GroupEntry;
 import com.android.systemui.statusbar.notification.collection.GroupEntryBuilder;
@@ -48,6 +48,7 @@ import com.android.systemui.statusbar.notification.collection.ListEntry;
 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.inflation.BindEventManagerImpl;
 import com.android.systemui.statusbar.notification.collection.inflation.NotifInflater;
 import com.android.systemui.statusbar.notification.collection.inflation.NotifUiAdjustmentProvider;
 import com.android.systemui.statusbar.notification.collection.listbuilder.NotifSection;
@@ -93,7 +94,7 @@ public class PreparationCoordinatorTest extends SysuiTestCase {
    @Mock private NotifSection mNotifSection;
    @Mock private NotifPipeline mNotifPipeline;
    @Mock private IStatusBarService mService;
-    @Mock private ConversationNotificationManager mConvoManager;
+    @Mock private BindEventManagerImpl mBindEventManagerImpl;
    @Spy private FakeNotifInflater mNotifInflater = new FakeNotifInflater();
    private final SectionClassifier mSectionClassifier = new SectionClassifier();
    private final NotifUiAdjustmentProvider mAdjustmentProvider =
@@ -121,7 +122,7 @@ public class PreparationCoordinatorTest extends SysuiTestCase {
                mock(NotifViewBarn.class),
                mAdjustmentProvider,
                mService,
-                mConvoManager,
+                mBindEventManagerImpl,
                TEST_CHILD_BIND_CUTOFF,
                TEST_MAX_GROUP_DELAY);

@@ -411,7 +412,8 @@ public class PreparationCoordinatorTest extends SysuiTestCase {
    public void testCallConversationManagerBindWhenInflated() {
        mBeforeFilterListener.onBeforeFinalizeFilter(List.of(mEntry));
        mNotifInflater.getInflateCallback(mEntry).onInflationFinished(mEntry, null);
-        verify(mConvoManager, times(1)).onEntryViewBound(eq(mEntry));
+        verify(mBindEventManagerImpl, times(1)).notifyViewBound(eq(mEntry));
+        verifyNoMoreInteractions(mBindEventManagerImpl);
    }

    @Test