diff --git a/services/core/java/com/android/server/notification/ManagedServices.java b/services/core/java/com/android/server/notification/ManagedServices.java index 202b315a7c822..a3daae4d854b1 100644 --- a/services/core/java/com/android/server/notification/ManagedServices.java +++ b/services/core/java/com/android/server/notification/ManagedServices.java @@ -101,6 +101,7 @@ abstract public class ManagedServices { protected static final String ENABLED_SERVICES_SEPARATOR = ":"; private static final String DB_VERSION_1 = "1"; private static final String DB_VERSION_2 = "2"; + private static final String DB_VERSION_3 = "3"; /** @@ -113,8 +114,9 @@ abstract public class ManagedServices { static final String ATT_VERSION = "version"; static final String ATT_DEFAULTS = "defaults"; static final String ATT_USER_SET = "user_set_services"; + static final String ATT_USER_CHANGED = "user_changed"; - static final int DB_VERSION = 3; + static final int DB_VERSION = 4; static final int APPROVAL_BY_PACKAGE = 0; static final int APPROVAL_BY_COMPONENT = 1; @@ -160,6 +162,8 @@ abstract public class ManagedServices { @GuardedBy("mApproved") protected ArrayMap> mUserSetServices = new ArrayMap<>(); + protected ArrayMap mIsUserChanged = new ArrayMap<>(); + // True if approved services are stored in xml, not settings. private boolean mUseXml; @@ -338,6 +342,7 @@ abstract public class ManagedServices { for (int i = 0; i < N; i++) { final int userId = mApproved.keyAt(i); final ArrayMap> approvedByType = mApproved.valueAt(i); + final Boolean userChanged = mIsUserChanged.get(userId); if (approvedByType != null) { final int M = approvedByType.size(); for (int j = 0; j < M; j++) { @@ -345,16 +350,20 @@ abstract public class ManagedServices { final ArraySet approved = approvedByType.valueAt(j); if (approvedByType != null && approvedByType.size() > 0) { pw.println(" " + String.join(ENABLED_SERVICES_SEPARATOR, approved) - + " (user: " + userId + " isPrimary: " + isPrimary + ")"); + + " (user: " + userId + " isPrimary: " + isPrimary + + (userChanged == null ? "" : " isUserChanged: " + + userChanged) + ")"); } } } } - pw.println(" Has user set:"); Set userIds = mUserSetServices.keySet(); for (int userId : userIds) { - pw.println(" userId=" + userId + " value=" + mUserSetServices.get(userId)); + if (mIsUserChanged.get(userId) == null) { + pw.println(" userId=" + userId + " value=" + + (mUserSetServices.get(userId))); + } } } @@ -489,13 +498,14 @@ abstract public class ManagedServices { continue; } final ArrayMap> approvedByType = mApproved.valueAt(i); + final Boolean isUserChanged = mIsUserChanged.get(approvedUserId); if (approvedByType != null) { final int M = approvedByType.size(); for (int j = 0; j < M; j++) { final boolean isPrimary = approvedByType.keyAt(j); final Set approved = approvedByType.valueAt(j); final Set userSet = mUserSetServices.get(approvedUserId); - if (approved != null || userSet != null) { + if (approved != null || userSet != null || isUserChanged != null) { String allowedItems = approved == null ? "" : String.join(ENABLED_SERVICES_SEPARATOR, approved); @@ -503,7 +513,9 @@ abstract public class ManagedServices { out.attribute(null, ATT_APPROVED_LIST, allowedItems); out.attributeInt(null, ATT_USER_ID, approvedUserId); out.attributeBoolean(null, ATT_IS_PRIMARY, isPrimary); - if (userSet != null) { + if (isUserChanged != null) { + out.attributeBoolean(null, ATT_USER_CHANGED, isUserChanged); + } else if (userSet != null) { String userSetItems = String.join(ENABLED_SERVICES_SEPARATOR, userSet); out.attribute(null, ATT_USER_SET, userSetItems); @@ -618,12 +630,21 @@ abstract public class ManagedServices { ? userId : parser.getAttributeInt(null, ATT_USER_ID, 0); final boolean isPrimary = parser.getAttributeBoolean(null, ATT_IS_PRIMARY, true); - final String userSet = XmlUtils.readStringAttribute(parser, ATT_USER_SET); + + final String isUserChanged = XmlUtils.readStringAttribute(parser, + ATT_USER_CHANGED); + String userSetComponent = null; + if (isUserChanged == null) { + userSetComponent = XmlUtils.readStringAttribute(parser, ATT_USER_SET); + } else { + mIsUserChanged.put(resolvedUserId, Boolean.valueOf(isUserChanged)); + } readExtraAttributes(tag, parser, resolvedUserId); if (allowedManagedServicePackages == null || allowedManagedServicePackages.test( - getPackageName(approved), resolvedUserId, getRequiredPermission())) { + getPackageName(approved), resolvedUserId, getRequiredPermission()) + || approved.isEmpty()) { if (mUm.getUserInfo(resolvedUserId) != null) { - addApprovedList(approved, resolvedUserId, isPrimary, userSet); + addApprovedList(approved, resolvedUserId, isPrimary, userSetComponent); } mUseXml = true; } @@ -634,10 +655,16 @@ abstract public class ManagedServices { } boolean isOldVersion = TextUtils.isEmpty(version) || DB_VERSION_1.equals(version) - || DB_VERSION_2.equals(version); + || DB_VERSION_2.equals(version) + || DB_VERSION_3.equals(version); + boolean needUpgradeUserset = DB_VERSION_3.equals(version); if (isOldVersion) { upgradeDefaultsXmlVersion(); } + if (needUpgradeUserset) { + upgradeUserSet(); + } + rebindServices(false, USER_ALL); } @@ -666,6 +693,8 @@ abstract public class ManagedServices { } } + protected void upgradeUserSet() {}; + /** * Read extra attributes in the {@link #TAG_MANAGED_SERVICES} tag. */ diff --git a/services/core/java/com/android/server/notification/NotificationManagerService.java b/services/core/java/com/android/server/notification/NotificationManagerService.java index 08a7d9e38d968..34abb39885ba5 100755 --- a/services/core/java/com/android/server/notification/NotificationManagerService.java +++ b/services/core/java/com/android/server/notification/NotificationManagerService.java @@ -106,7 +106,6 @@ import static android.service.notification.NotificationListenerService.TRIM_FULL import static android.service.notification.NotificationListenerService.TRIM_LIGHT; import static android.view.WindowManager.LayoutParams.TYPE_TOAST; -import static com.android.internal.util.CollectionUtils.emptyIfNull; import static com.android.internal.util.FrameworkStatsLog.DND_MODE_RULE; import static com.android.internal.util.FrameworkStatsLog.PACKAGE_NOTIFICATION_CHANNEL_GROUP_PREFERENCES; import static com.android.internal.util.FrameworkStatsLog.PACKAGE_NOTIFICATION_CHANNEL_PREFERENCES; @@ -324,7 +323,6 @@ import java.util.HashSet; import java.util.Iterator; import java.util.LinkedList; import java.util.List; -import java.util.Map; import java.util.Map.Entry; import java.util.Objects; import java.util.Set; @@ -9428,6 +9426,14 @@ public class NotificationManagerService extends SystemService { return mDefaultFromConfig; } + @Override + protected void upgradeUserSet() { + for (int userId: mApproved.keySet()) { + ArraySet userSetServices = mUserSetServices.get(userId); + mIsUserChanged.put(userId, (userSetServices != null && userSetServices.size() > 0)); + } + } + public NotificationAssistants(Context context, Object lock, UserProfiles up, IPackageManager pm) { super(context, lock, up, pm); @@ -9602,47 +9608,12 @@ public class NotificationManagerService extends SystemService { } boolean hasUserSet(int userId) { - synchronized (mLock) { - ArraySet userSetServices = mUserSetServices.get(userId); - if (userSetServices == null) { - // Legacy case - no data means user-set, unless no assistant is set - return !mApproved.isEmpty(); - } - Map> approvedByType = emptyIfNull(mApproved.get(userId)); - return userSetServices.containsAll(emptyIfNull(approvedByType.get(true))) - && userSetServices.containsAll(emptyIfNull(approvedByType.get(false))); - } + Boolean userSet = mIsUserChanged.get(userId); + return (userSet != null && userSet); } void setUserSet(int userId, boolean set) { - synchronized (mLock) { - ArraySet userSetServices = new ArraySet<>(); - if (set) { - ArrayMap> approvedByType = mApproved.get(userId); - if (approvedByType != null) { - for (int i = 0; i < approvedByType.size(); i++) { - userSetServices.addAll(approvedByType.valueAt(i)); - } - } - } - mUserSetServices.put(userId, userSetServices); - } - } - - @Override - protected void readExtraAttributes(String tag, TypedXmlPullParser parser, int userId) - throws IOException { - // TODO: this logic looks broken, since it's trying to convert a - // list into a boolean; for now we preserve the old parsing behavior - // to avoid a performance regression, but someone should investigate - final String value = parser.getAttributeValue(null, ATT_USER_SET); - final boolean userSet; - if (TextUtils.isEmpty(value)) { - userSet = false; - } else { - userSet = Boolean.parseBoolean(value); - } - setUserSet(userId, userSet); + mIsUserChanged.put(userId, set); } private void notifyCapabilitiesChanged(final ManagedServiceInfo info) { diff --git a/services/tests/uiservicestests/src/com/android/server/notification/ManagedServicesTest.java b/services/tests/uiservicestests/src/com/android/server/notification/ManagedServicesTest.java index c502ed5d5984d..825e53e15ea85 100644 --- a/services/tests/uiservicestests/src/com/android/server/notification/ManagedServicesTest.java +++ b/services/tests/uiservicestests/src/com/android/server/notification/ManagedServicesTest.java @@ -100,6 +100,7 @@ public class ManagedServicesTest extends UiServiceTestCase { private String mVersionString; private final Set mDefaults = new ArraySet(); private ManagedServices mService; + private String mUserSetString; private static final String SETTING = "setting"; private static final String SECONDARY_SETTING = "secondary_setting"; @@ -138,7 +139,7 @@ public class ManagedServicesTest extends UiServiceTestCase { profileIds.add(13); when(mUserProfiles.getCurrentProfileIds()).thenReturn(profileIds); - mVersionString = "2"; + mVersionString = "4"; mExpectedPrimary = new ArrayMap<>(); mExpectedSecondary = new ArrayMap<>(); mExpectedPrimaryPackages = new ArrayMap<>(); @@ -1363,6 +1364,7 @@ public class ManagedServicesTest extends UiServiceTestCase { public void loadDefaults_noVersionWithDefaults() throws Exception { resetComponentsAndPackages(); mDefaults.add(new ComponentName("default", "class")); + mVersionString = null; loadXml(mService); assertEquals(mService.getDefaultComponents(), mDefaults); } @@ -1421,12 +1423,34 @@ public class ManagedServicesTest extends UiServiceTestCase { resetComponentsAndPackages(); mDefaults.add(new ComponentName("default", "class")); mDefaultsString = "xml/class"; - mVersionString = String.valueOf(mService.DB_VERSION); loadXml(mService); assertEquals(mService.getDefaultComponents(), new ArraySet(Arrays.asList(new ComponentName("xml", "class")))); } + @Test + public void upgradeUserSet_versionThree() throws Exception { + resetComponentsAndPackages(); + + List users = new ArrayList<>(); + users.add(new UserInfo(98, "98", 0)); + users.add(new UserInfo(99, "99", 0)); + for (UserInfo user : users) { + when(mUm.getUserInfo(eq(user.id))).thenReturn(user); + } + + mDefaultsString = "xml/class"; + mVersionString = "3"; + mUserSetString = "xml/class"; + loadXml(mService); + + //Test services without overriding upgradeUserSet() remain unchanged + assertEquals(new ArraySet(Arrays.asList(mUserSetString)), + mService.mUserSetServices.get(98)); + assertEquals(new ArraySet(Arrays.asList(mUserSetString)), + mService.mUserSetServices.get(99)); + assertEquals(new ArrayMap(), mService.mIsUserChanged); + } private void resetComponentsAndPackages() { ArrayMap> empty = new ArrayMap(1); @@ -1468,11 +1492,17 @@ public class ManagedServicesTest extends UiServiceTestCase { xml.append("<" + ManagedServices.TAG_MANAGED_SERVICES + " " + ManagedServices.ATT_USER_ID + "=\"99\" " + ManagedServices.ATT_IS_PRIMARY + "=\"true\" " - + ManagedServices.ATT_APPROVED_LIST + "=\"990\" />\n"); + + ManagedServices.ATT_APPROVED_LIST + "=\"990\" " + + (mUserSetString != null ? ManagedServices.ATT_USER_SET + "=\"" + + mUserSetString + "\" " : "") + + "/>\n"); xml.append("<" + ManagedServices.TAG_MANAGED_SERVICES + " " + ManagedServices.ATT_USER_ID + "=\"98\" " + ManagedServices.ATT_IS_PRIMARY + "=\"false\" " - + ManagedServices.ATT_APPROVED_LIST + "=\"981\" />\n"); + + ManagedServices.ATT_APPROVED_LIST + "=\"981\" " + + (mUserSetString != null ? ManagedServices.ATT_USER_SET + "=\"" + + mUserSetString + "\" " : "") + + " />\n"); xml.append(""); return xml.toString(); diff --git a/services/tests/uiservicestests/src/com/android/server/notification/NotificationAssistantsTest.java b/services/tests/uiservicestests/src/com/android/server/notification/NotificationAssistantsTest.java index 6722fff80d0bb..feb924ce6cfa2 100644 --- a/services/tests/uiservicestests/src/com/android/server/notification/NotificationAssistantsTest.java +++ b/services/tests/uiservicestests/src/com/android/server/notification/NotificationAssistantsTest.java @@ -16,6 +16,7 @@ package com.android.server.notification; import static junit.framework.Assert.assertEquals; +import static junit.framework.Assert.assertNotNull; import static junit.framework.Assert.assertTrue; import static org.junit.Assert.assertNull; @@ -41,11 +42,13 @@ import android.content.pm.UserInfo; import android.os.UserHandle; import android.os.UserManager; import android.testing.TestableContext; +import android.util.ArrayMap; import android.util.ArraySet; import android.util.IntArray; import android.util.TypedXmlPullParser; import android.util.Xml; +import com.android.internal.util.function.TriPredicate; import com.android.server.UiServiceTestCase; import com.android.server.notification.NotificationManagerService.NotificationAssistants; @@ -131,6 +134,49 @@ public class NotificationAssistantsTest extends UiServiceTestCase { verify(mNm, times(mUm.getUsers().size())).setDefaultAssistantForUser(anyInt()); } + @Test + public void testReadXml_userDisabled() throws Exception { + String xml = "" + + "" + + ""; + + final TypedXmlPullParser parser = Xml.newFastPullParser(); + parser.setInput(new BufferedInputStream( + new ByteArrayInputStream(xml.toString().getBytes())), null); + TriPredicate allowedManagedServicePackages = + mNm::canUseManagedServices; + + parser.nextTag(); + mAssistants.readXml(parser, allowedManagedServicePackages, false, UserHandle.USER_ALL); + + ArrayMap> approved = mAssistants.mApproved.get(0); + + // approved should not be null + assertNotNull(approved); + assertEquals(new ArraySet<>(), approved.get(true)); + } + + @Test + public void testReadXml_upgradeUserSet() throws Exception { + String xml = "" + + "" + + ""; + + final TypedXmlPullParser parser = Xml.newFastPullParser(); + parser.setInput(new BufferedInputStream( + new ByteArrayInputStream(xml.toString().getBytes())), null); + TriPredicate allowedManagedServicePackages = + mNm::canUseManagedServices; + + parser.nextTag(); + mAssistants.readXml(parser, allowedManagedServicePackages, false, UserHandle.USER_ALL); + + verify(mAssistants, times(1)).upgradeUserSet(); + assertTrue(mAssistants.mIsUserChanged.get(0)); + } + @Test public void testXmlUpgradeExistingApprovedComponents() throws Exception { String xml = ""