From 72e6f5843445efaf4f8da968c93acaaf19b2f17d Mon Sep 17 00:00:00 2001 From: Yuri Lin Date: Tue, 7 Dec 2021 17:47:49 -0500 Subject: [PATCH] Set user_set_importance field in pullPackagePreferences. Bug: 194833441 Bug: 209838571 Test: PreferencesHelperTest Change-Id: I645b4875ff73acaefed45b7c9ff8a51dbd59ca46 --- .../notification/PreferencesHelper.java | 12 +++++++++- .../notification/PreferencesHelperTest.java | 24 ++++++++++++------- 2 files changed, 27 insertions(+), 9 deletions(-) diff --git a/services/core/java/com/android/server/notification/PreferencesHelper.java b/services/core/java/com/android/server/notification/PreferencesHelper.java index 7d4877cdc7ee2..258ae8c1c8496 100644 --- a/services/core/java/com/android/server/notification/PreferencesHelper.java +++ b/services/core/java/com/android/server/notification/PreferencesHelper.java @@ -2151,6 +2151,10 @@ public class PreferencesHelper implements RankingConfig { final PackagePreferences r = mPackagePreferences.valueAt(i); event.writeInt(r.uid); event.addBooleanAnnotation(ANNOTATION_ID_IS_UID, true); + + // collect whether this package's importance info was user-set for later, if needed + // before the migration is enabled, this will simply default to false in all cases. + boolean importanceIsUserSet = false; if (mPermissionHelper.isMigrationEnabled()) { // Even if this package's data is not present, we need to write something; // so default to IMPORTANCE_NONE, since if PM doesn't know about the package @@ -2158,8 +2162,12 @@ public class PreferencesHelper implements RankingConfig { int importance = IMPORTANCE_NONE; Pair key = new Pair<>(r.uid, r.pkg); if (pkgPermissions != null && pkgsWithPermissionsToHandle.contains(key)) { - importance = pkgPermissions.get(key).first + Pair permissionPair = pkgPermissions.get(key); + importance = permissionPair.first ? IMPORTANCE_DEFAULT : IMPORTANCE_NONE; + // cache the second value for writing later + importanceIsUserSet = permissionPair.second; + pkgsWithPermissionsToHandle.remove(key); } event.writeInt(importance); @@ -2168,6 +2176,7 @@ public class PreferencesHelper implements RankingConfig { } event.writeInt(r.visibility); event.writeInt(r.lockedAppFields); + event.writeBoolean(importanceIsUserSet); // optional bool user_set_importance = 5; events.add(event.build()); } } @@ -2189,6 +2198,7 @@ public class PreferencesHelper implements RankingConfig { // builder event.writeInt(DEFAULT_VISIBILITY); event.writeInt(DEFAULT_LOCKED_APP_FIELDS); + event.writeBoolean(pkgPermissions.get(p).second); // user_set_importance field events.add(event.build()); } } 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 e8a279907c5e5..d49cf670f471f 100644 --- a/services/tests/uiservicestests/src/com/android/server/notification/PreferencesHelperTest.java +++ b/services/tests/uiservicestests/src/com/android/server/notification/PreferencesHelperTest.java @@ -5112,6 +5112,11 @@ public class PreferencesHelperTest extends UiServiceTestCase { assertTrue(expected.containsKey(uid)); assertThat(expected.get(uid)).isEqualTo( builder.getInt(PackageNotificationPreferences.IMPORTANCE_FIELD_NUMBER)); + + // pre-migration, the userSet field will always default to false + boolean userSet = builder.getBoolean( + PackageNotificationPreferences.USER_SET_IMPORTANCE_FIELD_NUMBER); + assertFalse(userSet); } } } @@ -5123,8 +5128,8 @@ public class PreferencesHelperTest extends UiServiceTestCase { // build a collection of app permissions that should be passed in but ignored ArrayMap, Pair> appPermissions = new ArrayMap<>(); appPermissions.put(new Pair(1, "first"), new Pair(true, false)); // not in local prefs - appPermissions.put(new Pair(3, "third"), new Pair(false, false)); // not in local prefs - appPermissions.put(new Pair(UID_O, PKG_O), new Pair(false, false)); // in local prefs + appPermissions.put(new Pair(3, "third"), new Pair(false, true)); // not in local prefs + appPermissions.put(new Pair(UID_O, PKG_O), new Pair(false, true)); // in local prefs // package preferences: PKG_O not banned based on local importance, and PKG_P is mHelper.setImportance(PKG_O, UID_O, IMPORTANCE_HIGH); @@ -5132,11 +5137,11 @@ public class PreferencesHelperTest extends UiServiceTestCase { // expected output. format: uid -> importance, as only uid (and not package name) // is in PackageNotificationPreferences - ArrayMap expected = new ArrayMap<>(); - expected.put(1, IMPORTANCE_DEFAULT); - expected.put(3, IMPORTANCE_NONE); - expected.put(UID_O, IMPORTANCE_NONE); // banned by permissions - expected.put(UID_P, IMPORTANCE_NONE); // defaults to none + ArrayMap> expected = new ArrayMap<>(); + expected.put(1, new Pair(IMPORTANCE_DEFAULT, false)); + expected.put(3, new Pair(IMPORTANCE_NONE, true)); + expected.put(UID_O, new Pair(IMPORTANCE_NONE, true)); // banned by permissions + expected.put(UID_P, new Pair(IMPORTANCE_NONE, false)); // defaults to none, false ArrayList events = new ArrayList<>(); mHelper.pullPackagePreferencesStats(events, appPermissions); @@ -5144,11 +5149,14 @@ public class PreferencesHelperTest extends UiServiceTestCase { for (WrappedSysUiStatsEvent.WrappedBuilder builder : mStatsEventBuilderFactory.builders) { if (builder.getAtomId() == PACKAGE_NOTIFICATION_PREFERENCES) { int uid = builder.getInt(PackageNotificationPreferences.UID_FIELD_NUMBER); + boolean userSet = builder.getBoolean( + PackageNotificationPreferences.USER_SET_IMPORTANCE_FIELD_NUMBER); // if it's one of the expected ids, then make sure the importance matches assertTrue(expected.containsKey(uid)); - assertThat(expected.get(uid)).isEqualTo( + assertThat(expected.get(uid).first).isEqualTo( builder.getInt(PackageNotificationPreferences.IMPORTANCE_FIELD_NUMBER)); + assertThat(expected.get(uid).second).isEqualTo(userSet); } } }