From 4a823189a9e6021052acc835ceff7b200f4ad876 Mon Sep 17 00:00:00 2001 From: Jeff DeCew Date: Thu, 30 Jun 2022 16:15:42 +0000 Subject: Add the invalidation reason to existing log statements This ensures logs are richer while adding minimal memory usage, and without adding more log statements which might churn through our buffers faster. Bug: 236140753 Test: atest KeyguardCoordinatorTest SentitiveContentCoordinatorTest SmartspaceDedupingCoordinatorTest VisualStabilityCoordinatorTest ShadeListBuilderTest Change-Id: Ie16987a834039d038bb817e3886b0695c2f81976 --- .../notification/collection/ShadeListBuilder.java | 32 ++++--- .../collection/coordinator/BubbleCoordinator.java | 2 +- .../collection/coordinator/DebugModeCoordinator.kt | 8 +- .../coordinator/DeviceProvisionedCoordinator.java | 2 +- .../collection/coordinator/HeadsUpCoordinator.kt | 7 +- .../HideNotifsForOtherUsersCoordinator.java | 33 +++---- .../coordinator/KeyguardCoordinator.java | 6 +- .../coordinator/PreparationCoordinator.java | 11 ++- .../collection/coordinator/RankingCoordinator.java | 2 +- .../coordinator/SensitiveContentCoordinator.kt | 2 +- .../coordinator/SharedCoordinatorLogger.kt | 46 --------- .../coordinator/SmartspaceDedupingCoordinator.kt | 7 +- .../coordinator/VisualStabilityCoordinator.java | 17 ++-- .../listbuilder/ShadeListBuilderLogger.kt | 104 +++++++++++---------- .../listbuilder/pluggable/Pluggable.java | 8 +- .../collection/ShadeListBuilderTest.java | 30 +++--- .../HideNotifsForOtherUsersCoordinatorTest.java | 7 +- .../coordinator/KeyguardCoordinatorTest.kt | 2 - .../coordinator/SensitiveContentCoordinatorTest.kt | 5 +- .../SmartspaceDedupingCoordinatorTest.kt | 12 ++- .../VisualStabilityCoordinatorTest.java | 34 ++++--- 21 files changed, 171 insertions(+), 206 deletions(-) delete mode 100644 packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/SharedCoordinatorLogger.kt diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/ShadeListBuilder.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/ShadeListBuilder.java index 93761f580dd4..075a0dc7555e 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/ShadeListBuilder.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/ShadeListBuilder.java @@ -314,60 +314,62 @@ public class ShadeListBuilder implements Dumpable { } }; - private void onPreRenderInvalidated(Invalidator invalidator) { + private void onPreRenderInvalidated(Invalidator invalidator, @Nullable String reason) { Assert.isMainThread(); - mLogger.logPreRenderInvalidated(invalidator.getName(), mPipelineState.getState()); + mLogger.logPreRenderInvalidated(invalidator, mPipelineState.getState(), reason); rebuildListIfBefore(STATE_FINALIZING); } - private void onPreGroupFilterInvalidated(NotifFilter filter) { + private void onPreGroupFilterInvalidated(NotifFilter filter, @Nullable String reason) { Assert.isMainThread(); - mLogger.logPreGroupFilterInvalidated(filter.getName(), mPipelineState.getState()); + mLogger.logPreGroupFilterInvalidated(filter, mPipelineState.getState(), reason); rebuildListIfBefore(STATE_PRE_GROUP_FILTERING); } - private void onReorderingAllowedInvalidated(NotifStabilityManager stabilityManager) { + private void onReorderingAllowedInvalidated(NotifStabilityManager stabilityManager, + @Nullable String reason) { Assert.isMainThread(); mLogger.logReorderingAllowedInvalidated( - stabilityManager.getName(), - mPipelineState.getState()); + stabilityManager, + mPipelineState.getState(), + reason); rebuildListIfBefore(STATE_GROUPING); } - private void onPromoterInvalidated(NotifPromoter promoter) { + private void onPromoterInvalidated(NotifPromoter promoter, @Nullable String reason) { Assert.isMainThread(); - mLogger.logPromoterInvalidated(promoter.getName(), mPipelineState.getState()); + mLogger.logPromoterInvalidated(promoter, mPipelineState.getState(), reason); rebuildListIfBefore(STATE_TRANSFORMING); } - private void onNotifSectionInvalidated(NotifSectioner section) { + private void onNotifSectionInvalidated(NotifSectioner section, @Nullable String reason) { Assert.isMainThread(); - mLogger.logNotifSectionInvalidated(section.getName(), mPipelineState.getState()); + mLogger.logNotifSectionInvalidated(section, mPipelineState.getState(), reason); rebuildListIfBefore(STATE_SORTING); } - private void onFinalizeFilterInvalidated(NotifFilter filter) { + private void onFinalizeFilterInvalidated(NotifFilter filter, @Nullable String reason) { Assert.isMainThread(); - mLogger.logFinalizeFilterInvalidated(filter.getName(), mPipelineState.getState()); + mLogger.logFinalizeFilterInvalidated(filter, mPipelineState.getState(), reason); rebuildListIfBefore(STATE_FINALIZE_FILTERING); } - private void onNotifComparatorInvalidated(NotifComparator comparator) { + private void onNotifComparatorInvalidated(NotifComparator comparator, @Nullable String reason) { Assert.isMainThread(); - mLogger.logNotifComparatorInvalidated(comparator.getName(), mPipelineState.getState()); + mLogger.logNotifComparatorInvalidated(comparator, mPipelineState.getState(), reason); rebuildListIfBefore(STATE_SORTING); } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/BubbleCoordinator.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/BubbleCoordinator.java index b923fdfe6be8..77b6c6d9ae19 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/BubbleCoordinator.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/BubbleCoordinator.java @@ -150,7 +150,7 @@ public class BubbleCoordinator implements Coordinator { @Override public void invalidateNotifications(String reason) { - mNotifFilter.invalidateList(); + mNotifFilter.invalidateList(reason); } @Override diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/DebugModeCoordinator.kt b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/DebugModeCoordinator.kt index df54ccd85e73..240540515705 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/DebugModeCoordinator.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/DebugModeCoordinator.kt @@ -30,12 +30,12 @@ class DebugModeCoordinator @Inject constructor( ) : Coordinator { override fun attach(pipeline: NotifPipeline) { - pipeline.addPreGroupFilter(preGroupFilter) - debugModeFilterProvider.registerInvalidationListener(preGroupFilter::invalidateList) + pipeline.addPreGroupFilter(filter) + debugModeFilterProvider.registerInvalidationListener { filter.invalidateList(null) } } - private val preGroupFilter = object : NotifFilter("DebugModeCoordinator") { + private val filter = object : NotifFilter("DebugModeFilter") { override fun shouldFilterOut(entry: NotificationEntry, now: Long) = debugModeFilterProvider.shouldFilterOut(entry) } -} \ No newline at end of file +} diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/DeviceProvisionedCoordinator.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/DeviceProvisionedCoordinator.java index e8652493da6b..058042c4bccd 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/DeviceProvisionedCoordinator.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/DeviceProvisionedCoordinator.java @@ -90,7 +90,7 @@ public class DeviceProvisionedCoordinator implements Coordinator { new DeviceProvisionedController.DeviceProvisionedListener() { @Override public void onDeviceProvisionedChanged() { - mNotifFilter.invalidateList(); + mNotifFilter.invalidateList("onDeviceProvisionedChanged"); } }; } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/HeadsUpCoordinator.kt b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/HeadsUpCoordinator.kt index a61db4107c94..8278b549a7a0 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/HeadsUpCoordinator.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/HeadsUpCoordinator.kt @@ -35,6 +35,7 @@ import com.android.systemui.statusbar.notification.collection.render.NodeControl import com.android.systemui.statusbar.notification.dagger.IncomingHeader import com.android.systemui.statusbar.notification.interruption.HeadsUpViewBinder import com.android.systemui.statusbar.notification.interruption.NotificationInterruptStateProvider +import com.android.systemui.statusbar.notification.logKey import com.android.systemui.statusbar.notification.stack.BUCKET_HEADS_UP import com.android.systemui.statusbar.policy.HeadsUpManager import com.android.systemui.statusbar.policy.OnHeadsUpChangedListener @@ -278,8 +279,8 @@ class HeadsUpCoordinator @Inject constructor( .firstOrNull() ?.let { posted -> posted.entry.takeIf { entry -> - locationLookupByKey(entry.key) == GroupLocation.Isolated - && entry.sbn.notification.groupAlertBehavior == GROUP_ALERT_SUMMARY + locationLookupByKey(entry.key) == GroupLocation.Isolated && + entry.sbn.notification.groupAlertBehavior == GROUP_ALERT_SUMMARY } } @@ -512,7 +513,7 @@ class HeadsUpCoordinator @Inject constructor( private val mOnHeadsUpChangedListener = object : OnHeadsUpChangedListener { override fun onHeadsUpStateChanged(entry: NotificationEntry, isHeadsUp: Boolean) { if (!isHeadsUp) { - mNotifPromoter.invalidateList() + mNotifPromoter.invalidateList("headsUpEnded: ${entry.logKey}") mHeadsUpViewBinder.unbindHeadsUpView(entry) endNotifLifetimeExtensionIfExtended(entry) } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/HideNotifsForOtherUsersCoordinator.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/HideNotifsForOtherUsersCoordinator.java index 7b5cf8511900..e4e2bc16b77e 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/HideNotifsForOtherUsersCoordinator.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/HideNotifsForOtherUsersCoordinator.java @@ -41,14 +41,11 @@ import javax.inject.Inject; @CoordinatorScope public class HideNotifsForOtherUsersCoordinator implements Coordinator { private final NotificationLockscreenUserManager mLockscreenUserManager; - private final SharedCoordinatorLogger mLogger; @Inject public HideNotifsForOtherUsersCoordinator( - NotificationLockscreenUserManager lockscreenUserManager, - SharedCoordinatorLogger logger) { + NotificationLockscreenUserManager lockscreenUserManager) { mLockscreenUserManager = lockscreenUserManager; - mLogger = logger; } @Override @@ -70,23 +67,19 @@ public class HideNotifsForOtherUsersCoordinator implements Coordinator { // changes @Override public void onCurrentProfilesChanged(SparseArray currentProfiles) { - mLogger.logUserOrProfileChanged( - mLockscreenUserManager.getCurrentUserId(), - profileIdsToStr(currentProfiles)); - mFilter.invalidateList(); + StringBuilder sb = new StringBuilder("onCurrentProfilesChanged:"); + sb.append(" user=").append(mLockscreenUserManager.getCurrentUserId()); + sb.append(" profiles="); + sb.append("{"); + for (int i = 0; i < currentProfiles.size(); i++) { + if (i != 0) { + sb.append(","); + } + sb.append(currentProfiles.keyAt(i)); + } + sb.append("}"); + mFilter.invalidateList(sb.toString()); } }; - private String profileIdsToStr(SparseArray currentProfiles) { - StringBuilder sb = new StringBuilder(); - sb.append("{"); - for (int i = 0; i < currentProfiles.size(); i++) { - sb.append(currentProfiles.keyAt(i)); - if (i < currentProfiles.size() - 1) { - sb.append(","); - } - } - sb.append("}"); - return sb.toString(); - } } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/KeyguardCoordinator.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/KeyguardCoordinator.java index ef63be0633bf..e3d71c8b29d9 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/KeyguardCoordinator.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/KeyguardCoordinator.java @@ -38,18 +38,15 @@ public class KeyguardCoordinator implements Coordinator { private static final String TAG = "KeyguardCoordinator"; private final KeyguardNotificationVisibilityProvider mKeyguardNotificationVisibilityProvider; private final SectionHeaderVisibilityProvider mSectionHeaderVisibilityProvider; - private final SharedCoordinatorLogger mLogger; private final StatusBarStateController mStatusBarStateController; @Inject public KeyguardCoordinator( KeyguardNotificationVisibilityProvider keyguardNotificationVisibilityProvider, SectionHeaderVisibilityProvider sectionHeaderVisibilityProvider, - SharedCoordinatorLogger logger, StatusBarStateController statusBarStateController) { mKeyguardNotificationVisibilityProvider = keyguardNotificationVisibilityProvider; mSectionHeaderVisibilityProvider = sectionHeaderVisibilityProvider; - mLogger = logger; mStatusBarStateController = statusBarStateController; } @@ -78,9 +75,8 @@ public class KeyguardCoordinator implements Coordinator { } private void invalidateListFromFilter(String reason) { - mLogger.logKeyguardCoordinatorInvalidated(reason); updateSectionHeadersVisibility(); - mNotifFilter.invalidateList(); + mNotifFilter.invalidateList(reason); } private void updateSectionHeadersVisibility() { 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 023c4ef2b8b7..a2506d554e7f 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 @@ -16,6 +16,7 @@ package com.android.systemui.statusbar.notification.collection.coordinator; +import static com.android.systemui.statusbar.notification.NotificationUtils.logKey; import static com.android.systemui.statusbar.notification.stack.NotificationChildrenContainer.NUMBER_OF_CHILDREN_WHEN_CHILDREN_EXPANDED; import static java.util.Objects.requireNonNull; @@ -147,7 +148,8 @@ public class PreparationCoordinator implements Coordinator { @Override public void attach(NotifPipeline pipeline) { mNotifErrorManager.addInflationErrorListener(mInflationErrorListener); - mAdjustmentProvider.addDirtyListener(mNotifInflatingFilter::invalidateList); + mAdjustmentProvider.addDirtyListener( + () -> mNotifInflatingFilter.invalidateList("adjustmentProviderChanged")); pipeline.addCollectionListener(mNotifCollectionListener); // Inflate after grouping/sorting since that affects what views to inflate. @@ -245,12 +247,13 @@ public class PreparationCoordinator implements Coordinator { } catch (RemoteException ex) { // System server is dead, nothing to do about that } - mNotifInflationErrorFilter.invalidateList(); + mNotifInflationErrorFilter.invalidateList("onNotifInflationError for " + logKey(entry)); } @Override public void onNotifInflationErrorCleared(NotificationEntry entry) { - mNotifInflationErrorFilter.invalidateList(); + mNotifInflationErrorFilter.invalidateList( + "onNotifInflationErrorCleared for " + logKey(entry)); } }; @@ -371,7 +374,7 @@ public class PreparationCoordinator implements Coordinator { mViewBarn.registerViewForEntry(entry, controller); mInflationStates.put(entry, STATE_INFLATED); mBindEventManager.notifyViewBound(entry); - mNotifInflatingFilter.invalidateList(); + mNotifInflatingFilter.invalidateList("onInflationFinished for " + logKey(entry)); } private void freeNotifViews(NotificationEntry entry) { diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/RankingCoordinator.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/RankingCoordinator.java index 5ac481341d43..67a8a63ad7da 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/RankingCoordinator.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/RankingCoordinator.java @@ -199,7 +199,7 @@ public class RankingCoordinator implements Coordinator { new StatusBarStateController.StateListener() { @Override public void onDozingChanged(boolean isDozing) { - mDndVisualEffectsFilter.invalidateList(); + mDndVisualEffectsFilter.invalidateList("onDozingChanged to " + isDozing); } }; } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/SensitiveContentCoordinator.kt b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/SensitiveContentCoordinator.kt index 3f8a39f62dfb..aeeeb4fb054d 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/SensitiveContentCoordinator.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/SensitiveContentCoordinator.kt @@ -64,7 +64,7 @@ private class SensitiveContentCoordinatorImpl @Inject constructor( pipeline.addPreRenderInvalidator(this) } - override fun onDynamicPrivacyChanged(): Unit = invalidateList() + override fun onDynamicPrivacyChanged(): Unit = invalidateList("onDynamicPrivacyChanged") override fun onBeforeRenderList(entries: List) { if (keyguardStateController.isKeyguardGoingAway() || diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/SharedCoordinatorLogger.kt b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/SharedCoordinatorLogger.kt deleted file mode 100644 index 25bc641f0a84..000000000000 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/SharedCoordinatorLogger.kt +++ /dev/null @@ -1,46 +0,0 @@ -/* - * 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.coordinator - -import com.android.systemui.log.LogBuffer -import com.android.systemui.log.LogLevel -import com.android.systemui.log.dagger.NotificationLog -import javax.inject.Inject - -/** - * Shared logging class for coordinators that don't log enough to merit their own logger. - */ -class SharedCoordinatorLogger @Inject constructor( - @NotificationLog private val buffer: LogBuffer -) { - fun logUserOrProfileChanged(userId: Int, profiles: String) { - buffer.log("NotCurrentUserFilter", LogLevel.INFO, { - int1 = userId - str1 = profiles - }, { - "Current user or profiles changed. Current user is $int1; profiles are $str1" - }) - } - - fun logKeyguardCoordinatorInvalidated(reason: String) { - buffer.log("KeyguardCoordinator", LogLevel.DEBUG, { - str1 = reason - }, { - "KeyguardCoordinator invalidated: $str1" - }) - } -} diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/SmartspaceDedupingCoordinator.kt b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/SmartspaceDedupingCoordinator.kt index 48f00ac4bf55..861c2514db0f 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/SmartspaceDedupingCoordinator.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/SmartspaceDedupingCoordinator.kt @@ -30,6 +30,7 @@ import com.android.systemui.statusbar.notification.collection.NotificationEntry import com.android.systemui.statusbar.notification.collection.coordinator.dagger.CoordinatorScope import com.android.systemui.statusbar.notification.collection.listbuilder.pluggable.NotifFilter import com.android.systemui.statusbar.notification.collection.notifcollection.NotifCollectionListener +import com.android.systemui.statusbar.notification.logKey import com.android.systemui.util.concurrency.DelayableExecutor import com.android.systemui.util.time.SystemClock import java.util.concurrent.TimeUnit.SECONDS @@ -137,7 +138,7 @@ class SmartspaceDedupingCoordinator @Inject constructor( } if (changed) { - filter.invalidateList() + filter.invalidateList("onNewSmartspaceTargets") notificationEntryManager.updateNotifications("Smartspace targets changed") } @@ -174,7 +175,7 @@ class SmartspaceDedupingCoordinator @Inject constructor( target.cancelTimeoutRunnable = executor.executeDelayed({ target.cancelTimeoutRunnable = null target.shouldFilter = true - filter.invalidateList() + filter.invalidateList("updateAlertException: ${entry.logKey}") notificationEntryManager.updateNotifications("deduping timeout expired") }, alertExceptionExpires - now) } @@ -191,7 +192,7 @@ class SmartspaceDedupingCoordinator @Inject constructor( isOnLockscreen = newState == StatusBarState.KEYGUARD if (isOnLockscreen != wasOnLockscreen) { - filter.invalidateList() + filter.invalidateList("recordStatusBarState: " + StatusBarState.toString(newState)) // No need to call notificationEntryManager.updateNotifications; something else already // does it for us when the keyguard state changes } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/VisualStabilityCoordinator.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/VisualStabilityCoordinator.java index b9d23b4a344a..0833df505bf3 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/VisualStabilityCoordinator.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/VisualStabilityCoordinator.java @@ -192,11 +192,16 @@ public class VisualStabilityCoordinator implements Coordinator, Dumpable, + " reorderingAllowed " + wasReorderingAllowed + "->" + mReorderingAllowed + " when setting " + field + "=" + value); } - if ((mPipelineRunAllowed && mIsSuppressingPipelineRun) - || (mReorderingAllowed && (mIsSuppressingGroupChange - || isSuppressingSectionChange() - || mIsSuppressingEntryReorder))) { - mNotifStabilityManager.invalidateList(); + if (mPipelineRunAllowed && mIsSuppressingPipelineRun) { + mNotifStabilityManager.invalidateList("pipeline run suppression ended"); + } else if (mReorderingAllowed && (mIsSuppressingGroupChange + || isSuppressingSectionChange() + || mIsSuppressingEntryReorder)) { + String reason = "reorder suppression ended for" + + " group=" + mIsSuppressingGroupChange + + " section=" + isSuppressingSectionChange() + + " sort=" + mIsSuppressingEntryReorder; + mNotifStabilityManager.invalidateList(reason); } mVisualStabilityProvider.setReorderingAllowed(mReorderingAllowed); } @@ -241,7 +246,7 @@ public class VisualStabilityCoordinator implements Coordinator, Dumpable, now + ALLOW_SECTION_CHANGE_TIMEOUT)); if (!wasSectionChangeAllowed) { - mNotifStabilityManager.invalidateList(); + mNotifStabilityManager.invalidateList("temporarilyAllowSectionChanges"); } } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/listbuilder/ShadeListBuilderLogger.kt b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/listbuilder/ShadeListBuilderLogger.kt index 10a627d65b83..4c406e3ba0b4 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/listbuilder/ShadeListBuilderLogger.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/listbuilder/ShadeListBuilderLogger.kt @@ -25,8 +25,15 @@ import com.android.systemui.statusbar.notification.NotifPipelineFlags import com.android.systemui.statusbar.notification.collection.GroupEntry import com.android.systemui.statusbar.notification.collection.ListEntry import com.android.systemui.statusbar.notification.collection.NotificationEntry +import com.android.systemui.statusbar.notification.collection.listbuilder.PipelineState.StateName +import com.android.systemui.statusbar.notification.collection.listbuilder.PipelineState.getStateName +import com.android.systemui.statusbar.notification.collection.listbuilder.pluggable.Invalidator +import com.android.systemui.statusbar.notification.collection.listbuilder.pluggable.NotifComparator import com.android.systemui.statusbar.notification.collection.listbuilder.pluggable.NotifFilter import com.android.systemui.statusbar.notification.collection.listbuilder.pluggable.NotifPromoter +import com.android.systemui.statusbar.notification.collection.listbuilder.pluggable.NotifSectioner +import com.android.systemui.statusbar.notification.collection.listbuilder.pluggable.NotifStabilityManager +import com.android.systemui.statusbar.notification.collection.listbuilder.pluggable.Pluggable import com.android.systemui.statusbar.notification.logKey import com.android.systemui.util.Compile import javax.inject.Inject @@ -60,68 +67,63 @@ class ShadeListBuilderLogger @Inject constructor( }) } - fun logPreRenderInvalidated(filterName: String, pipelineState: Int) { + private fun logPluggableInvalidated( + type: String, + pluggable: Pluggable<*>, + @StateName pipelineState: Int, + reason: String? + ) { buffer.log(TAG, DEBUG, { - str1 = filterName + str1 = type + str2 = pluggable.name int1 = pipelineState + str3 = reason }, { - """Pre-render Invalidator "$str1" invalidated; pipeline state is $int1""" + """Invalidated while ${getStateName(int1)} by $str1 "$str2" because $str3""" }) } - fun logPreGroupFilterInvalidated(filterName: String, pipelineState: Int) { - buffer.log(TAG, DEBUG, { - str1 = filterName - int1 = pipelineState - }, { - """Pre-group NotifFilter "$str1" invalidated; pipeline state is $int1""" - }) - } + fun logPreRenderInvalidated( + invalidator: Invalidator, + @StateName pipelineState: Int, + reason: String? + ) = logPluggableInvalidated("Pre-render Invalidator", invalidator, pipelineState, reason) - fun logReorderingAllowedInvalidated(name: String, pipelineState: Int) { - buffer.log(TAG, DEBUG, { - str1 = name - int1 = pipelineState - }, { - """ReorderingNowAllowed "$str1" invalidated; pipeline state is $int1""" - }) - } + fun logPreGroupFilterInvalidated( + filter: NotifFilter, + @StateName pipelineState: Int, + reason: String? + ) = logPluggableInvalidated("Pre-group NotifFilter", filter, pipelineState, reason) - fun logPromoterInvalidated(name: String, pipelineState: Int) { - buffer.log(TAG, DEBUG, { - str1 = name - int1 = pipelineState - }, { - """NotifPromoter "$str1" invalidated; pipeline state is $int1""" - }) - } + fun logReorderingAllowedInvalidated( + stabilityManager: NotifStabilityManager, + @StateName pipelineState: Int, + reason: String? + ) = logPluggableInvalidated("ReorderingNowAllowed", stabilityManager, pipelineState, reason) - fun logNotifSectionInvalidated(name: String, pipelineState: Int) { - buffer.log(TAG, DEBUG, { - str1 = name - int1 = pipelineState - }, { - """NotifSection "$str1" invalidated; pipeline state is $int1""" - }) - } + fun logPromoterInvalidated( + promoter: NotifPromoter, + @StateName pipelineState: Int, + reason: String? + ) = logPluggableInvalidated("NotifPromoter", promoter, pipelineState, reason) - fun logNotifComparatorInvalidated(name: String, pipelineState: Int) { - buffer.log(TAG, DEBUG, { - str1 = name - int1 = pipelineState - }, { - """NotifComparator "$str1" invalidated; pipeline state is $int1""" - }) - } + fun logNotifSectionInvalidated( + sectioner: NotifSectioner, + @StateName pipelineState: Int, + reason: String? + ) = logPluggableInvalidated("NotifSection", sectioner, pipelineState, reason) - fun logFinalizeFilterInvalidated(name: String, pipelineState: Int) { - buffer.log(TAG, DEBUG, { - str1 = name - int1 = pipelineState - }, { - """Finalize NotifFilter "$str1" invalidated; pipeline state is $int1""" - }) - } + fun logNotifComparatorInvalidated( + comparator: NotifComparator, + @StateName pipelineState: Int, + reason: String? + ) = logPluggableInvalidated("NotifComparator", comparator, pipelineState, reason) + + fun logFinalizeFilterInvalidated( + filter: NotifFilter, + @StateName pipelineState: Int, + reason: String? + ) = logPluggableInvalidated("Finalize NotifFilter", filter, pipelineState, reason) fun logDuplicateSummary( buildId: Int, diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/listbuilder/pluggable/Pluggable.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/listbuilder/pluggable/Pluggable.java index b981a9621526..966ab4c61b50 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/listbuilder/pluggable/Pluggable.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/listbuilder/pluggable/Pluggable.java @@ -49,10 +49,10 @@ public abstract class Pluggable { * Call this method when something has caused this pluggable's behavior to change. The pipeline * will be re-run. */ - public final void invalidateList() { + public final void invalidateList(@Nullable String reason) { if (mListener != null) { Trace.beginSection("Pluggable<" + mName + ">.invalidateList"); - mListener.onPluggableInvalidated((This) this); + mListener.onPluggableInvalidated((This) this, reason); Trace.endSection(); } } @@ -74,7 +74,7 @@ public abstract class Pluggable { * @param The type of pluggable that is being listened to. */ public interface PluggableListener { - /** Called whenever {@link #invalidateList()} is called on this pluggable. */ - void onPluggableInvalidated(T pluggable); + /** Called whenever {@link #invalidateList(String)} is called on this pluggable. */ + void onPluggableInvalidated(T pluggable, @Nullable String reason); } } diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/ShadeListBuilderTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/ShadeListBuilderTest.java index 555adfdfdc31..dfa38abc1ff8 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/ShadeListBuilderTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/ShadeListBuilderTest.java @@ -1028,37 +1028,37 @@ public class ShadeListBuilderTest extends SysuiTestCase { // WHEN each pluggable is invalidated THEN the list is re-rendered clearInvocations(mOnRenderListListener); - packageFilter.invalidateList(); + packageFilter.invalidateList(null); assertTrue(mPipelineChoreographer.isScheduled()); mPipelineChoreographer.runIfScheduled(); verify(mOnRenderListListener).onRenderList(anyList()); clearInvocations(mOnRenderListListener); - idPromoter.invalidateList(); + idPromoter.invalidateList(null); assertTrue(mPipelineChoreographer.isScheduled()); mPipelineChoreographer.runIfScheduled(); verify(mOnRenderListListener).onRenderList(anyList()); clearInvocations(mOnRenderListListener); - section.invalidateList(); + section.invalidateList(null); assertTrue(mPipelineChoreographer.isScheduled()); mPipelineChoreographer.runIfScheduled(); verify(mOnRenderListListener).onRenderList(anyList()); clearInvocations(mOnRenderListListener); - hypeComparator.invalidateList(); + hypeComparator.invalidateList(null); assertTrue(mPipelineChoreographer.isScheduled()); mPipelineChoreographer.runIfScheduled(); verify(mOnRenderListListener).onRenderList(anyList()); clearInvocations(mOnRenderListListener); - sectionComparator.invalidateList(); + sectionComparator.invalidateList(null); assertTrue(mPipelineChoreographer.isScheduled()); mPipelineChoreographer.runIfScheduled(); verify(mOnRenderListListener).onRenderList(anyList()); clearInvocations(mOnRenderListListener); - preRenderInvalidator.invalidateList(); + preRenderInvalidator.invalidateList(null); assertTrue(mPipelineChoreographer.isScheduled()); mPipelineChoreographer.runIfScheduled(); verify(mOnRenderListListener).onRenderList(anyList()); @@ -1584,7 +1584,7 @@ public class ShadeListBuilderTest extends SysuiTestCase { // WHEN visual stability manager allows group changes again mStabilityManager.setAllowGroupChanges(true); - mStabilityManager.invalidateList(); + mStabilityManager.invalidateList(null); mPipelineChoreographer.runIfScheduled(); // THEN entries are grouped @@ -1623,7 +1623,7 @@ public class ShadeListBuilderTest extends SysuiTestCase { // WHEN section changes are allowed again mStabilityManager.setAllowSectionChanges(true); - mStabilityManager.invalidateList(); + mStabilityManager.invalidateList(null); mPipelineChoreographer.runIfScheduled(); // THEN the section updates @@ -1719,7 +1719,7 @@ public class ShadeListBuilderTest extends SysuiTestCase { public void testOutOfOrderPreGroupFilterInvalidationThrows() { // GIVEN a PreGroupNotifFilter that gets invalidated during the grouping stage NotifFilter filter = new PackageFilter(PACKAGE_5); - OnBeforeTransformGroupsListener listener = (list) -> filter.invalidateList(); + OnBeforeTransformGroupsListener listener = (list) -> filter.invalidateList(null); mListBuilder.addPreGroupFilter(filter); mListBuilder.addOnBeforeTransformGroupsListener(listener); @@ -1735,7 +1735,7 @@ public class ShadeListBuilderTest extends SysuiTestCase { // GIVEN a NotifPromoter that gets invalidated during the sorting stage NotifPromoter promoter = new IdPromoter(47); OnBeforeSortListener listener = - (list) -> promoter.invalidateList(); + (list) -> promoter.invalidateList(null); mListBuilder.addPromoter(promoter); mListBuilder.addOnBeforeSortListener(listener); @@ -1751,7 +1751,7 @@ public class ShadeListBuilderTest extends SysuiTestCase { // GIVEN a NotifComparator that gets invalidated during the finalizing stage NotifComparator comparator = new HypeComparator(PACKAGE_5); OnBeforeRenderListListener listener = - (list) -> comparator.invalidateList(); + (list) -> comparator.invalidateList(null); mListBuilder.setComparators(singletonList(comparator)); mListBuilder.addOnBeforeRenderListListener(listener); @@ -1766,7 +1766,7 @@ public class ShadeListBuilderTest extends SysuiTestCase { public void testOutOfOrderPreRenderFilterInvalidationThrows() { // GIVEN a PreRenderNotifFilter that gets invalidated during the finalizing stage NotifFilter filter = new PackageFilter(PACKAGE_5); - OnBeforeRenderListListener listener = (list) -> filter.invalidateList(); + OnBeforeRenderListListener listener = (list) -> filter.invalidateList(null); mListBuilder.addFinalizeFilter(filter); mListBuilder.addOnBeforeRenderListListener(listener); @@ -1903,7 +1903,7 @@ public class ShadeListBuilderTest extends SysuiTestCase { public void testInOrderPreRenderFilter() { // GIVEN a PreRenderFilter that gets invalidated during the grouping stage NotifFilter filter = new PackageFilter(PACKAGE_5); - OnBeforeTransformGroupsListener listener = (list) -> filter.invalidateList(); + OnBeforeTransformGroupsListener listener = (list) -> filter.invalidateList(null); mListBuilder.addFinalizeFilter(filter); mListBuilder.addOnBeforeTransformGroupsListener(listener); @@ -1936,8 +1936,8 @@ public class ShadeListBuilderTest extends SysuiTestCase { mListBuilder.addFinalizeFilter(filter2); // WHEN both filters invalidate - filter1.invalidateList(); - filter2.invalidateList(); + filter1.invalidateList(null); + filter2.invalidateList(null); // THEN the pipeline choreographer is scheduled to evaluate, AND the pipeline hasn't // actually run. diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coordinator/HideNotifsForOtherUsersCoordinatorTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coordinator/HideNotifsForOtherUsersCoordinatorTest.java index d21053bcd691..27542a462d36 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coordinator/HideNotifsForOtherUsersCoordinatorTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coordinator/HideNotifsForOtherUsersCoordinatorTest.java @@ -18,7 +18,9 @@ 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.anyInt; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -51,7 +53,6 @@ public class HideNotifsForOtherUsersCoordinatorTest extends SysuiTestCase { @Mock private NotificationLockscreenUserManager mLockscreenUserManager; @Mock private NotifPipeline mNotifPipeline; @Mock private PluggableListener mInvalidationListener; - @Mock private SharedCoordinatorLogger mLogger; @Captor private ArgumentCaptor mUserChangedListenerCaptor; @Captor private ArgumentCaptor mNotifFilterCaptor; @@ -66,7 +67,7 @@ public class HideNotifsForOtherUsersCoordinatorTest extends SysuiTestCase { MockitoAnnotations.initMocks(this); HideNotifsForOtherUsersCoordinator coordinator = - new HideNotifsForOtherUsersCoordinator(mLockscreenUserManager, mLogger); + new HideNotifsForOtherUsersCoordinator(mLockscreenUserManager); coordinator.attach(mNotifPipeline); verify(mLockscreenUserManager).addUserChangedListener(mUserChangedListenerCaptor.capture()); @@ -102,6 +103,6 @@ public class HideNotifsForOtherUsersCoordinatorTest extends SysuiTestCase { mCapturedUserChangeListener.onCurrentProfilesChanged(new SparseArray<>()); // THEN the filter is invalidated - verify(mInvalidationListener).onPluggableInvalidated(mCapturedNotifFilter); + verify(mInvalidationListener).onPluggableInvalidated(eq(mCapturedNotifFilter), any()); } } diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coordinator/KeyguardCoordinatorTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coordinator/KeyguardCoordinatorTest.kt index 8c506a6d16ae..7e2e6f619946 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coordinator/KeyguardCoordinatorTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coordinator/KeyguardCoordinatorTest.kt @@ -41,7 +41,6 @@ class KeyguardCoordinatorTest : SysuiTestCase() { private val notifPipeline: NotifPipeline = mock() private val keyguardNotifVisibilityProvider: KeyguardNotificationVisibilityProvider = mock() private val sectionHeaderVisibilityProvider: SectionHeaderVisibilityProvider = mock() - private val sharedCoordinatorLogger: SharedCoordinatorLogger = mock() private val statusBarStateController: StatusBarStateController = mock() private lateinit var onStateChangeListener: Consumer @@ -52,7 +51,6 @@ class KeyguardCoordinatorTest : SysuiTestCase() { val keyguardCoordinator = KeyguardCoordinator( keyguardNotifVisibilityProvider, sectionHeaderVisibilityProvider, - sharedCoordinatorLogger, statusBarStateController ) keyguardCoordinator.attach(notifPipeline) diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coordinator/SensitiveContentCoordinatorTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coordinator/SensitiveContentCoordinatorTest.kt index a2d8e3ddba24..27be4c802aec 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coordinator/SensitiveContentCoordinatorTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coordinator/SensitiveContentCoordinatorTest.kt @@ -34,6 +34,7 @@ import com.android.systemui.statusbar.notification.collection.listbuilder.plugga import com.android.systemui.statusbar.notification.collection.listbuilder.pluggable.Pluggable import com.android.systemui.statusbar.policy.KeyguardStateController import com.android.systemui.util.mockito.any +import com.android.systemui.util.mockito.eq import com.android.systemui.util.mockito.mock import com.android.systemui.util.mockito.withArgCaptor import dagger.BindsInstance @@ -79,7 +80,7 @@ class SensitiveContentCoordinatorTest : SysuiTestCase() { dynamicPrivacyListener.onDynamicPrivacyChanged() - verify(invalidationListener).onPluggableInvalidated(invalidator) + verify(invalidationListener).onPluggableInvalidated(eq(invalidator), any()) } @Test @@ -265,4 +266,4 @@ interface TestSensitiveContentCoordinatorComponent { @BindsInstance keyguardStateController: KeyguardStateController ): TestSensitiveContentCoordinatorComponent } -} \ No newline at end of file +} diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coordinator/SmartspaceDedupingCoordinatorTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coordinator/SmartspaceDedupingCoordinatorTest.kt index fdff6e9a52f3..d4f05050b6dd 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coordinator/SmartspaceDedupingCoordinatorTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coordinator/SmartspaceDedupingCoordinatorTest.kt @@ -35,21 +35,23 @@ import com.android.systemui.statusbar.notification.collection.listbuilder.plugga import com.android.systemui.statusbar.notification.collection.listbuilder.pluggable.Pluggable import com.android.systemui.statusbar.notification.collection.notifcollection.NotifCollectionListener import com.android.systemui.util.concurrency.FakeExecutor +import com.android.systemui.util.mockito.any +import com.android.systemui.util.mockito.eq import com.android.systemui.util.mockito.withArgCaptor import com.android.systemui.util.time.FakeSystemClock +import java.util.concurrent.TimeUnit import org.junit.Assert.assertEquals import org.junit.Assert.assertFalse import org.junit.Assert.assertTrue import org.junit.Before import org.junit.Test import org.mockito.Mock -import org.mockito.Mockito.`when` import org.mockito.Mockito.anyString import org.mockito.Mockito.clearInvocations import org.mockito.Mockito.never import org.mockito.Mockito.verify +import org.mockito.Mockito.`when` import org.mockito.MockitoAnnotations -import java.util.concurrent.TimeUnit @SmallTest class SmartspaceDedupingCoordinatorTest : SysuiTestCase() { @@ -349,7 +351,7 @@ class SmartspaceDedupingCoordinatorTest : SysuiTestCase() { // THEN the new pipeline is invalidated (but the old one isn't because it's not // necessary) because the notif should no longer be filtered out - verify(pluggableListener).onPluggableInvalidated(filter) + verify(pluggableListener).onPluggableInvalidated(eq(filter), any()) verify(notificationEntryManager, never()).updateNotifications(anyString()) assertFalse(filter.shouldFilterOut(entry2HasNotRecentlyAlerted, now)) } @@ -387,7 +389,7 @@ class SmartspaceDedupingCoordinatorTest : SysuiTestCase() { } private fun verifyPipelinesInvalidated() { - verify(pluggableListener).onPluggableInvalidated(filter) + verify(pluggableListener).onPluggableInvalidated(eq(filter), any()) verify(notificationEntryManager).updateNotifications(anyString()) } @@ -396,7 +398,7 @@ class SmartspaceDedupingCoordinatorTest : SysuiTestCase() { } private fun verifyPipelinesNotInvalidated() { - verify(pluggableListener, never()).onPluggableInvalidated(filter) + verify(pluggableListener, never()).onPluggableInvalidated(eq(filter), any()) verify(notificationEntryManager, never()).updateNotifications(anyString()) } diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coordinator/VisualStabilityCoordinatorTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coordinator/VisualStabilityCoordinatorTest.java index 2f37f8928e79..96c40ecd3d81 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coordinator/VisualStabilityCoordinatorTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coordinator/VisualStabilityCoordinatorTest.java @@ -19,6 +19,7 @@ package com.android.systemui.statusbar.notification.collection.coordinator; import static junit.framework.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.doAnswer; import static org.mockito.Mockito.never; @@ -55,6 +56,7 @@ import org.mockito.ArgumentCaptor; import org.mockito.Captor; import org.mockito.Mock; import org.mockito.MockitoAnnotations; +import org.mockito.verification.VerificationMode; @SmallTest @RunWith(AndroidTestingRunner.class) @@ -130,7 +132,7 @@ public class VisualStabilityCoordinatorTest extends SysuiTestCase { doAnswer(i -> { mNotifStabilityManager.onBeginRun(); return null; - }).when(mInvalidateListener).onPluggableInvalidated(eq(mNotifStabilityManager)); + }).when(mInvalidateListener).onPluggableInvalidated(eq(mNotifStabilityManager), any()); } @Test @@ -280,7 +282,7 @@ public class VisualStabilityCoordinatorTest extends SysuiTestCase { mCoordinator.temporarilyAllowSectionChanges(mEntry, mFakeSystemClock.uptimeMillis()); // THEN the notification list is invalidated - verify(mInvalidateListener, times(1)).onPluggableInvalidated(mNotifStabilityManager); + verifyStabilityManagerWasInvalidated(times(1)); } @Test @@ -295,7 +297,7 @@ public class VisualStabilityCoordinatorTest extends SysuiTestCase { mCoordinator.temporarilyAllowSectionChanges(mEntry, mFakeSystemClock.currentTimeMillis()); // THEN invalidate is not called because this entry was never suppressed from reordering - verify(mInvalidateListener, never()).onPluggableInvalidated(mNotifStabilityManager); + verifyStabilityManagerWasInvalidated(never()); } @Test @@ -312,7 +314,7 @@ public class VisualStabilityCoordinatorTest extends SysuiTestCase { // THEN invalidate is not called because this entry was never suppressed from reordering; // THEN section changes are allowed for this notification - verify(mInvalidateListener, never()).onPluggableInvalidated(mNotifStabilityManager); + verifyStabilityManagerWasInvalidated(never()); assertTrue(mNotifStabilityManager.isSectionChangeAllowed(mEntry)); // WHEN we're pulsing (now disallowing reordering) @@ -341,13 +343,13 @@ public class VisualStabilityCoordinatorTest extends SysuiTestCase { // WHEN we temporarily allow section changes for this notification entry mCoordinator.temporarilyAllowSectionChanges(mEntry, mFakeSystemClock.currentTimeMillis()); // can now reorder, so invalidates - verify(mInvalidateListener, times(1)).onPluggableInvalidated(mNotifStabilityManager); + verifyStabilityManagerWasInvalidated(times(1)); // WHEN reordering is now allowed because device isn't pulsing anymore setPulsing(false); // THEN invalidate isn't called a second time since reordering was already allowed - verify(mInvalidateListener, times(1)).onPluggableInvalidated(mNotifStabilityManager); + verifyStabilityManagerWasInvalidated(times(1)); } @Test @@ -368,7 +370,7 @@ public class VisualStabilityCoordinatorTest extends SysuiTestCase { // THEN we never see any calls to invalidate since there weren't any notifications that // were being suppressed from grouping or section changes - verify(mInvalidateListener, never()).onPluggableInvalidated(mNotifStabilityManager); + verifyStabilityManagerWasInvalidated(never()); } @Test @@ -386,7 +388,7 @@ public class VisualStabilityCoordinatorTest extends SysuiTestCase { setPanelExpanded(false); // invalidate is called because we were previously suppressing a group change - verify(mInvalidateListener, times(1)).onPluggableInvalidated(mNotifStabilityManager); + verifyStabilityManagerWasInvalidated(times(1)); } @Test @@ -400,7 +402,7 @@ public class VisualStabilityCoordinatorTest extends SysuiTestCase { setActivityLaunching(false); // invalidate is called, b/c we were previously suppressing the pipeline from running - verify(mInvalidateListener, times(1)).onPluggableInvalidated(mNotifStabilityManager); + verifyStabilityManagerWasInvalidated(times(1)); } @Test @@ -414,7 +416,7 @@ public class VisualStabilityCoordinatorTest extends SysuiTestCase { setPanelCollapsing(false); // invalidate is called, b/c we were previously suppressing the pipeline from running - verify(mInvalidateListener, times(1)).onPluggableInvalidated(mNotifStabilityManager); + verifyStabilityManagerWasInvalidated(times(1)); } @Test @@ -426,7 +428,7 @@ public class VisualStabilityCoordinatorTest extends SysuiTestCase { setPanelCollapsing(false); // THEN invalidate is not called, b/c nothing has been suppressed - verify(mInvalidateListener, never()).onPluggableInvalidated(mNotifStabilityManager); + verifyStabilityManagerWasInvalidated(never()); } @Test @@ -438,7 +440,7 @@ public class VisualStabilityCoordinatorTest extends SysuiTestCase { setActivityLaunching(false); // THEN invalidate is not called, b/c nothing has been suppressed - verify(mInvalidateListener, never()).onPluggableInvalidated(mNotifStabilityManager); + verifyStabilityManagerWasInvalidated(never()); } @Test @@ -457,7 +459,7 @@ public class VisualStabilityCoordinatorTest extends SysuiTestCase { setPanelExpanded(false); // invalidate is called because we were previously suppressing an entry reorder - verify(mInvalidateListener, times(1)).onPluggableInvalidated(mNotifStabilityManager); + verifyStabilityManagerWasInvalidated(times(1)); } @Test @@ -474,7 +476,7 @@ public class VisualStabilityCoordinatorTest extends SysuiTestCase { setPanelExpanded(false); // invalidate is not called because we were not told that an entry reorder was suppressed - verify(mInvalidateListener, never()).onPluggableInvalidated(mNotifStabilityManager); + verifyStabilityManagerWasInvalidated(never()); } @Test @@ -499,6 +501,10 @@ public class VisualStabilityCoordinatorTest extends SysuiTestCase { assertFalse(mNotifStabilityManager.isGroupPruneAllowed(mGroupEntry)); } + private void verifyStabilityManagerWasInvalidated(VerificationMode mode) { + verify(mInvalidateListener, mode).onPluggableInvalidated(eq(mNotifStabilityManager), any()); + } + private void setActivityLaunching(boolean activityLaunching) { mNotifPanelEventsCallback.onLaunchingActivityChanged(activityLaunching); } -- cgit v1.2.3-59-g8ed1b From 3caf79a3ead78c75b6ea87e1b2996a763a76d00f Mon Sep 17 00:00:00 2001 From: Jeff DeCew Date: Fri, 1 Jul 2022 17:24:16 +0000 Subject: Only log NOTIF INFLATION ABORTED if we may have actually aborted something. Bug: 236140753 Test: atest PreparationCoodinatorTest Change-Id: I60fb07e47da7bcb5e81c7a97c9fa7b73310a0335 --- .../statusbar/notification/collection/NotifInflaterImpl.java | 4 ++-- .../statusbar/notification/collection/NotificationEntry.java | 4 +++- .../collection/coordinator/PreparationCoordinator.java | 8 +++++--- .../statusbar/notification/collection/inflation/NotifInflater.kt | 6 +++--- .../collection/coordinator/PreparationCoordinatorTest.java | 3 ++- 5 files changed, 15 insertions(+), 10 deletions(-) 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 6a3799b38dc5..d1aa01bb2125 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 @@ -75,8 +75,8 @@ public class NotifInflaterImpl implements NotifInflater { } @Override - public void abortInflation(NotificationEntry entry) { - entry.abortTask(); + public boolean abortInflation(NotificationEntry entry) { + return entry.abortTask(); } @Override 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 0a16fb65b1ac..b6392f705c49 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 @@ -476,11 +476,13 @@ public final class NotificationEntry extends ListEntry { /** * Abort all existing inflation tasks */ - public void abortTask() { + public boolean abortTask() { if (mRunningTask != null) { mRunningTask.abort(); mRunningTask = null; + return true; } + return false; } public void setInflationTask(InflationTask abortableTask) { 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 a2506d554e7f..ef1e57b4cd3b 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 @@ -363,9 +363,11 @@ public class PreparationCoordinator implements Coordinator { } private void abortInflation(NotificationEntry entry, String reason) { - mLogger.logInflationAborted(entry, reason); - mNotifInflater.abortInflation(entry); - mInflatingNotifs.remove(entry); + final boolean taskAborted = mNotifInflater.abortInflation(entry); + final boolean wasInflating = mInflatingNotifs.remove(entry); + if (taskAborted || wasInflating) { + mLogger.logInflationAborted(entry, reason); + } } private void onInflationFinished(NotificationEntry entry, NotifViewController controller) { diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/inflation/NotifInflater.kt b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/inflation/NotifInflater.kt index 567ec85cf6c7..08e21e8f668e 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/inflation/NotifInflater.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/inflation/NotifInflater.kt @@ -42,9 +42,9 @@ interface NotifInflater { /** * Request to stop the inflation of an entry. For example, called when a notification is - * removed and no longer needs to be inflated. + * removed and no longer needs to be inflated. Returns whether anything may have been aborted. */ - fun abortInflation(entry: NotificationEntry) + fun abortInflation(entry: NotificationEntry): Boolean /** * Called to let the system remove the content views from the notification row. @@ -62,4 +62,4 @@ interface NotifInflater { * A class holding parameters used when inflating the notification row */ class Params(val isLowPriority: Boolean, val reason: String) -} \ No newline at end of file +} 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 a6d3719de5cc..f4adf6927e31 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 @@ -463,7 +463,8 @@ public class PreparationCoordinatorTest extends SysuiTestCase { } @Override - public void abortInflation(@NonNull NotificationEntry entry) { + public boolean abortInflation(@NonNull NotificationEntry entry) { + return false; } public InflationCallback getInflateCallback(NotificationEntry entry) { -- cgit v1.2.3-59-g8ed1b