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

Commit 4ce06695 authored by Julia Reynolds's avatar Julia Reynolds
Browse files

Fix binder error when an app has many channels 

By using a ParceledListSlice when sending the data
between processes.

Test: PreferencesHelperTest
Test: NotificationListenersTest (new since last attempt to land)
Test: NotificationManagerServiceTest
Test: view the notification settings page for an app with hundreds of
notification channels
Bug: 215072888

Change-Id: Iac1ba92b6636e86cf506f1ab3c12dcf803101322
parent b956e1ee

core/java/android/app/NotificationChannelGroup.java

+14 −3
Original line number Diff line number Diff line
@@ -20,6 +20,7 @@ import android.annotation.SystemApi; 
import android.annotation.TestApi;

import android.compat.annotation.UnsupportedAppUsage;

import android.content.Intent;

import android.content.pm.ParceledListSlice;

import android.os.Parcel;

import android.os.Parcelable;

import android.text.TextUtils;
@@ -68,7 +69,7 @@ public final class NotificationChannelGroup implements Parcelable { 
    private CharSequence mName;

    private String mDescription;

    private boolean mBlocked;

    private List<NotificationChannel> mChannels = new ArrayList<>();

    private List<NotificationChannel> mChannels = new ArrayList();

    // Bitwise representation of fields that have been changed by the user

    private int mUserLockedFields;
@@ -106,7 +107,12 @@ public final class NotificationChannelGroup implements Parcelable { 
        } else {

            mDescription = null;

        }

        in.readParcelableList(mChannels, NotificationChannel.class.getClassLoader(), android.app.NotificationChannel.class);

        if (in.readByte() != 0) {

            mChannels = in.readParcelable(NotificationChannelGroup.class.getClassLoader(),

                    ParceledListSlice.class).getList();

        } else {

            mChannels = new ArrayList<>();

        }

        mBlocked = in.readBoolean();

        mUserLockedFields = in.readInt();

    }
@@ -138,7 +144,12 @@ public final class NotificationChannelGroup implements Parcelable { 
        } else {

            dest.writeByte((byte) 0);

        }

        dest.writeParcelableList(mChannels, flags);

        if (mChannels != null) {

            dest.writeByte((byte) 1);

            dest.writeParcelable(new ParceledListSlice<>(mChannels), flags);

        } else {

            dest.writeByte((byte) 0);

        }

        dest.writeBoolean(mBlocked);

        dest.writeInt(mUserLockedFields);

    }

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

