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

Commit 7e455036 authored by Yuri Lin's avatar Yuri Lin
Browse files

Log unknown package importance as IMPORTANCE_UNSPECIFIED 

In the case of packages where for some reason we have package preferences but not any recorded package permissions, log it as such to distinguish from ones that are actually banned by the system (IMPORTANCE_NONE).

This change might help debug cases where packages appear to be banned but we're not sure why.

Bug: 275389727
Test: atest PreferencesHelperTest
Change-Id: Ie22efda4300f63e5f48e08d3148515f04c62643b
parent 5b12bf18

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

+4 −3
Original line number Diff line number Diff line
@@ -2045,9 +2045,10 @@ public class PreferencesHelper implements RankingConfig { 
                // before the migration is enabled, this will simply default to false in all cases.

                boolean importanceIsUserSet = false;

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

                // for some reason, notifications are not allowed.

                int importance = IMPORTANCE_NONE;

                // default to IMPORTANCE_UNSPECIFIED. If PM doesn't know about the package

                // for some reason, notifications are not allowed, but in logged output we want

                // to distinguish this case from the actually-banned packages.

                int importance = IMPORTANCE_UNSPECIFIED;

                Pair<Integer, String> key = new Pair<>(r.uid, r.pkg);

                if (pkgPermissions != null && pkgsWithPermissionsToHandle.contains(key)) {

                    Pair<Boolean, Boolean> permissionPair = pkgPermissions.get(key);

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

+16 −7
Original line number Diff line number Diff line
@@ -4967,11 +4967,17 @@ public class PreferencesHelperTest extends UiServiceTestCase { 


    @Test

    public void testPullPackagePreferencesStats_postPermissionMigration() {

        // make sure there's at least one channel for each package we want to test

        NotificationChannel channelA = new NotificationChannel("a", "a", IMPORTANCE_DEFAULT);

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

        NotificationChannel channelB = new NotificationChannel("b", "b", IMPORTANCE_DEFAULT);

        mHelper.createNotificationChannel(PKG_O, UID_O, channelB, true, false);

        NotificationChannel channelC = new NotificationChannel("c", "c", IMPORTANCE_DEFAULT);

        mHelper.createNotificationChannel(PKG_P, UID_P, channelC, true, false);



        // build a collection of app permissions that should be passed in but ignored

        // build a collection of app permissions that should be passed in and used

        ArrayMap<Pair<Integer, String>, Pair<Boolean, Boolean>> 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, true)); // not in local prefs

        appPermissions.put(new Pair<>(UID_N_MR1, PKG_N_MR1), new Pair<>(true, false));

        appPermissions.put(new Pair<>(UID_O, PKG_O), new Pair<>(false, true)); // in local prefs



        // local preferences
@@ -4981,16 +4987,17 @@ public class PreferencesHelperTest extends UiServiceTestCase { 
        // expected output. format: uid -> importance, as only uid (and not package name)

        // is in PackageNotificationPreferences

        ArrayMap<Integer, Pair<Integer, Boolean>> expected = new ArrayMap<>();

        expected.put(1, new Pair<>(IMPORTANCE_DEFAULT, false));

        expected.put(3, new Pair<>(IMPORTANCE_NONE, true));

        expected.put(UID_N_MR1, new Pair<>(IMPORTANCE_DEFAULT, false));

        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

        expected.put(UID_P, new Pair<>(IMPORTANCE_UNSPECIFIED, false)); // default: unspecified



        ArrayList<StatsEvent> events = new ArrayList<>();

        mHelper.pullPackagePreferencesStats(events, appPermissions);



        int found = 0;

        for (WrappedSysUiStatsEvent.WrappedBuilder builder : mStatsEventBuilderFactory.builders) {

            if (builder.getAtomId() == PACKAGE_NOTIFICATION_PREFERENCES) {

                ++found;

                int uid = builder.getInt(PackageNotificationPreferences.UID_FIELD_NUMBER);

                boolean userSet = builder.getBoolean(

                        PackageNotificationPreferences.USER_SET_IMPORTANCE_FIELD_NUMBER);
@@ -5002,6 +5009,8 @@ public class PreferencesHelperTest extends UiServiceTestCase { 
                assertThat(expected.get(uid).second).isEqualTo(userSet);

            }

        }

        // should have at least one entry for each of the packages we expected to see

        assertThat(found).isAtLeast(3);

    }



    @Test