Android/frameworks_base
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Dedup smart reply updates for bubble flyout

Browse Source 
Smart reply updates are not visually interruptive and bubbles should not
show flyout for them, since flyout text remains the same.

1) Modify NoMan.isVisuallyInterruptive to skip evaluation of fields
irrelevant to bubbles

2) Modify NotificationComparator to rank interruptive notifs higher

3) Pipe bool (isInterruptive) from system_process to SysUI
	NoMan --- set bool on notif record and ranking
	Ranking --- parcel bool for cross-process transport
	SysUI notif entry --- get bool from ranking
	SysUI bubble data --- on notif entry update, suppress flyout if isInterruptive=false

	Considered adding "isInterruptive" bool to StatusBarNotification.
	Did not because "visually-interruptive" is additional information that the
	system figured out and SBNs should be limited to info from the app.

4) NoMan --- schedule ranking update if interruptive changes for bubble

Fixes: 138755533
Test: manual - send one sms => flyout appears once
Test: manual - send multiple sms in a row => flyout appears for each one
Test: atest FrameworksUiServicesTests
Test: atest NotificationComparatorTest
Test: atest SystemUITests
Change-Id: Id4b855054689ee73a109bb7cd18004531b41f28c
(cherry picked from commit 0dddc61824d091e48962e36939828cae3cde2aa9)
This commit is contained in:
Lyn Han
2019-08-13 16:07:55 -07:00
committed by Julia Reynolds
parent 1d35b3cd99
commit 405e0b705e
10 changed files with 101 additions and 21 deletions
Download Patch File Download Diff File Expand all files Collapse all files

15
core/java/android/service/notification/NotificationListenerService.java
View File