+8 −0
Original line number Diff line number Diff line
@@ -1718,6 +1718,14 @@ abstract public class ManagedServices { 
            return ManagedServices.this;

        }



        public IInterface getService() {

            return service;

        }



        public boolean isSystem() {

            return isSystem;

        }



        @Override

        public String toString() {

            return new StringBuilder("ManagedServiceInfo[")

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

+5 −4
Original line number Diff line number Diff line
@@ -9962,8 +9962,9 @@ public class NotificationManagerService extends SystemService { 
     * instance per user, we want to filter out interactions that are not for the user that the

     * given NAS is bound in.

     */

    private boolean isInteractionVisibleToListener(ManagedServiceInfo info, int userId) {

        boolean isAssistantService = isServiceTokenValid(info.service);

    @VisibleForTesting

    boolean isInteractionVisibleToListener(ManagedServiceInfo info, int userId) {

        boolean isAssistantService = isServiceTokenValid(info.getService());

        return !isAssistantService || info.isSameUser(userId);

    }










@@ -11265,7 +11266,7 @@ public class NotificationManagerService extends SystemService {













                }





























                BackgroundThread.getHandler().post(() -> {













                    if (info.isSystem || hasCompanionDevice(info)) {













                    if (info.isSystem() || hasCompanionDevice(info)) {















                        notifyNotificationChannelGroupChanged(















                                info, pkg, user, group, modificationType);















                    }










@@ -11348,7 +11349,7 @@ public class NotificationManagerService extends SystemService {













        private void notifyNotificationChannelGroupChanged(ManagedServiceInfo info,















                final String pkg, final UserHandle user, final NotificationChannelGroup group,















                final int modificationType) {













            final INotificationListener listener = (INotificationListener) info.service;













            final INotificationListener listener = (INotificationListener) info.getService();















            try {















                listener.onNotificationChannelGroupModification(pkg, user, group, modificationType);















            } catch (RemoteException ex) {

































services/tests/uiservicestests/src/com/android/server/notification/NotificationListenersTest.java



+52
−2




























Original line number
Diff line number
Diff line








@@ -28,24 +28,29 @@ import static com.google.common.truth.Truth.assertThat;













import static junit.framework.Assert.assertFalse;















import static junit.framework.Assert.assertTrue;





























import static org.junit.Assert.fail;















import static org.mockito.ArgumentMatchers.any;















import static org.mockito.ArgumentMatchers.anyInt;













import static org.mockito.ArgumentMatchers.anyString;















import static org.mockito.ArgumentMatchers.eq;















import static org.mockito.ArgumentMatchers.intThat;















import static org.mockito.ArgumentMatchers.nullable;















import static org.mockito.Mockito.atLeast;













import static org.mockito.Mockito.doAnswer;















import static org.mockito.Mockito.doNothing;















import static org.mockito.Mockito.doReturn;















import static org.mockito.Mockito.mock;















import static org.mockito.Mockito.never;















import static org.mockito.Mockito.reset;















import static org.mockito.Mockito.spy;













import static org.mockito.Mockito.times;















import static org.mockito.Mockito.verify;















import static org.mockito.Mockito.when;































import android.app.INotificationManager;















import android.app.Notification;















import android.app.NotificationChannel;













import android.app.NotificationChannelGroup;















import android.app.NotificationManager;















import android.content.ComponentName;















import android.content.pm.IPackageManager;









@@ -54,7 +59,10 @@ import android.content.pm.ServiceInfo;













import android.content.pm.VersionedPackage;















import android.content.res.Resources;















import android.os.Bundle;













import android.os.Parcel;













import android.os.RemoteException;















import android.os.UserHandle;













import android.service.notification.INotificationListener;















import android.service.notification.NotificationListenerFilter;















import android.service.notification.NotificationListenerService;















import android.service.notification.NotificationRankingUpdate;









@@ -63,10 +71,10 @@ import android.service.notification.StatusBarNotification;













import android.testing.TestableContext;















import android.util.ArraySet;















import android.util.Pair;













import android.util.Xml;





























import com.android.modules.utils.TypedXmlPullParser;















import com.android.modules.utils.TypedXmlSerializer;













import android.util.Xml;





























import com.android.server.UiServiceTestCase;































import com.google.common.collect.ImmutableList;










@@ -113,6 +121,8 @@ public class NotificationListenersTest extends UiServiceTestCase {













        getContext().setMockPackageManager(mPm);















        doNothing().when(mContext).sendBroadcastAsUser(any(), any(), any());





























        when(mNm.isInteractionVisibleToListener(any(), anyInt())).thenReturn(true);





























        mListeners = spy(mNm.new NotificationListeners(















                mContext, new Object(), mock(ManagedServices.UserProfiles.class), miPm));















        when(mNm.getBinderService()).thenReturn(mINm);










@@ -596,4 +606,44 @@ public class NotificationListenersTest extends UiServiceTestCase {













        verify(mPmi).grantImplicitAccess(sbn.getUserId(), null, UserHandle.getAppId(33),















                sbn.getUid(), false, false);















    }



























    @Test













    public void testUpdateGroup_notifyTwoListeners() throws Exception {













        final NotificationChannelGroup updated = new NotificationChannelGroup("id", "name");













        updated.setChannels(ImmutableList.of(













                new NotificationChannel("a", "a", 1), new NotificationChannel("b", "b", 2)));













        updated.setBlocked(true);



























        ManagedServices.ManagedServiceInfo i1 = getParcelingListener(updated);













        ManagedServices.ManagedServiceInfo i2= getParcelingListener(updated);













        when(mListeners.getServices()).thenReturn(ImmutableList.of(i1, i2));













        NotificationChannelGroup existing = new NotificationChannelGroup("id", "name");



























        mListeners.notifyNotificationChannelGroupChanged("pkg", UserHandle.of(0), updated, 0);













        Thread.sleep(500);



























        verify(((INotificationListener) i1.getService()), times(1))













                .onNotificationChannelGroupModification(anyString(), any(), any(), anyInt());













    }



























    private ManagedServices.ManagedServiceInfo getParcelingListener(













            final NotificationChannelGroup toParcel)













            throws RemoteException {













        ManagedServices.ManagedServiceInfo i1 = mock(ManagedServices.ManagedServiceInfo.class);













        when(i1.isSystem()).thenReturn(true);













        INotificationListener l1 = mock(INotificationListener.class);













        when(i1.enabledAndUserMatches(anyInt())).thenReturn(true);













        doAnswer(invocationOnMock -> {













            try {













                toParcel.writeToParcel(Parcel.obtain(), 0);













            } catch (Exception e) {













                fail("Failed to parcel group to listener");













                return e;



























            }













            return null;













        }).when(l1).onNotificationChannelGroupModification(anyString(), any(), any(), anyInt());













        when(i1.getService()).thenReturn(l1);













        return i1;













    }















}
























services/tests/uiservicestests/src/com/android/server/notification/PreferencesHelperTest.java



+30
−0




























Original line number
Diff line number
Diff line








@@ -93,6 +93,7 @@ import android.net.Uri;













import android.os.AsyncTask;















import android.os.Build;















import android.os.Bundle;













import android.os.Parcel;















import android.os.RemoteCallback;















import android.os.RemoteException;















import android.os.UserHandle;










@@ -2446,6 +2447,35 @@ public class PreferencesHelperTest extends UiServiceTestCase {













                mLogger.get(6).event);  // Final log is the deletion of the channel.















    }





























    @Test













    public void testGetNotificationChannelGroup() throws Exception {













        NotificationChannelGroup notDeleted = new NotificationChannelGroup("not", "deleted");













        NotificationChannel base =













                new NotificationChannel("not deleted", "belongs to notDeleted", IMPORTANCE_DEFAULT);













        base.setGroup("not");













        NotificationChannel convo =













                new NotificationChannel("convo", "belongs to notDeleted", IMPORTANCE_DEFAULT);













        convo.setGroup("not");













        convo.setConversationId("not deleted", "banana");



























        mHelper.createNotificationChannelGroup(PKG_N_MR1, UID_N_MR1, notDeleted, true);













        mHelper.createNotificationChannel(PKG_N_MR1, UID_N_MR1, base, true, false);













        mHelper.createNotificationChannel(PKG_N_MR1, UID_N_MR1, convo, true, false);













        mHelper.createNotificationChannelGroup(PKG_N_MR1, UID_N_MR1, notDeleted, true);



























        NotificationChannelGroup g













                = mHelper.getNotificationChannelGroup(notDeleted.getId(), PKG_N_MR1, UID_N_MR1);













        Parcel parcel = Parcel.obtain();













        g.writeToParcel(parcel, 0);













        parcel.setDataPosition(0);



























        NotificationChannelGroup g2













                = mHelper.getNotificationChannelGroup(notDeleted.getId(), PKG_N_MR1, UID_N_MR1);













        Parcel parcel2 = Parcel.obtain();













        g2.writeToParcel(parcel2, 0);













        parcel2.setDataPosition(0);













    }





























    @Test















    public void testOnUserRemoved() throws Exception {















        int[] user0Uids = {98, 235, 16, 3782};