diff options
6 files changed, 85 insertions, 26 deletions
diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/NotificationInfo.java b/packages/SystemUI/src/com/android/systemui/statusbar/NotificationInfo.java index 735f4fd82dec..afe906c28ea5 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/NotificationInfo.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/NotificationInfo.java @@ -50,7 +50,6 @@ import com.android.settingslib.Utils; import com.android.systemui.Interpolators; import com.android.systemui.R; -import java.lang.IllegalArgumentException; import java.util.List; import java.util.Set; @@ -274,7 +273,7 @@ public class NotificationInfo extends LinearLayout implements NotificationGuts.G } private void saveImportance() { - if (mNonblockable || !hasImportanceChanged()) { + if (mNonblockable) { return; } MetricsLogger.action(mContext, MetricsEvent.ACTION_SAVE_IMPORTANCE, @@ -409,7 +408,7 @@ public class NotificationInfo extends LinearLayout implements NotificationGuts.G final int centerY = v.getHeight() / 2; final int x = targetLoc[0] - parentLoc[0] + centerX; final int y = targetLoc[1] - parentLoc[1] + centerY; - mGutsContainer.closeControls(x, y, false /* save */, false /* force */); + mGutsContainer.closeControls(x, y, true /* save */, false /* force */); } @Override @@ -429,7 +428,9 @@ public class NotificationInfo extends LinearLayout implements NotificationGuts.G @Override public boolean handleCloseControls(boolean save, boolean force) { - if (save && hasImportanceChanged()) { + // Save regardless of the importance so we can lock the importance field if the user wants + // to keep getting notifications + if (save) { if (mCheckSaveListener != null) { mCheckSaveListener.checkSave(this::saveImportance, mSbn); } else { 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 8e8b3e0f131b..b8d9b192aac4 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/NotificationInfoTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/NotificationInfoTest.java @@ -16,7 +16,9 @@ package com.android.systemui.statusbar; +import static android.app.NotificationChannel.USER_LOCKED_IMPORTANCE; import static android.app.NotificationManager.IMPORTANCE_LOW; +import static android.app.NotificationManager.IMPORTANCE_UNSPECIFIED; import static android.print.PrintManager.PRINT_SPOOLER_PACKAGE_NAME; import static android.view.View.GONE; import static android.view.View.VISIBLE; @@ -95,6 +97,7 @@ public class NotificationInfoTest extends SysuiTestCase { final LayoutInflater layoutInflater = LayoutInflater.from(mContext); mNotificationInfo = (NotificationInfo) layoutInflater.inflate(R.layout.notification_info, null); + mNotificationInfo.setGutsParent(mock(NotificationGuts.class)); // PackageManager must return a packageInfo and applicationInfo. final PackageInfo packageInfo = new PackageInfo(); @@ -323,24 +326,27 @@ public class NotificationInfoTest extends SysuiTestCase { @Test public void testHandleCloseControls_DoesNotUpdateNotificationChannelIfUnchanged() throws Exception { + int originalImportance = mNotificationChannel.getImportance(); mNotificationInfo.bindNotification(mMockPackageManager, mMockINotificationManager, TEST_PACKAGE_NAME, mNotificationChannel, 1, mSbn, null, null, null, null); mNotificationInfo.handleCloseControls(true, false); - verify(mMockINotificationManager, never()).updateNotificationChannelForPackage( + verify(mMockINotificationManager, times(1)).updateNotificationChannelForPackage( anyString(), eq(TEST_UID), any()); + assertEquals(originalImportance, mNotificationChannel.getImportance()); } @Test public void testHandleCloseControls_DoesNotUpdateNotificationChannelIfUnspecified() throws Exception { - mNotificationChannel.setImportance(NotificationManager.IMPORTANCE_UNSPECIFIED); + mNotificationChannel.setImportance(IMPORTANCE_UNSPECIFIED); mNotificationInfo.bindNotification(mMockPackageManager, mMockINotificationManager, TEST_PACKAGE_NAME, mNotificationChannel, 1, mSbn, null, null, null, null); mNotificationInfo.handleCloseControls(true, false); - verify(mMockINotificationManager, never()).updateNotificationChannelForPackage( + verify(mMockINotificationManager, times(1)).updateNotificationChannelForPackage( anyString(), eq(TEST_UID), any()); + assertEquals(IMPORTANCE_UNSPECIFIED, mNotificationChannel.getImportance()); } @Test @@ -370,16 +376,30 @@ public class NotificationInfoTest extends SysuiTestCase { verify(mMockINotificationManager, times(1)).updateNotificationChannelForPackage( anyString(), eq(TEST_UID), updated.capture()); assertTrue((updated.getValue().getUserLockedFields() - & NotificationChannel.USER_LOCKED_IMPORTANCE) != 0); + & USER_LOCKED_IMPORTANCE) != 0); } @Test - public void testBlockUndoDoesNotCallUpdateNotificationChannel() throws Exception { + public void testKeepUpdatesNotificationChannel() throws Exception { mNotificationChannel.setImportance(IMPORTANCE_LOW); mNotificationInfo.bindNotification(mMockPackageManager, mMockINotificationManager, - TEST_PACKAGE_NAME, mNotificationChannel, 1, mSbn, null, null, null, - Collections.singleton(TEST_PACKAGE_NAME)); + TEST_PACKAGE_NAME, mNotificationChannel, 1, mSbn, null, null, null, null); + + mNotificationInfo.handleCloseControls(true, false); + + ArgumentCaptor<NotificationChannel> updated = + ArgumentCaptor.forClass(NotificationChannel.class); + verify(mMockINotificationManager, times(1)).updateNotificationChannelForPackage( + anyString(), eq(TEST_UID), updated.capture()); + assertTrue(0 != (mNotificationChannel.getUserLockedFields() & USER_LOCKED_IMPORTANCE)); + assertEquals(IMPORTANCE_LOW, mNotificationChannel.getImportance()); + } + @Test + public void testBlockUndoDoesNotBlockNotificationChannel() throws Exception { + mNotificationChannel.setImportance(IMPORTANCE_LOW); + mNotificationInfo.bindNotification(mMockPackageManager, mMockINotificationManager, + TEST_PACKAGE_NAME, mNotificationChannel, 1, mSbn, null, null, null, null); mNotificationInfo.findViewById(R.id.block).performClick(); waitForUndoButton(); @@ -389,8 +409,9 @@ public class NotificationInfoTest extends SysuiTestCase { ArgumentCaptor<NotificationChannel> updated = ArgumentCaptor.forClass(NotificationChannel.class); - verify(mMockINotificationManager, never()).updateNotificationChannelForPackage( + verify(mMockINotificationManager, times(1)).updateNotificationChannelForPackage( anyString(), eq(TEST_UID), updated.capture()); + assertTrue(0 != (mNotificationChannel.getUserLockedFields() & USER_LOCKED_IMPORTANCE)); assertEquals(IMPORTANCE_LOW, mNotificationChannel.getImportance()); } diff --git a/services/core/java/com/android/server/notification/NotificationRecord.java b/services/core/java/com/android/server/notification/NotificationRecord.java index 1ad8c74a831c..8d2f0dd3c00c 100644 --- a/services/core/java/com/android/server/notification/NotificationRecord.java +++ b/services/core/java/com/android/server/notification/NotificationRecord.java @@ -15,6 +15,7 @@ */ package com.android.server.notification; +import static android.app.NotificationChannel.USER_LOCKED_IMPORTANCE; import static android.app.NotificationManager.IMPORTANCE_MIN; import static android.app.NotificationManager.IMPORTANCE_UNSPECIFIED; import static android.app.NotificationManager.IMPORTANCE_DEFAULT; @@ -22,6 +23,8 @@ import static android.app.NotificationManager.IMPORTANCE_HIGH; import static android.app.NotificationManager.IMPORTANCE_LOW; import static android.service.notification.NotificationListenerService.Ranking .USER_SENTIMENT_NEUTRAL; +import static android.service.notification.NotificationListenerService.Ranking + .USER_SENTIMENT_POSITIVE; import android.app.Notification; import android.app.NotificationChannel; @@ -163,6 +166,7 @@ public final class NotificationRecord { mLight = calculateLights(); mAdjustments = new ArrayList<>(); mStats = new NotificationStats(); + calculateUserSentiment(); } private boolean isPreChannelsNotification() { @@ -320,7 +324,7 @@ public final class NotificationRecord { if (mPreChannelsNotification && (importance == IMPORTANCE_UNSPECIFIED || (getChannel().getUserLockedFields() - & NotificationChannel.USER_LOCKED_IMPORTANCE) == 0)) { + & USER_LOCKED_IMPORTANCE) == 0)) { if (!stats.isNoisy && requestedImportance > IMPORTANCE_LOW) { requestedImportance = IMPORTANCE_LOW; } @@ -585,8 +589,12 @@ public final class NotificationRecord { setOverrideGroupKey(groupOverrideKey); } if (signals.containsKey(Adjustment.KEY_USER_SENTIMENT)) { - setUserSentiment(adjustment.getSignals().getInt( - Adjustment.KEY_USER_SENTIMENT, USER_SENTIMENT_NEUTRAL)); + // 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) { + setUserSentiment(adjustment.getSignals().getInt( + Adjustment.KEY_USER_SENTIMENT, USER_SENTIMENT_NEUTRAL)); + } } } } @@ -845,10 +853,6 @@ public final class NotificationRecord { } } - public boolean isImportanceFromUser() { - return mImportance == mUserImportance; - } - public NotificationChannel getChannel() { return mChannel; } @@ -857,6 +861,7 @@ public final class NotificationRecord { if (channel != null) { mChannel = channel; calculateImportance(); + calculateUserSentiment(); } } @@ -900,6 +905,12 @@ public final class NotificationRecord { mSnoozeCriteria = snoozeCriteria; } + private void calculateUserSentiment() { + if ((getChannel().getUserLockedFields() & USER_LOCKED_IMPORTANCE) != 0) { + mUserSentiment = USER_SENTIMENT_POSITIVE; + } + } + private void setUserSentiment(int userSentiment) { mUserSentiment = userSentiment; } diff --git a/services/core/java/com/android/server/notification/RankingHelper.java b/services/core/java/com/android/server/notification/RankingHelper.java index b0e3820f5887..dc936d247dbf 100644 --- a/services/core/java/com/android/server/notification/RankingHelper.java +++ b/services/core/java/com/android/server/notification/RankingHelper.java @@ -640,9 +640,11 @@ public class RankingHelper implements RankingConfig { if (updatedChannel.getLockscreenVisibility() == Notification.VISIBILITY_PUBLIC) { updatedChannel.setLockscreenVisibility(Ranking.VISIBILITY_NO_OVERRIDE); } - updatedChannel.unlockFields(updatedChannel.getUserLockedFields()); - updatedChannel.lockFields(channel.getUserLockedFields()); + if (!fromUser) { + updatedChannel.unlockFields(updatedChannel.getUserLockedFields()); + } if (fromUser) { + updatedChannel.lockFields(channel.getUserLockedFields()); lockFieldsForUpdate(channel, updatedChannel); } r.channels.put(updatedChannel.getId(), updatedChannel); 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 a5fa903c4f00..a566327c77cc 100644 --- a/services/tests/uiservicestests/src/com/android/server/notification/NotificationRecordTest.java +++ b/services/tests/uiservicestests/src/com/android/server/notification/NotificationRecordTest.java @@ -15,10 +15,13 @@ */ package com.android.server.notification; +import static android.app.NotificationChannel.USER_LOCKED_IMPORTANCE; import static android.service.notification.NotificationListenerService.Ranking .USER_SENTIMENT_NEGATIVE; import static android.service.notification.NotificationListenerService.Ranking .USER_SENTIMENT_NEUTRAL; +import static android.service.notification.NotificationListenerService.Ranking + .USER_SENTIMENT_POSITIVE; import static junit.framework.Assert.assertEquals; import static junit.framework.Assert.assertFalse; @@ -315,7 +318,7 @@ public class NotificationRecordTest extends UiServiceTestCase { @Test public void testImportance_locked_preUpgrade() throws Exception { defaultChannel.setImportance(NotificationManager.IMPORTANCE_LOW); - defaultChannel.lockFields(NotificationChannel.USER_LOCKED_IMPORTANCE); + defaultChannel.lockFields(USER_LOCKED_IMPORTANCE); StatusBarNotification sbn = getNotification(true /*preO */, true /* noisy */, true /* defaultSound */, false /* buzzy */, false /* defaultBuzz */, false /* lights */, false /* defaultLights */, null /* group */); @@ -327,7 +330,7 @@ public class NotificationRecordTest extends UiServiceTestCase { @Test public void testImportance_locked_unspecified_preUpgrade() throws Exception { defaultChannel.setImportance(NotificationManager.IMPORTANCE_UNSPECIFIED); - defaultChannel.lockFields(NotificationChannel.USER_LOCKED_IMPORTANCE); + defaultChannel.lockFields(USER_LOCKED_IMPORTANCE); StatusBarNotification sbn = getNotification(true /*preO */, true /* noisy */, true /* defaultSound */, false /* buzzy */, false /* defaultBuzz */, false /* lights */, false /* defaultLights */, null /* group */); @@ -549,4 +552,23 @@ public class NotificationRecordTest extends UiServiceTestCase { assertEquals(USER_SENTIMENT_NEGATIVE, record.getUserSentiment()); } + + @Test + public void testUserSentiment_userLocked() throws Exception { + channel.lockFields(USER_LOCKED_IMPORTANCE); + 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_POSITIVE, record.getUserSentiment()); + + 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()); + } } 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 abfc54d19068..9ebce71e8900 100644 --- a/services/tests/uiservicestests/src/com/android/server/notification/RankingHelperTest.java +++ b/services/tests/uiservicestests/src/com/android/server/notification/RankingHelperTest.java @@ -914,16 +914,18 @@ public class RankingHelperTest extends UiServiceTestCase { final NotificationChannel update1 = getChannel(); update1.setSound(new Uri.Builder().scheme("test").build(), new AudioAttributes.Builder().build()); - update1.lockFields(NotificationChannel.USER_LOCKED_PRIORITY); // should be ignored + update1.lockFields(NotificationChannel.USER_LOCKED_PRIORITY); mHelper.updateNotificationChannel(PKG, UID, update1, true); - assertEquals(NotificationChannel.USER_LOCKED_SOUND, + assertEquals(NotificationChannel.USER_LOCKED_PRIORITY + | NotificationChannel.USER_LOCKED_SOUND, mHelper.getNotificationChannel(PKG, UID, update1.getId(), false) .getUserLockedFields()); NotificationChannel update2 = getChannel(); update2.enableVibration(true); mHelper.updateNotificationChannel(PKG, UID, update2, true); - assertEquals(NotificationChannel.USER_LOCKED_SOUND + assertEquals(NotificationChannel.USER_LOCKED_PRIORITY + | NotificationChannel.USER_LOCKED_SOUND | NotificationChannel.USER_LOCKED_VIBRATION, mHelper.getNotificationChannel(PKG, UID, update2.getId(), false) .getUserLockedFields()); |