From 590e1b2aa5ba09aaa4c3562430c56113e694da9d Mon Sep 17 00:00:00 2001 From: Rohan Shah Date: Tue, 10 Apr 2018 23:48:47 -0400 Subject: [PATCH] [Notif] Allow locking importance on notification Currently locking only works on channel - this CL allows doing so on the overarching notification too. Added locking field in appropriate places in record and surfaced it to other bits via the RankingHelper. Test: Visually, reproduced organically (+ deleting /data/ files) Fixes: 77775657 Change-Id: Ie46093921dd6c1ae3533ded7b87faaa475a631e4 --- .../android/app/INotificationManager.aidl | 7 +++ .../systemui/statusbar/NotificationInfo.java | 2 +- .../statusbar/NotificationInfoTest.java | 60 +++++++++++++++++++ .../NotificationManagerService.java | 23 +++++++ .../notification/NotificationRecord.java | 32 +++++++--- .../server/notification/RankingHelper.java | 59 ++++++++++++++++-- .../notification/NotificationRecordTest.java | 42 +++++++++++++ .../notification/RankingHelperTest.java | 5 ++ 8 files changed, 217 insertions(+), 13 deletions(-) diff --git a/core/java/android/app/INotificationManager.aidl b/core/java/android/app/INotificationManager.aidl index 5067e19f1ce43..5461c0c9118e0 100644 --- a/core/java/android/app/INotificationManager.aidl +++ b/core/java/android/app/INotificationManager.aidl @@ -54,6 +54,13 @@ interface INotificationManager void setShowBadge(String pkg, int uid, boolean showBadge); boolean canShowBadge(String pkg, int uid); void setNotificationsEnabledForPackage(String pkg, int uid, boolean enabled); + /** + * Updates the notification's enabled state. Additionally locks importance for all of the + * notifications belonging to the app, such that future notifications aren't reconsidered for + * blocking helper. + */ + void setNotificationsEnabledWithImportanceLockForPackage(String pkg, int uid, boolean enabled); + boolean areNotificationsEnabledForPackage(String pkg, int uid); boolean areNotificationsEnabled(String pkg); int getPackageImportance(String pkg); diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/NotificationInfo.java b/packages/SystemUI/src/com/android/systemui/statusbar/NotificationInfo.java index 81dd9e8c3d874..806206448dcdb 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/NotificationInfo.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/NotificationInfo.java @@ -523,7 +523,7 @@ public class NotificationInfo extends LinearLayout implements NotificationGuts.G } else { // For notifications with more than one channel, update notification enabled // state. If the importance was lowered, we disable notifications. - mINotificationManager.setNotificationsEnabledForPackage( + mINotificationManager.setNotificationsEnabledWithImportanceLockForPackage( mPackageName, mAppUid, mNewImportance >= mCurrentImportance); } } catch (RemoteException e) { diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/NotificationInfoTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/NotificationInfoTest.java index 3cbe27430d47d..b0530c85781f5 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/NotificationInfoTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/NotificationInfoTest.java @@ -420,6 +420,66 @@ public class NotificationInfoTest extends SysuiTestCase { assertEquals(IMPORTANCE_UNSPECIFIED, mNotificationChannel.getImportance()); } + @Test + public void testHandleCloseControls_setsNotificationsDisabledForMultipleChannelNotifications() + throws Exception { + mNotificationChannel.setImportance(IMPORTANCE_LOW); + mNotificationInfo.bindNotification(mMockPackageManager, mMockINotificationManager, + TEST_PACKAGE_NAME, mNotificationChannel /* notificationChannel */, + 10 /* numUniqueChannelsInRow */, mSbn, null /* checkSaveListener */, + null /* onSettingsClick */, null /* onAppSettingsClick */ , + false /* isNonblockable */); + + mNotificationInfo.findViewById(R.id.block).performClick(); + waitForUndoButton(); + mNotificationInfo.handleCloseControls(true, false); + + mTestableLooper.processAllMessages(); + verify(mMockINotificationManager, times(1)) + .setNotificationsEnabledWithImportanceLockForPackage( + anyString(), eq(TEST_UID), eq(false)); + } + + + @Test + public void testHandleCloseControls_keepsNotificationsEnabledForMultipleChannelNotifications() + throws Exception { + mNotificationChannel.setImportance(IMPORTANCE_LOW); + mNotificationInfo.bindNotification(mMockPackageManager, mMockINotificationManager, + TEST_PACKAGE_NAME, mNotificationChannel /* notificationChannel */, + 10 /* numUniqueChannelsInRow */, mSbn, null /* checkSaveListener */, + null /* onSettingsClick */, null /* onAppSettingsClick */ , + false /* isNonblockable */); + + mNotificationInfo.findViewById(R.id.block).performClick(); + waitForUndoButton(); + mNotificationInfo.handleCloseControls(true, false); + + mTestableLooper.processAllMessages(); + verify(mMockINotificationManager, times(1)) + .setNotificationsEnabledWithImportanceLockForPackage( + anyString(), eq(TEST_UID), eq(false)); + } + + @Test + public void testCloseControls_blockingHelperSavesImportanceForMultipleChannelNotifications() + throws Exception { + mNotificationInfo.bindNotification(mMockPackageManager, mMockINotificationManager, + TEST_PACKAGE_NAME, mNotificationChannel /* notificationChannel */, + 10 /* numUniqueChannelsInRow */, mSbn, null /* checkSaveListener */, + null /* onSettingsClick */, null /* onAppSettingsClick */ , + false /* isNonblockable */, true /* isForBlockingHelper */, + true /* isUserSentimentNegative */); + + mNotificationInfo.findViewById(R.id.keep).performClick(); + + verify(mBlockingHelperManager).dismissCurrentBlockingHelper(); + mTestableLooper.processAllMessages(); + verify(mMockINotificationManager, times(1)) + .setNotificationsEnabledWithImportanceLockForPackage( + anyString(), eq(TEST_UID), eq(true)); + } + @Test public void testCloseControls_blockingHelperDismissedIfShown() throws Exception { mNotificationInfo.bindNotification( diff --git a/services/core/java/com/android/server/notification/NotificationManagerService.java b/services/core/java/com/android/server/notification/NotificationManagerService.java index 7254acf3438e9..3ecf6b5591ea1 100644 --- a/services/core/java/com/android/server/notification/NotificationManagerService.java +++ b/services/core/java/com/android/server/notification/NotificationManagerService.java @@ -2130,6 +2130,26 @@ public class NotificationManagerService extends SystemService { savePolicyFile(); } + /** + * Updates the enabled state for notifications for the given package (and uid). + * Additionally, this method marks the app importance as locked by the user, which means + * that notifications from the app will not be considered for showing a + * blocking helper. + * + * @param pkg package that owns the notifications to update + * @param uid uid of the app providing notifications + * @param enabled whether notifications should be enabled for the app + * + * @see #setNotificationsEnabledForPackage(String, int, boolean) + */ + @Override + public void setNotificationsEnabledWithImportanceLockForPackage( + String pkg, int uid, boolean enabled) { + setNotificationsEnabledForPackage(pkg, uid, enabled); + + mRankingHelper.setAppImportanceLocked(pkg, uid); + } + /** * Use this when you just want to know if notifications are OK for this package. */ @@ -3616,6 +3636,8 @@ public class NotificationManagerService extends SystemService { System.currentTimeMillis()); summaryRecord = new NotificationRecord(getContext(), summarySbn, notificationRecord.getChannel()); + summaryRecord.setIsAppImportanceLocked( + notificationRecord.getIsAppImportanceLocked()); summaries.put(pkg, summarySbn.getKey()); } } @@ -4012,6 +4034,7 @@ public class NotificationManagerService extends SystemService { pkg, opPkg, id, tag, notificationUid, callingPid, notification, user, null, System.currentTimeMillis()); final NotificationRecord r = new NotificationRecord(getContext(), n, channel); + r.setIsAppImportanceLocked(mRankingHelper.getIsAppImportanceLocked(pkg, callingUid)); if ((notification.flags & Notification.FLAG_FOREGROUND_SERVICE) != 0 && (channel.getUserLockedFields() & NotificationChannel.USER_LOCKED_IMPORTANCE) == 0 diff --git a/services/core/java/com/android/server/notification/NotificationRecord.java b/services/core/java/com/android/server/notification/NotificationRecord.java index 9bd3e529cb29e..9745be3c5c2f1 100644 --- a/services/core/java/com/android/server/notification/NotificationRecord.java +++ b/services/core/java/com/android/server/notification/NotificationRecord.java @@ -151,11 +151,14 @@ public final class NotificationRecord { private boolean mIsInterruptive; private int mNumberOfSmartRepliesAdded; private boolean mHasSeenSmartReplies; + /** + * Whether this notification (and its channels) should be considered user locked. Used in + * conjunction with user sentiment calculation. + */ + private boolean mIsAppImportanceLocked; - @VisibleForTesting public NotificationRecord(Context context, StatusBarNotification sbn, - NotificationChannel channel) - { + NotificationChannel channel) { this.sbn = sbn; mOriginalFlags = sbn.getNotification().flags; mRankingTimeMs = calculateRankingTimeMs(0L); @@ -503,6 +506,7 @@ public final class NotificationRecord { pw.println(prefix + "mImportance=" + NotificationListenerService.Ranking.importanceToString(mImportance)); pw.println(prefix + "mImportanceExplanation=" + mImportanceExplanation); + pw.println(prefix + "mIsAppImportanceLocked=" + mIsAppImportanceLocked); pw.println(prefix + "mIntercept=" + mIntercept); pw.println(prefix + "mHidden==" + mHidden); pw.println(prefix + "mGlobalSortKey=" + mGlobalSortKey); @@ -564,11 +568,11 @@ public final class NotificationRecord { public final String toString() { return String.format( "NotificationRecord(0x%08x: pkg=%s user=%s id=%d tag=%s importance=%d key=%s" + - ": %s)", + "appImportanceLocked=%s: %s)", System.identityHashCode(this), this.sbn.getPackageName(), this.sbn.getUser(), this.sbn.getId(), this.sbn.getTag(), this.mImportance, this.sbn.getKey(), - this.sbn.getNotification()); + mIsAppImportanceLocked, this.sbn.getNotification()); } public void addAdjustment(Adjustment adjustment) { @@ -600,7 +604,8 @@ public final class NotificationRecord { if (signals.containsKey(Adjustment.KEY_USER_SENTIMENT)) { // Only allow user sentiment update from assistant if user hasn't already // expressed a preference for this channel - if ((getChannel().getUserLockedFields() & USER_LOCKED_IMPORTANCE) == 0) { + if (!mIsAppImportanceLocked + && (getChannel().getUserLockedFields() & USER_LOCKED_IMPORTANCE) == 0) { setUserSentiment(adjustment.getSignals().getInt( Adjustment.KEY_USER_SENTIMENT, USER_SENTIMENT_NEUTRAL)); } @@ -609,6 +614,11 @@ public final class NotificationRecord { } } + public void setIsAppImportanceLocked(boolean isAppImportanceLocked) { + mIsAppImportanceLocked = isAppImportanceLocked; + calculateUserSentiment(); + } + public void setContactAffinity(float contactAffinity) { mContactAffinity = contactAffinity; if (mImportance < IMPORTANCE_DEFAULT && @@ -870,6 +880,13 @@ public final class NotificationRecord { return mChannel; } + /** + * @see RankingHelper#getIsAppImportanceLocked(String, int) + */ + public boolean getIsAppImportanceLocked() { + return mIsAppImportanceLocked; + } + protected void updateNotificationChannel(NotificationChannel channel) { if (channel != null) { mChannel = channel; @@ -927,7 +944,8 @@ public final class NotificationRecord { } private void calculateUserSentiment() { - if ((getChannel().getUserLockedFields() & USER_LOCKED_IMPORTANCE) != 0) { + if ((getChannel().getUserLockedFields() & USER_LOCKED_IMPORTANCE) != 0 + || mIsAppImportanceLocked) { mUserSentiment = USER_SENTIMENT_POSITIVE; } } diff --git a/services/core/java/com/android/server/notification/RankingHelper.java b/services/core/java/com/android/server/notification/RankingHelper.java index 43d393a237ca9..c2ec79b8b9f47 100644 --- a/services/core/java/com/android/server/notification/RankingHelper.java +++ b/services/core/java/com/android/server/notification/RankingHelper.java @@ -24,6 +24,7 @@ import com.android.internal.logging.nano.MetricsProto; import com.android.internal.util.Preconditions; import com.android.internal.util.XmlUtils; +import android.annotation.IntDef; import android.annotation.NonNull; import android.app.Notification; import android.app.NotificationChannel; @@ -36,7 +37,6 @@ import android.content.pm.PackageManager; import android.content.pm.PackageManager.NameNotFoundException; import android.content.pm.ParceledListSlice; import android.content.pm.Signature; -import android.content.res.Resources; import android.metrics.LogMaker; import android.os.Build; import android.os.UserHandle; @@ -89,11 +89,25 @@ public class RankingHelper implements RankingConfig { private static final String ATT_VISIBILITY = "visibility"; private static final String ATT_IMPORTANCE = "importance"; private static final String ATT_SHOW_BADGE = "show_badge"; + private static final String ATT_APP_USER_LOCKED_FIELDS = "app_user_locked_fields"; private static final int DEFAULT_PRIORITY = Notification.PRIORITY_DEFAULT; private static final int DEFAULT_VISIBILITY = NotificationManager.VISIBILITY_NO_OVERRIDE; private static final int DEFAULT_IMPORTANCE = NotificationManager.IMPORTANCE_UNSPECIFIED; private static final boolean DEFAULT_SHOW_BADGE = true; + /** + * Default value for what fields are user locked. See {@link LockableAppFields} for all lockable + * fields. + */ + private static final int DEFAULT_LOCKED_APP_FIELDS = 0; + + /** + * All user-lockable fields for a given application. + */ + @IntDef({LockableAppFields.USER_LOCKED_IMPORTANCE}) + public @interface LockableAppFields { + int USER_LOCKED_IMPORTANCE = 0x00000001; + } private final NotificationSignalExtractor[] mSignalExtractors; private final NotificationComparator mPreliminaryComparator; @@ -218,6 +232,8 @@ public class RankingHelper implements RankingConfig { parser, ATT_VISIBILITY, DEFAULT_VISIBILITY); r.showBadge = XmlUtils.readBooleanAttribute( parser, ATT_SHOW_BADGE, DEFAULT_SHOW_BADGE); + r.lockedAppFields = XmlUtils.readIntAttribute(parser, + ATT_APP_USER_LOCKED_FIELDS, DEFAULT_LOCKED_APP_FIELDS); final int innerDepth = parser.getDepth(); while ((type = parser.next()) != XmlPullParser.END_DOCUMENT @@ -388,10 +404,14 @@ public class RankingHelper implements RankingConfig { if (forBackup && UserHandle.getUserId(r.uid) != UserHandle.USER_SYSTEM) { continue; } - final boolean hasNonDefaultSettings = r.importance != DEFAULT_IMPORTANCE - || r.priority != DEFAULT_PRIORITY || r.visibility != DEFAULT_VISIBILITY - || r.showBadge != DEFAULT_SHOW_BADGE || r.channels.size() > 0 - || r.groups.size() > 0; + final boolean hasNonDefaultSettings = + r.importance != DEFAULT_IMPORTANCE + || r.priority != DEFAULT_PRIORITY + || r.visibility != DEFAULT_VISIBILITY + || r.showBadge != DEFAULT_SHOW_BADGE + || r.lockedAppFields != DEFAULT_LOCKED_APP_FIELDS + || r.channels.size() > 0 + || r.groups.size() > 0; if (hasNonDefaultSettings) { out.startTag(null, TAG_PACKAGE); out.attribute(null, ATT_NAME, r.pkg); @@ -405,6 +425,8 @@ public class RankingHelper implements RankingConfig { out.attribute(null, ATT_VISIBILITY, Integer.toString(r.visibility)); } out.attribute(null, ATT_SHOW_BADGE, Boolean.toString(r.showBadge)); + out.attribute(null, ATT_APP_USER_LOCKED_FIELDS, + Integer.toString(r.lockedAppFields)); if (!forBackup) { out.attribute(null, ATT_UID, Integer.toString(r.uid)); @@ -511,6 +533,17 @@ public class RankingHelper implements RankingConfig { return getOrCreateRecord(packageName, uid).importance; } + + /** + * Returns whether the importance of the corresponding notification is user-locked and shouldn't + * be adjusted by an assistant (via means of a blocking helper, for example). For the channel + * locking field, see {@link NotificationChannel#USER_LOCKED_IMPORTANCE}. + */ + public boolean getIsAppImportanceLocked(String packageName, int uid) { + int userLockedFields = getOrCreateRecord(packageName, uid).lockedAppFields; + return (userLockedFields & LockableAppFields.USER_LOCKED_IMPORTANCE) != 0; + } + @Override public boolean canShowBadge(String packageName, int uid) { return getOrCreateRecord(packageName, uid).showBadge; @@ -996,6 +1029,21 @@ public class RankingHelper implements RankingConfig { enabled ? DEFAULT_IMPORTANCE : IMPORTANCE_NONE); } + /** + * Sets whether any notifications from the app, represented by the given {@code pkgName} and + * {@code uid}, have their importance locked by the user. Locked notifications don't get + * considered for sentiment adjustments (and thus never show a blocking helper). + */ + public void setAppImportanceLocked(String packageName, int uid) { + Record record = getOrCreateRecord(packageName, uid); + if ((record.lockedAppFields & LockableAppFields.USER_LOCKED_IMPORTANCE) != 0) { + return; + } + + record.lockedAppFields = record.lockedAppFields | LockableAppFields.USER_LOCKED_IMPORTANCE; + updateConfig(); + } + @VisibleForTesting void lockFieldsForUpdate(NotificationChannel original, NotificationChannel update) { if (original.canBypassDnd() != update.canBypassDnd()) { @@ -1413,6 +1461,7 @@ public class RankingHelper implements RankingConfig { int priority = DEFAULT_PRIORITY; int visibility = DEFAULT_VISIBILITY; boolean showBadge = DEFAULT_SHOW_BADGE; + int lockedAppFields = DEFAULT_LOCKED_APP_FIELDS; ArrayMap channels = new ArrayMap<>(); Map groups = new ConcurrentHashMap<>(); diff --git a/services/tests/uiservicestests/src/com/android/server/notification/NotificationRecordTest.java b/services/tests/uiservicestests/src/com/android/server/notification/NotificationRecordTest.java index a566327c77cc6..6303184d26049 100644 --- a/services/tests/uiservicestests/src/com/android/server/notification/NotificationRecordTest.java +++ b/services/tests/uiservicestests/src/com/android/server/notification/NotificationRecordTest.java @@ -553,6 +553,34 @@ public class NotificationRecordTest extends UiServiceTestCase { assertEquals(USER_SENTIMENT_NEGATIVE, record.getUserSentiment()); } + @Test + public void testUserSentiment_appImportanceUpdatesSentiment() throws Exception { + StatusBarNotification sbn = getNotification(false /*preO */, true /* noisy */, + true /* defaultSound */, false /* buzzy */, false /* defaultBuzz */, + false /* lights */, false /* defaultLights */, groupId /* group */); + NotificationRecord record = new NotificationRecord(mMockContext, sbn, channel); + assertEquals(USER_SENTIMENT_NEUTRAL, record.getUserSentiment()); + + record.setIsAppImportanceLocked(true); + assertEquals(USER_SENTIMENT_POSITIVE, record.getUserSentiment()); + } + + @Test + public void testUserSentiment_appImportanceBlocksNegativeSentimentUpdate() throws Exception { + StatusBarNotification sbn = getNotification(false /*preO */, true /* noisy */, + true /* defaultSound */, false /* buzzy */, false /* defaultBuzz */, + false /* lights */, false /* defaultLights */, groupId /* group */); + NotificationRecord record = new NotificationRecord(mMockContext, sbn, channel); + record.setIsAppImportanceLocked(true); + + Bundle signals = new Bundle(); + signals.putInt(Adjustment.KEY_USER_SENTIMENT, USER_SENTIMENT_NEGATIVE); + record.addAdjustment(new Adjustment(pkg, record.getKey(), signals, null, sbn.getUserId())); + record.applyAdjustments(); + + assertEquals(USER_SENTIMENT_POSITIVE, record.getUserSentiment()); + } + @Test public void testUserSentiment_userLocked() throws Exception { channel.lockFields(USER_LOCKED_IMPORTANCE); @@ -571,4 +599,18 @@ public class NotificationRecordTest extends UiServiceTestCase { assertEquals(USER_SENTIMENT_POSITIVE, record.getUserSentiment()); } + + @Test + public void testAppImportance_returnsCorrectly() throws Exception { + StatusBarNotification sbn = getNotification(false /*preO */, true /* noisy */, + true /* defaultSound */, false /* buzzy */, false /* defaultBuzz */, + false /* lights */, false /* defaultLights */, groupId /* group */); + NotificationRecord record = new NotificationRecord(mMockContext, sbn, channel); + + record.setIsAppImportanceLocked(true); + assertEquals(true, record.getIsAppImportanceLocked()); + + record.setIsAppImportanceLocked(false); + assertEquals(false, record.getIsAppImportanceLocked()); + } } diff --git a/services/tests/uiservicestests/src/com/android/server/notification/RankingHelperTest.java b/services/tests/uiservicestests/src/com/android/server/notification/RankingHelperTest.java index 54ed1e6f84b72..78aa9653e2b87 100644 --- a/services/tests/uiservicestests/src/com/android/server/notification/RankingHelperTest.java +++ b/services/tests/uiservicestests/src/com/android/server/notification/RankingHelperTest.java @@ -371,6 +371,7 @@ public class RankingHelperTest extends UiServiceTestCase { mHelper.createNotificationChannel(PKG, UID, channel2, false, false); mHelper.setShowBadge(PKG, UID, true); + mHelper.setAppImportanceLocked(PKG, UID); ByteArrayOutputStream baos = writeXmlAndPurge(PKG, UID, false, channel1.getId(), channel2.getId(), NotificationChannel.DEFAULT_CHANNEL_ID); @@ -379,6 +380,7 @@ public class RankingHelperTest extends UiServiceTestCase { loadStreamXml(baos, false); assertTrue(mHelper.canShowBadge(PKG, UID)); + assertTrue(mHelper.getIsAppImportanceLocked(PKG, UID)); assertEquals(channel1, mHelper.getNotificationChannel(PKG, UID, channel1.getId(), false)); compareChannels(channel2, mHelper.getNotificationChannel(PKG, UID, channel2.getId(), false)); @@ -805,6 +807,7 @@ public class RankingHelperTest extends UiServiceTestCase { assertEquals(Notification.PRIORITY_DEFAULT, mHelper.getPackagePriority(PKG, UID)); assertEquals(NotificationManager.VISIBILITY_NO_OVERRIDE, mHelper.getPackageVisibility(PKG, UID)); + assertFalse(mHelper.getIsAppImportanceLocked(PKG, UID)); NotificationChannel defaultChannel = mHelper.getNotificationChannel( PKG, UID, NotificationChannel.DEFAULT_CHANNEL_ID, false); @@ -814,6 +817,7 @@ public class RankingHelperTest extends UiServiceTestCase { defaultChannel.setBypassDnd(true); defaultChannel.setLockscreenVisibility(Notification.VISIBILITY_SECRET); + mHelper.setAppImportanceLocked(PKG, UID); mHelper.updateNotificationChannel(PKG, UID, defaultChannel, true); // ensure app level fields are changed @@ -821,6 +825,7 @@ public class RankingHelperTest extends UiServiceTestCase { assertEquals(Notification.PRIORITY_MAX, mHelper.getPackagePriority(PKG, UID)); assertEquals(Notification.VISIBILITY_SECRET, mHelper.getPackageVisibility(PKG, UID)); assertEquals(IMPORTANCE_NONE, mHelper.getImportance(PKG, UID)); + assertTrue(mHelper.getIsAppImportanceLocked(PKG, UID)); } @Test