@@ -1522,6 +1522,7 @@ public abstract class NotificationListenerService extends Service {
private ArrayList<Notification.Action> mSmartActions;
private ArrayList<CharSequence> mSmartReplies;
private boolean mCanBubble;
private boolean mVisuallyInterruptive;
private static final int PARCEL_VERSION = 2;
@@ -1553,6 +1554,7 @@ public abstract class NotificationListenerService extends Service {
out.writeTypedList(mSmartActions, flags);
out.writeCharSequenceList(mSmartReplies);
out.writeBoolean(mCanBubble);
out.writeBoolean(mVisuallyInterruptive);
}
/** @hide */
@@ -1585,6 +1587,7 @@ public abstract class NotificationListenerService extends Service {
mSmartActions = in.createTypedArrayList(Notification.Action.CREATOR);
mSmartReplies = in.readCharSequenceList();
mCanBubble = in.readBoolean();
mVisuallyInterruptive = in.readBoolean();
}
@@ -1771,6 +1774,11 @@ public abstract class NotificationListenerService extends Service {
return mCanBubble;
}
/** @hide */
public boolean visuallyInterruptive() {
return mVisuallyInterruptive;
}
/** @hide */
public boolean isNoisy() {
return mNoisy;
@@ -1787,7 +1795,8 @@ public abstract class NotificationListenerService extends Service {
ArrayList<SnoozeCriterion> snoozeCriteria, boolean showBadge,
int userSentiment, boolean hidden, long lastAudiblyAlertedMs,
boolean noisy, ArrayList<Notification.Action> smartActions,
ArrayList<CharSequence> smartReplies, boolean canBubble) {
ArrayList<CharSequence> smartReplies, boolean canBubble,
boolean visuallyInterruptive) {
mKey = key;
mRank = rank;
mIsAmbient = importance < NotificationManager.IMPORTANCE_LOW;
@@ -1808,6 +1817,7 @@ public abstract class NotificationListenerService extends Service {
mSmartActions = smartActions;
mSmartReplies = smartReplies;
mCanBubble = canBubble;
mVisuallyInterruptive = visuallyInterruptive;
}
/**
@@ -1832,7 +1842,8 @@ public abstract class NotificationListenerService extends Service {
other.mNoisy,
other.mSmartActions,
other.mSmartReplies,
other.mCanBubble);
other.mCanBubble,
other.mVisuallyInterruptive);
}
/**

4
packages/SystemUI/src/com/android/systemui/bubbles/BubbleData.java
View File

@@ -184,6 +184,8 @@ public class BubbleData {
Log.d(TAG, "notificationEntryUpdated: " + entry);
}
Bubble bubble = getBubbleWithKey(entry.key);
suppressFlyout = !entry.isVisuallyInterruptive || suppressFlyout;
if (bubble == null) {
// Create a new bubble
bubble = new Bubble(mContext, entry);
@@ -193,8 +195,10 @@ public class BubbleData {
} else {
// Updates an existing bubble
bubble.updateEntry(entry);
bubble.setSuppressFlyout(suppressFlyout);
doUpdate(bubble);
}
if (bubble.shouldAutoExpand()) {
setSelectedBubbleInternal(bubble);
if (!mExpanded) {

8
packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotificationEntry.java
View File

@@ -140,6 +140,12 @@ public final class NotificationEntry {
*/
private boolean hasSentReply;
/**
* Whether this notification has changed in visual appearance since the previous post.
* New notifications are interruptive by default.
*/
public boolean isVisuallyInterruptive;
/**
* Whether this notification is shown to the user as a high priority notification: visible on
* the lock screen/status bar and in the top section in the shade.
@@ -205,6 +211,7 @@ public final class NotificationEntry {
+ " doesn't match existing key " + key);
}
mRanking = ranking;
isVisuallyInterruptive = ranking.visuallyInterruptive();
}
public NotificationChannel getChannel() {
@@ -244,6 +251,7 @@ public final class NotificationEntry {
return mRanking.canBubble();
}
public @NonNull List<Notification.Action> getSmartActions() {
return mRanking.getSmartActions();
}

4
packages/SystemUI/tests/src/com/android/systemui/statusbar/RankingBuilder.java
View File

@@ -51,6 +51,7 @@ public class RankingBuilder {
private ArrayList<Notification.Action> mSmartActions = new ArrayList<>();
private ArrayList<CharSequence> mSmartReplies = new ArrayList<>();
private boolean mCanBubble = false;
private boolean mIsVisuallyInterruptive = false;
public RankingBuilder() {
}
@@ -98,7 +99,8 @@ public class RankingBuilder {
mNoisy,
mSmartActions,
mSmartReplies,
mCanBubble);
mCanBubble,
mIsVisuallyInterruptive);
return ranking;
}

4
packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/NotificationEntryManagerTest.java
View File

@@ -183,7 +183,7 @@ public class NotificationEntryManagerTest extends SysuiTestCase {
0,
NotificationManager.IMPORTANCE_DEFAULT,
null, null,
null, null, null, true, sentiment, false, -1, false, null, null, false);
null, null, null, true, sentiment, false, -1, false, null, null, false, false);
return true;
}).when(mRankingMap).getRanking(eq(key), any(NotificationListenerService.Ranking.class));
}
@@ -202,7 +202,7 @@ public class NotificationEntryManagerTest extends SysuiTestCase {
null, null,
null, null, null, true,
NotificationListenerService.Ranking.USER_SENTIMENT_NEUTRAL, false, -1,
false, smartActions, null, false);
false, smartActions, null, false, false);
return true;
}).when(mRankingMap).getRanking(eq(key), any(NotificationListenerService.Ranking.class));
}

5
packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/NotificationDataTest.java
View File

@@ -653,6 +653,7 @@ public class NotificationDataTest extends SysuiTestCase {
public static final String OVERRIDE_SMART_ACTIONS = "sa";
public static final String OVERRIDE_SMART_REPLIES = "sr";
public static final String OVERRIDE_BUBBLE = "cb";
public static final String OVERRIDE_VISUALLY_INTERRUPTIVE = "vi";
public Map<String, Bundle> rankingOverrides = new HashMap<>();
@@ -713,7 +714,9 @@ public class NotificationDataTest extends SysuiTestCase {
overrides.containsKey(OVERRIDE_SMART_REPLIES)
? overrides.getCharSequenceArrayList(OVERRIDE_SMART_REPLIES)
: currentReplies,
overrides.getBoolean(OVERRIDE_BUBBLE, outRanking.canBubble()));
overrides.getBoolean(OVERRIDE_BUBBLE, outRanking.canBubble()),
overrides.getBoolean(OVERRIDE_VISUALLY_INTERRUPTIVE,
outRanking.visuallyInterruptive()));
} else {
outRanking.populate(
new RankingBuilder()

6
services/core/java/com/android/server/notification/NotificationComparator.java
View File

@@ -129,6 +129,12 @@ public class NotificationComparator
return -1 * Integer.compare(leftPriority, rightPriority);
}
final boolean leftInterruptive = left.isInterruptive();
final boolean rightInterruptive = right.isInterruptive();
if (leftInterruptive != rightInterruptive) {
return -1 * Boolean.compare(leftInterruptive, rightInterruptive);
}
// then break ties by time, most recent first
return -1 * Long.compare(left.getRankingTimeMs(), right.getRankingTimeMs());
}

46
services/core/java/com/android/server/notification/NotificationManagerService.java
View File

@@ -5759,7 +5759,9 @@ public class NotificationManagerService extends SystemService {
notification.flags |=
old.getNotification().flags & FLAG_FOREGROUND_SERVICE;
r.isUpdate = true;
r.setTextChanged(isVisuallyInterruptive(old, r));
final boolean isInterruptive = isVisuallyInterruptive(old, r);
r.setTextChanged(isInterruptive);
r.setInterruptive(isInterruptive);
}
mNotificationsByKey.put(n.getKey(), r);
@@ -5858,7 +5860,6 @@ public class NotificationManagerService extends SystemService {
Notification oldN = old.sbn.getNotification();
Notification newN = r.sbn.getNotification();
if (oldN.extras == null || newN.extras == null) {
if (DEBUG_INTERRUPTIVENESS) {
Slog.v(TAG, "INTERRUPTIVENESS: "
@@ -5890,6 +5891,7 @@ public class NotificationManagerService extends SystemService {
}
return true;
}
// Do not compare Spannables (will always return false); compare unstyled Strings
final String oldText = String.valueOf(oldN.extras.get(Notification.EXTRA_TEXT));
final String newText = String.valueOf(newN.extras.get(Notification.EXTRA_TEXT));
@@ -5904,6 +5906,7 @@ public class NotificationManagerService extends SystemService {
}
return true;
}
if (oldN.hasCompletedProgress() != newN.hasCompletedProgress()) {
if (DEBUG_INTERRUPTIVENESS) {
Slog.v(TAG, "INTERRUPTIVENESS: "
@@ -5911,6 +5914,16 @@ public class NotificationManagerService extends SystemService {
}
return true;
}
// Fields below are invisible to bubbles.
if (r.canBubble()) {
if (DEBUG_INTERRUPTIVENESS) {
Slog.v(TAG, "INTERRUPTIVENESS: "
+ r.getKey() + " is not interruptive: bubble");
}
return false;
}
// Actions
if (Notification.areActionsVisiblyDifferent(oldN, newN)) {
if (DEBUG_INTERRUPTIVENESS) {
@@ -5944,7 +5957,6 @@ public class NotificationManagerService extends SystemService {
} catch (Exception e) {
Slog.w(TAG, "error recovering builder", e);
}
return false;
}
@@ -6139,12 +6151,17 @@ public class NotificationManagerService extends SystemService {
Slog.v(TAG, "INTERRUPTIVENESS: "
+ record.getKey() + " is not interruptive: summary");
}
} else if (record.canBubble()) {
if (DEBUG_INTERRUPTIVENESS) {
Slog.v(TAG, "INTERRUPTIVENESS: "
+ record.getKey() + " is not interruptive: bubble");
}
} else {
record.setInterruptive(true);
if (DEBUG_INTERRUPTIVENESS) {
Slog.v(TAG, "INTERRUPTIVENESS: "
+ record.getKey() + " is interruptive: alerted");
}
record.setInterruptive(true);
}
MetricsLogger.action(record.getLogMaker()
.setCategory(MetricsEvent.NOTIFICATION_ALERT)
@@ -6503,15 +6520,21 @@ public class NotificationManagerService extends SystemService {
int indexBefore = findNotificationRecordIndexLocked(record);
boolean interceptBefore = record.isIntercepted();
int visibilityBefore = record.getPackageVisibilityOverride();
boolean interruptiveBefore = record.isInterruptive();
recon.applyChangesLocked(record);
applyZenModeLocked(record);
mRankingHelper.sort(mNotificationList);
int indexAfter = findNotificationRecordIndexLocked(record);
boolean interceptAfter = record.isIntercepted();
int visibilityAfter = record.getPackageVisibilityOverride();
changed = indexBefore != indexAfter || interceptBefore != interceptAfter
|| visibilityBefore != visibilityAfter;
if (interceptBefore && !interceptAfter
boolean indexChanged = indexBefore != findNotificationRecordIndexLocked(record);
boolean interceptChanged = interceptBefore != record.isIntercepted();
boolean visibilityChanged = visibilityBefore != record.getPackageVisibilityOverride();
// Broadcast isInterruptive changes for bubbles.
boolean interruptiveChanged =
record.canBubble() && (interruptiveBefore != record.isInterruptive());
changed = indexChanged || interceptChanged || visibilityChanged || interruptiveChanged;
if (interceptBefore && !record.isIntercepted()
&& record.isNewEnoughForAlerting(System.currentTimeMillis())) {
buzzBeepBlinkLocked(record);
}
@@ -7661,7 +7684,8 @@ public class NotificationManagerService extends SystemService {
record.getSound() != null || record.getVibration() != null,
record.getSystemGeneratedSmartActions(),
record.getSmartReplies(),
record.canBubble()
record.canBubble(),
record.isInterruptive()
);
rankings.add(ranking);
}

15
services/tests/uiservicestests/src/com/android/server/notification/NotificationComparatorTest.java
View File

@@ -68,6 +68,7 @@ public class NotificationComparatorTest extends UiServiceTestCase {
private final int uid2 = 1111111;
private static final String TEST_CHANNEL_ID = "test_channel_id";
private NotificationRecord mRecordMinCallNonInterruptive;
private NotificationRecord mRecordMinCall;
private NotificationRecord mRecordHighCall;
private NotificationRecord mRecordDefaultMedia;
@@ -105,6 +106,18 @@ public class NotificationComparatorTest extends UiServiceTestCase {
smsPkg = Settings.Secure.getString(mContext.getContentResolver(),
Settings.Secure.SMS_DEFAULT_APPLICATION);
Notification nonInterruptiveNotif = new Notification.Builder(mContext, TEST_CHANNEL_ID)
.setCategory(Notification.CATEGORY_CALL)
.setFlag(Notification.FLAG_FOREGROUND_SERVICE, true)
.build();
mRecordMinCallNonInterruptive = new NotificationRecord(mContext,
new StatusBarNotification(callPkg,
callPkg, 1, "mRecordMinCallNonInterruptive", callUid, callUid,
nonInterruptiveNotif,
new UserHandle(userId), "", 2000), getDefaultChannel());
mRecordMinCallNonInterruptive.setSystemImportance(NotificationManager.IMPORTANCE_MIN);
mRecordMinCallNonInterruptive.setInterruptive(false);
Notification n1 = new Notification.Builder(mContext, TEST_CHANNEL_ID)
.setCategory(Notification.CATEGORY_CALL)
.setFlag(Notification.FLAG_FOREGROUND_SERVICE, true)
@@ -113,6 +126,7 @@ public class NotificationComparatorTest extends UiServiceTestCase {
callPkg, 1, "minCall", callUid, callUid, n1,
new UserHandle(userId), "", 2000), getDefaultChannel());
mRecordMinCall.setSystemImportance(NotificationManager.IMPORTANCE_MIN);
mRecordMinCall.setInterruptive(true);
Notification n2 = new Notification.Builder(mContext, TEST_CHANNEL_ID)
.setCategory(Notification.CATEGORY_CALL)
@@ -245,6 +259,7 @@ public class NotificationComparatorTest extends UiServiceTestCase {
expected.add(mRecordCheater);
expected.add(mRecordCheaterColorized);
expected.add(mRecordMinCall);
expected.add(mRecordMinCallNonInterruptive);
List<NotificationRecord> actual = new ArrayList<>();
actual.addAll(expected);

15
services/tests/uiservicestests/src/com/android/server/notification/NotificationListenerServiceTest.java
View File

@@ -51,6 +51,8 @@ import android.service.notification.NotificationRankingUpdate;
import android.service.notification.SnoozeCriterion;
import android.test.suitebuilder.annotation.SmallTest;
import androidx.test.runner.AndroidJUnit4;
import com.android.server.UiServiceTestCase;
import org.junit.After;
@@ -61,8 +63,6 @@ import org.junit.runner.RunWith;
import java.util.ArrayList;
import java.util.List;
import androidx.test.runner.AndroidJUnit4;
@SmallTest
@RunWith(AndroidJUnit4.class)
public class NotificationListenerServiceTest extends UiServiceTestCase {
@@ -116,6 +116,7 @@ public class NotificationListenerServiceTest extends UiServiceTestCase {
assertActionsEqual(getSmartActions(key, i), ranking.getSmartActions());
assertEquals(getSmartReplies(key, i), ranking.getSmartReplies());
assertEquals(canBubble(i), ranking.canBubble());
assertEquals(visuallyInterruptive(i), ranking.visuallyInterruptive());
}
}
@@ -182,7 +183,8 @@ public class NotificationListenerServiceTest extends UiServiceTestCase {
tweak.isNoisy(),
(ArrayList) tweak.getSmartActions(),
(ArrayList) tweak.getSmartReplies(),
tweak.canBubble()
tweak.canBubble(),
tweak.visuallyInterruptive()
);
assertNotEquals(nru, nru2);
}
@@ -258,7 +260,8 @@ public class NotificationListenerServiceTest extends UiServiceTestCase {
getNoisy(i),
getSmartActions(key, i),
getSmartReplies(key, i),
canBubble(i)
canBubble(i),
visuallyInterruptive(i)
);
rankings[i] = ranking;
}
@@ -363,6 +366,10 @@ public class NotificationListenerServiceTest extends UiServiceTestCase {
return index % 4 == 0;
}
private boolean visuallyInterruptive(int index) {
return index % 4 == 0;
}
private void assertActionsEqual(
List<Notification.Action> expecteds, List<Notification.Action> actuals) {
assertEquals(expecteds.size(), actuals.size());