Allow conversation channels to inherit canBypassDnd from their parent.
This change passes through a more correct value of fromTargetApp to PreferencesHelper.createNotificationChannel. The reason this fixes the inheritance of canBypassDnd for conversation channels is that that field cannot be modified by apps; but when the system creates a conversation channel, it should be able to inherit all its properties from its parent.
Bug: 253848209
Test: atest NotificationManagerServiceTest, atest NotificationManagerTest (CTS)
Change-Id: I407a239376afac18afd6c2cf2471a0e5399253a3
diff --git a/services/core/java/com/android/server/notification/NotificationManagerService.java b/services/core/java/com/android/server/notification/NotificationManagerService.java
index c2df904d..8a7fd82 100755
--- a/services/core/java/com/android/server/notification/NotificationManagerService.java
+++ b/services/core/java/com/android/server/notification/NotificationManagerService.java
@@ -3796,13 +3796,13 @@
}
private void createNotificationChannelsImpl(String pkg, int uid,
- ParceledListSlice channelsList) {
- createNotificationChannelsImpl(pkg, uid, channelsList,
+ ParceledListSlice channelsList, boolean fromTargetApp) {
+ createNotificationChannelsImpl(pkg, uid, channelsList, fromTargetApp,
ActivityTaskManager.INVALID_TASK_ID);
}
private void createNotificationChannelsImpl(String pkg, int uid,
- ParceledListSlice channelsList, int startingTaskId) {
+ ParceledListSlice channelsList, boolean fromTargetApp, int startingTaskId) {
List<NotificationChannel> channels = channelsList.getList();
final int channelsSize = channels.size();
ParceledListSlice<NotificationChannel> oldChannels =
@@ -3814,7 +3814,7 @@
final NotificationChannel channel = channels.get(i);
Objects.requireNonNull(channel, "channel in list is null");
needsPolicyFileChange = mPreferencesHelper.createNotificationChannel(pkg, uid,
- channel, true /* fromTargetApp */,
+ channel, fromTargetApp,
mConditionProviders.isPackageOrComponentAllowed(
pkg, UserHandle.getUserId(uid)));
if (needsPolicyFileChange) {
@@ -3850,6 +3850,7 @@
@Override
public void createNotificationChannels(String pkg, ParceledListSlice channelsList) {
checkCallerIsSystemOrSameApp(pkg);
+ boolean fromTargetApp = !isCallerSystemOrPhone(); // if not system, it's from the app
int taskId = ActivityTaskManager.INVALID_TASK_ID;
try {
int uid = mPackageManager.getPackageUid(pkg, 0,
@@ -3858,14 +3859,15 @@
} catch (RemoteException e) {
// Do nothing
}
- createNotificationChannelsImpl(pkg, Binder.getCallingUid(), channelsList, taskId);
+ createNotificationChannelsImpl(pkg, Binder.getCallingUid(), channelsList, fromTargetApp,
+ taskId);
}
@Override
public void createNotificationChannelsForPackage(String pkg, int uid,
ParceledListSlice channelsList) {
enforceSystemOrSystemUI("only system can call this");
- createNotificationChannelsImpl(pkg, uid, channelsList);
+ createNotificationChannelsImpl(pkg, uid, channelsList, false /* fromTargetApp */);
}
@Override
@@ -3880,7 +3882,8 @@
CONVERSATION_CHANNEL_ID_FORMAT, parentId, conversationId));
conversationChannel.setConversationId(parentId, conversationId);
createNotificationChannelsImpl(
- pkg, uid, new ParceledListSlice(Arrays.asList(conversationChannel)));
+ pkg, uid, new ParceledListSlice(Arrays.asList(conversationChannel)),
+ false /* fromTargetApp */);
mRankingHandler.requestSort();
handleSavePolicyFile();
}
diff --git a/services/core/java/com/android/server/notification/PreferencesHelper.java b/services/core/java/com/android/server/notification/PreferencesHelper.java
index d8aa469..9791158 100644
--- a/services/core/java/com/android/server/notification/PreferencesHelper.java
+++ b/services/core/java/com/android/server/notification/PreferencesHelper.java
@@ -916,7 +916,7 @@
throw new IllegalArgumentException("Reserved id");
}
NotificationChannel existing = r.channels.get(channel.getId());
- if (existing != null && fromTargetApp) {
+ if (existing != null) {
// Actually modifying an existing channel - keep most of the existing settings
if (existing.isDeleted()) {
// The existing channel was deleted - undelete it.
@@ -1002,9 +1002,7 @@
}
if (fromTargetApp) {
channel.setLockscreenVisibility(r.visibility);
- channel.setAllowBubbles(existing != null
- ? existing.getAllowBubbles()
- : NotificationChannel.DEFAULT_ALLOW_BUBBLE);
+ channel.setAllowBubbles(NotificationChannel.DEFAULT_ALLOW_BUBBLE);
}
clearLockedFieldsLocked(channel);
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 7df4b57..6edd87c 100755
--- a/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java
+++ b/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java
@@ -1101,6 +1101,10 @@
new NotificationChannel("id", "name", IMPORTANCE_HIGH);
mBinderService.updateNotificationChannelForPackage(PKG, mUid, updatedChannel);
+ // pretend only this following part is called by the app (system permissions are required to
+ // update the notification channel on behalf of the user above)
+ mService.isSystemUid = false;
+
// Recreating with a lower importance leaves channel unchanged.
final NotificationChannel dupeChannel =
new NotificationChannel("id", "name", NotificationManager.IMPORTANCE_LOW);
@@ -1126,6 +1130,46 @@
}
@Test
+ public void testCreateNotificationChannels_fromAppCannotSetFields() throws Exception {
+ // Confirm that when createNotificationChannels is called from the relevant app and not
+ // system, then it cannot set fields that can't be set by apps
+ mService.isSystemUid = false;
+
+ final NotificationChannel channel =
+ new NotificationChannel("id", "name", IMPORTANCE_DEFAULT);
+ channel.setBypassDnd(true);
+ channel.setAllowBubbles(true);
+
+ mBinderService.createNotificationChannels(PKG,
+ new ParceledListSlice(Arrays.asList(channel)));
+
+ final NotificationChannel createdChannel =
+ mBinderService.getNotificationChannel(PKG, mContext.getUserId(), PKG, "id");
+ assertFalse(createdChannel.canBypassDnd());
+ assertFalse(createdChannel.canBubble());
+ }
+
+ @Test
+ public void testCreateNotificationChannels_fromSystemCanSetFields() throws Exception {
+ // Confirm that when createNotificationChannels is called from system,
+ // then it can set fields that can't be set by apps
+ mService.isSystemUid = true;
+
+ final NotificationChannel channel =
+ new NotificationChannel("id", "name", IMPORTANCE_DEFAULT);
+ channel.setBypassDnd(true);
+ channel.setAllowBubbles(true);
+
+ mBinderService.createNotificationChannels(PKG,
+ new ParceledListSlice(Arrays.asList(channel)));
+
+ final NotificationChannel createdChannel =
+ mBinderService.getNotificationChannel(PKG, mContext.getUserId(), PKG, "id");
+ assertTrue(createdChannel.canBypassDnd());
+ assertTrue(createdChannel.canBubble());
+ }
+
+ @Test
public void testBlockedNotifications_suspended() throws Exception {
when(mPackageManager.isPackageSuspendedForUser(anyString(), anyInt())).thenReturn(true);
@@ -3088,6 +3132,8 @@
@Test
public void testDeleteChannelGroupChecksForFgses() throws Exception {
+ // the setup for this test requires it to seem like it's coming from the app
+ mService.isSystemUid = false;
when(mCompanionMgr.getAssociations(PKG, UserHandle.getUserId(mUid)))
.thenReturn(singletonList(mock(AssociationInfo.class)));
CountDownLatch latch = new CountDownLatch(2);
@@ -3100,7 +3146,7 @@
ParceledListSlice<NotificationChannel> pls =
new ParceledListSlice(ImmutableList.of(notificationChannel));
try {
- mBinderService.createNotificationChannelsForPackage(PKG, mUid, pls);
+ mBinderService.createNotificationChannels(PKG, pls);
} catch (RemoteException e) {
throw new RuntimeException(e);
}
@@ -3119,8 +3165,10 @@
ParceledListSlice<NotificationChannel> pls =
new ParceledListSlice(ImmutableList.of(notificationChannel));
try {
- mBinderService.createNotificationChannelsForPackage(PKG, mUid, pls);
- mBinderService.deleteNotificationChannelGroup(PKG, "group");
+ // Because existing channels won't have their groups overwritten when the call
+ // is from the app, this call won't take the channel out of the group
+ mBinderService.createNotificationChannels(PKG, pls);
+ mBinderService.deleteNotificationChannelGroup(PKG, "group");
} catch (RemoteException e) {
throw new RuntimeException(e);
}
@@ -8566,7 +8614,7 @@
assertEquals("friend", friendChannel.getConversationId());
assertEquals(null, original.getConversationId());
assertEquals(original.canShowBadge(), friendChannel.canShowBadge());
- assertFalse(friendChannel.canBubble()); // can't be modified by app
+ assertEquals(original.canBubble(), friendChannel.canBubble()); // called by system
assertFalse(original.getId().equals(friendChannel.getId()));
assertNotNull(friendChannel.getId());
}