From bac3f7da332f593fb9e6faf32a20bc87b08ae1cc Mon Sep 17 00:00:00 2001 From: Yuri Lin Date: Thu, 16 Sep 2021 13:58:22 -0400 Subject: [PATCH] Log a notification update when fields such as importance or ranking score are updated. This update happens in the function performing the actual sort because while adjustments are queued, the actual updates to fields in the NotificationRecord don't happen until sorting time. To avoid spurious extra logging, we only log an update to notifications that were updated via assistant adjustments (by tracking those keys in a set). Note that there are two ways adjustments can be applied to notifications: synchronously to an enqueued notification or asynchronously (prompting the sort). Given that enqueued notification (posts or updates) are logged once the post or update goes through, this change only logs the updates to the ones updated asynchronously. (Otherwise, we may see a notification "update" before the notification is ever even visible to the user.) Bug: 195579280 Test: manual via statsd_testdrive, atest NotificationManagerServiceTest Change-Id: I97ef8865e31653f0fee0ac943251fecc21e5e05f Merged-In: I97ef8865e31653f0fee0ac943251fecc21e5e05f --- .../com/android/server/EventLogTags.logtags | 2 + .../NotificationManagerService.java | 178 +++++++++++++----- .../notification/NotificationRecord.java | 43 +++++ .../NotificationRecordLogger.java | 21 ++- .../NotificationRecordLoggerImpl.java | 40 ++-- .../NotificationManagerServiceTest.java | 74 ++++++++ .../NotificationRecordLoggerFake.java | 15 ++ 7 files changed, 315 insertions(+), 58 deletions(-) diff --git a/services/core/java/com/android/server/EventLogTags.logtags b/services/core/java/com/android/server/EventLogTags.logtags index 483250ad22577..68fd0c12bea84 100644 --- a/services/core/java/com/android/server/EventLogTags.logtags +++ b/services/core/java/com/android/server/EventLogTags.logtags @@ -92,6 +92,8 @@ option java_package com.android.server 27533 notification_autogrouped (key|3) # notification was removed from an autogroup 275534 notification_unautogrouped (key|3) +# when a notification is adjusted via assistant +27535 notification_adjusted (key|3),(adjustment_type|3),(new_value|3) # --------------------------- # Watchdog.java diff --git a/services/core/java/com/android/server/notification/NotificationManagerService.java b/services/core/java/com/android/server/notification/NotificationManagerService.java index 78c909d92c582..7ba0f04a435f7 100755 --- a/services/core/java/com/android/server/notification/NotificationManagerService.java +++ b/services/core/java/com/android/server/notification/NotificationManagerService.java @@ -5397,6 +5397,7 @@ public class NotificationManagerService extends SystemService { == IMPORTANCE_NONE) { cancelNotificationsFromListener(token, new String[]{r.getKey()}); } else { + r.setPendingLogUpdate(true); needsSort = true; } } @@ -8057,64 +8058,151 @@ public class NotificationManagerService extends SystemService { } } + static class NotificationRecordExtractorData { + // Class that stores any field in a NotificationRecord that can change via an extractor. + // Used to cache previous data used in a sort. + int mPosition; + int mVisibility; + boolean mShowBadge; + boolean mAllowBubble; + boolean mIsBubble; + NotificationChannel mChannel; + String mGroupKey; + ArrayList mOverridePeople; + ArrayList mSnoozeCriteria; + Integer mUserSentiment; + Integer mSuppressVisually; + ArrayList mSystemSmartActions; + ArrayList mSmartReplies; + int mImportance; + + // These fields may not trigger a reranking but diffs here may be logged. + float mRankingScore; + boolean mIsConversation; + + NotificationRecordExtractorData(int position, int visibility, boolean showBadge, + boolean allowBubble, boolean isBubble, NotificationChannel channel, String groupKey, + ArrayList overridePeople, ArrayList snoozeCriteria, + Integer userSentiment, Integer suppressVisually, + ArrayList systemSmartActions, + ArrayList smartReplies, int importance, float rankingScore, + boolean isConversation) { + mPosition = position; + mVisibility = visibility; + mShowBadge = showBadge; + mAllowBubble = allowBubble; + mIsBubble = isBubble; + mChannel = channel; + mGroupKey = groupKey; + mOverridePeople = overridePeople; + mSnoozeCriteria = snoozeCriteria; + mUserSentiment = userSentiment; + mSuppressVisually = suppressVisually; + mSystemSmartActions = systemSmartActions; + mSmartReplies = smartReplies; + mImportance = importance; + mRankingScore = rankingScore; + mIsConversation = isConversation; + } + + // Returns whether the provided NotificationRecord differs from the cached data in any way. + // Should be guarded by mNotificationLock; not annotated here as this class is static. + boolean hasDiffForRankingLocked(NotificationRecord r, int newPosition) { + return mPosition != newPosition + || mVisibility != r.getPackageVisibilityOverride() + || mShowBadge != r.canShowBadge() + || mAllowBubble != r.canBubble() + || mIsBubble != r.getNotification().isBubbleNotification() + || !Objects.equals(mChannel, r.getChannel()) + || !Objects.equals(mGroupKey, r.getGroupKey()) + || !Objects.equals(mOverridePeople, r.getPeopleOverride()) + || !Objects.equals(mSnoozeCriteria, r.getSnoozeCriteria()) + || !Objects.equals(mUserSentiment, r.getUserSentiment()) + || !Objects.equals(mSuppressVisually, r.getSuppressedVisualEffects()) + || !Objects.equals(mSystemSmartActions, r.getSystemGeneratedSmartActions()) + || !Objects.equals(mSmartReplies, r.getSmartReplies()) + || mImportance != r.getImportance(); + } + + // Returns whether the NotificationRecord has a change from this data for which we should + // log an update. This method specifically targets fields that may be changed via + // adjustments from the assistant. + // + // Fields here are the union of things in NotificationRecordLogger.shouldLogReported + // and NotificationRecord.applyAdjustments. + // + // Should be guarded by mNotificationLock; not annotated here as this class is static. + boolean hasDiffForLoggingLocked(NotificationRecord r, int newPosition) { + return mPosition != newPosition + || !Objects.equals(mChannel, r.getChannel()) + || !Objects.equals(mGroupKey, r.getGroupKey()) + || !Objects.equals(mOverridePeople, r.getPeopleOverride()) + || !Objects.equals(mSnoozeCriteria, r.getSnoozeCriteria()) + || !Objects.equals(mUserSentiment, r.getUserSentiment()) + || !Objects.equals(mSystemSmartActions, r.getSystemGeneratedSmartActions()) + || !Objects.equals(mSmartReplies, r.getSmartReplies()) + || mImportance != r.getImportance() + || !r.rankingScoreMatches(mRankingScore) + || mIsConversation != r.isConversation(); + } + } + void handleRankingSort() { if (mRankingHelper == null) return; synchronized (mNotificationLock) { final int N = mNotificationList.size(); // Any field that can change via one of the extractors needs to be added here. - ArrayList orderBefore = new ArrayList<>(N); - int[] visibilities = new int[N]; - boolean[] showBadges = new boolean[N]; - boolean[] allowBubbles = new boolean[N]; - boolean[] isBubble = new boolean[N]; - ArrayList channelBefore = new ArrayList<>(N); - ArrayList groupKeyBefore = new ArrayList<>(N); - ArrayList> overridePeopleBefore = new ArrayList<>(N); - ArrayList> snoozeCriteriaBefore = new ArrayList<>(N); - ArrayList userSentimentBefore = new ArrayList<>(N); - ArrayList suppressVisuallyBefore = new ArrayList<>(N); - ArrayList> systemSmartActionsBefore = new ArrayList<>(N); - ArrayList> smartRepliesBefore = new ArrayList<>(N); - int[] importancesBefore = new int[N]; + ArrayMap extractorDataBefore = + new ArrayMap<>(N); for (int i = 0; i < N; i++) { final NotificationRecord r = mNotificationList.get(i); - orderBefore.add(r.getKey()); - visibilities[i] = r.getPackageVisibilityOverride(); - showBadges[i] = r.canShowBadge(); - allowBubbles[i] = r.canBubble(); - isBubble[i] = r.getNotification().isBubbleNotification(); - channelBefore.add(r.getChannel()); - groupKeyBefore.add(r.getGroupKey()); - overridePeopleBefore.add(r.getPeopleOverride()); - snoozeCriteriaBefore.add(r.getSnoozeCriteria()); - userSentimentBefore.add(r.getUserSentiment()); - suppressVisuallyBefore.add(r.getSuppressedVisualEffects()); - systemSmartActionsBefore.add(r.getSystemGeneratedSmartActions()); - smartRepliesBefore.add(r.getSmartReplies()); - importancesBefore[i] = r.getImportance(); + NotificationRecordExtractorData extractorData = new NotificationRecordExtractorData( + i, + r.getPackageVisibilityOverride(), + r.canShowBadge(), + r.canBubble(), + r.getNotification().isBubbleNotification(), + r.getChannel(), + r.getGroupKey(), + r.getPeopleOverride(), + r.getSnoozeCriteria(), + r.getUserSentiment(), + r.getSuppressedVisualEffects(), + r.getSystemGeneratedSmartActions(), + r.getSmartReplies(), + r.getImportance(), + r.getRankingScore(), + r.isConversation()); + extractorDataBefore.put(r.getKey(), extractorData); mRankingHelper.extractSignals(r); } mRankingHelper.sort(mNotificationList); for (int i = 0; i < N; i++) { final NotificationRecord r = mNotificationList.get(i); - if (!orderBefore.get(i).equals(r.getKey()) - || visibilities[i] != r.getPackageVisibilityOverride() - || showBadges[i] != r.canShowBadge() - || allowBubbles[i] != r.canBubble() - || isBubble[i] != r.getNotification().isBubbleNotification() - || !Objects.equals(channelBefore.get(i), r.getChannel()) - || !Objects.equals(groupKeyBefore.get(i), r.getGroupKey()) - || !Objects.equals(overridePeopleBefore.get(i), r.getPeopleOverride()) - || !Objects.equals(snoozeCriteriaBefore.get(i), r.getSnoozeCriteria()) - || !Objects.equals(userSentimentBefore.get(i), r.getUserSentiment()) - || !Objects.equals(suppressVisuallyBefore.get(i), - r.getSuppressedVisualEffects()) - || !Objects.equals(systemSmartActionsBefore.get(i), - r.getSystemGeneratedSmartActions()) - || !Objects.equals(smartRepliesBefore.get(i), r.getSmartReplies()) - || importancesBefore[i] != r.getImportance()) { + if (!extractorDataBefore.containsKey(r.getKey())) { + // This shouldn't happen given that we just built this with all the + // notifications, but check just to be safe. + continue; + } + if (extractorDataBefore.get(r.getKey()).hasDiffForRankingLocked(r, i)) { mHandler.scheduleSendRankingUpdate(); - return; + } + + // If this notification is one for which we wanted to log an update, and + // sufficient relevant bits are different, log update. + if (r.hasPendingLogUpdate()) { + // We need to acquire the previous data associated with this specific + // notification, as the one at the current index may be unrelated if + // notification order has changed. + NotificationRecordExtractorData prevData = extractorDataBefore.get(r.getKey()); + if (prevData.hasDiffForLoggingLocked(r, i)) { + mNotificationRecordLogger.logNotificationAdjusted(r, i, 0, + getGroupInstanceId(r.getSbn().getGroupKey())); + } + + // Remove whether there was a diff or not; we've sorted the key, so if it + // turns out there was nothing to log, that's fine too. + r.setPendingLogUpdate(false); } } } diff --git a/services/core/java/com/android/server/notification/NotificationRecord.java b/services/core/java/com/android/server/notification/NotificationRecord.java index f66cfa99ce986..b4ca5118e10f1 100644 --- a/services/core/java/com/android/server/notification/NotificationRecord.java +++ b/services/core/java/com/android/server/notification/NotificationRecord.java @@ -200,6 +200,10 @@ public final class NotificationRecord { private boolean mIsAppImportanceLocked; private ArraySet mGrantableUris; + // Whether this notification record should have an update logged the next time notifications + // are sorted. + private boolean mPendingLogUpdate = false; + public NotificationRecord(Context context, StatusBarNotification sbn, NotificationChannel channel) { this.sbn = sbn; @@ -648,17 +652,23 @@ public final class NotificationRecord { final ArrayList people = adjustment.getSignals().getStringArrayList(Adjustment.KEY_PEOPLE); setPeopleOverride(people); + EventLogTags.writeNotificationAdjusted( + getKey(), Adjustment.KEY_PEOPLE, people.toString()); } if (signals.containsKey(Adjustment.KEY_SNOOZE_CRITERIA)) { final ArrayList snoozeCriterionList = adjustment.getSignals().getParcelableArrayList( Adjustment.KEY_SNOOZE_CRITERIA); setSnoozeCriteria(snoozeCriterionList); + EventLogTags.writeNotificationAdjusted(getKey(), Adjustment.KEY_SNOOZE_CRITERIA, + snoozeCriterionList.toString()); } if (signals.containsKey(Adjustment.KEY_GROUP_KEY)) { final String groupOverrideKey = adjustment.getSignals().getString(Adjustment.KEY_GROUP_KEY); setOverrideGroupKey(groupOverrideKey); + EventLogTags.writeNotificationAdjusted(getKey(), Adjustment.KEY_GROUP_KEY, + groupOverrideKey); } if (signals.containsKey(Adjustment.KEY_USER_SENTIMENT)) { // Only allow user sentiment update from assistant if user hasn't already @@ -667,27 +677,42 @@ public final class NotificationRecord { && (getChannel().getUserLockedFields() & USER_LOCKED_IMPORTANCE) == 0) { setUserSentiment(adjustment.getSignals().getInt( Adjustment.KEY_USER_SENTIMENT, USER_SENTIMENT_NEUTRAL)); + EventLogTags.writeNotificationAdjusted(getKey(), + Adjustment.KEY_USER_SENTIMENT, + Integer.toString(getUserSentiment())); } } if (signals.containsKey(Adjustment.KEY_CONTEXTUAL_ACTIONS)) { setSystemGeneratedSmartActions( signals.getParcelableArrayList(Adjustment.KEY_CONTEXTUAL_ACTIONS)); + EventLogTags.writeNotificationAdjusted(getKey(), + Adjustment.KEY_CONTEXTUAL_ACTIONS, + getSystemGeneratedSmartActions().toString()); } if (signals.containsKey(Adjustment.KEY_TEXT_REPLIES)) { setSmartReplies(signals.getCharSequenceArrayList(Adjustment.KEY_TEXT_REPLIES)); + EventLogTags.writeNotificationAdjusted(getKey(), Adjustment.KEY_TEXT_REPLIES, + getSmartReplies().toString()); } if (signals.containsKey(Adjustment.KEY_IMPORTANCE)) { int importance = signals.getInt(Adjustment.KEY_IMPORTANCE); importance = Math.max(IMPORTANCE_UNSPECIFIED, importance); importance = Math.min(IMPORTANCE_HIGH, importance); setAssistantImportance(importance); + EventLogTags.writeNotificationAdjusted(getKey(), Adjustment.KEY_IMPORTANCE, + Integer.toString(importance)); } if (signals.containsKey(Adjustment.KEY_RANKING_SCORE)) { mRankingScore = signals.getFloat(Adjustment.KEY_RANKING_SCORE); + EventLogTags.writeNotificationAdjusted(getKey(), Adjustment.KEY_RANKING_SCORE, + Float.toString(mRankingScore)); } if (signals.containsKey(Adjustment.KEY_NOT_CONVERSATION)) { mIsNotConversationOverride = signals.getBoolean( Adjustment.KEY_NOT_CONVERSATION); + EventLogTags.writeNotificationAdjusted(getKey(), + Adjustment.KEY_NOT_CONVERSATION, + Boolean.toString(mIsNotConversationOverride)); } if (!signals.isEmpty() && adjustment.getIssuer() != null) { mAdjustmentIssuer = adjustment.getIssuer(); @@ -1478,6 +1503,24 @@ public final class NotificationRecord { return sbn; } + /** + * Returns whether this record's ranking score is approximately equal to otherScore + * (the difference must be within 0.0001). + */ + public boolean rankingScoreMatches(float otherScore) { + return Math.abs(mRankingScore - otherScore) < 0.0001; + } + + protected void setPendingLogUpdate(boolean pendingLogUpdate) { + mPendingLogUpdate = pendingLogUpdate; + } + + // If a caller of this function subsequently logs the update, they should also call + // setPendingLogUpdate to false to make sure other callers don't also do so. + protected boolean hasPendingLogUpdate() { + return mPendingLogUpdate; + } + @VisibleForTesting static final class Light { public final int color; diff --git a/services/core/java/com/android/server/notification/NotificationRecordLogger.java b/services/core/java/com/android/server/notification/NotificationRecordLogger.java index 0e2ff7523c85e..7d8564fe11905 100644 --- a/services/core/java/com/android/server/notification/NotificationRecordLogger.java +++ b/services/core/java/com/android/server/notification/NotificationRecordLogger.java @@ -58,6 +58,20 @@ public interface NotificationRecordLogger { int position, int buzzBeepBlink, InstanceId groupId); + /** + * Logs a NotificationReported atom reflecting an adjustment to a notification. + * Unlike maybeLogNotificationPosted, this method is guaranteed to log a notification update, + * so the caller must take responsibility for checking that that logging update is necessary, + * and that the notification is meaningfully changed. + * @param r The NotificationRecord. If null, no action is taken. + * @param position The position at which this notification is ranked. + * @param buzzBeepBlink Logging code reflecting whether this notification alerted the user. + * @param groupId The instance Id of the group summary notification, or null. + */ + void logNotificationAdjusted(@Nullable NotificationRecord r, + int position, int buzzBeepBlink, + InstanceId groupId); + /** * Logs a notification cancel / dismiss event using UiEventReported (event ids from the * NotificationCancelledEvents enum). @@ -96,7 +110,9 @@ public interface NotificationRecordLogger { @UiEvent(doc = "New notification enqueued to post") NOTIFICATION_POSTED(162), @UiEvent(doc = "Notification substantially updated, or alerted again.") - NOTIFICATION_UPDATED(163); + NOTIFICATION_UPDATED(163), + @UiEvent(doc = "Notification adjusted by assistant.") + NOTIFICATION_ADJUSTED(908); private final int mId; NotificationReportedEvent(int id) { @@ -349,7 +365,8 @@ public interface NotificationRecordLogger { && Objects.equals(r.getSbn().getNotification().category, old.getSbn().getNotification().category) && (r.getImportance() == old.getImportance()) - && (getLoggingImportance(r) == getLoggingImportance(old))); + && (getLoggingImportance(r) == getLoggingImportance(old)) + && r.rankingScoreMatches(old.getRankingScore())); } /** diff --git a/services/core/java/com/android/server/notification/NotificationRecordLoggerImpl.java b/services/core/java/com/android/server/notification/NotificationRecordLoggerImpl.java index 1a99fb0e55f3f..1859ec4816a31 100644 --- a/services/core/java/com/android/server/notification/NotificationRecordLoggerImpl.java +++ b/services/core/java/com/android/server/notification/NotificationRecordLoggerImpl.java @@ -16,6 +16,8 @@ package com.android.server.notification; +import android.annotation.Nullable; + import com.android.internal.logging.InstanceId; import com.android.internal.logging.UiEventLogger; import com.android.internal.logging.UiEventLoggerImpl; @@ -37,33 +39,49 @@ public class NotificationRecordLoggerImpl implements NotificationRecordLogger { if (!p.shouldLogReported(buzzBeepBlink)) { return; } + writeNotificationReportedAtom(p, NotificationReportedEvent.fromRecordPair(p), + position, buzzBeepBlink, groupId); + } + + @Override + public void logNotificationAdjusted(@Nullable NotificationRecord r, + int position, int buzzBeepBlink, + InstanceId groupId) { + NotificationRecordPair p = new NotificationRecordPair(r, null); + writeNotificationReportedAtom(p, NotificationReportedEvent.NOTIFICATION_ADJUSTED, + position, buzzBeepBlink, groupId); + } + + private void writeNotificationReportedAtom(NotificationRecordPair p, + NotificationReportedEvent eventType, int position, int buzzBeepBlink, + InstanceId groupId) { FrameworkStatsLog.write(FrameworkStatsLog.NOTIFICATION_REPORTED, - /* int32 event_id = 1 */ NotificationReportedEvent.fromRecordPair(p).getId(), - /* int32 uid = 2 */ r.getUid(), - /* string package_name = 3 */ r.getSbn().getPackageName(), + /* int32 event_id = 1 */ eventType.getId(), + /* int32 uid = 2 */ p.r.getUid(), + /* string package_name = 3 */ p.r.getSbn().getPackageName(), /* int32 instance_id = 4 */ p.getInstanceId(), /* int32 notification_id_hash = 5 */ p.getNotificationIdHash(), /* int32 channel_id_hash = 6 */ p.getChannelIdHash(), /* string group_id_hash = 7 */ p.getGroupIdHash(), /* int32 group_instance_id = 8 */ (groupId == null) ? 0 : groupId.getId(), - /* bool is_group_summary = 9 */ r.getSbn().getNotification().isGroupSummary(), - /* string category = 10 */ r.getSbn().getNotification().category, + /* bool is_group_summary = 9 */ p.r.getSbn().getNotification().isGroupSummary(), + /* string category = 10 */ p.r.getSbn().getNotification().category, /* int32 style = 11 */ p.getStyle(), /* int32 num_people = 12 */ p.getNumPeople(), /* int32 position = 13 */ position, /* android.stats.sysui.NotificationImportance importance = 14 */ - NotificationRecordLogger.getLoggingImportance(r), + NotificationRecordLogger.getLoggingImportance(p.r), /* int32 alerting = 15 */ buzzBeepBlink, /* NotificationImportanceExplanation importance_source = 16 */ - r.getImportanceExplanationCode(), + p.r.getImportanceExplanationCode(), /* android.stats.sysui.NotificationImportance importance_initial = 17 */ - r.getInitialImportance(), + p.r.getInitialImportance(), /* NotificationImportanceExplanation importance_initial_source = 18 */ - r.getInitialImportanceExplanationCode(), + p.r.getInitialImportanceExplanationCode(), /* android.stats.sysui.NotificationImportance importance_asst = 19 */ - r.getAssistantImportance(), + p.r.getAssistantImportance(), /* int32 assistant_hash = 20 */ p.getAssistantHash(), - /* float assistant_ranking_score = 21 */ r.getRankingScore() + /* float assistant_ranking_score = 21 */ p.r.getRankingScore() ); } diff --git a/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java b/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java index f660af02a0f69..1ae219db77267 100755 --- a/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java +++ b/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java @@ -4010,6 +4010,80 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { verify(mRankingHandler, times(1)).requestSort(); } + @Test + public void testApplyAdjustmentsLogged() throws Exception { + NotificationManagerService.WorkerHandler handler = mock( + NotificationManagerService.WorkerHandler.class); + mService.setHandler(handler); + when(mAssistants.isSameUser(eq(null), anyInt())).thenReturn(true); + + // Set up notifications that will be adjusted + final NotificationRecord r1 = generateNotificationRecord( + mTestNotificationChannel, 1, null, true); + r1.getSbn().setInstanceId(mNotificationInstanceIdSequence.newInstanceId()); + mService.addNotification(r1); + final NotificationRecord r2 = generateNotificationRecord( + mTestNotificationChannel, 2, null, true); + r2.getSbn().setInstanceId(mNotificationInstanceIdSequence.newInstanceId()); + mService.addNotification(r2); + + // Third notification that's NOT adjusted, just to make sure that doesn't get spuriously + // logged. + final NotificationRecord r3 = generateNotificationRecord( + mTestNotificationChannel, 3, null, true); + r3.getSbn().setInstanceId(mNotificationInstanceIdSequence.newInstanceId()); + mService.addNotification(r3); + + List adjustments = new ArrayList<>(); + + // Test an adjustment that's associated with a ranking change and one that's not + Bundle signals1 = new Bundle(); + signals1.putInt(Adjustment.KEY_IMPORTANCE, IMPORTANCE_HIGH); + Adjustment adjustment1 = new Adjustment( + r1.getSbn().getPackageName(), r1.getKey(), signals1, "", + r1.getUser().getIdentifier()); + adjustments.add(adjustment1); + + // This one wouldn't trigger a ranking change, but should still trigger a log. + Bundle signals2 = new Bundle(); + signals2.putFloat(Adjustment.KEY_RANKING_SCORE, -0.5f); + Adjustment adjustment2 = new Adjustment( + r2.getSbn().getPackageName(), r2.getKey(), signals2, "", + r2.getUser().getIdentifier()); + adjustments.add(adjustment2); + + mBinderService.applyAdjustmentsFromAssistant(null, adjustments); + verify(mRankingHandler, times(1)).requestSort(); + + // Actually apply the adjustments & recalculate importance when run + doAnswer(invocationOnMock -> { + ((NotificationRecord) invocationOnMock.getArguments()[0]) + .applyAdjustments(); + ((NotificationRecord) invocationOnMock.getArguments()[0]) + .calculateImportance(); + return null; + }).when(mRankingHelper).extractSignals(any(NotificationRecord.class)); + + // Now make sure that when the sort happens, we actually log the changes. + mService.handleRankingSort(); + + // Even though the ranking score change is not meant to trigger a ranking update, + // during this process the package visibility & canShowBadge values are changing + // in all notifications, so all 3 seem to trigger a ranking change. Here we check instead + // that scheduleSendRankingUpdate is sent and that the relevant fields have been changed + // accordingly to confirm the adjustments happened to the 2 relevant notifications. + verify(handler, times(3)).scheduleSendRankingUpdate(); + assertEquals(IMPORTANCE_HIGH, r1.getImportance()); + assertTrue(r2.rankingScoreMatches(-0.5f)); + assertEquals(2, mNotificationRecordLogger.numCalls()); + assertEquals(NotificationRecordLogger.NotificationReportedEvent.NOTIFICATION_ADJUSTED, + mNotificationRecordLogger.event(0)); + assertEquals(NotificationRecordLogger.NotificationReportedEvent.NOTIFICATION_ADJUSTED, + mNotificationRecordLogger.event(1)); + assertEquals(1, mNotificationRecordLogger.get(0).getInstanceId()); + assertEquals(2, mNotificationRecordLogger.get(1).getInstanceId()); + } + @Test public void testEnqueuedAdjustmentAppliesAdjustments() throws Exception { final NotificationRecord r = generateNotificationRecord(mTestNotificationChannel); diff --git a/services/tests/uiservicestests/src/com/android/server/notification/NotificationRecordLoggerFake.java b/services/tests/uiservicestests/src/com/android/server/notification/NotificationRecordLoggerFake.java index 64fd19e69009c..8a11798bbf198 100644 --- a/services/tests/uiservicestests/src/com/android/server/notification/NotificationRecordLoggerFake.java +++ b/services/tests/uiservicestests/src/com/android/server/notification/NotificationRecordLoggerFake.java @@ -45,6 +45,15 @@ class NotificationRecordLoggerFake implements NotificationRecordLogger { groupInstanceId = groupId; } + CallRecord(NotificationRecord r, int position, int buzzBeepBlink, InstanceId groupId) { + super(r, null); + this.position = position; + this.buzzBeepBlink = buzzBeepBlink; + wasLogged = true; + event = NotificationReportedEvent.NOTIFICATION_ADJUSTED; + groupInstanceId = groupId; + } + CallRecord(NotificationRecord r, UiEventLogger.UiEventEnum event) { super(r, null); wasLogged = true; @@ -74,6 +83,12 @@ class NotificationRecordLoggerFake implements NotificationRecordLogger { mCalls.add(new CallRecord(r, old, position, buzzBeepBlink, groupId)); } + @Override + public void logNotificationAdjusted(NotificationRecord r, int position, int buzzBeepBlink, + InstanceId groupId) { + mCalls.add(new CallRecord(r, position, buzzBeepBlink, groupId)); + } + @Override public void log(UiEventLogger.UiEventEnum event, NotificationRecord r) { mCalls.add(new CallRecord(r, event));