Fix issue with media notifs being misbucketed

Previous code assumed that "isHighPriority" == "is in top section",
which is not always true. Media notifs and some other notifs can appear
in the top section even if they're not high priority. Because we detect
section boundaries by iterating through the list until we find the first
notif where isHighPriority == false, we were sometimes drawing the
section boundary way too high. This change creates a new propery,
isInTopSection() that accurately tracks this state.

Setting this value in the proper location would require some seriously
destabilizing refactors, so instead we set it in the list comparator,
which is awful but here are.

Test: manual
Bug: 136709564
Change-Id: I19223720bac534ab92219a2962169097819e8efb
This commit is contained in:
Ned Burns
2019-07-19 14:26:15 -04:00
parent 0555f98949
commit 8c1b763dcf
10 changed files with 45 additions and 25 deletions

View File

@@ -15,7 +15,6 @@
package com.android.systemui.statusbar;
import android.content.pm.UserInfo;
import android.service.notification.StatusBarNotification;
import android.util.SparseArray;
import com.android.systemui.statusbar.notification.collection.NotificationEntry;
@@ -58,7 +57,7 @@ public interface NotificationLockscreenUserManager {
boolean shouldHideNotifications(int userId);
boolean shouldHideNotifications(String key);
boolean shouldShowOnKeyguard(StatusBarNotification sbn);
boolean shouldShowOnKeyguard(NotificationEntry entry);
boolean isAnyProfilePublicMode();

View File

@@ -33,7 +33,6 @@ import android.os.ServiceManager;
import android.os.UserHandle;
import android.os.UserManager;
import android.provider.Settings;
import android.service.notification.StatusBarNotification;
import android.util.Log;
import android.util.SparseArray;
import android.util.SparseBooleanArray;
@@ -302,7 +301,7 @@ public class NotificationLockscreenUserManagerImpl implements
Notification.VISIBILITY_SECRET;
}
public boolean shouldShowOnKeyguard(StatusBarNotification sbn) {
public boolean shouldShowOnKeyguard(NotificationEntry entry) {
if (getEntryManager() == null) {
Log.wtf(TAG, "mEntryManager was null!", new Throwable());
return false;
@@ -310,10 +309,10 @@ public class NotificationLockscreenUserManagerImpl implements
boolean exceedsPriorityThreshold;
if (NotificationUtils.useNewInterruptionModel(mContext)
&& hideSilentNotificationsOnLockscreen()) {
exceedsPriorityThreshold = getEntryManager().getNotificationData().isHighPriority(sbn);
exceedsPriorityThreshold = entry.isTopBucket();
} else {
exceedsPriorityThreshold =
!getEntryManager().getNotificationData().isAmbient(sbn.getKey());
!getEntryManager().getNotificationData().isAmbient(entry.key);
}
return mShowLockscreenNotifications && exceedsPriorityThreshold;
}

View File

@@ -397,15 +397,13 @@ public class NotificationViewHierarchyManager implements DynamicPrivacyControlle
int userId = entry.notification.getUserId();
boolean suppressedSummary = mGroupManager.isSummaryOfSuppressedGroup(
entry.notification) && !entry.isRowRemoved();
boolean showOnKeyguard = mLockscreenUserManager.shouldShowOnKeyguard(entry
.notification);
boolean showOnKeyguard = mLockscreenUserManager.shouldShowOnKeyguard(entry);
if (!showOnKeyguard) {
// min priority notifications should show if their summary is showing
if (mGroupManager.isChildInGroupWithSummary(entry.notification)) {
NotificationEntry summary = mGroupManager.getLogicalGroupSummary(
entry.notification);
if (summary != null && mLockscreenUserManager.shouldShowOnKeyguard(
summary.notification)) {
if (summary != null && mLockscreenUserManager.shouldShowOnKeyguard(summary)) {
showOnKeyguard = true;
}
}

View File

@@ -25,7 +25,6 @@ import android.service.notification.NotificationListenerService.RankingMap;
import android.service.notification.SnoozeCriterion;
import android.service.notification.StatusBarNotification;
import android.util.ArrayMap;
import android.util.Slog;
import com.android.internal.annotations.VisibleForTesting;
import com.android.systemui.Dependency;
@@ -108,10 +107,19 @@ public class NotificationData {
boolean bSystemMax = bImportance >= NotificationManager.IMPORTANCE_HIGH
&& isSystemNotification(nb);
boolean isHeadsUp = a.getRow().isHeadsUp();
if (isHeadsUp != b.getRow().isHeadsUp()) {
return isHeadsUp ? -1 : 1;
} else if (isHeadsUp) {
boolean aHeadsUp = a.getRow().isHeadsUp();
boolean bHeadsUp = b.getRow().isHeadsUp();
// HACK: This should really go elsewhere, but it's currently not straightforward to
// extract the comparison code and we're guaranteed to touch every element, so this is
// the best place to set the buckets for the moment.
a.setIsTopBucket(aHeadsUp || aMedia || aSystemMax || a.isHighPriority());
b.setIsTopBucket(bHeadsUp || bMedia || bSystemMax || b.isHighPriority());
if (aHeadsUp != bHeadsUp) {
return aHeadsUp ? -1 : 1;
} else if (aHeadsUp) {
// Provide consistent ranking with headsUpManager
return mHeadsUpManager.compare(a, b);
} else if (aMedia != bMedia) {

View File

@@ -173,6 +173,9 @@ public final class NotificationEntry {
* the lock screen/status bar and in the top section in the shade.
*/
private boolean mHighPriority;
private boolean mIsTopBucket;
private boolean mSensitive = true;
private Runnable mOnSensitiveChangedListener;
private boolean mAutoHeadsUp;
@@ -224,6 +227,18 @@ public final class NotificationEntry {
this.mHighPriority = highPriority;
}
/**
* @return True if the notif should appear in the "top" or "important" section of notifications
* (as opposed to the "bottom" or "silent" section). This is usually the same as
* {@link #isHighPriority()}, but there are certain exceptions, such as media notifs.
*/
public boolean isTopBucket() {
return mIsTopBucket;
}
public void setIsTopBucket(boolean isTopBucket) {
mIsTopBucket = isTopBucket;
}
public boolean isBubble() {
return (notification.getNotification().flags & FLAG_BUBBLE) != 0;
}

View File

@@ -133,7 +133,7 @@ class NotificationSectionsManager implements StackScrollAlgorithm.SectionProvide
if (child instanceof ExpandableNotificationRow
&& child.getVisibility() != View.GONE) {
ExpandableNotificationRow row = (ExpandableNotificationRow) child;
if (!row.getEntry().isHighPriority()) {
if (!row.getEntry().isTopBucket()) {
firstGentleNotifIndex = i;
mFirstGentleNotif = row;
break;
@@ -248,7 +248,7 @@ class NotificationSectionsManager implements StackScrollAlgorithm.SectionProvide
View child = mParent.getChildAt(i);
if (child.getVisibility() != View.GONE && child instanceof ExpandableNotificationRow) {
ExpandableNotificationRow row = (ExpandableNotificationRow) child;
if (!row.getEntry().isHighPriority()) {
if (!row.getEntry().isTopBucket()) {
break;
} else {
lastChildBeforeGap = row;

View File

@@ -5746,7 +5746,7 @@ public class NotificationStackScrollLayout extends ViewGroup implements ScrollAd
currentIndex++;
boolean beforeSpeedBump;
if (mHighPriorityBeforeSpeedBump) {
beforeSpeedBump = row.getEntry().isHighPriority();
beforeSpeedBump = row.getEntry().isTopBucket();
} else {
beforeSpeedBump = !row.getEntry().ambient;
}
@@ -5804,9 +5804,9 @@ public class NotificationStackScrollLayout extends ViewGroup implements ScrollAd
case ROWS_ALL:
return true;
case ROWS_HIGH_PRIORITY:
return row.getEntry().isHighPriority();
return row.getEntry().isTopBucket();
case ROWS_GENTLE:
return !row.getEntry().isHighPriority();
return !row.getEntry().isTopBucket();
default:
throw new IllegalArgumentException("Unknown selection: " + selection);
}

View File

@@ -803,8 +803,7 @@ public class NotificationPanelView extends PanelView implements
if (suppressedSummary) {
continue;
}
if (!mLockscreenUserManager.shouldShowOnKeyguard(
row.getStatusBarNotification())) {
if (!mLockscreenUserManager.shouldShowOnKeyguard(row.getEntry())) {
continue;
}
if (row.isRemoved()) {

View File

@@ -38,7 +38,6 @@ import android.os.Handler;
import android.os.Looper;
import android.os.UserManager;
import android.provider.Settings;
import android.service.notification.StatusBarNotification;
import android.testing.AndroidTestingRunner;
import android.testing.TestableLooper;
@@ -48,6 +47,7 @@ import com.android.systemui.Dependency;
import com.android.systemui.SysuiTestCase;
import com.android.systemui.statusbar.notification.NotificationEntryManager;
import com.android.systemui.statusbar.notification.collection.NotificationData;
import com.android.systemui.statusbar.notification.collection.NotificationEntry;
import com.android.systemui.statusbar.phone.StatusBarKeyguardViewManager;
import com.android.systemui.statusbar.policy.DeviceProvisionedController;
@@ -166,7 +166,7 @@ public class NotificationLockscreenUserManagerTest extends SysuiTestCase {
Settings.Secure.LOCK_SCREEN_SHOW_SILENT_NOTIFICATIONS, 1);
when(mNotificationData.isHighPriority(any())).thenReturn(false);
assertTrue(mLockscreenUserManager.shouldShowOnKeyguard(mock(StatusBarNotification.class)));
assertTrue(mLockscreenUserManager.shouldShowOnKeyguard(mock(NotificationEntry.class)));
}
@Test
@@ -179,7 +179,7 @@ public class NotificationLockscreenUserManagerTest extends SysuiTestCase {
Settings.Secure.LOCK_SCREEN_SHOW_SILENT_NOTIFICATIONS, 0);
when(mNotificationData.isHighPriority(any())).thenReturn(false);
assertFalse(mLockscreenUserManager.shouldShowOnKeyguard(mock(StatusBarNotification.class)));
assertFalse(mLockscreenUserManager.shouldShowOnKeyguard(mock(NotificationEntry.class)));
}
private class TestNotificationLockscreenUserManager

View File

@@ -263,6 +263,8 @@ public class NotificationSectionsManagerTest extends SysuiTestCase {
when(notifRow.getVisibility()).thenReturn(View.VISIBLE);
when(notifRow.getEntry().isHighPriority())
.thenReturn(children[i] == ChildType.HIPRI);
when(notifRow.getEntry().isTopBucket())
.thenReturn(children[i] == ChildType.HIPRI);
when(notifRow.getParent()).thenReturn(mNssl);
child = notifRow;
break;