Merge "Always call getOrCreatePackagePreferences in getNotificationChannels" into main

6 files changed, 54 insertions, 96 deletions

diff --git a/core/java/android/app/INotificationManager.aidl b/core/java/android/app/INotificationManager.aidl
index 8fa2362139a1..ff39329a0d2d 100644
--- a/core/java/android/app/INotificationManager.aidl
+++ b/core/java/android/app/INotificationManager.aidl
@@ -114,7 +114,6 @@ interface INotificationManager
     NotificationChannel getNotificationChannelForPackage(String pkg, int uid, String channelId, String conversationId, boolean includeDeleted);
     void deleteNotificationChannel(String pkg, String channelId);
     ParceledListSlice getNotificationChannels(String callingPkg, String targetPkg, int userId);
-    ParceledListSlice getOrCreateNotificationChannels(String callingPkg, String targetPkg, int userId, boolean createPrefsIfNeeded);
     ParceledListSlice getNotificationChannelsForPackage(String pkg, int uid, boolean includeDeleted);
     int getNumNotificationChannelsForPackage(String pkg, int uid, boolean includeDeleted);
     int getDeletedChannelCount(String pkg, int uid);
diff --git a/core/java/android/app/NotificationManager.java b/core/java/android/app/NotificationManager.java
index 1e1ec602d0a2..21dad28560df 100644
--- a/core/java/android/app/NotificationManager.java
+++ b/core/java/android/app/NotificationManager.java
@@ -1264,8 +1264,7 @@ public class NotificationManager {
                     mNotificationChannelListCache.query(new NotificationChannelQuery(
                             mContext.getOpPackageName(),
                             mContext.getPackageName(),
-                            mContext.getUserId(),
-                            true)));  // create (default channel) if needed
+                            mContext.getUserId())));
         } else {
             INotificationManager service = service();
             try {
@@ -1293,8 +1292,7 @@ public class NotificationManager {
                     mNotificationChannelListCache.query(new NotificationChannelQuery(
                             mContext.getOpPackageName(),
                             mContext.getPackageName(),
-                            mContext.getUserId(),
-                            true)));  // create (default channel) if needed
+                            mContext.getUserId())));
         } else {
             INotificationManager service = service();
             try {
@@ -1320,8 +1318,7 @@ public class NotificationManager {
             return mNotificationChannelListCache.query(new NotificationChannelQuery(
                     mContext.getOpPackageName(),
                     mContext.getPackageName(),
-                    mContext.getUserId(),
-                    false));
+                    mContext.getUserId()));
         } else {
             INotificationManager service = service();
             try {
@@ -1461,8 +1458,8 @@ public class NotificationManager {
                 public List<NotificationChannel> apply(NotificationChannelQuery query) {
                     INotificationManager service = service();
                     try {
-                        return service.getOrCreateNotificationChannels(query.callingPkg,
-                                query.targetPkg, query.userId, query.createIfNeeded).getList();
+                        return service.getNotificationChannels(query.callingPkg,
+                                query.targetPkg, query.userId).getList();
                     } catch (RemoteException e) {
                         throw e.rethrowFromSystemServer();
                     }
@@ -1490,8 +1487,7 @@ public class NotificationManager {
     private record NotificationChannelQuery(
             String callingPkg,
             String targetPkg,
-            int userId,
-            boolean createIfNeeded) {}
+            int userId) {}
 
     /**
      * @hide
diff --git a/core/tests/coretests/src/android/app/NotificationManagerTest.java b/core/tests/coretests/src/android/app/NotificationManagerTest.java
index 41432294b3c2..9b97c8feaf12 100644
--- a/core/tests/coretests/src/android/app/NotificationManagerTest.java
+++ b/core/tests/coretests/src/android/app/NotificationManagerTest.java
@@ -19,7 +19,6 @@ package android.app;
 import static com.google.common.truth.Truth.assertThat;
 
 import static org.mockito.ArgumentMatchers.any;
-import static org.mockito.ArgumentMatchers.anyBoolean;
 import static org.mockito.ArgumentMatchers.anyInt;
 import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.atLeast;
@@ -304,9 +303,8 @@ public class NotificationManagerTest {
 
         // It doesn't matter what the returned contents are, as long as we return a channel.
         // This setup must set up getNotificationChannels(), as that's the method called.
-        when(mNotificationManager.mBackendService.getOrCreateNotificationChannels(any(), any(),
-                anyInt(), anyBoolean())).thenReturn(
-                    new ParceledListSlice<>(List.of(exampleChannel())));
+        when(mNotificationManager.mBackendService.getNotificationChannels(any(), any(),
+                anyInt())).thenReturn(new ParceledListSlice<>(List.of(exampleChannel())));
 
         // ask for the same channel 100 times without invalidating the cache
         for (int i = 0; i < 100; i++) {
@@ -318,7 +316,7 @@ public class NotificationManagerTest {
         NotificationChannel unused = mNotificationManager.getNotificationChannel("id");
 
         verify(mNotificationManager.mBackendService, times(2))
-                .getOrCreateNotificationChannels(any(), any(), anyInt(), anyBoolean());
+                .getNotificationChannels(any(), any(), anyInt());
     }
 
     @Test
@@ -331,24 +329,23 @@ public class NotificationManagerTest {
         NotificationChannel c2 = new NotificationChannel("id2", "name2",
                 NotificationManager.IMPORTANCE_NONE);
 
-        when(mNotificationManager.mBackendService.getOrCreateNotificationChannels(any(), any(),
-                anyInt(), anyBoolean())).thenReturn(new ParceledListSlice<>(List.of(c1, c2)));
+        when(mNotificationManager.mBackendService.getNotificationChannels(any(), any(),
+                anyInt())).thenReturn(new ParceledListSlice<>(List.of(c1, c2)));
 
         assertThat(mNotificationManager.getNotificationChannel("id1")).isEqualTo(c1);
         assertThat(mNotificationManager.getNotificationChannel("id2")).isEqualTo(c2);
         assertThat(mNotificationManager.getNotificationChannel("id3")).isNull();
 
         verify(mNotificationManager.mBackendService, times(1))
-                .getOrCreateNotificationChannels(any(), any(), anyInt(), anyBoolean());
+                .getNotificationChannels(any(), any(), anyInt());
     }
 
     @Test
     @EnableFlags(Flags.FLAG_NM_BINDER_PERF_CACHE_CHANNELS)
     public void getNotificationChannels_cachedUntilInvalidated() throws Exception {
         NotificationManager.invalidateNotificationChannelCache();
-        when(mNotificationManager.mBackendService.getOrCreateNotificationChannels(any(), any(),
-                anyInt(), anyBoolean())).thenReturn(
-                    new ParceledListSlice<>(List.of(exampleChannel())));
+        when(mNotificationManager.mBackendService.getNotificationChannels(any(), any(),
+                anyInt())).thenReturn(new ParceledListSlice<>(List.of(exampleChannel())));
 
         // ask for channels 100 times without invalidating the cache
         for (int i = 0; i < 100; i++) {
@@ -360,7 +357,7 @@ public class NotificationManagerTest {
         List<NotificationChannel> res = mNotificationManager.getNotificationChannels();
 
         verify(mNotificationManager.mBackendService, times(2))
-                .getOrCreateNotificationChannels(any(), any(), anyInt(), anyBoolean());
+                .getNotificationChannels(any(), any(), anyInt());
         assertThat(res).containsExactlyElementsIn(List.of(exampleChannel()));
     }
 
@@ -378,9 +375,8 @@ public class NotificationManagerTest {
         NotificationChannel c2 = new NotificationChannel("other", "name2",
                 NotificationManager.IMPORTANCE_DEFAULT);
 
-        when(mNotificationManager.mBackendService.getOrCreateNotificationChannels(any(), any(),
-                anyInt(), anyBoolean())).thenReturn(
-                    new ParceledListSlice<>(List.of(c1, conv1, c2)));
+        when(mNotificationManager.mBackendService.getNotificationChannels(any(), any(),
+                anyInt())).thenReturn(new ParceledListSlice<>(List.of(c1, conv1, c2)));
 
         // Lookup for channel c1 and c2: returned as expected
         assertThat(mNotificationManager.getNotificationChannel("id")).isEqualTo(c1);
@@ -397,9 +393,9 @@ public class NotificationManagerTest {
         // Lookup of a nonexistent channel is null
         assertThat(mNotificationManager.getNotificationChannel("id3")).isNull();
 
-        // All of that should have been one call to getOrCreateNotificationChannels()
+        // All of that should have been one call to getNotificationChannels()
         verify(mNotificationManager.mBackendService, times(1))
-                .getOrCreateNotificationChannels(any(), any(), anyInt(), anyBoolean());
+                .getNotificationChannels(any(), any(), anyInt());
     }
 
     @Test
@@ -419,12 +415,12 @@ public class NotificationManagerTest {
         NotificationChannel channel3 = channel1.copy();
         channel3.setName("name3");
 
-        when(mNotificationManager.mBackendService.getOrCreateNotificationChannels(any(), eq(pkg1),
-                eq(userId), anyBoolean())).thenReturn(new ParceledListSlice<>(List.of(channel1)));
-        when(mNotificationManager.mBackendService.getOrCreateNotificationChannels(any(), eq(pkg2),
-                eq(userId), anyBoolean())).thenReturn(new ParceledListSlice<>(List.of(channel2)));
-        when(mNotificationManager.mBackendService.getOrCreateNotificationChannels(any(), eq(pkg1),
-                eq(userId1), anyBoolean())).thenReturn(new ParceledListSlice<>(List.of(channel3)));
+        when(mNotificationManager.mBackendService.getNotificationChannels(any(), eq(pkg1),
+                eq(userId))).thenReturn(new ParceledListSlice<>(List.of(channel1)));
+        when(mNotificationManager.mBackendService.getNotificationChannels(any(), eq(pkg2),
+                eq(userId))).thenReturn(new ParceledListSlice<>(List.of(channel2)));
+        when(mNotificationManager.mBackendService.getNotificationChannels(any(), eq(pkg1),
+                eq(userId1))).thenReturn(new ParceledListSlice<>(List.of(channel3)));
 
         // set our context to pretend to be from package 1 and userId 0
         mContext.setParameters(pkg1, pkg1, userId);
@@ -440,7 +436,7 @@ public class NotificationManagerTest {
 
         // Those should have been three different calls
         verify(mNotificationManager.mBackendService, times(3))
-                .getOrCreateNotificationChannels(any(), any(), anyInt(), anyBoolean());
+                .getNotificationChannels(any(), any(), anyInt());
     }
 
     @Test
diff --git a/services/core/java/com/android/server/notification/NotificationManagerService.java b/services/core/java/com/android/server/notification/NotificationManagerService.java
index d19eb235c9d5..b4a58ac72394 100644
--- a/services/core/java/com/android/server/notification/NotificationManagerService.java
+++ b/services/core/java/com/android/server/notification/NotificationManagerService.java
@@ -5056,14 +5056,8 @@ public class NotificationManagerService extends SystemService {
         }
 
         @Override
-        public ParceledListSlice<NotificationChannel> getNotificationChannels(
-                String callingPkg, String targetPkg, int userId) {
-            return getOrCreateNotificationChannels(callingPkg, targetPkg, userId, false);
-        }
-
-        @Override
-        public ParceledListSlice<NotificationChannel> getOrCreateNotificationChannels(
-                String callingPkg, String targetPkg, int userId, boolean createPrefsIfNeeded) {
+        public ParceledListSlice<NotificationChannel> getNotificationChannels(String callingPkg,
+                String targetPkg, int userId) {
             if (canNotifyAsPackage(callingPkg, targetPkg, userId)
                 || isCallingUidSystem()) {
                 int targetUid = -1;
@@ -5073,8 +5067,7 @@ public class NotificationManagerService extends SystemService {
                     /* ignore */
                 }
                 return mPreferencesHelper.getNotificationChannels(
-                        targetPkg, targetUid, false /* includeDeleted */, true,
-                        createPrefsIfNeeded);
+                        targetPkg, targetUid, false /* includeDeleted */, true);
             }
             throw new SecurityException("Pkg " + callingPkg
                     + " cannot read channels for " + targetPkg + " in " + userId);
diff --git a/services/core/java/com/android/server/notification/PreferencesHelper.java b/services/core/java/com/android/server/notification/PreferencesHelper.java
index 3b34dcd17705..14cc91b6305f 100644
--- a/services/core/java/com/android/server/notification/PreferencesHelper.java
+++ b/services/core/java/com/android/server/notification/PreferencesHelper.java
@@ -16,8 +16,8 @@
 
 package com.android.server.notification;
 
-import static android.app.Flags.notificationClassificationUi;
 import static android.app.AppOpsManager.OP_SYSTEM_ALERT_WINDOW;
+import static android.app.Flags.notificationClassificationUi;
 import static android.app.NotificationChannel.DEFAULT_CHANNEL_ID;
 import static android.app.NotificationChannel.NEWS_ID;
 import static android.app.NotificationChannel.PLACEHOLDER_CONVERSATION_ID;
@@ -89,9 +89,10 @@ import android.util.SparseBooleanArray;
 import android.util.StatsEvent;
 import android.util.proto.ProtoOutputStream;
 
+import androidx.annotation.VisibleForTesting;
+
 import com.android.internal.R;
 import com.android.internal.annotations.GuardedBy;
-import com.android.internal.annotations.VisibleForTesting;
 import com.android.internal.config.sysui.SystemUiSystemPropertiesFlags;
 import com.android.internal.config.sysui.SystemUiSystemPropertiesFlags.NotificationFlags;
 import com.android.internal.logging.MetricsLogger;
@@ -1962,21 +1963,11 @@ public class PreferencesHelper implements RankingConfig {
     @Override
     public ParceledListSlice<NotificationChannel> getNotificationChannels(String pkg, int uid,
             boolean includeDeleted, boolean includeBundles) {
-        return getNotificationChannels(pkg, uid, includeDeleted, includeBundles, false);
-    }
-
-    protected ParceledListSlice<NotificationChannel> getNotificationChannels(String pkg, int uid,
-            boolean includeDeleted, boolean includeBundles, boolean createPrefsIfNeeded) {
-        if (createPrefsIfNeeded && !android.app.Flags.nmBinderPerfCacheChannels()) {
-            Slog.wtf(TAG,
-                    "getNotificationChannels called with createPrefsIfNeeded=true and flag off");
-            createPrefsIfNeeded = false;
-        }
         Objects.requireNonNull(pkg);
         List<NotificationChannel> channels = new ArrayList<>();
         synchronized (mLock) {
             PackagePreferences r;
-            if (createPrefsIfNeeded) {
+            if (android.app.Flags.nmBinderPerfCacheChannels()) {
                 r = getOrCreatePackagePreferencesLocked(pkg, uid);
             } else {
                 r = getPackagePreferencesLocked(pkg, uid);
@@ -1997,6 +1988,18 @@ public class PreferencesHelper implements RankingConfig {
         }
     }
 
+    @VisibleForTesting(otherwise = VisibleForTesting.NONE)
+    // Gets the entire list of notification channels for this package, with no filtering and
+    // without creating package preferences. For testing only, specifically to confirm the
+    // notification channels of a removed/deleted package.
+    protected List<NotificationChannel> getRemovedPkgNotificationChannels(String pkg, int uid) {
+        PackagePreferences r = getPackagePreferencesLocked(pkg, uid);
+        if (r == null || r.channels == null) {
+            return new ArrayList<>();
+        }
+        return new ArrayList<>(r.channels.values());
+    }
+
     /**
      * Gets all notification channels associated with the given pkg and uid that can bypass dnd
      */
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 704b580a80b0..832ca51ae580 100644
--- a/services/tests/uiservicestests/src/com/android/server/notification/PreferencesHelperTest.java
+++ b/services/tests/uiservicestests/src/com/android/server/notification/PreferencesHelperTest.java
@@ -260,7 +260,7 @@ public class PreferencesHelperTest extends UiServiceTestCase {
         return FlagsParameterization.allCombinationsOf(
                 android.app.Flags.FLAG_API_RICH_ONGOING,
                 FLAG_NOTIFICATION_CLASSIFICATION, FLAG_NOTIFICATION_CLASSIFICATION_UI,
-                FLAG_MODES_UI);
+                FLAG_MODES_UI, android.app.Flags.FLAG_NM_BINDER_PERF_CACHE_CHANNELS);
     }
 
     public PreferencesHelperTest(FlagsParameterization flags) {
@@ -3381,13 +3381,12 @@ public class PreferencesHelperTest extends UiServiceTestCase {
         // user 0 records remain
         for (int i = 0; i < user0Uids.length; i++) {
             assertEquals(1,
-                    mHelper.getNotificationChannels(PKG_N_MR1, user0Uids[i], false, true)
-                            .getList().size());
+                    mHelper.getRemovedPkgNotificationChannels(PKG_N_MR1, user0Uids[i]).size());
         }
         // user 1 records are gone
         for (int i = 0; i < user1Uids.length; i++) {
-            assertEquals(0, mHelper.getNotificationChannels(PKG_N_MR1, user1Uids[i], false, true)
-                    .getList().size());
+            assertEquals(0,
+                    mHelper.getRemovedPkgNotificationChannels(PKG_N_MR1, user1Uids[i]).size());
         }
     }
 
@@ -3402,8 +3401,7 @@ public class PreferencesHelperTest extends UiServiceTestCase {
         assertTrue(mHelper.onPackagesChanged(true, USER_SYSTEM, new String[]{PKG_N_MR1},
                 new int[]{UID_N_MR1}));
 
-        assertEquals(0, mHelper.getNotificationChannels(
-                PKG_N_MR1, UID_N_MR1, true, true).getList().size());
+        assertEquals(0, mHelper.getRemovedPkgNotificationChannels(PKG_N_MR1, UID_N_MR1).size());
 
         // Not deleted
         mHelper.createNotificationChannel(PKG_N_MR1, UID_N_MR1, channel1, true, false,
@@ -3472,7 +3470,7 @@ public class PreferencesHelperTest extends UiServiceTestCase {
         assertTrue(mHelper.canShowBadge(PKG_O, UID_O));
         assertNull(mHelper.getNotificationDelegate(PKG_O, UID_O));
         assertEquals(0, mHelper.getAppLockedFields(PKG_O, UID_O));
-        assertEquals(0, mHelper.getNotificationChannels(PKG_O, UID_O, true, true).getList().size());
+        assertEquals(0, mHelper.getRemovedPkgNotificationChannels(PKG_O, UID_O).size());
         assertEquals(0, mHelper.getNotificationChannelGroups(PKG_O, UID_O).size());
 
         NotificationChannel channel = getChannel();
@@ -6836,38 +6834,11 @@ public class PreferencesHelperTest extends UiServiceTestCase {
     }
 
     @Test
-    @EnableFlags(android.app.Flags.FLAG_NM_BINDER_PERF_CACHE_CHANNELS)
-    public void testGetNotificationChannels_createIfNeeded() {
-        // Test setup hasn't created any channels or read package preferences yet.
-        // If we ask for notification channels _without_ creating, we should get no result.
-        ParceledListSlice<NotificationChannel> channels = mHelper.getNotificationChannels(PKG_N_MR1,
-                UID_N_MR1, false, false, /* createPrefsIfNeeded= */ false);
-        assertThat(channels.getList().size()).isEqualTo(0);
-
-        // If we ask it to create package preferences, we expect the default channel to be created
-        // for N_MR1.
-        channels = mHelper.getNotificationChannels(PKG_N_MR1, UID_N_MR1, false,
-                false, /* createPrefsIfNeeded= */ true);
-        assertThat(channels.getList().size()).isEqualTo(1);
-        assertThat(channels.getList().getFirst().getId()).isEqualTo(
-                NotificationChannel.DEFAULT_CHANNEL_ID);
-    }
-
-    @Test
     @DisableFlags(android.app.Flags.FLAG_NM_BINDER_PERF_CACHE_CHANNELS)
     public void testGetNotificationChannels_neverCreatesWhenFlagOff() {
-        ParceledListSlice<NotificationChannel> channels;
-        try {
-            channels = mHelper.getNotificationChannels(PKG_N_MR1, UID_N_MR1, false,
-                    false, /* createPrefsIfNeeded= */ true);
-        } catch (Exception e) {
-            // Slog.wtf kicks in, presumably
-        } finally {
-            channels = mHelper.getNotificationChannels(PKG_N_MR1, UID_N_MR1, false,
-                    false, /* createPrefsIfNeeded= */ false);
-            assertThat(channels.getList().size()).isEqualTo(0);
-        }
-
+        ParceledListSlice<NotificationChannel> channels = mHelper.getNotificationChannels(PKG_N_MR1,
+                UID_N_MR1, false, false);
+        assertThat(channels.getList().size()).isEqualTo(0);
     }
 
     // Test version of PreferencesHelper whose only functional difference is that it does not