From bc23c7e813f76a51e0d1f462138b4564c91b46a7 Mon Sep 17 00:00:00 2001 From: Julia Reynolds Date: Wed, 13 May 2020 18:16:32 -0400 Subject: [PATCH] Handle apps that upgrade to full conversations - Apps that have sent incomplete conversations only are allowed into the conversation section, but not allowed to have full controls. Users can also demote these apps entirely from the converstion space - Once an app starts using complete notifications, it can no longer be fully demoted out of the conversation space, it's only demoted on a per conversation basis. - If an app has sent full conversation notifications, and then sends an incomplete one, the incomplete notification will not be shown in the conversation space. Test: atest Bug: 155276427 Change-Id: Iba9b01c53949632b6db2834511165e3571387ac9 --- .../android/app/INotificationManager.aidl | 4 +- .../NotificationManagerService.java | 35 ++++++-- .../notification/NotificationRecord.java | 16 ++++ .../notification/PreferencesHelper.java | 90 ++++++++++++++++--- .../NotificationManagerServiceTest.java | 46 ++++++++-- .../notification/NotificationRecordTest.java | 20 +++++ .../notification/PreferencesHelperTest.java | 52 +++++++++-- 7 files changed, 233 insertions(+), 30 deletions(-) diff --git a/core/java/android/app/INotificationManager.aidl b/core/java/android/app/INotificationManager.aidl index 8dfce14af5f76..4c3e888157ee8 100644 --- a/core/java/android/app/INotificationManager.aidl +++ b/core/java/android/app/INotificationManager.aidl @@ -58,7 +58,9 @@ interface INotificationManager void setShowBadge(String pkg, int uid, boolean showBadge); boolean canShowBadge(String pkg, int uid); - boolean hasSentMessage(String pkg, int uid); + boolean isInInvalidMsgState(String pkg, int uid); + boolean hasUserDemotedInvalidMsgApp(String pkg, int uid); + void setInvalidMsgAppDemoted(String pkg, int uid, boolean isDemoted); void setNotificationsEnabledForPackage(String pkg, int uid, boolean enabled); /** * Updates the notification's enabled state. Additionally locks importance for all of the diff --git a/services/core/java/com/android/server/notification/NotificationManagerService.java b/services/core/java/com/android/server/notification/NotificationManagerService.java index bc7bd23551957..86e8734177f04 100755 --- a/services/core/java/com/android/server/notification/NotificationManagerService.java +++ b/services/core/java/com/android/server/notification/NotificationManagerService.java @@ -2725,9 +2725,18 @@ public class NotificationManagerService extends SystemService { Context appContext = r.getSbn().getPackageContext(getContext()); Notification.Builder nb = Notification.Builder.recoverBuilder(appContext, r.getNotification()); - if (nb.getStyle() instanceof Notification.MessagingStyle && r.getShortcutInfo() == null) { - mPreferencesHelper.setMessageSent(r.getSbn().getPackageName(), r.getUid()); - handleSavePolicyFile(); + if (nb.getStyle() instanceof Notification.MessagingStyle) { + if (r.getShortcutInfo() != null) { + if (mPreferencesHelper.setValidMessageSent( + r.getSbn().getPackageName(), r.getUid())) { + handleSavePolicyFile(); + } + } else { + if (mPreferencesHelper.setInvalidMessageSent( + r.getSbn().getPackageName(), r.getUid())) { + handleSavePolicyFile(); + } + } } } @@ -3158,9 +3167,22 @@ public class NotificationManagerService extends SystemService { } @Override - public boolean hasSentMessage(String pkg, int uid) { + public boolean isInInvalidMsgState(String pkg, int uid) { checkCallerIsSystem(); - return mPreferencesHelper.hasSentMessage(pkg, uid); + return mPreferencesHelper.isInInvalidMsgState(pkg, uid); + } + + @Override + public boolean hasUserDemotedInvalidMsgApp(String pkg, int uid) { + checkCallerIsSystem(); + return mPreferencesHelper.hasUserDemotedInvalidMsgApp(pkg, uid); + } + + @Override + public void setInvalidMsgAppDemoted(String pkg, int uid, boolean isDemoted) { + checkCallerIsSystem(); + mPreferencesHelper.setInvalidMsgAppDemoted(pkg, uid, isDemoted); + handleSavePolicyFile(); } @Override @@ -5698,6 +5720,9 @@ public class NotificationManagerService extends SystemService { Slog.w(TAG, "notification " + r.getKey() + " added an invalid shortcut"); } r.setShortcutInfo(info); + r.setHasSentValidMsg(mPreferencesHelper.hasSentValidMsg(pkg, notificationUid)); + r.userDemotedAppFromConvoSpace( + mPreferencesHelper.hasUserDemotedInvalidMsgApp(pkg, notificationUid)); if (!checkDisqualifyingFeatures(userId, notificationUid, id, tag, r, r.getSbn().getOverrideGroupKey() != null)) { diff --git a/services/core/java/com/android/server/notification/NotificationRecord.java b/services/core/java/com/android/server/notification/NotificationRecord.java index a9fa2b1bd491c..c10782242faa1 100644 --- a/services/core/java/com/android/server/notification/NotificationRecord.java +++ b/services/core/java/com/android/server/notification/NotificationRecord.java @@ -188,6 +188,8 @@ public final class NotificationRecord { private boolean mHasSeenSmartReplies; private boolean mFlagBubbleRemoved; private boolean mPostSilently; + private boolean mHasSentValidMsg; + private boolean mAppDemotedFromConvo; /** * Whether this notification (and its channels) should be considered user locked. Used in * conjunction with user sentiment calculation. @@ -1377,6 +1379,14 @@ public final class NotificationRecord { return mShortcutInfo; } + public void setHasSentValidMsg(boolean hasSentValidMsg) { + mHasSentValidMsg = hasSentValidMsg; + } + + public void userDemotedAppFromConvoSpace(boolean userDemoted) { + mAppDemotedFromConvo = userDemoted; + } + /** * Whether this notification is a conversation notification. */ @@ -1397,6 +1407,12 @@ public final class NotificationRecord { && mShortcutInfo == null) { return false; } + if (mHasSentValidMsg && mShortcutInfo == null) { + return false; + } + if (mAppDemotedFromConvo) { + return false; + } return true; } diff --git a/services/core/java/com/android/server/notification/PreferencesHelper.java b/services/core/java/com/android/server/notification/PreferencesHelper.java index ec0fc4a343879..38c65f11a7170 100644 --- a/services/core/java/com/android/server/notification/PreferencesHelper.java +++ b/services/core/java/com/android/server/notification/PreferencesHelper.java @@ -116,7 +116,9 @@ public class PreferencesHelper implements RankingConfig { private static final String ATT_ENABLED = "enabled"; private static final String ATT_USER_ALLOWED = "allowed"; private static final String ATT_HIDE_SILENT = "hide_gentle"; - private static final String ATT_SENT_MESSAGE = "sent_invalid_msg"; + private static final String ATT_SENT_INVALID_MESSAGE = "sent_invalid_msg"; + private static final String ATT_SENT_VALID_MESSAGE = "sent_valid_msg"; + private static final String ATT_USER_DEMOTED_INVALID_MSG_APP = "user_demote_msg_app"; private static final int DEFAULT_PRIORITY = Notification.PRIORITY_DEFAULT; private static final int DEFAULT_VISIBILITY = NotificationManager.VISIBILITY_NO_OVERRIDE; @@ -253,8 +255,12 @@ public class PreferencesHelper implements RankingConfig { parser, ATT_SHOW_BADGE, DEFAULT_SHOW_BADGE); r.lockedAppFields = XmlUtils.readIntAttribute(parser, ATT_APP_USER_LOCKED_FIELDS, DEFAULT_LOCKED_APP_FIELDS); - r.hasSentMessage = XmlUtils.readBooleanAttribute( - parser, ATT_SENT_MESSAGE, false); + r.hasSentInvalidMessage = XmlUtils.readBooleanAttribute( + parser, ATT_SENT_INVALID_MESSAGE, false); + r.hasSentValidMessage = XmlUtils.readBooleanAttribute( + parser, ATT_SENT_VALID_MESSAGE, false); + r.userDemotedMsgApp = XmlUtils.readBooleanAttribute( + parser, ATT_USER_DEMOTED_INVALID_MSG_APP, false); final int innerDepth = parser.getDepth(); while ((type = parser.next()) != XmlPullParser.END_DOCUMENT @@ -497,7 +503,9 @@ public class PreferencesHelper implements RankingConfig { || r.groups.size() > 0 || r.delegate != null || r.bubblePreference != DEFAULT_BUBBLE_PREFERENCE - || r.hasSentMessage; + || r.hasSentInvalidMessage + || r.userDemotedMsgApp + || r.hasSentValidMessage; if (hasNonDefaultSettings) { out.startTag(null, TAG_PACKAGE); out.attribute(null, ATT_NAME, r.pkg); @@ -516,7 +524,12 @@ public class PreferencesHelper implements RankingConfig { out.attribute(null, ATT_SHOW_BADGE, Boolean.toString(r.showBadge)); out.attribute(null, ATT_APP_USER_LOCKED_FIELDS, Integer.toString(r.lockedAppFields)); - out.attribute(null, ATT_SENT_MESSAGE, Boolean.toString(r.hasSentMessage)); + out.attribute(null, ATT_SENT_INVALID_MESSAGE, + Boolean.toString(r.hasSentInvalidMessage)); + out.attribute(null, ATT_SENT_VALID_MESSAGE, + Boolean.toString(r.hasSentValidMessage)); + out.attribute(null, ATT_USER_DEMOTED_INVALID_MSG_APP, + Boolean.toString(r.userDemotedMsgApp)); if (!forBackup) { out.attribute(null, ATT_UID, Integer.toString(r.uid)); @@ -635,15 +648,68 @@ public class PreferencesHelper implements RankingConfig { updateConfig(); } - public boolean hasSentMessage(String packageName, int uid) { + public boolean isInInvalidMsgState(String packageName, int uid) { synchronized (mPackagePreferences) { - return getOrCreatePackagePreferencesLocked(packageName, uid).hasSentMessage; + PackagePreferences r = getOrCreatePackagePreferencesLocked(packageName, uid); + return r.hasSentInvalidMessage && !r.hasSentValidMessage; } } - public void setMessageSent(String packageName, int uid) { + public boolean hasUserDemotedInvalidMsgApp(String packageName, int uid) { synchronized (mPackagePreferences) { - getOrCreatePackagePreferencesLocked(packageName, uid).hasSentMessage = true; + PackagePreferences r = getOrCreatePackagePreferencesLocked(packageName, uid); + return isInInvalidMsgState(packageName, uid) ? r.userDemotedMsgApp : false; + } + } + + public void setInvalidMsgAppDemoted(String packageName, int uid, boolean isDemoted) { + synchronized (mPackagePreferences) { + PackagePreferences r = getOrCreatePackagePreferencesLocked(packageName, uid); + r.userDemotedMsgApp = isDemoted; + } + } + + public boolean setInvalidMessageSent(String packageName, int uid) { + synchronized (mPackagePreferences) { + PackagePreferences r = getOrCreatePackagePreferencesLocked(packageName, uid); + boolean valueChanged = r.hasSentInvalidMessage == false; + r.hasSentInvalidMessage = true; + + return valueChanged; + } + } + + public boolean setValidMessageSent(String packageName, int uid) { + synchronized (mPackagePreferences) { + PackagePreferences r = getOrCreatePackagePreferencesLocked(packageName, uid); + boolean valueChanged = r.hasSentValidMessage == false; + r.hasSentValidMessage = true; + + return valueChanged; + } + } + + @VisibleForTesting + boolean hasSentInvalidMsg(String packageName, int uid) { + synchronized (mPackagePreferences) { + PackagePreferences r = getOrCreatePackagePreferencesLocked(packageName, uid); + return r.hasSentInvalidMessage; + } + } + + @VisibleForTesting + boolean hasSentValidMsg(String packageName, int uid) { + synchronized (mPackagePreferences) { + PackagePreferences r = getOrCreatePackagePreferencesLocked(packageName, uid); + return r.hasSentValidMessage; + } + } + + @VisibleForTesting + boolean didUserEverDemoteInvalidMsgApp(String packageName, int uid) { + synchronized (mPackagePreferences) { + PackagePreferences r = getOrCreatePackagePreferencesLocked(packageName, uid); + return r.userDemotedMsgApp; } } @@ -2273,7 +2339,11 @@ public class PreferencesHelper implements RankingConfig { boolean oemLockedImportance = DEFAULT_OEM_LOCKED_IMPORTANCE; List oemLockedChannels = new ArrayList<>(); boolean defaultAppLockedImportance = DEFAULT_APP_LOCKED_IMPORTANCE; - boolean hasSentMessage = false; + + boolean hasSentInvalidMessage = false; + boolean hasSentValidMessage = false; + // notE: only valid while hasSentMessage is false and hasSentInvalidMessage is true + boolean userDemotedMsgApp = false; Delegate delegate = null; ArrayMap channels = new ArrayMap<>(); 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 289933e5ecb28..d45ecc9a33292 100755 --- a/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java +++ b/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java @@ -6650,18 +6650,14 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { nr.getSbn().getId(), nr.getSbn().getNotification(), nr.getSbn().getUserId()); waitForIdle(); - assertTrue(mBinderService.hasSentMessage(PKG, mUid)); + assertTrue(mBinderService.isInInvalidMsgState(PKG, mUid)); } @Test public void testRecordMessages_validMsg() throws RemoteException { - // Messaging notification with shortcut info - Notification.BubbleMetadata metadata = - new Notification.BubbleMetadata.Builder("id").build(); Notification.Builder nb = getMessageStyleNotifBuilder(false /* addDefaultMetadata */, null /* groupKey */, false /* isSummary */); - nb.setShortcutId("id"); - nb.setBubbleMetadata(metadata); + nb.setShortcutId(null); StatusBarNotification sbn = new StatusBarNotification(PKG, PKG, 1, "testRecordMessages_validMsg", mUid, 0, nb.build(), new UserHandle(mUid), null, 0); NotificationRecord nr = new NotificationRecord(mContext, sbn, mTestNotificationChannel); @@ -6670,7 +6666,43 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { nr.getSbn().getId(), nr.getSbn().getNotification(), nr.getSbn().getUserId()); waitForIdle(); - assertFalse(mBinderService.hasSentMessage(PKG, mUid)); + assertTrue(mBinderService.isInInvalidMsgState(PKG, mUid)); + + nr = generateMessageBubbleNotifRecord(mTestNotificationChannel, + "testRecordMessages_validMsg"); + + mBinderService.enqueueNotificationWithTag(PKG, PKG, nr.getSbn().getTag(), + nr.getSbn().getId(), nr.getSbn().getNotification(), nr.getSbn().getUserId()); + waitForIdle(); + + assertFalse(mBinderService.isInInvalidMsgState(PKG, mUid)); + } + + @Test + public void testRecordMessages_invalidMsg_afterValidMsg() throws RemoteException { + NotificationRecord nr = generateMessageBubbleNotifRecord(mTestNotificationChannel, + "testRecordMessages_invalidMsg_afterValidMsg_1"); + mBinderService.enqueueNotificationWithTag(PKG, PKG, nr.getSbn().getTag(), + nr.getSbn().getId(), nr.getSbn().getNotification(), nr.getSbn().getUserId()); + waitForIdle(); + assertTrue(mService.getNotificationRecord(nr.getKey()).isConversation()); + + mBinderService.cancelAllNotifications(PKG, mUid); + waitForIdle(); + + Notification.Builder nb = getMessageStyleNotifBuilder(false /* addDefaultMetadata */, + null /* groupKey */, false /* isSummary */); + nb.setShortcutId(null); + StatusBarNotification sbn = new StatusBarNotification(PKG, PKG, 1, + "testRecordMessages_invalidMsg_afterValidMsg_2", mUid, 0, nb.build(), + new UserHandle(mUid), null, 0); + nr = new NotificationRecord(mContext, sbn, mTestNotificationChannel); + + mBinderService.enqueueNotificationWithTag(PKG, PKG, nr.getSbn().getTag(), + nr.getSbn().getId(), nr.getSbn().getNotification(), nr.getSbn().getUserId()); + waitForIdle(); + + assertFalse(mService.getNotificationRecord(nr.getKey()).isConversation()); } @Test 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 b03596a35c32c..6df3c7b69d15a 100644 --- a/services/tests/uiservicestests/src/com/android/server/notification/NotificationRecordTest.java +++ b/services/tests/uiservicestests/src/com/android/server/notification/NotificationRecordTest.java @@ -1136,6 +1136,26 @@ public class NotificationRecordTest extends UiServiceTestCase { assertTrue(record.isConversation()); } + @Test + public void testIsConversation_noShortcut_appHasPreviousSentFullConversation() { + StatusBarNotification sbn = getMessagingStyleNotification(); + NotificationRecord record = new NotificationRecord(mMockContext, sbn, channel); + record.setShortcutInfo(null); + record.setHasSentValidMsg(true); + + assertFalse(record.isConversation()); + } + + @Test + public void testIsConversation_noShortcut_userDemotedApp() { + StatusBarNotification sbn = getMessagingStyleNotification(); + NotificationRecord record = new NotificationRecord(mMockContext, sbn, channel); + record.setShortcutInfo(null); + record.userDemotedAppFromConvoSpace(true); + + assertFalse(record.isConversation()); + } + @Test public void testIsConversation_noShortcut_targetsR() { StatusBarNotification sbn = getMessagingStyleNotification(PKG_R); diff --git a/services/tests/uiservicestests/src/com/android/server/notification/PreferencesHelperTest.java b/services/tests/uiservicestests/src/com/android/server/notification/PreferencesHelperTest.java index 4320f1c3c8964..f4e5d569512a6 100644 --- a/services/tests/uiservicestests/src/com/android/server/notification/PreferencesHelperTest.java +++ b/services/tests/uiservicestests/src/com/android/server/notification/PreferencesHelperTest.java @@ -454,7 +454,9 @@ public class PreferencesHelperTest extends UiServiceTestCase { mHelper.createNotificationChannel(PKG_O, UID_O, getChannel(), true, false); mHelper.setShowBadge(PKG_N_MR1, UID_N_MR1, true); - mHelper.setMessageSent(PKG_P, UID_P); + mHelper.setInvalidMessageSent(PKG_P, UID_P); + mHelper.setValidMessageSent(PKG_P, UID_P); + mHelper.setInvalidMsgAppDemoted(PKG_P, UID_P, true); mHelper.setImportance(PKG_O, UID_O, IMPORTANCE_NONE); @@ -470,8 +472,10 @@ public class PreferencesHelperTest extends UiServiceTestCase { assertEquals(IMPORTANCE_NONE, mHelper.getImportance(PKG_O, UID_O)); assertTrue(mHelper.canShowBadge(PKG_N_MR1, UID_N_MR1)); - assertTrue(mHelper.hasSentMessage(PKG_P, UID_P)); - assertFalse(mHelper.hasSentMessage(PKG_N_MR1, UID_N_MR1)); + assertTrue(mHelper.hasSentInvalidMsg(PKG_P, UID_P)); + assertFalse(mHelper.hasSentInvalidMsg(PKG_N_MR1, UID_N_MR1)); + assertTrue(mHelper.hasSentValidMsg(PKG_P, UID_P)); + assertTrue(mHelper.didUserEverDemoteInvalidMsgApp(PKG_P, UID_P)); assertEquals(channel1, mHelper.getNotificationChannel(PKG_N_MR1, UID_N_MR1, channel1.getId(), false)); compareChannels(channel2, @@ -3380,15 +3384,49 @@ public class PreferencesHelperTest extends UiServiceTestCase { } @Test - public void testMessageSent() { + public void testInvalidMessageSent() { // create package preferences mHelper.canShowBadge(PKG_P, UID_P); // check default value - assertFalse(mHelper.hasSentMessage(PKG_P, UID_P)); + assertFalse(mHelper.isInInvalidMsgState(PKG_P, UID_P)); // change it - mHelper.setMessageSent(PKG_P, UID_P); - assertTrue(mHelper.hasSentMessage(PKG_P, UID_P)); + mHelper.setInvalidMessageSent(PKG_P, UID_P); + assertTrue(mHelper.isInInvalidMsgState(PKG_P, UID_P)); + assertTrue(mHelper.hasSentInvalidMsg(PKG_P, UID_P)); + } + + @Test + public void testValidMessageSent() { + // create package preferences + mHelper.canShowBadge(PKG_P, UID_P); + + // get into the bad state + mHelper.setInvalidMessageSent(PKG_P, UID_P); + + // and then fix it + mHelper.setValidMessageSent(PKG_P, UID_P); + + assertTrue(mHelper.hasSentValidMsg(PKG_P, UID_P)); + assertFalse(mHelper.isInInvalidMsgState(PKG_P, UID_P)); + } + + @Test + public void testUserDemotedInvalidMsgApp() { + // create package preferences + mHelper.canShowBadge(PKG_P, UID_P); + + // demotion means nothing before msg notif sent + mHelper.setInvalidMsgAppDemoted(PKG_P, UID_P, true); + assertFalse(mHelper.hasUserDemotedInvalidMsgApp(PKG_P, UID_P)); + + // it's valid when incomplete msgs have been sent + mHelper.setInvalidMessageSent(PKG_P, UID_P); + assertTrue(mHelper.hasUserDemotedInvalidMsgApp(PKG_P, UID_P)); + + // and is invalid once complete msgs are sent + mHelper.setValidMessageSent(PKG_P, UID_P); + assertFalse(mHelper.hasUserDemotedInvalidMsgApp(PKG_P, UID_P)); } }