From 3742322d040fb4a7c48eab7c9b3635a94a7e1e0c Mon Sep 17 00:00:00 2001 From: Kevin Han Date: Tue, 10 Mar 2020 16:05:07 -0700 Subject: [PATCH 1/2] Clean up NotificationRowBinderImpl Clean up a few things in NotificationRowBinderImpl *Remove a bunch of dead parameters and methodsd *Split logic into three main helper methods 1) bindRow - called once when row is made 2) updateRow - called to update row view state whenever the notification is updated 3) inflateContentViews - called to initiate row content inflation whenever notification is updated *Try to move other logic to appropriate places -Mainly move targetSdk resolution to be right after setting the sbn instead of after the row is inflated. This also makes icons neater to manage since we can immediately set the tag Bug: 145749521 Test: atest SystemUITests Change-Id: Id59a28c0fbbe1df5ffc4ec8eef58d760b8c6df05 --- .../NotificationEntryManager.java | 8 +- .../collection/NotifCollection.java | 3 + .../collection/NotificationEntry.java | 9 -- .../collection/TargetSdkResolver.kt | 56 +++++++++++ .../inflation/NotificationRowBinderImpl.java | 94 +++++++++---------- .../NotifCollectionListener.java | 10 ++ .../collection/notifcollection/NotifEvent.kt | 10 ++ .../notification/icon/IconManager.kt | 24 +---- .../ExpandableNotificationRowController.java | 3 - .../phone/StatusBarNotificationPresenter.java | 4 +- .../TestableNotificationEntryManager.kt | 9 +- ...NotificationEntryManagerInflationTest.java | 4 +- 12 files changed, 139 insertions(+), 95 deletions(-) create mode 100644 packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/TargetSdkResolver.kt diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/NotificationEntryManager.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/NotificationEntryManager.java index 9324b14bf2111..eb97169d24833 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/NotificationEntryManager.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/NotificationEntryManager.java @@ -558,6 +558,10 @@ public class NotificationEntryManager implements ranking, mFgsFeatureController.isForegroundServiceDismissalEnabled(), SystemClock.uptimeMillis()); + for (NotifCollectionListener listener : mNotifCollectionListeners) { + listener.onEntryBind(entry, notification); + } + mAllNotifications.add(entry); mLeakDetector.trackInstance(entry); @@ -612,7 +616,9 @@ public class NotificationEntryManager implements updateRankingAndSort(ranking, "updateNotificationInternal"); StatusBarNotification oldSbn = entry.getSbn(); entry.setSbn(notification); - mGroupManager.onEntryUpdated(entry, oldSbn); + for (NotifCollectionListener listener : mNotifCollectionListeners) { + listener.onEntryBind(entry, notification); + } mGroupManager.onEntryUpdated(entry, oldSbn); mLogger.logNotifUpdated(entry.getKey()); for (NotificationEntryListener listener : mNotificationEntryListeners) { diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotifCollection.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotifCollection.java index 365862bef871e..a3621b6cabf60 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotifCollection.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotifCollection.java @@ -65,6 +65,7 @@ import com.android.systemui.statusbar.FeatureFlags; import com.android.systemui.statusbar.notification.collection.coalescer.CoalescedEvent; import com.android.systemui.statusbar.notification.collection.coalescer.GroupCoalescer; import com.android.systemui.statusbar.notification.collection.coalescer.GroupCoalescer.BatchableNotificationHandler; +import com.android.systemui.statusbar.notification.collection.notifcollection.BindEntryEvent; import com.android.systemui.statusbar.notification.collection.notifcollection.CleanUpEntryEvent; import com.android.systemui.statusbar.notification.collection.notifcollection.CollectionReadyForBuildListener; import com.android.systemui.statusbar.notification.collection.notifcollection.DismissedByUserStats; @@ -389,6 +390,7 @@ public class NotifCollection implements Dumpable { if (entry == null) { // A new notification! entry = new NotificationEntry(sbn, ranking, SystemClock.uptimeMillis()); + mEventQueue.add(new BindEntryEvent(entry, sbn)); mNotificationSet.put(sbn.getKey(), entry); mLogger.logNotifPosted(sbn.getKey()); @@ -409,6 +411,7 @@ public class NotifCollection implements Dumpable { entry.mCancellationReason = REASON_NOT_CANCELED; entry.setSbn(sbn); + mEventQueue.add(new BindEntryEvent(entry, sbn)); mLogger.logNotifUpdated(sbn.getKey()); mEventQueue.add(new EntryUpdatedEvent(entry)); diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotificationEntry.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotificationEntry.java index 68ec34e907602..749c3e4c9d0d3 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotificationEntry.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotificationEntry.java @@ -408,15 +408,6 @@ public final class NotificationEntry extends ListEntry { return wasBubble != isBubble(); } - /** - * Resets the notification entry to be re-used. - */ - public void reset() { - if (row != null) { - row.reset(); - } - } - @NotificationSectionsManager.PriorityBucket public int getBucket() { return mBucket; diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/TargetSdkResolver.kt b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/TargetSdkResolver.kt new file mode 100644 index 0000000000000..1c1b2bb087f0a --- /dev/null +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/TargetSdkResolver.kt @@ -0,0 +1,56 @@ +/* + * Copyright (C) 2020 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 + +import android.content.Context +import android.content.pm.PackageManager +import android.service.notification.StatusBarNotification +import android.util.Log +import com.android.systemui.statusbar.notification.collection.notifcollection.CommonNotifCollection +import com.android.systemui.statusbar.notification.collection.notifcollection.NotifCollectionListener +import com.android.systemui.statusbar.phone.StatusBar +import javax.inject.Inject +import javax.inject.Singleton + +@Singleton +class TargetSdkResolver @Inject constructor( + private val context: Context, + private val collection: CommonNotifCollection +) { + init { + collection.addCollectionListener(object : NotifCollectionListener { + override fun onEntryBind(entry: NotificationEntry, sbn: StatusBarNotification) { + entry.targetSdk = resolveNotificationSdk(sbn) + } + }) + } + + private fun resolveNotificationSdk(sbn: StatusBarNotification): Int { + val pmUser = StatusBar.getPackageManagerForUser(context, sbn.user.identifier) + var targetSdk = 0 + // Extract target SDK version. + try { + val info = pmUser.getApplicationInfo(sbn.packageName, 0) + targetSdk = info.targetSdkVersion + } catch (ex: PackageManager.NameNotFoundException) { + Log.e(TAG, "Failed looking up ApplicationInfo for " + sbn.packageName, ex) + } + return targetSdk + } + + private val TAG = "TargetSdkResolver" +} \ No newline at end of file diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/inflation/NotificationRowBinderImpl.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/inflation/NotificationRowBinderImpl.java index 32f1822804f8f..74cf7511aec7b 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/inflation/NotificationRowBinderImpl.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/inflation/NotificationRowBinderImpl.java @@ -16,13 +16,11 @@ package com.android.systemui.statusbar.notification.collection.inflation; +import static java.util.Objects.requireNonNull; + import android.annotation.Nullable; import android.content.Context; -import android.content.pm.ApplicationInfo; -import android.content.pm.PackageManager; import android.os.Build; -import android.service.notification.StatusBarNotification; -import android.util.Log; import android.view.ViewGroup; import com.android.internal.util.NotificationMessagingUtil; @@ -44,9 +42,6 @@ import com.android.systemui.statusbar.notification.row.RowContentBindStage; import com.android.systemui.statusbar.notification.row.RowInflaterTask; import com.android.systemui.statusbar.notification.row.dagger.ExpandableNotificationRowComponent; import com.android.systemui.statusbar.notification.stack.NotificationListContainer; -import com.android.systemui.statusbar.phone.StatusBar; - -import java.util.Objects; import javax.inject.Inject; import javax.inject.Provider; @@ -128,14 +123,13 @@ public class NotificationRowBinderImpl implements NotificationRowBinder { public void inflateViews(NotificationEntry entry, Runnable onDismissRunnable) throws InflationException { ViewGroup parent = mListContainer.getViewParentForNotification(entry); - PackageManager pmUser = StatusBar.getPackageManagerForUser(mContext, - entry.getSbn().getUser().getIdentifier()); - final StatusBarNotification sbn = entry.getSbn(); if (entry.rowExists()) { mIconManager.updateIcons(entry); - entry.reset(); - updateNotification(entry, pmUser, sbn, entry.getRow()); + ExpandableNotificationRow row = entry.getRow(); + row.reset(); + updateRow(entry, row); + inflateContentViews(entry, row); entry.getRowController().setOnDismissRunnable(onDismissRunnable); } else { mIconManager.createIcons(entry); @@ -155,21 +149,27 @@ public class NotificationRowBinderImpl implements NotificationRowBinder { component.getExpandableNotificationRowController(); rowController.init(); entry.setRowController(rowController); - bindRow(entry, pmUser, sbn, row); - updateNotification(entry, pmUser, sbn, row); + bindRow(entry, row); + updateRow(entry, row); + inflateContentViews(entry, row); }); } } - //TODO: This method associates a row with an entry, but eventually needs to not do that - private void bindRow(NotificationEntry entry, PackageManager pmUser, - StatusBarNotification sbn, ExpandableNotificationRow row) { + /** + * Bind row to various controllers and managers. This is only called when the row is first + * created. + * + * TODO: This method associates a row with an entry, but eventually needs to not do that + */ + private void bindRow(NotificationEntry entry, ExpandableNotificationRow row) { mListContainer.bindRow(row); mNotificationRemoteInputManager.bindRow(row); + row.setOnActivatedListener(mPresenter); entry.setRow(row); row.setEntry(entry); mNotifBindPipeline.manageRow(entry, row); - mBindRowCallback.onBindRow(entry, pmUser, sbn, row); + mBindRowCallback.onBindRow(row); } /** @@ -184,11 +184,10 @@ public class NotificationRowBinderImpl implements NotificationRowBinder { NotificationUiAdjustment newAdjustment) { if (NotificationUiAdjustment.needReinflate(oldAdjustment, newAdjustment)) { if (entry.rowExists()) { - entry.reset(); - PackageManager pmUser = StatusBar.getPackageManagerForUser( - mContext, - entry.getSbn().getUser().getIdentifier()); - updateNotification(entry, pmUser, entry.getSbn(), entry.getRow()); + ExpandableNotificationRow row = entry.getRow(); + row.reset(); + updateRow(entry, row); + inflateContentViews(entry, row); } else { // Once the RowInflaterTask is done, it will pick up the updated entry, so // no-op here. @@ -202,59 +201,52 @@ public class NotificationRowBinderImpl implements NotificationRowBinder { } } - private void updateNotification( + /** + * Update row after the notification has updated. + * + * @param entry notification that has updated + */ + private void updateRow( NotificationEntry entry, - PackageManager pmUser, - StatusBarNotification sbn, ExpandableNotificationRow row) { - - // Extract target SDK version. - try { - ApplicationInfo info = pmUser.getApplicationInfo(sbn.getPackageName(), 0); - entry.targetSdk = info.targetSdkVersion; - } catch (PackageManager.NameNotFoundException ex) { - Log.e(TAG, "Failed looking up ApplicationInfo for " + sbn.getPackageName(), ex); - } row.setLegacy(entry.targetSdk >= Build.VERSION_CODES.GINGERBREAD && entry.targetSdk < Build.VERSION_CODES.LOLLIPOP); - // TODO: should this be happening somewhere else? - mIconManager.updateIconTags(entry, entry.targetSdk); - - row.setOnActivatedListener(mPresenter); + // bind the click event to the content area + requireNonNull(mNotificationClicker).register(row, entry.getSbn()); + } + /** + * Inflate the row's basic content views. + */ + private void inflateContentViews( + NotificationEntry entry, + ExpandableNotificationRow row) { final boolean useIncreasedCollapsedHeight = - mMessagingUtil.isImportantMessaging(sbn, entry.getImportance()); + mMessagingUtil.isImportantMessaging(entry.getSbn(), entry.getImportance()); final boolean isLowPriority = entry.isAmbient(); RowContentBindParams params = mRowContentBindStage.getStageParams(entry); params.setUseIncreasedCollapsedHeight(useIncreasedCollapsedHeight); params.setUseLowPriority(entry.isAmbient()); - //TODO: Replace this API with RowContentBindParams directly + // TODO: Replace this API with RowContentBindParams directly. Also move to a separate + // redaction controller. row.setNeedsRedaction(mNotificationLockscreenUserManager.needsRedaction(entry)); + params.rebindAllContentViews(); mRowContentBindStage.requestRebind(entry, en -> { row.setUsesIncreasedCollapsedHeight(useIncreasedCollapsedHeight); row.setIsLowPriority(isLowPriority); mInflationCallback.onAsyncInflationFinished(en); }); - - // bind the click event to the content area - Objects.requireNonNull(mNotificationClicker).register(row, sbn); } /** Callback for when a row is bound to an entry. */ public interface BindRowCallback { /** - * Called when a new notification and row is created. - * - * @param entry entry for the notification - * @param pmUser package manager for user - * @param sbn notification - * @param row row for the notification + * Called when a new row is created and bound to a notification. */ - void onBindRow(NotificationEntry entry, PackageManager pmUser, - StatusBarNotification sbn, ExpandableNotificationRow row); + void onBindRow(ExpandableNotificationRow row); } } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/notifcollection/NotifCollectionListener.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/notifcollection/NotifCollectionListener.java index 41ca52d5a6267..59f119e987b4a 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/notifcollection/NotifCollectionListener.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/notifcollection/NotifCollectionListener.java @@ -17,6 +17,7 @@ package com.android.systemui.statusbar.notification.collection.notifcollection; import android.service.notification.NotificationListenerService; +import android.service.notification.StatusBarNotification; import com.android.systemui.statusbar.notification.collection.NotifCollection.CancellationReason; import com.android.systemui.statusbar.notification.collection.NotificationEntry; @@ -25,6 +26,15 @@ import com.android.systemui.statusbar.notification.collection.NotificationEntry; * Listener interface for {@link NotificationEntry} events. */ public interface NotifCollectionListener { + + /** + * Called when the entry is having a new status bar notification bound to it. This should + * be used to initialize any derivative state on the entry that needs to update when the + * notification is updated. + */ + default void onEntryBind(NotificationEntry entry, StatusBarNotification sbn) { + } + /** * Called whenever a new {@link NotificationEntry} is initialized. This should be used for * initializing any decorated state tied to the notification. diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/notifcollection/NotifEvent.kt b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/notifcollection/NotifEvent.kt index 2ef0368061ba1..2810b891373ff 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/notifcollection/NotifEvent.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/notifcollection/NotifEvent.kt @@ -17,6 +17,7 @@ package com.android.systemui.statusbar.notification.collection.notifcollection import android.service.notification.NotificationListenerService.RankingMap +import android.service.notification.StatusBarNotification import com.android.systemui.statusbar.notification.collection.NotifCollection import com.android.systemui.statusbar.notification.collection.NotificationEntry @@ -37,6 +38,15 @@ sealed class NotifEvent { abstract fun dispatchToListener(listener: NotifCollectionListener) } +data class BindEntryEvent( + val entry: NotificationEntry, + val sbn: StatusBarNotification +) : NotifEvent() { + override fun dispatchToListener(listener: NotifCollectionListener) { + listener.onEntryBind(entry, sbn) + } +} + data class InitEntryEvent( val entry: NotificationEntry ) : NotifEvent() { diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/icon/IconManager.kt b/packages/SystemUI/src/com/android/systemui/statusbar/notification/icon/IconManager.kt index da8ad2da5c871..08be4f8724153 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/icon/IconManager.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/icon/IconManager.kt @@ -172,18 +172,6 @@ class IconManager @Inject constructor( } } - /** - * Updates tags on the icon views to match the posting app's target SDK level - * - * Note that this method MUST be called after both [createIcons] and [updateIcons]. - */ - fun updateIconTags(entry: NotificationEntry, targetSdk: Int) { - setTagOnIconViews( - entry.icons, - R.id.icon_is_pre_L, - targetSdk < Build.VERSION_CODES.LOLLIPOP) - } - private fun updateIconsSafe(entry: NotificationEntry) { try { updateIcons(entry) @@ -259,6 +247,7 @@ class IconManager @Inject constructor( iconView: StatusBarIconView ) { iconView.setShowsConversation(showsConversation(entry, iconView, iconDescriptor)) + iconView.setTag(R.id.icon_is_pre_L, entry.targetSdk < Build.VERSION_CODES.LOLLIPOP) if (!iconView.set(iconDescriptor)) { throw InflationException("Couldn't create icon $iconDescriptor") } @@ -326,20 +315,13 @@ class IconManager @Inject constructor( val usedInSensitiveContext = iconView === entry.icons.shelfIcon || iconView === entry.icons.aodIcon val isSmallIcon = iconDescriptor.icon.equals(entry.sbn.notification.smallIcon) - return isImportantConversation(entry) && !isSmallIcon - && (!usedInSensitiveContext || !entry.isSensitive) + return isImportantConversation(entry) && !isSmallIcon && + (!usedInSensitiveContext || !entry.isSensitive) } private fun isImportantConversation(entry: NotificationEntry): Boolean { return entry.ranking.channel != null && entry.ranking.channel.isImportantConversation } - - private fun setTagOnIconViews(icons: IconPack, key: Int, tag: Any) { - icons.statusBarIcon?.setTag(key, tag) - icons.shelfIcon?.setTag(key, tag) - icons.aodIcon?.setTag(key, tag) - icons.centeredIcon?.setTag(key, tag) - } } private const val TAG = "IconManager" \ No newline at end of file diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/ExpandableNotificationRowController.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/ExpandableNotificationRowController.java index 8b3d06b97882c..f8d9c4648ac9e 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/ExpandableNotificationRowController.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/ExpandableNotificationRowController.java @@ -60,7 +60,6 @@ public class ExpandableNotificationRowController { private final HeadsUpManager mHeadsUpManager; private final ExpandableNotificationRow.OnExpandClickListener mOnExpandClickListener; private final StatusBarStateController mStatusBarStateController; - private final NotificationRowContentBinder.InflationCallback mInflationCallback; private final ExpandableNotificationRow.ExpansionLogger mExpansionLogger = this::logNotificationExpansion; @@ -82,7 +81,6 @@ public class ExpandableNotificationRowController { NotificationLogger notificationLogger, HeadsUpManager headsUpManager, ExpandableNotificationRow.OnExpandClickListener onExpandClickListener, StatusBarStateController statusBarStateController, - NotificationRowContentBinder.InflationCallback inflationCallback, NotificationGutsManager notificationGutsManager, @Named(ALLOW_NOTIFICATION_LONG_PRESS_NAME) boolean allowLongPress, @DismissRunnable Runnable onDismissRunnable, FalsingManager falsingManager, @@ -101,7 +99,6 @@ public class ExpandableNotificationRowController { mHeadsUpManager = headsUpManager; mOnExpandClickListener = onExpandClickListener; mStatusBarStateController = statusBarStateController; - mInflationCallback = inflationCallback; mNotificationGutsManager = notificationGutsManager; mOnDismissRunnable = onDismissRunnable; mOnAppOpsClickListener = mNotificationGutsManager::openGuts; diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/phone/StatusBarNotificationPresenter.java b/packages/SystemUI/src/com/android/systemui/statusbar/phone/StatusBarNotificationPresenter.java index 79cea91b8612c..aecbb9097c7a9 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/phone/StatusBarNotificationPresenter.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/StatusBarNotificationPresenter.java @@ -22,7 +22,6 @@ import static com.android.systemui.statusbar.phone.StatusBar.SPEW; import android.annotation.Nullable; import android.app.KeyguardManager; import android.content.Context; -import android.content.pm.PackageManager; import android.os.RemoteException; import android.os.ServiceManager; import android.service.notification.StatusBarNotification; @@ -355,8 +354,7 @@ public class StatusBarNotificationPresenter implements NotificationPresenter, } @Override - public void onBindRow(NotificationEntry entry, PackageManager pmUser, - StatusBarNotification sbn, ExpandableNotificationRow row) { + public void onBindRow(ExpandableNotificationRow row) { row.setAboveShelfChangedListener(mAboveShelfObserver); row.setSecureStateProvider(mKeyguardStateController::canDismissLockScreen); } diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/TestableNotificationEntryManager.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/TestableNotificationEntryManager.kt index d522f903c83ac..698cae4d8bc19 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/TestableNotificationEntryManager.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/TestableNotificationEntryManager.kt @@ -24,6 +24,7 @@ import com.android.systemui.statusbar.notification.collection.NotificationRankin import com.android.systemui.statusbar.notification.collection.inflation.NotificationRowBinder import com.android.systemui.statusbar.phone.NotificationGroupManager import com.android.systemui.util.leak.LeakDetector +import dagger.Lazy import java.util.concurrent.CountDownLatch /** @@ -35,12 +36,12 @@ class TestableNotificationEntryManager( rm: NotificationRankingManager, ke: KeyguardEnvironment, ff: FeatureFlags, - rb: dagger.Lazy, - notificationRemoteInputManagerLazy: dagger.Lazy, + rb: Lazy, + notificationRemoteInputManagerLazy: Lazy, leakDetector: LeakDetector, fgsFeatureController: ForegroundServiceDismissalFeatureController -) : NotificationEntryManager(logger, gm, rm, ke, ff, rb, - notificationRemoteInputManagerLazy, leakDetector, fgsFeatureController) { +) : NotificationEntryManager(logger, gm, rm, ke, ff, rb, notificationRemoteInputManagerLazy, + leakDetector, fgsFeatureController) { public var countDownLatch: CountDownLatch = CountDownLatch(1) diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/row/NotificationEntryManagerInflationTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/row/NotificationEntryManagerInflationTest.java index 1c6e5a36bd8a6..774b5eddc6865 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/row/NotificationEntryManagerInflationTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/row/NotificationEntryManagerInflationTest.java @@ -23,7 +23,6 @@ import static com.android.systemui.statusbar.notification.row.NotificationRowCon import static junit.framework.Assert.assertNotNull; import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; @@ -243,7 +242,6 @@ public class NotificationEntryManagerInflationTest extends SysuiTestCase { mHeadsUpManager, mPresenter, mStatusBarStateController, - mEntryManager, mGutsManager, true, null, @@ -330,7 +328,7 @@ public class NotificationEntryManagerInflationTest extends SysuiTestCase { assertNotNull(entry.getRow().getPrivateLayout().getContractedChild()); // THEN inflation callbacks are called - verify(mBindCallback).onBindRow(eq(entry), any(), eq(mSbn), any()); + verify(mBindCallback).onBindRow(entry.getRow()); verify(mEntryListener, never()).onInflationError(any(), any()); verify(mEntryListener).onEntryInflated(entry); verify(mEntryListener).onNotificationAdded(entry); From bd142939eb9f674d41cb9fabf8a5d52cd2b9e238 Mon Sep 17 00:00:00 2001 From: Kevin Han Date: Tue, 10 Mar 2020 18:27:50 -0700 Subject: [PATCH 2/2] Remove setInflationCallback The current use cases for NotificationRowBinderImpl and NotifInflater's API don't really need a persistent inflation callback stored as opposed to a one-shot callback. By explicitly putting the callback in the inflate methods, it also makes it more clear which calls inflate and expect a callback to be called. Bug: 145749521 Test: atest SystemUITests Test: smoke test (add, update, remove notification) Change-Id: I001413f17b7472ea2ef637be21d9d454ee17015e --- .../NotificationEntryManager.java | 71 ++++++++++--------- .../collection/NotifInflaterImpl.java | 58 ++++++++------- .../coordinator/PreparationCoordinator.java | 5 +- .../collection/inflation/NotifInflater.java | 14 ++-- .../inflation/NotificationRowBinder.java | 4 +- .../inflation/NotificationRowBinderImpl.java | 24 +++---- .../init/NotificationsControllerImpl.kt | 2 - .../ExpandableNotificationRowComponent.java | 3 - .../TestableNotificationEntryManager.kt | 63 ---------------- .../PreparationCoordinatorTest.java | 21 +++--- ...NotificationEntryManagerInflationTest.java | 4 -- .../NotificationStackScrollLayoutTest.java | 7 +- 12 files changed, 103 insertions(+), 173 deletions(-) delete mode 100644 packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/TestableNotificationEntryManager.kt diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/NotificationEntryManager.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/NotificationEntryManager.java index eb97169d24833..d37e16b176200 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/NotificationEntryManager.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/NotificationEntryManager.java @@ -88,14 +88,11 @@ import dagger.Lazy; * @see #getActiveNotificationUnfiltered(String) to check if a key exists * @see #getPendingNotificationsIterator() for an iterator over the pending notifications * @see #getPendingOrActiveNotif(String) to find a notification exists for that key in any list - * @see #getPendingAndActiveNotifications() to get the entire set of Notifications that we're - * aware of * @see #getActiveNotificationsForCurrentUser() to see every notification that the current user owns */ public class NotificationEntryManager implements CommonNotifCollection, Dumpable, - InflationCallback, VisualStabilityManager.Callback { private static final String TAG = "NotificationEntryMgr"; private static final boolean DEBUG = Log.isLoggable(TAG, Log.DEBUG); @@ -309,12 +306,7 @@ public class NotificationEntryManager implements * * WARNING: this will call back into us. Don't hold any locks. */ - @Override - public void handleInflationException(NotificationEntry n, Exception e) { - handleInflationException(n.getSbn(), e); - } - - public void handleInflationException(StatusBarNotification n, Exception e) { + private void handleInflationException(StatusBarNotification n, Exception e) { removeNotificationInternal( n.getKey(), null, null, true /* forceRemove */, false /* removedByUser */, REASON_ERROR); @@ -323,30 +315,37 @@ public class NotificationEntryManager implements } } - @Override - public void onAsyncInflationFinished(NotificationEntry entry) { - mPendingNotifications.remove(entry.getKey()); - // If there was an async task started after the removal, we don't want to add it back to - // the list, otherwise we might get leaks. - if (!entry.isRowRemoved()) { - boolean isNew = getActiveNotificationUnfiltered(entry.getKey()) == null; - mLogger.logNotifInflated(entry.getKey(), isNew); - if (isNew) { - for (NotificationEntryListener listener : mNotificationEntryListeners) { - listener.onEntryInflated(entry); - } - addActiveNotification(entry); - updateNotifications("onAsyncInflationFinished"); - for (NotificationEntryListener listener : mNotificationEntryListeners) { - listener.onNotificationAdded(entry); - } - } else { - for (NotificationEntryListener listener : mNotificationEntryListeners) { - listener.onEntryReinflated(entry); + private final InflationCallback mInflationCallback = new InflationCallback() { + @Override + public void handleInflationException(NotificationEntry entry, Exception e) { + NotificationEntryManager.this.handleInflationException(entry.getSbn(), e); + } + + @Override + public void onAsyncInflationFinished(NotificationEntry entry) { + mPendingNotifications.remove(entry.getKey()); + // If there was an async task started after the removal, we don't want to add it back to + // the list, otherwise we might get leaks. + if (!entry.isRowRemoved()) { + boolean isNew = getActiveNotificationUnfiltered(entry.getKey()) == null; + mLogger.logNotifInflated(entry.getKey(), isNew); + if (isNew) { + for (NotificationEntryListener listener : mNotificationEntryListeners) { + listener.onEntryInflated(entry); + } + addActiveNotification(entry); + updateNotifications("onAsyncInflationFinished"); + for (NotificationEntryListener listener : mNotificationEntryListeners) { + listener.onNotificationAdded(entry); + } + } else { + for (NotificationEntryListener listener : mNotificationEntryListeners) { + listener.onEntryReinflated(entry); + } } } } - } + }; private final NotificationHandler mNotifListener = new NotificationHandler() { @Override @@ -572,8 +571,10 @@ public class NotificationEntryManager implements // Construct the expanded view. if (!mFeatureFlags.isNewNotifPipelineRenderingEnabled()) { mNotificationRowBinderLazy.get() - .inflateViews(entry, () -> performRemoveNotification(notification, - REASON_CANCEL)); + .inflateViews( + entry, + () -> performRemoveNotification(notification, REASON_CANCEL), + mInflationCallback); } abortExistingInflation(key, "addNotification"); @@ -630,8 +631,10 @@ public class NotificationEntryManager implements if (!mFeatureFlags.isNewNotifPipelineRenderingEnabled()) { mNotificationRowBinderLazy.get() - .inflateViews(entry, () -> performRemoveNotification(notification, - REASON_CANCEL)); + .inflateViews( + entry, + () -> performRemoveNotification(notification, REASON_CANCEL), + mInflationCallback); } updateNotifications("updateNotificationInternal"); diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotifInflaterImpl.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotifInflaterImpl.java index 1f6413b525cb1..d081e114855eb 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotifInflaterImpl.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotifInflaterImpl.java @@ -47,7 +47,6 @@ public class NotifInflaterImpl implements NotifInflater { private final NotifPipeline mNotifPipeline; private NotificationRowBinderImpl mNotificationRowBinder; - private InflationCallback mExternalInflationCallback; @Inject public NotifInflaterImpl( @@ -66,17 +65,11 @@ public class NotifInflaterImpl implements NotifInflater { */ public void setRowBinder(NotificationRowBinderImpl rowBinder) { mNotificationRowBinder = rowBinder; - mNotificationRowBinder.setInflationCallback(mInflationCallback); } @Override - public void setInflationCallback(InflationCallback callback) { - mExternalInflationCallback = callback; - } - - @Override - public void rebindViews(NotificationEntry entry) { - inflateViews(entry); + public void rebindViews(NotificationEntry entry, InflationCallback callback) { + inflateViews(entry, callback); } /** @@ -84,11 +77,14 @@ public class NotifInflaterImpl implements NotifInflater { * views are bound. */ @Override - public void inflateViews(NotificationEntry entry) { + public void inflateViews(NotificationEntry entry, InflationCallback callback) { try { - requireBinder().inflateViews(entry, getDismissCallback(entry)); + requireBinder().inflateViews( + entry, + getDismissCallback(entry), + wrapInflationCallback(callback)); } catch (InflationException e) { - // logged in mInflationCallback.handleInflationException + mNotifErrorManager.setInflationError(entry, e); } } @@ -121,6 +117,26 @@ public class NotifInflaterImpl implements NotifInflater { }; } + private NotificationContentInflater.InflationCallback wrapInflationCallback( + InflationCallback callback) { + return new NotificationContentInflater.InflationCallback() { + @Override + public void handleInflationException( + NotificationEntry entry, + Exception e) { + mNotifErrorManager.setInflationError(entry, e); + } + + @Override + public void onAsyncInflationFinished(NotificationEntry entry) { + mNotifErrorManager.clearInflationError(entry); + if (callback != null) { + callback.onInflationFinished(entry); + } + } + }; + } + private NotificationRowBinderImpl requireBinder() { if (mNotificationRowBinder == null) { throw new RuntimeException("NotificationRowBinder must be attached before using " @@ -128,22 +144,4 @@ public class NotifInflaterImpl implements NotifInflater { } return mNotificationRowBinder; } - - private final NotificationContentInflater.InflationCallback mInflationCallback = - new NotificationContentInflater.InflationCallback() { - @Override - public void handleInflationException( - NotificationEntry entry, - Exception e) { - mNotifErrorManager.setInflationError(entry, e); - } - - @Override - public void onAsyncInflationFinished(NotificationEntry entry) { - mNotifErrorManager.clearInflationError(entry); - if (mExternalInflationCallback != null) { - mExternalInflationCallback.onInflationFinished(entry); - } - } - }; } diff --git 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 index 9973ef9ae14ea..0e8dd5e24e910 100644 --- 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 @@ -72,7 +72,6 @@ public class PreparationCoordinator implements Coordinator { ) { mLogger = logger; mNotifInflater = notifInflater; - mNotifInflater.setInflationCallback(mInflationCallback); mNotifErrorManager = errorManager; mNotifErrorManager.addInflationErrorListener(mInflationErrorListener); mViewBarn = viewBarn; @@ -218,11 +217,11 @@ public class PreparationCoordinator implements Coordinator { private void inflateEntry(NotificationEntry entry, String reason) { abortInflation(entry, reason); - mNotifInflater.inflateViews(entry); + mNotifInflater.inflateViews(entry, mInflationCallback); } private void rebind(NotificationEntry entry, String reason) { - mNotifInflater.rebindViews(entry); + mNotifInflater.rebindViews(entry, mInflationCallback); } private void abortInflation(NotificationEntry entry, String reason) { diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/inflation/NotifInflater.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/inflation/NotifInflater.java index ea0ece444a679..e3d76113d5378 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/inflation/NotifInflater.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/inflation/NotifInflater.java @@ -24,22 +24,20 @@ import com.android.systemui.statusbar.notification.collection.coordinator.Prepar * main thread. When the inflation is finished, NotifInflater will trigger its InflationCallback. */ public interface NotifInflater { - - /** - * Callback used when inflation is finished. - */ - void setInflationCallback(InflationCallback callback); - /** * Called to rebind the entry's views. + * + * @param callback callback called after inflation finishes */ - void rebindViews(NotificationEntry entry); + void rebindViews(NotificationEntry entry, InflationCallback callback); /** * Called to inflate the views of an entry. Views are not considered inflated until all of its * views are bound. Once all views are inflated, the InflationCallback is triggered. + * + * @param callback callback called after inflation finishes */ - void inflateViews(NotificationEntry entry); + void inflateViews(NotificationEntry entry, InflationCallback callback); /** * Request to stop the inflation of an entry. For example, called when a notification is diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/inflation/NotificationRowBinder.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/inflation/NotificationRowBinder.java index 3f500644b1846..f4c4924b4b9ae 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/inflation/NotificationRowBinder.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/inflation/NotificationRowBinder.java @@ -22,6 +22,7 @@ import com.android.systemui.statusbar.NotificationUiAdjustment; import com.android.systemui.statusbar.notification.InflationException; import com.android.systemui.statusbar.notification.NotificationEntryManager; import com.android.systemui.statusbar.notification.collection.NotificationEntry; +import com.android.systemui.statusbar.notification.row.NotificationRowContentBinder; /** * Used by the {@link NotificationEntryManager}. When notifications are added or updated, the binder @@ -37,7 +38,8 @@ public interface NotificationRowBinder { */ void inflateViews( NotificationEntry entry, - Runnable onDismissRunnable) + Runnable onDismissRunnable, + NotificationRowContentBinder.InflationCallback callback) throws InflationException; /** diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/inflation/NotificationRowBinderImpl.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/inflation/NotificationRowBinderImpl.java index 74cf7511aec7b..73f12f86e52e5 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/inflation/NotificationRowBinderImpl.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/inflation/NotificationRowBinderImpl.java @@ -67,7 +67,6 @@ public class NotificationRowBinderImpl implements NotificationRowBinder { private NotificationPresenter mPresenter; private NotificationListContainer mListContainer; - private NotificationRowContentBinder.InflationCallback mInflationCallback; private BindRowCallback mBindRowCallback; private NotificationClicker mNotificationClicker; @@ -108,10 +107,6 @@ public class NotificationRowBinderImpl implements NotificationRowBinder { mIconManager.attach(); } - public void setInflationCallback(NotificationRowContentBinder.InflationCallback callback) { - mInflationCallback = callback; - } - public void setNotificationClicker(NotificationClicker clicker) { mNotificationClicker = clicker; } @@ -120,7 +115,10 @@ public class NotificationRowBinderImpl implements NotificationRowBinder { * Inflates the views for the given entry (possibly asynchronously). */ @Override - public void inflateViews(NotificationEntry entry, Runnable onDismissRunnable) + public void inflateViews( + NotificationEntry entry, + Runnable onDismissRunnable, + NotificationRowContentBinder.InflationCallback callback) throws InflationException { ViewGroup parent = mListContainer.getViewParentForNotification(entry); @@ -129,7 +127,7 @@ public class NotificationRowBinderImpl implements NotificationRowBinder { ExpandableNotificationRow row = entry.getRow(); row.reset(); updateRow(entry, row); - inflateContentViews(entry, row); + inflateContentViews(entry, row, callback); entry.getRowController().setOnDismissRunnable(onDismissRunnable); } else { mIconManager.createIcons(entry); @@ -141,7 +139,6 @@ public class NotificationRowBinderImpl implements NotificationRowBinder { .expandableNotificationRow(row) .notificationEntry(entry) .onDismissRunnable(onDismissRunnable) - .inflationCallback(mInflationCallback) .rowContentBindStage(mRowContentBindStage) .onExpandClickListener(mPresenter) .build(); @@ -151,7 +148,7 @@ public class NotificationRowBinderImpl implements NotificationRowBinder { entry.setRowController(rowController); bindRow(entry, row); updateRow(entry, row); - inflateContentViews(entry, row); + inflateContentViews(entry, row, callback); }); } } @@ -175,6 +172,8 @@ public class NotificationRowBinderImpl implements NotificationRowBinder { /** * Updates the views bound to an entry when the entry's ranking changes, either in-place or by * reinflating them. + * + * TODO: Should this method be in this class? */ @Override public void onNotificationRankingUpdated( @@ -187,7 +186,7 @@ public class NotificationRowBinderImpl implements NotificationRowBinder { ExpandableNotificationRow row = entry.getRow(); row.reset(); updateRow(entry, row); - inflateContentViews(entry, row); + inflateContentViews(entry, row, null /* callback */); } else { // Once the RowInflaterTask is done, it will pick up the updated entry, so // no-op here. @@ -221,7 +220,8 @@ public class NotificationRowBinderImpl implements NotificationRowBinder { */ private void inflateContentViews( NotificationEntry entry, - ExpandableNotificationRow row) { + ExpandableNotificationRow row, + NotificationRowContentBinder.InflationCallback inflationCallback) { final boolean useIncreasedCollapsedHeight = mMessagingUtil.isImportantMessaging(entry.getSbn(), entry.getImportance()); final boolean isLowPriority = entry.isAmbient(); @@ -238,7 +238,7 @@ public class NotificationRowBinderImpl implements NotificationRowBinder { mRowContentBindStage.requestRebind(entry, en -> { row.setUsesIncreasedCollapsedHeight(useIncreasedCollapsedHeight); row.setIsLowPriority(isLowPriority); - mInflationCallback.onAsyncInflationFinished(en); + inflationCallback.onAsyncInflationFinished(en); }); } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/init/NotificationsControllerImpl.kt b/packages/SystemUI/src/com/android/systemui/statusbar/notification/init/NotificationsControllerImpl.kt index d1cceaeb6dd52..5fac5b1cf159b 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/init/NotificationsControllerImpl.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/init/NotificationsControllerImpl.kt @@ -108,8 +108,6 @@ class NotificationsControllerImpl @Inject constructor( if (featureFlags.isNewNotifPipelineRenderingEnabled) { // TODO } else { - notificationRowBinder.setInflationCallback(entryManager) - remoteInputUriController.attach(entryManager) groupAlertTransferHelper.bind(entryManager, groupManager) headsUpManager.addListener(groupManager) diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/dagger/ExpandableNotificationRowComponent.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/dagger/ExpandableNotificationRowComponent.java index 6d6d3e446f536..9846f2dcd1705 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/dagger/ExpandableNotificationRowComponent.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/dagger/ExpandableNotificationRowComponent.java @@ -25,7 +25,6 @@ import com.android.systemui.statusbar.notification.collection.NotificationEntry; import com.android.systemui.statusbar.notification.row.ActivatableNotificationView; import com.android.systemui.statusbar.notification.row.ExpandableNotificationRow; import com.android.systemui.statusbar.notification.row.ExpandableNotificationRowController; -import com.android.systemui.statusbar.notification.row.NotificationRowContentBinder; import com.android.systemui.statusbar.notification.row.RowContentBindStage; import com.android.systemui.statusbar.phone.StatusBar; @@ -60,8 +59,6 @@ public interface ExpandableNotificationRowComponent { @BindsInstance Builder rowContentBindStage(RowContentBindStage rowContentBindStage); @BindsInstance - Builder inflationCallback(NotificationRowContentBinder.InflationCallback inflationCallback); - @BindsInstance Builder onExpandClickListener(ExpandableNotificationRow.OnExpandClickListener presenter); ExpandableNotificationRowComponent build(); } diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/TestableNotificationEntryManager.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/TestableNotificationEntryManager.kt deleted file mode 100644 index 698cae4d8bc19..0000000000000 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/TestableNotificationEntryManager.kt +++ /dev/null @@ -1,63 +0,0 @@ -/* - * Copyright (C) 2019 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 - -import com.android.systemui.statusbar.FeatureFlags -import com.android.systemui.statusbar.NotificationPresenter -import com.android.systemui.statusbar.NotificationRemoteInputManager -import com.android.systemui.statusbar.notification.collection.NotificationEntry -import com.android.systemui.statusbar.notification.collection.NotificationRankingManager -import com.android.systemui.statusbar.notification.collection.inflation.NotificationRowBinder -import com.android.systemui.statusbar.phone.NotificationGroupManager -import com.android.systemui.util.leak.LeakDetector -import dagger.Lazy -import java.util.concurrent.CountDownLatch - -/** - * Enable some test capabilities for NEM without making everything public on the base class - */ -class TestableNotificationEntryManager( - logger: NotificationEntryManagerLogger, - gm: NotificationGroupManager, - rm: NotificationRankingManager, - ke: KeyguardEnvironment, - ff: FeatureFlags, - rb: Lazy, - notificationRemoteInputManagerLazy: Lazy, - leakDetector: LeakDetector, - fgsFeatureController: ForegroundServiceDismissalFeatureController -) : NotificationEntryManager(logger, gm, rm, ke, ff, rb, notificationRemoteInputManagerLazy, - leakDetector, fgsFeatureController) { - - public var countDownLatch: CountDownLatch = CountDownLatch(1) - - override fun onAsyncInflationFinished(entry: NotificationEntry) { - super.onAsyncInflationFinished(entry) - countDownLatch.countDown() - } - - fun setUpForTest( - presenter: NotificationPresenter? - ) { - super.setUpWithPresenter(presenter) - } - - fun setActiveNotificationList(activeList: List) { - mSortedAndFiltered.clear() - mSortedAndFiltered.addAll(activeList) - } -} diff --git 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 index 6b9e43bcb2900..35b31c01fd9cd 100644 --- 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 @@ -18,6 +18,7 @@ package com.android.systemui.statusbar.notification.collection.coordinator; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; +import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; @@ -62,7 +63,6 @@ public class PreparationCoordinatorTest extends SysuiTestCase { private OnBeforeFinalizeFilterListener mBeforeFilterListener; private NotifFilter mUninflatedFilter; private NotifFilter mInflationErrorFilter; - private NotifInflaterImpl.InflationCallback mCallback; private NotifInflationErrorManager mErrorManager; private NotificationEntry mEntry; private Exception mInflationError; @@ -104,9 +104,6 @@ public class PreparationCoordinatorTest extends SysuiTestCase { mBeforeFilterListenerCaptor.capture()); mBeforeFilterListener = mBeforeFilterListenerCaptor.getValue(); - verify(mNotifInflater).setInflationCallback(mCallbackCaptor.capture()); - mCallback = mCallbackCaptor.getValue(); - mCollectionListener.onEntryInit(mEntry); } @@ -142,7 +139,7 @@ public class PreparationCoordinatorTest extends SysuiTestCase { mBeforeFilterListener.onBeforeFinalizeFilter(List.of(mEntry)); // THEN we inflate it - verify(mNotifInflater).inflateViews(mEntry); + verify(mNotifInflater).inflateViews(eq(mEntry), any()); // THEN we filter it out until it's done inflating. assertTrue(mUninflatedFilter.shouldFilterOut(mEntry, 0)); @@ -151,14 +148,17 @@ public class PreparationCoordinatorTest extends SysuiTestCase { @Test public void testRebindsInflatedNotificationsOnUpdate() { // GIVEN an inflated notification - mCallback.onInflationFinished(mEntry); + mCollectionListener.onEntryAdded(mEntry); + mBeforeFilterListener.onBeforeFinalizeFilter(List.of(mEntry)); + verify(mNotifInflater).inflateViews(eq(mEntry), mCallbackCaptor.capture()); + mCallbackCaptor.getValue().onInflationFinished(mEntry); // WHEN notification is updated mCollectionListener.onEntryUpdated(mEntry); mBeforeFilterListener.onBeforeFinalizeFilter(List.of(mEntry)); // THEN we rebind it - verify(mNotifInflater).rebindViews(mEntry); + verify(mNotifInflater).rebindViews(eq(mEntry), any()); // THEN we do not filter it because it's not the first inflation. assertFalse(mUninflatedFilter.shouldFilterOut(mEntry, 0)); @@ -166,8 +166,11 @@ public class PreparationCoordinatorTest extends SysuiTestCase { @Test public void testDoesntFilterInflatedNotifs() { - // WHEN a notification is inflated - mCallback.onInflationFinished(mEntry); + // GIVEN an inflated notification + mCollectionListener.onEntryAdded(mEntry); + mBeforeFilterListener.onBeforeFinalizeFilter(List.of(mEntry)); + verify(mNotifInflater).inflateViews(eq(mEntry), mCallbackCaptor.capture()); + mCallbackCaptor.getValue().onInflationFinished(mEntry); // THEN it isn't filtered from shade list assertFalse(mUninflatedFilter.shouldFilterOut(mEntry, 0)); diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/row/NotificationEntryManagerInflationTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/row/NotificationEntryManagerInflationTest.java index 774b5eddc6865..855f524db3f82 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/row/NotificationEntryManagerInflationTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/row/NotificationEntryManagerInflationTest.java @@ -214,9 +214,6 @@ public class NotificationEntryManagerInflationTest extends SysuiTestCase { when(mExpandableNotificationRowComponentBuilder .onDismissRunnable(any())) .thenReturn(mExpandableNotificationRowComponentBuilder); - when(mExpandableNotificationRowComponentBuilder - .inflationCallback(any())) - .thenReturn(mExpandableNotificationRowComponentBuilder); when(mExpandableNotificationRowComponentBuilder .rowContentBindStage(any())) .thenReturn(mExpandableNotificationRowComponentBuilder); @@ -273,7 +270,6 @@ public class NotificationEntryManagerInflationTest extends SysuiTestCase { mEntryManager.addNotificationEntryListener(mEntryListener); mRowBinder.setUpWithPresenter(mPresenter, mListContainer, mBindCallback); - mRowBinder.setInflationCallback(mEntryManager); mRowBinder.setNotificationClicker(mock(NotificationClicker.class)); Ranking ranking = new Ranking(); diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/stack/NotificationStackScrollLayoutTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/stack/NotificationStackScrollLayoutTest.java index ef2071ef090e9..657bc8d614cf5 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/stack/NotificationStackScrollLayoutTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/stack/NotificationStackScrollLayoutTest.java @@ -69,7 +69,6 @@ import com.android.systemui.statusbar.notification.NotificationEntryManager; import com.android.systemui.statusbar.notification.NotificationEntryManagerLogger; import com.android.systemui.statusbar.notification.NotificationFilter; import com.android.systemui.statusbar.notification.NotificationSectionsFeatureManager; -import com.android.systemui.statusbar.notification.TestableNotificationEntryManager; import com.android.systemui.statusbar.notification.VisualStabilityManager; import com.android.systemui.statusbar.notification.collection.NotifCollection; import com.android.systemui.statusbar.notification.collection.NotifPipeline; @@ -138,7 +137,7 @@ public class NotificationStackScrollLayoutTest extends SysuiTestCase { @Mock private NotificationLockscreenUserManager mLockscreenUserManager; @Mock private FeatureFlags mFeatureFlags; private UserChangedListener mUserChangedListener; - private TestableNotificationEntryManager mEntryManager; + private NotificationEntryManager mEntryManager; private int mOriginalInterruptionModelSetting; private UiEventLoggerFake mUiEventLoggerFake = new UiEventLoggerFake(); @@ -167,7 +166,7 @@ public class NotificationStackScrollLayoutTest extends SysuiTestCase { ArgumentCaptor userChangedCaptor = ArgumentCaptor .forClass(UserChangedListener.class); - mEntryManager = new TestableNotificationEntryManager( + mEntryManager = new NotificationEntryManager( mock(NotificationEntryManagerLogger.class), mock(NotificationGroupManager.class), new NotificationRankingManager( @@ -187,7 +186,7 @@ public class NotificationStackScrollLayoutTest extends SysuiTestCase { mock(LeakDetector.class), mock(ForegroundServiceDismissalFeatureController.class) ); - mEntryManager.setUpForTest(mock(NotificationPresenter.class)); + mEntryManager.setUpWithPresenter(mock(NotificationPresenter.class)); when(mFeatureFlags.isNewNotifPipelineRenderingEnabled()).thenReturn(false); NotificationShelf notificationShelf = mock(NotificationShelf.class);