From f6d322577af7231c28d398096b3ca9b88260fa49 Mon Sep 17 00:00:00 2001 From: Julia Reynolds Date: Wed, 10 Aug 2022 17:40:20 +0000 Subject: [PATCH] Revert^2 "Fix binder error when an app has many channels" 34b3e4b51c6565cf84084c7533e1d9308fd39b10 Test: NotificationChannelGroupTest Test: NotificationManagerServiceTest Test: PreferencesHelperTest (new test for revert: testGetNotificationChannelGroup) Test: long press on a conversation notification in the shade Bug: 215072888 Change-Id: Ib9c671b0fd1f97e01ef02957839ebad2cadb5e44 --- .../android/app/NotificationChannelGroup.java | 21 +++---- .../row/ChannelEditorDialogControllerTest.kt | 2 +- .../notification/PreferencesHelper.java | 55 +++++++++++-------- .../notification/PreferencesHelperTest.java | 30 ++++++++++ 4 files changed, 71 insertions(+), 37 deletions(-) diff --git a/core/java/android/app/NotificationChannelGroup.java b/core/java/android/app/NotificationChannelGroup.java index f97415ca20c8a..1769993e0e075 100644 --- a/core/java/android/app/NotificationChannelGroup.java +++ b/core/java/android/app/NotificationChannelGroup.java @@ -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; @@ -66,7 +67,7 @@ public final class NotificationChannelGroup implements Parcelable { private CharSequence mName; private String mDescription; private boolean mBlocked; - private List mChannels = new ArrayList<>(); + private ParceledListSlice mChannels; // Bitwise representation of fields that have been changed by the user private int mUserLockedFields; @@ -100,7 +101,8 @@ public final class NotificationChannelGroup implements Parcelable { } else { mDescription = null; } - in.readParcelableList(mChannels, NotificationChannel.class.getClassLoader(), android.app.NotificationChannel.class); + mChannels = in.readParcelable( + NotificationChannelGroup.class.getClassLoader(), ParceledListSlice.class); mBlocked = in.readBoolean(); mUserLockedFields = in.readInt(); } @@ -127,7 +129,7 @@ public final class NotificationChannelGroup implements Parcelable { } else { dest.writeByte((byte) 0); } - dest.writeParcelableList(mChannels, flags); + dest.writeParcelable(mChannels, flags); dest.writeBoolean(mBlocked); dest.writeInt(mUserLockedFields); } @@ -157,7 +159,7 @@ public final class NotificationChannelGroup implements Parcelable { * Returns the list of channels that belong to this group */ public List getChannels() { - return mChannels; + return mChannels == null ? new ArrayList<>() : mChannels.getList(); } /** @@ -188,18 +190,11 @@ public final class NotificationChannelGroup implements Parcelable { mBlocked = blocked; } - /** - * @hide - */ - public void addChannel(NotificationChannel channel) { - mChannels.add(channel); - } - /** * @hide */ public void setChannels(List channels) { - mChannels = channels; + mChannels = new ParceledListSlice<>(channels); } /** @@ -334,7 +329,7 @@ public final class NotificationChannelGroup implements Parcelable { proto.write(NotificationChannelGroupProto.NAME, mName.toString()); proto.write(NotificationChannelGroupProto.DESCRIPTION, mDescription); proto.write(NotificationChannelGroupProto.IS_BLOCKED, mBlocked); - for (NotificationChannel channel : mChannels) { + for (NotificationChannel channel : mChannels.getList()) { channel.dumpDebug(proto, NotificationChannelGroupProto.CHANNELS); } proto.end(token); diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/row/ChannelEditorDialogControllerTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/row/ChannelEditorDialogControllerTest.kt index 64d0256287541..214ba16dfc44b 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/row/ChannelEditorDialogControllerTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/row/ChannelEditorDialogControllerTest.kt @@ -98,7 +98,7 @@ class ChannelEditorDialogControllerTest : SysuiTestCase() { @Test fun testPrepareDialogForApp_onlyDefaultChannel() { - group.addChannel(channelDefault) + group.channels = listOf(channelDefault) controller.prepareDialogForApp(TEST_APP_NAME, TEST_PACKAGE_NAME, TEST_UID, setOf(channelDefault), appIcon, clickListener) diff --git a/services/core/java/com/android/server/notification/PreferencesHelper.java b/services/core/java/com/android/server/notification/PreferencesHelper.java index 477b8da61e0f5..729c521a4ce07 100644 --- a/services/core/java/com/android/server/notification/PreferencesHelper.java +++ b/services/core/java/com/android/server/notification/PreferencesHelper.java @@ -86,6 +86,7 @@ import java.io.PrintWriter; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; +import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Objects; @@ -1327,16 +1328,17 @@ public class PreferencesHelper implements RankingConfig { return null; } NotificationChannelGroup group = r.groups.get(groupId).clone(); - group.setChannels(new ArrayList<>()); + ArrayList channels = new ArrayList(); int N = r.channels.size(); for (int i = 0; i < N; i++) { final NotificationChannel nc = r.channels.valueAt(i); if (includeDeleted || !nc.isDeleted()) { if (groupId.equals(nc.getGroup())) { - group.addChannel(nc); + channels.add(nc); } } } + group.setChannels(channels); return group; } } @@ -1349,7 +1351,10 @@ public class PreferencesHelper implements RankingConfig { if (r == null) { return null; } - return r.groups.get(groupId); + if (r.groups.get(groupId) != null) { + return r.groups.get(groupId).clone(); + } + return null; } } @@ -1357,44 +1362,48 @@ public class PreferencesHelper implements RankingConfig { public ParceledListSlice getNotificationChannelGroups(String pkg, int uid, boolean includeDeleted, boolean includeNonGrouped, boolean includeEmpty) { Objects.requireNonNull(pkg); - Map groups = new ArrayMap<>(); + List groups = new ArrayList<>(); synchronized (mPackagePreferences) { PackagePreferences r = getPackagePreferencesLocked(pkg, uid); if (r == null) { return ParceledListSlice.emptyList(); } - NotificationChannelGroup nonGrouped = new NotificationChannelGroup(null, null); + Map> groupedChannels = new HashMap(); int N = r.channels.size(); for (int i = 0; i < N; i++) { final NotificationChannel nc = r.channels.valueAt(i); if (includeDeleted || !nc.isDeleted()) { if (nc.getGroup() != null) { if (r.groups.get(nc.getGroup()) != null) { - NotificationChannelGroup ncg = groups.get(nc.getGroup()); - if (ncg == null) { - ncg = r.groups.get(nc.getGroup()).clone(); - ncg.setChannels(new ArrayList<>()); - groups.put(nc.getGroup(), ncg); - - } - ncg.addChannel(nc); + ArrayList channels = groupedChannels.getOrDefault( + nc.getGroup(), new ArrayList<>()); + channels.add(nc); + groupedChannels.put(nc.getGroup(), channels); } } else { - nonGrouped.addChannel(nc); + ArrayList channels = groupedChannels.getOrDefault( + null, new ArrayList<>()); + channels.add(nc); + groupedChannels.put(null, channels); } } } - if (includeNonGrouped && nonGrouped.getChannels().size() > 0) { - groups.put(null, nonGrouped); - } - if (includeEmpty) { - for (NotificationChannelGroup group : r.groups.values()) { - if (!groups.containsKey(group.getId())) { - groups.put(group.getId(), group); - } + for (NotificationChannelGroup group : r.groups.values()) { + ArrayList channels = + groupedChannels.getOrDefault(group.getId(), new ArrayList<>()); + if (includeEmpty || !channels.isEmpty()) { + NotificationChannelGroup clone = group.clone(); + clone.setChannels(channels); + groups.add(clone); } } - return new ParceledListSlice<>(new ArrayList<>(groups.values())); + + if (includeNonGrouped && groupedChannels.containsKey(null)) { + NotificationChannelGroup nonGrouped = new NotificationChannelGroup(null, null); + nonGrouped.setChannels(groupedChannels.get(null)); + groups.add(nonGrouped); + } + return new ParceledListSlice<>(groups); } } 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 598a22bbde393..d62ac99e530bf 100644 --- a/services/tests/uiservicestests/src/com/android/server/notification/PreferencesHelperTest.java +++ b/services/tests/uiservicestests/src/com/android/server/notification/PreferencesHelperTest.java @@ -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};