From 6a5ca92e75738e605090feab3b6f95bf4ff37d84 Mon Sep 17 00:00:00 2001 From: Ruslan Tkhakokhov Date: Tue, 4 May 2021 09:38:53 +0100 Subject: [PATCH] Don't restore notification channel if its sound is unavailable If the sound we're trying to restore is unavailable on the device, we set the sound for that channel to system default. Instead, skip restore of that channel altogether. Bug: 158351264 Test: 1. Make sure the notification sound in Calendar is set to 'App provided sound' -> Run backup -> Wipe the device -> Restore -> Make sure notification sound is still 'App provided sound'. 2. Set notification sound in Calendar to some pre-bundled sound. -> Run backup -> Wipe the device -> Restore -> Make sure notification sound for Calendar is restored correctly. Change-Id: Id0d0a762b381ca1ef7f4cb8305f03e9716282161 --- .../java/android/app/NotificationChannel.java | 14 ++++++- .../notification/PreferencesHelper.java | 3 +- .../notification/PreferencesHelperTest.java | 41 +++++++++++-------- 3 files changed, 39 insertions(+), 19 deletions(-) diff --git a/core/java/android/app/NotificationChannel.java b/core/java/android/app/NotificationChannel.java index 6553b61ebbc2b..1837fb860b737 100644 --- a/core/java/android/app/NotificationChannel.java +++ b/core/java/android/app/NotificationChannel.java @@ -260,6 +260,8 @@ public final class NotificationChannel implements Parcelable { private boolean mDemoted = false; private boolean mImportantConvo = false; private long mDeletedTime = DEFAULT_DELETION_TIME_MS; + // If the sound for this channel is missing, e.g. after restore. + private boolean mIsSoundMissing; /** * Creates a notification channel. @@ -714,6 +716,13 @@ public final class NotificationChannel implements Parcelable { return mSound; } + /** + * @hide + */ + public boolean isSoundMissing() { + return mIsSoundMissing; + } + /** * Returns the audio attributes for sound played by notifications posted to this channel. */ @@ -998,8 +1007,9 @@ public final class NotificationChannel implements Parcelable { // according to the docs because canonicalize method has to handle canonical uris as well. Uri canonicalizedUri = contentResolver.canonicalize(uri); if (canonicalizedUri == null) { - // We got a null because the uri in the backup does not exist here, so we return default - return Settings.System.DEFAULT_NOTIFICATION_URI; + // We got a null because the uri in the backup does not exist here. + mIsSoundMissing = true; + return null; } return contentResolver.uncanonicalize(canonicalizedUri); } diff --git a/services/core/java/com/android/server/notification/PreferencesHelper.java b/services/core/java/com/android/server/notification/PreferencesHelper.java index 55a0949fb7c7d..03676b5556d97 100644 --- a/services/core/java/com/android/server/notification/PreferencesHelper.java +++ b/services/core/java/com/android/server/notification/PreferencesHelper.java @@ -330,7 +330,8 @@ public class PreferencesHelper implements RankingConfig { } } - if (isShortcutOk(channel) && isDeletionOk(channel)) { + if (isShortcutOk(channel) && isDeletionOk(channel) + && !channel.isSoundMissing()) { r.channels.put(id, channel); } } 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 1b26352abe358..3a51ff2fb5fa0 100644 --- a/services/tests/uiservicestests/src/com/android/server/notification/PreferencesHelperTest.java +++ b/services/tests/uiservicestests/src/com/android/server/notification/PreferencesHelperTest.java @@ -375,19 +375,27 @@ public class PreferencesHelperTest extends UiServiceTestCase { when(mPm.getPackageUidAsUser(eq(packageName), anyInt())).thenReturn(uid); } + private static NotificationChannel createNotificationChannel(String id, String name, + int importance) { + NotificationChannel channel = new NotificationChannel(id, name, importance); + channel.setSound(SOUND_URI, Notification.AUDIO_ATTRIBUTES_DEFAULT); + return channel; + } + @Test public void testWriteXml_onlyBackupsTargetUser() throws Exception { // Setup package notifications. String package0 = "test.package.user0"; int uid0 = 1001; setUpPackageWithUid(package0, uid0); - NotificationChannel channel0 = new NotificationChannel("id0", "name0", IMPORTANCE_HIGH); + NotificationChannel channel0 = createNotificationChannel("id0", "name0", IMPORTANCE_HIGH); assertTrue(mHelper.createNotificationChannel(package0, uid0, channel0, true, false)); String package10 = "test.package.user10"; int uid10 = 1001001; setUpPackageWithUid(package10, uid10); - NotificationChannel channel10 = new NotificationChannel("id10", "name10", IMPORTANCE_HIGH); + NotificationChannel channel10 = createNotificationChannel("id10", "name10", + IMPORTANCE_HIGH); assertTrue(mHelper.createNotificationChannel(package10, uid10, channel10, true, false)); ByteArrayOutputStream baos = writeXmlAndPurge(package10, uid10, true, 10); @@ -412,7 +420,7 @@ public class PreferencesHelperTest extends UiServiceTestCase { String package0 = "test.package.user0"; int uid0 = 1001; setUpPackageWithUid(package0, uid0); - NotificationChannel channel0 = new NotificationChannel("id0", "name0", IMPORTANCE_HIGH); + NotificationChannel channel0 = createNotificationChannel("id0", "name0", IMPORTANCE_HIGH); assertTrue(mHelper.createNotificationChannel(package0, uid0, channel0, true, false)); ByteArrayOutputStream baos = writeXmlAndPurge(package0, uid0, true, 0); @@ -505,9 +513,8 @@ public class PreferencesHelperTest extends UiServiceTestCase { NotificationChannelGroup ncg = new NotificationChannelGroup("1", "bye"); NotificationChannelGroup ncg2 = new NotificationChannelGroup("2", "hello"); NotificationChannel channel1 = - new NotificationChannel("id1", "name1", NotificationManager.IMPORTANCE_HIGH); - NotificationChannel channel2 = - new NotificationChannel("id2", "name2", IMPORTANCE_LOW); + createNotificationChannel("id1", "name1", NotificationManager.IMPORTANCE_HIGH); + NotificationChannel channel2 = createNotificationChannel("id2", "name2", IMPORTANCE_LOW); channel2.setDescription("descriptions for all"); channel2.setSound(SOUND_URI, mAudioAttributes); channel2.enableLights(true); @@ -516,7 +523,7 @@ public class PreferencesHelperTest extends UiServiceTestCase { channel2.enableVibration(false); channel2.setGroup(ncg.getId()); channel2.setLightColor(Color.BLUE); - NotificationChannel channel3 = new NotificationChannel("id3", "NAM3", IMPORTANCE_HIGH); + NotificationChannel channel3 = createNotificationChannel("id3", "NAM3", IMPORTANCE_HIGH); channel3.enableVibration(true); mHelper.createNotificationChannelGroup(PKG_N_MR1, UID_N_MR1, ncg, true); @@ -623,7 +630,8 @@ public class PreferencesHelperTest extends UiServiceTestCase { } @Test - public void testRestoreXml_withNonExistentCanonicalizedSoundUri() throws Exception { + public void testRestoreXml_withNonExistentCanonicalizedSoundUri_ignoreChannel() + throws Exception { Thread.sleep(3000); doReturn(null) .when(mTestIContentProvider).canonicalize(any(), eq(CANONICAL_SOUND_URI)); @@ -641,7 +649,7 @@ public class PreferencesHelperTest extends UiServiceTestCase { NotificationChannel actualChannel = mHelper.getNotificationChannel( PKG_N_MR1, UID_N_MR1, channel.getId(), false); - assertEquals(Settings.System.DEFAULT_NOTIFICATION_URI, actualChannel.getSound()); + assertNull(actualChannel); } @@ -650,7 +658,8 @@ public class PreferencesHelperTest extends UiServiceTestCase { * handle its restore properly. */ @Test - public void testRestoreXml_withUncanonicalizedNonLocalSoundUri() throws Exception { + public void testRestoreXml_withUncanonicalizedNonLocalSoundUri_ignoreChannel() + throws Exception { // Not a local uncanonicalized uri, simulating that it fails to exist locally doReturn(null) .when(mTestIContentProvider).canonicalize(any(), eq(SOUND_URI)); @@ -669,7 +678,7 @@ public class PreferencesHelperTest extends UiServiceTestCase { backupWithUncanonicalizedSoundUri.getBytes(), true, UserHandle.USER_SYSTEM); NotificationChannel actualChannel = mHelper.getNotificationChannel(PKG_N_MR1, UID_N_MR1, id, false); - assertEquals(Settings.System.DEFAULT_NOTIFICATION_URI, actualChannel.getSound()); + assertNull(actualChannel); } @Test @@ -693,11 +702,11 @@ public class PreferencesHelperTest extends UiServiceTestCase { NotificationChannelGroup ncg = new NotificationChannelGroup("1", "bye"); NotificationChannelGroup ncg2 = new NotificationChannelGroup("2", "hello"); NotificationChannel channel1 = - new NotificationChannel("id1", "name1", NotificationManager.IMPORTANCE_HIGH); + createNotificationChannel("id1", "name1", NotificationManager.IMPORTANCE_HIGH); NotificationChannel channel2 = - new NotificationChannel("id2", "name2", IMPORTANCE_HIGH); + createNotificationChannel("id2", "name2", IMPORTANCE_HIGH); NotificationChannel channel3 = - new NotificationChannel("id3", "name3", IMPORTANCE_LOW); + createNotificationChannel("id3", "name3", IMPORTANCE_LOW); channel3.setGroup(ncg.getId()); mHelper.createNotificationChannelGroup(PKG_N_MR1, UID_N_MR1, ncg, true); @@ -3048,7 +3057,7 @@ public class PreferencesHelperTest extends UiServiceTestCase { @Test public void testChannelXml_backupDefaultApp() throws Exception { NotificationChannel channel1 = - new NotificationChannel("id1", "name1", NotificationManager.IMPORTANCE_HIGH); + createNotificationChannel("id1", "name1", NotificationManager.IMPORTANCE_HIGH); mHelper.createNotificationChannel(PKG_O, UID_O, channel1, true, false); @@ -3329,7 +3338,7 @@ public class PreferencesHelperTest extends UiServiceTestCase { mAppOpsManager, mStatsEventBuilderFactory); mHelper.createNotificationChannel( - PKG_P, UID_P, new NotificationChannel("id", "id", 2), true, false); + PKG_P, UID_P, createNotificationChannel("id", "id", 2), true, false); mHelper.deleteNotificationChannel(PKG_P, UID_P, "id"); NotificationChannel nc1 = mHelper.getNotificationChannel(PKG_P, UID_P, "id", true); assertTrue(DateUtils.isToday(nc1.getDeletedTimeMs()));