diff options
3 files changed, 58 insertions, 76 deletions
diff --git a/packages/SystemUI/multivalentTests/src/com/android/systemui/statusbar/notification/headsup/HeadsUpManagerImplTest.kt b/packages/SystemUI/multivalentTests/src/com/android/systemui/statusbar/notification/headsup/HeadsUpManagerImplTest.kt index 98bf0e6170d4..739a9c956178 100644 --- a/packages/SystemUI/multivalentTests/src/com/android/systemui/statusbar/notification/headsup/HeadsUpManagerImplTest.kt +++ b/packages/SystemUI/multivalentTests/src/com/android/systemui/statusbar/notification/headsup/HeadsUpManagerImplTest.kt @@ -45,7 +45,6 @@ import com.android.systemui.statusbar.notification.collection.NotificationEntry import com.android.systemui.statusbar.notification.collection.NotificationEntryBuilder import com.android.systemui.statusbar.notification.collection.provider.visualStabilityProvider import com.android.systemui.statusbar.notification.collection.render.GroupMembershipManager -import com.android.systemui.statusbar.notification.headsup.HeadsUpManagerImpl.HeadsUpEntry import com.android.systemui.statusbar.notification.row.ExpandableNotificationRow import com.android.systemui.statusbar.notification.row.NotificationTestHelper import com.android.systemui.statusbar.notification.shared.NotificationThrottleHun diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/headsup/HeadsUpManagerImpl.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/headsup/HeadsUpManagerImpl.java index 17f6a3f7dfd6..6756077e5444 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/headsup/HeadsUpManagerImpl.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/headsup/HeadsUpManagerImpl.java @@ -47,6 +47,7 @@ import com.android.systemui.shade.domain.interactor.ShadeInteractor; import com.android.systemui.statusbar.StatusBarState; import com.android.systemui.statusbar.chips.notification.shared.StatusBarNotifChips; import com.android.systemui.statusbar.notification.collection.NotificationEntry; +import com.android.systemui.statusbar.notification.collection.coordinator.HeadsUpCoordinator; import com.android.systemui.statusbar.notification.collection.provider.OnReorderingAllowedListener; import com.android.systemui.statusbar.notification.collection.provider.OnReorderingBannedListener; import com.android.systemui.statusbar.notification.collection.provider.VisualStabilityProvider; @@ -65,6 +66,11 @@ import com.android.systemui.util.kotlin.JavaAdapter; import com.android.systemui.util.settings.GlobalSettings; import com.android.systemui.util.time.SystemClock; +import kotlinx.coroutines.flow.Flow; +import kotlinx.coroutines.flow.MutableStateFlow; +import kotlinx.coroutines.flow.StateFlow; +import kotlinx.coroutines.flow.StateFlowKt; + import org.jetbrains.annotations.NotNull; import java.io.PrintWriter; @@ -78,11 +84,6 @@ import java.util.stream.Stream; import javax.inject.Inject; -import kotlinx.coroutines.flow.Flow; -import kotlinx.coroutines.flow.MutableStateFlow; -import kotlinx.coroutines.flow.StateFlow; -import kotlinx.coroutines.flow.StateFlowKt; - /** * A manager which handles heads up notifications which is a special mode where * they simply peek from the top of the screen. @@ -384,9 +385,7 @@ public class HeadsUpManagerImpl HeadsUpEntry headsUpEntry = mHeadsUpEntryMap.get(key); mLogger.logUpdateNotificationRequest(key, requestedPinnedStatus, headsUpEntry != null); - Runnable runnable = () -> { - updateNotificationInternal(key, requestedPinnedStatus); - }; + Runnable runnable = () -> updateNotificationInternal(key, requestedPinnedStatus); mAvalancheController.update(headsUpEntry, runnable, "updateNotification"); } @@ -408,9 +407,7 @@ public class HeadsUpManagerImpl headsUpEntry.updateEntry(true /* updatePostTime */, "updateNotification"); PinnedStatus pinnedStatus = getNewPinnedStatusForEntry(headsUpEntry, requestedPinnedStatus); - if (headsUpEntry != null) { - setEntryPinned(headsUpEntry, pinnedStatus, "updateNotificationInternal"); - } + setEntryPinned(headsUpEntry, pinnedStatus, "updateNotificationInternal"); } } @@ -516,8 +513,7 @@ public class HeadsUpManagerImpl } /** - * @param key - * @return When a HUN entry should be removed in milliseconds from now + * @return When a HUN entry with the given key should be removed in milliseconds from now */ @Override public long getEarliestRemovalTime(String key) { @@ -529,7 +525,10 @@ public class HeadsUpManagerImpl } @VisibleForTesting - boolean shouldHeadsUpBecomePinned(@NonNull NotificationEntry entry) { + boolean shouldHeadsUpBecomePinned(@Nullable NotificationEntry entry) { + if (entry == null) { + return false; + } boolean pin = mStatusBarState == StatusBarState.SHADE && !mIsShadeOrQsExpanded; if (SceneContainerFlag.isEnabled()) { pin |= mIsQsExpanded; @@ -551,12 +550,6 @@ public class HeadsUpManagerImpl } private boolean hasFullScreenIntent(@NonNull NotificationEntry entry) { - if (entry == null) { - return false; - } - if (entry.getSbn() == null) { - return false; - } if (entry.getSbn().getNotification() == null) { return false; } @@ -566,8 +559,8 @@ public class HeadsUpManagerImpl private void setEntryPinned( @NonNull HeadsUpManagerImpl.HeadsUpEntry headsUpEntry, PinnedStatus pinnedStatus, String reason) { - mLogger.logSetEntryPinned(headsUpEntry.mEntry, pinnedStatus, reason); - NotificationEntry entry = headsUpEntry.mEntry; + NotificationEntry entry = headsUpEntry.requireEntry(); + mLogger.logSetEntryPinned(entry, pinnedStatus, reason); boolean isPinned = pinnedStatus.isPinned(); if (!isPinned) { headsUpEntry.mWasUnpinned = true; @@ -575,7 +568,7 @@ public class HeadsUpManagerImpl if (headsUpEntry.getPinnedStatus().getValue() != pinnedStatus) { headsUpEntry.setRowPinnedStatus(pinnedStatus); updatePinnedMode(); - if (isPinned && entry.getSbn() != null) { + if (isPinned) { mUiEventLogger.logWithInstanceId( NotificationPeekEvent.NOTIFICATION_PEEK, entry.getSbn().getUid(), entry.getSbn().getPackageName(), entry.getSbn().getInstanceId()); @@ -596,8 +589,8 @@ public class HeadsUpManagerImpl * @param headsUpEntry entry added */ @VisibleForTesting - void onEntryAdded(HeadsUpEntry headsUpEntry, PinnedStatus requestedPinnedStatus) { - NotificationEntry entry = headsUpEntry.mEntry; + void onEntryAdded(HeadsUpEntry headsUpEntry, PinnedStatus requestedPinnedStatus) { + NotificationEntry entry = headsUpEntry.requireEntry(); entry.setHeadsUp(true); PinnedStatus pinnedStatus = getNewPinnedStatusForEntry(headsUpEntry, requestedPinnedStatus); @@ -653,10 +646,10 @@ public class HeadsUpManagerImpl if (finalHeadsUpEntry == null) { return; } - NotificationEntry entry = finalHeadsUpEntry.mEntry; + NotificationEntry entry = finalHeadsUpEntry.requireEntry(); // If the notification is animating, we will remove it at the end of the animation. - if (entry != null && entry.isExpandAnimationRunning()) { + if (entry.isExpandAnimationRunning()) { return; } entry.demoteStickyHun(); @@ -679,8 +672,8 @@ public class HeadsUpManagerImpl * @param reason why onEntryRemoved was called */ @VisibleForTesting - protected void onEntryRemoved(HeadsUpEntry headsUpEntry, String reason) { - NotificationEntry entry = headsUpEntry.mEntry; + void onEntryRemoved(@NonNull HeadsUpEntry headsUpEntry, String reason) { + NotificationEntry entry = headsUpEntry.requireEntry(); entry.setHeadsUp(false); setEntryPinned(headsUpEntry, PinnedStatus.NotPinned, "onEntryRemoved"); EventLogTags.writeSysuiHeadsUpStatus(entry.getKey(), 0 /* visible */); @@ -702,15 +695,14 @@ public class HeadsUpManagerImpl // mEntriesToRemoveWhenReorderingAllowed, we should not remove from this list (and cause // ArrayIndexOutOfBoundsException). We don't need to in this case anyway, because we // clear mEntriesToRemoveWhenReorderingAllowed after removing these entries. - if (!reason.equals(REASON_REORDER_ALLOWED) - && mEntriesToRemoveWhenReorderingAllowed.contains(notifEntry)) { + if (!reason.equals(REASON_REORDER_ALLOWED)) { mEntriesToRemoveWhenReorderingAllowed.remove(notifEntry); } } } private void updateTopHeadsUpFlow() { - mTopHeadsUpRow.setValue((HeadsUpRowRepository) getTopHeadsUpEntry()); + mTopHeadsUpRow.setValue(getTopHeadsUpEntry()); } private void updateHeadsUpFlow() { @@ -755,17 +747,6 @@ public class HeadsUpManagerImpl } } - /** - * Manager-specific logic, that should occur, when the entry is updated, and its posted time has - * changed. - * - * @param headsUpEntry entry updated - */ - private void onEntryUpdated(HeadsUpEntry headsUpEntry) { - // no need to update the list here - updateTopHeadsUpFlow(); - } - private void updatePinnedMode() { boolean hasPinnedNotification = hasPinnedNotificationInternal(); mPinnedNotificationStatus = pinnedNotificationStatusInternal(); @@ -808,7 +789,7 @@ public class HeadsUpManagerImpl keySet.addAll(mAvalancheController.getWaitingKeys()); for (String key : keySet) { HeadsUpEntry entry = getHeadsUpEntry(key); - if (entry.mEntry == null) { + if (entry == null || entry.mEntry == null) { continue; } String packageName = entry.mEntry.getSbn().getPackageName(); @@ -981,7 +962,7 @@ public class HeadsUpManagerImpl private boolean hasPinnedNotificationInternal() { for (String key : mHeadsUpEntryMap.keySet()) { HeadsUpEntry entry = getHeadsUpEntry(key); - if (entry.mEntry != null && entry.mEntry.isRowPinned()) { + if (entry != null && entry.mEntry != null && entry.mEntry.isRowPinned()) { return true; } } @@ -1006,6 +987,10 @@ public class HeadsUpManagerImpl public void unpinAll(boolean userUnPinned) { for (String key : mHeadsUpEntryMap.keySet()) { HeadsUpEntry headsUpEntry = getHeadsUpEntry(key); + if (headsUpEntry == null) { + Log.wtf(TAG, "Couldn't find entry " + key + " in unpinAll"); + continue; + } mLogger.logUnpinEntryRequest(key); Runnable runnable = () -> { mLogger.logUnpinEntry(key); @@ -1016,10 +1001,10 @@ public class HeadsUpManagerImpl // when the user unpinned all of HUNs by moving one HUN, all of HUNs should not stay // on the screen. - if (userUnPinned && headsUpEntry.mEntry != null) { - if (headsUpEntry.mEntry != null && headsUpEntry.mEntry.mustStayOnScreen()) { - headsUpEntry.mEntry.setHeadsUpIsVisible(); - } + if (userUnPinned + && headsUpEntry.mEntry != null + && headsUpEntry.mEntry.mustStayOnScreen()) { + headsUpEntry.mEntry.setHeadsUpIsVisible(); } }; mAvalancheController.delete(headsUpEntry, runnable, "unpinAll"); @@ -1040,7 +1025,7 @@ public class HeadsUpManagerImpl } else { headsUpEntry.updateEntry(false /* updatePostTime */, "setRemoteInputActive(false)"); } - onEntryUpdated(headsUpEntry); + updateTopHeadsUpFlow(); } } @@ -1116,7 +1101,7 @@ public class HeadsUpManagerImpl * * @param entry the entry that might be indirectly removed by the user's action * - * @see HeadsUpCoordinator#mActionPressListener + * @see HeadsUpCoordinator.mActionPressListener * @see #canRemoveImmediately(String) */ public void setUserActionMayIndirectlyRemove(@NonNull NotificationEntry entry) { @@ -1155,8 +1140,8 @@ public class HeadsUpManagerImpl } /** - * @param key - * @return true if the entry is (pinned and expanded) or (has an active remote input) + * @return true if the entry with the given key is (pinned and expanded) or (has an active + * remote input) */ @Override public boolean isSticky(String key) { @@ -1356,7 +1341,7 @@ public class HeadsUpManagerImpl /** * An interface that returns the amount of time left this HUN should show. */ - interface FinishTimeUpdater { + private interface FinishTimeUpdater { long updateAndGetTimeRemaining(); } @@ -1376,6 +1361,10 @@ public class HeadsUpManagerImpl public void updateEntry(boolean updatePostTime, boolean updateEarliestRemovalTime, @Nullable String reason) { Runnable runnable = () -> { + if (mEntry == null) { + Log.wtf(TAG, "#updateEntry called with null mEntry; returning early"); + return; + } mLogger.logUpdateEntry(mEntry, updatePostTime, reason); final long now = mSystemClock.elapsedRealtime(); @@ -1397,23 +1386,18 @@ public class HeadsUpManagerImpl FinishTimeUpdater finishTimeCalculator = () -> { final long finishTime = calculateFinishTime(); final long now = mSystemClock.elapsedRealtime(); - final long timeLeft = NotificationThrottleHun.isEnabled() + return NotificationThrottleHun.isEnabled() ? Math.max(finishTime, mEarliestRemovalTime) - now : Math.max(finishTime - now, mMinimumDisplayTime); - return timeLeft; }; scheduleAutoRemovalCallback(finishTimeCalculator, "updateEntry (not sticky)"); // Notify the manager, that the posted time has changed. - onEntryUpdated(this); + updateTopHeadsUpFlow(); - if (mEntriesToRemoveAfterExpand.contains(mEntry)) { - mEntriesToRemoveAfterExpand.remove(mEntry); - } + mEntriesToRemoveAfterExpand.remove(mEntry); if (!NotificationThrottleHun.isEnabled()) { - if (mEntriesToRemoveWhenReorderingAllowed.contains(mEntry)) { - mEntriesToRemoveWhenReorderingAllowed.remove(mEntry); - } + mEntriesToRemoveWhenReorderingAllowed.remove(mEntry); } } @@ -1534,8 +1518,7 @@ public class HeadsUpManagerImpl @Override public boolean equals(@Nullable Object o) { if (this == o) return true; - if (o == null || !(o instanceof HeadsUpEntry)) return false; - HeadsUpEntry otherHeadsUpEntry = (HeadsUpEntry) o; + if (!(o instanceof HeadsUpEntry otherHeadsUpEntry)) return false; if (mEntry != null && otherHeadsUpEntry.mEntry != null) { return mEntry.getKey().equals(otherHeadsUpEntry.mEntry.getKey()); } @@ -1599,10 +1582,13 @@ public class HeadsUpManagerImpl } } - public void scheduleAutoRemovalCallback(FinishTimeUpdater finishTimeCalculator, + private void scheduleAutoRemovalCallback(FinishTimeUpdater finishTimeCalculator, @NonNull String reason) { - - mLogger.logAutoRemoveRequest(this.mEntry, reason); + if (mEntry == null) { + Log.wtf(TAG, "#scheduleAutoRemovalCallback with null mEntry; returning early"); + return; + } + mLogger.logAutoRemoveRequest(mEntry, reason); Runnable runnable = () -> { long delayMs = finishTimeCalculator.updateAndGetTimeRemaining(); @@ -1642,10 +1628,8 @@ public class HeadsUpManagerImpl public void removeAsSoonAsPossible() { if (mRemoveRunnable != null) { - FinishTimeUpdater finishTimeCalculator = () -> { - final long timeLeft = mEarliestRemovalTime - mSystemClock.elapsedRealtime(); - return timeLeft; - }; + FinishTimeUpdater finishTimeCalculator = () -> + mEarliestRemovalTime - mSystemClock.elapsedRealtime(); scheduleAutoRemovalCallback(finishTimeCalculator, "removeAsSoonAsPossible"); } } @@ -1698,7 +1682,6 @@ public class HeadsUpManagerImpl /** * Get user-preferred or default timeout duration. The larger one will be returned. * @return milliseconds before auto-dismiss - * @param requestedTimeout */ private int getRecommendedHeadsUpTimeoutMs(int requestedTimeout) { return mAccessibilityMgr.getRecommendedTimeoutMillis( diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/headsup/HeadsUpManagerLogger.kt b/packages/SystemUI/src/com/android/systemui/statusbar/notification/headsup/HeadsUpManagerLogger.kt index 1ccc45b9c385..e3ca7c81582f 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/headsup/HeadsUpManagerLogger.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/headsup/HeadsUpManagerLogger.kt @@ -156,12 +156,12 @@ constructor(@NotificationHeadsUpLog private val buffer: LogBuffer) { ) } - fun logAutoRemoveCanceled(entry: NotificationEntry, reason: String?) { + fun logAutoRemoveCanceled(entry: NotificationEntry?, reason: String?) { buffer.log( TAG, INFO, { - str1 = entry.logKey + str1 = entry?.logKey str2 = reason ?: "unknown" }, { "cancel auto remove of $str1 reason: $str2" }, |