Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit c41dcd7b authored by Yuri Lin's avatar Yuri Lin
Browse files

Always call getOrCreatePackagePreferences in getNotificationChannels 

Once flag is on, we always call getOrCreatePackagePreferences when an app calls getNotificationChannels. This affects the overall behavior for a very small slice of apps (ones that qualify for shouldHaveDefaultChannel) in the specific time before the app has interacted with notification settings at all.

Bug: 381131846
Test: NotificationManagerTest, PreferencesHelperTest, presubmit
Flag: android.app.nm_binder_perf_cache_channels
Change-Id: Ie6e13edcf0c164feed8f5cf4dcd656ede82171e7
parent 4bd1fe00

core/java/android/app/INotificationManager.aidl

+0 −1
Original line number Diff line number Diff line
@@ -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);

core/java/android/app/NotificationManager.java

+6 −10
Original line number Diff line number Diff line
@@ -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

core/tests/coretests/src/android/app/NotificationManagerTest.java

+20 −24
Original line number Diff line number Diff line
@@ -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

services/core/java/com/android/server/notification/NotificationManagerService.java

+3 −10
Original line number Diff line number Diff line
@@ -5018,14 +5018,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;









@@ -5035,8 +5029,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);

































services/core/java/com/android/server/notification/PreferencesHelper.java



+16
−13




























Original line number
Diff line number
Diff line








@@ -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















     */