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

Commit 54369237 authored by Julia Reynolds's avatar Julia Reynolds
Browse files

Be more strict about triggering notification lights

Don't trigger lights if they should be suppressed by notification flags.

Don't log that notification lights happened if global or temporary
settings have turned them off

Test: runtest systemui-notification, verify
notification_itnerruptiveness numbers for a fresh 3p app that uses
lights
Bug: 111069748

Change-Id: I68750bf39eb76ef0bda40ddacfd90e1be79e8575
(cherry picked from commit 28149f65)
parent 5c75b5b6
Loading
Loading
Loading
Loading
+63 −8
Original line number Diff line number Diff line
@@ -329,6 +329,8 @@ public class NotificationManagerService extends SystemService {

    private long[] mFallbackVibrationPattern;
    private boolean mUseAttentionLight;
    boolean mHasLight = true;
    boolean mLightEnabled;
    boolean mSystemReady;

    private boolean mDisableNotificationEffects;
@@ -343,9 +345,9 @@ public class NotificationManagerService extends SystemService {
    private int mInterruptionFilter = NotificationListenerService.INTERRUPTION_FILTER_UNKNOWN;

    // for enabling and disabling notification pulse behavior
    private boolean mScreenOn = true;
    boolean mScreenOn = true;
    protected boolean mInCall = false;
    private boolean mNotificationPulseEnabled;
    boolean mNotificationPulseEnabled;

    private Uri mInCallNotificationUri;
    private AudioAttributes mInCallNotificationAudioAttributes;
@@ -1198,7 +1200,8 @@ public class NotificationManagerService extends SystemService {
            ContentResolver resolver = getContext().getContentResolver();
            if (uri == null || NOTIFICATION_LIGHT_PULSE_URI.equals(uri)) {
                boolean pulseEnabled = Settings.System.getIntForUser(resolver,
                            Settings.System.NOTIFICATION_LIGHT_PULSE, 0, UserHandle.USER_CURRENT) != 0;
                            Settings.System.NOTIFICATION_LIGHT_PULSE, 0, UserHandle.USER_CURRENT)
                        != 0;
                if (mNotificationPulseEnabled != pulseEnabled) {
                    mNotificationPulseEnabled = pulseEnabled;
                    updateNotificationPulse();
@@ -1460,6 +1463,8 @@ public class NotificationManagerService extends SystemService {
        mInCallNotificationVolume = resources.getFloat(R.dimen.config_inCallNotificationVolume);

        mUseAttentionLight = resources.getBoolean(R.bool.config_useAttentionLight);
        mHasLight =
                resources.getBoolean(com.android.internal.R.bool.config_intrusiveNotificationLed);

        // Don't start allowing notifications until the setup wizard has run once.
        // After that, including subsequent boots, init with notifications turned on.
@@ -3825,6 +3830,7 @@ public class NotificationManagerService extends SystemService {
                        pw.println("  ");
                    }
                    pw.println("  mUseAttentionLight=" + mUseAttentionLight);
                    pw.println("  mHasLight=" + mHasLight);
                    pw.println("  mNotificationPulseEnabled=" + mNotificationPulseEnabled);
                    pw.println("  mSoundNotificationKey=" + mSoundNotificationKey);
                    pw.println("  mVibrateNotificationKey=" + mVibrateNotificationKey);
@@ -4822,8 +4828,7 @@ public class NotificationManagerService extends SystemService {
        // light
        // release the light
        boolean wasShowLights = mLights.remove(key);
        if (record.getLight() != null && aboveThreshold
                && ((record.getSuppressedVisualEffects() & SUPPRESSED_EFFECT_LIGHTS) == 0)) {
        if (canShowLightsLocked(record, aboveThreshold)) {
            mLights.add(key);
            updateLightsLocked();
            if (mUseAttentionLight) {
@@ -4834,7 +4839,19 @@ public class NotificationManagerService extends SystemService {
            updateLightsLocked();
        }
        if (buzz || beep || blink) {
            // Ignore summary updates because we don't display most of the information.
            if (record.sbn.isGroup() && record.sbn.getNotification().isGroupSummary()) {
                if (DEBUG_INTERRUPTIVENESS) {
                    Log.v(TAG, "INTERRUPTIVENESS: "
                            + record.getKey() + " is not interruptive: summary");
                }
            } else {
                if (DEBUG_INTERRUPTIVENESS) {
                    Log.v(TAG, "INTERRUPTIVENESS: "
                            + record.getKey() + " is interruptive: alerted");
                }
                record.setInterruptive(true);
            }
            MetricsLogger.action(record.getLogMaker()
                    .setCategory(MetricsEvent.NOTIFICATION_ALERT)
                    .setType(MetricsEvent.TYPE_OPEN)
@@ -4843,12 +4860,50 @@ public class NotificationManagerService extends SystemService {
        }
    }

    @GuardedBy("mNotificationLock")
    boolean canShowLightsLocked(final NotificationRecord record, boolean aboveThreshold) {
        // device lacks light
        if (!mHasLight) {
            return false;
        }
        // user turned lights off globally
        if (!mNotificationPulseEnabled) {
            return false;
        }
        // the notification/channel has no light
        if (record.getLight() == null) {
            return false;
        }
        // unimportant notification
        if (!aboveThreshold) {
            return false;
        }
        // suppressed due to DND
        if ((record.getSuppressedVisualEffects() & SUPPRESSED_EFFECT_LIGHTS) != 0) {
            return false;
        }
        // Suppressed because it's a silent update
        final Notification notification = record.getNotification();
        if (record.isUpdate && (notification.flags & Notification.FLAG_ONLY_ALERT_ONCE) != 0) {
            return false;
        }
        // Suppressed because another notification in its group handles alerting
        if (record.sbn.isGroup() && record.getNotification().suppressAlertingDueToGrouping()) {
            return false;
        }
        // not if in call or the screen's on
        if (mInCall || mScreenOn) {
            return false;
        }

        return true;
    }

    @GuardedBy("mNotificationLock")
    boolean shouldMuteNotificationLocked(final NotificationRecord record) {
        // Suppressed because it's a silent update
        final Notification notification = record.getNotification();
        if(record.isUpdate
                && (notification.flags & Notification.FLAG_ONLY_ALERT_ONCE) != 0) {
        if (record.isUpdate && (notification.flags & Notification.FLAG_ONLY_ALERT_ONCE) != 0) {
            return true;
        }

+173 −2
Original line number Diff line number Diff line
@@ -19,7 +19,9 @@ import static android.app.Notification.GROUP_ALERT_ALL;
import static android.app.Notification.GROUP_ALERT_CHILDREN;
import static android.app.Notification.GROUP_ALERT_SUMMARY;
import static android.app.NotificationManager.IMPORTANCE_HIGH;
import static android.app.NotificationManager.IMPORTANCE_LOW;
import static android.app.NotificationManager.IMPORTANCE_MIN;
import static android.app.NotificationManager.Policy.SUPPRESSED_EFFECT_LIGHTS;

import static junit.framework.Assert.assertFalse;
import static junit.framework.Assert.assertNull;
@@ -149,6 +151,9 @@ public class BuzzBeepBlinkTest extends UiServiceTestCase {
        mService.setFallbackVibrationPattern(FALLBACK_VIBRATION_PATTERN);
        mService.setUsageStats(mUsageStats);
        mService.setAccessibilityManager(accessibilityManager);
        mService.mScreenOn = false;
        mService.mInCall = false;
        mService.mNotificationPulseEnabled = true;
    }

    //
@@ -216,8 +221,13 @@ public class BuzzBeepBlinkTest extends UiServiceTestCase {
    }

    private NotificationRecord getLightsNotification() {
        return getNotificationRecord(mId, false /* insistent */, false /* once */,
                false /* noisy */, false /* buzzy*/, true /* lights */);
    }

    private NotificationRecord getLightsOnceNotification() {
        return getNotificationRecord(mId, false /* insistent */, true /* once */,
                false /* noisy */, true /* buzzy*/, true /* lights */);
                false /* noisy */, false /* buzzy*/, true /* lights */);
    }

    private NotificationRecord getCustomLightsNotification() {
@@ -244,6 +254,12 @@ public class BuzzBeepBlinkTest extends UiServiceTestCase {
                groupKey, groupAlertBehavior, false);
    }

    private NotificationRecord getLightsNotificationRecord(String groupKey,
            int groupAlertBehavior) {
        return getNotificationRecord(mId, false, false, false, false, true /*lights*/, true, true,
                true, groupKey, groupAlertBehavior, false);
    }

    private NotificationRecord getNotificationRecord(int id, boolean insistent, boolean once,
            boolean noisy, boolean buzzy, boolean lights, boolean defaultVibration,
            boolean defaultSound, boolean defaultLights, String groupKey, int groupAlertBehavior,
@@ -369,6 +385,10 @@ public class BuzzBeepBlinkTest extends UiServiceTestCase {
        verify(mVibrator, never()).cancel();
    }

    private void verifyNeverLights() {
        verify(mLight, never()).setFlashing(anyInt(), anyInt(), anyInt(), anyInt());
    }

    private void verifyLights() {
        verify(mLight, times(1)).setFlashing(anyInt(), anyInt(), anyInt(), anyInt());
    }
@@ -712,7 +732,8 @@ public class BuzzBeepBlinkTest extends UiServiceTestCase {
        mService.buzzBeepBlinkLocked(summary);

        verifyBeepLooped();
        assertTrue(summary.isInterruptive());
        // summaries are never interruptive for notification counts
        assertFalse(summary.isInterruptive());
    }

    @Test
@@ -990,6 +1011,156 @@ public class BuzzBeepBlinkTest extends UiServiceTestCase {
        verify(mAccessibilityService, times(1)).sendAccessibilityEvent(any(), anyInt());
    }

    @Test
    public void testLightsScreenOn() {
        mService.mScreenOn = true;
        NotificationRecord r = getLightsNotification();
        mService.buzzBeepBlinkLocked(r);
        verifyNeverLights();
        assertFalse(r.isInterruptive());
    }

    @Test
    public void testLightsInCall() {
        mService.mInCall = true;
        NotificationRecord r = getLightsNotification();
        mService.buzzBeepBlinkLocked(r);
        verifyNeverLights();
        assertFalse(r.isInterruptive());
    }

    @Test
    public void testLightsSilentUpdate() {
        NotificationRecord r = getLightsOnceNotification();
        mService.buzzBeepBlinkLocked(r);
        verifyLights();
        assertTrue(r.isInterruptive());

        r = getLightsOnceNotification();
        r.isUpdate = true;
        mService.buzzBeepBlinkLocked(r);
        // checks that lights happened once, i.e. this new call didn't trigger them again
        verifyLights();
        assertFalse(r.isInterruptive());
    }

    @Test
    public void testLightsUnimportant() {
        NotificationRecord r = getLightsNotification();
        r.setImportance(IMPORTANCE_LOW, "testing");
        mService.buzzBeepBlinkLocked(r);
        verifyNeverLights();
        assertFalse(r.isInterruptive());
    }

    @Test
    public void testLightsNoLights() {
        NotificationRecord r = getQuietNotification();
        mService.buzzBeepBlinkLocked(r);
        verifyNeverLights();
        assertFalse(r.isInterruptive());
    }

    @Test
    public void testLightsNoLightOnDevice() {
        mService.mHasLight = false;
        NotificationRecord r = getLightsNotification();
        mService.buzzBeepBlinkLocked(r);
        verifyNeverLights();
        assertFalse(r.isInterruptive());
    }

    @Test
    public void testLightsLightsOffGlobally() {
        mService.mNotificationPulseEnabled = false;
        NotificationRecord r = getLightsNotification();
        mService.buzzBeepBlinkLocked(r);
        verifyNeverLights();
        assertFalse(r.isInterruptive());
    }

    @Test
    public void testLightsDndIntercepted() {
        NotificationRecord r = getLightsNotification();
        r.setSuppressedVisualEffects(SUPPRESSED_EFFECT_LIGHTS);
        mService.buzzBeepBlinkLocked(r);
        verifyNeverLights();
        assertFalse(r.isInterruptive());
    }

    @Test
    public void testGroupAlertSummaryNoLightsChild() {
        NotificationRecord child = getLightsNotificationRecord("a", GROUP_ALERT_SUMMARY);

        mService.buzzBeepBlinkLocked(child);

        verifyNeverLights();
        assertFalse(child.isInterruptive());
    }

    @Test
    public void testGroupAlertSummaryLightsSummary() {
        NotificationRecord summary = getLightsNotificationRecord("a", GROUP_ALERT_SUMMARY);
        summary.getNotification().flags |= Notification.FLAG_GROUP_SUMMARY;

        mService.buzzBeepBlinkLocked(summary);

        verifyLights();
        // summaries should never count for interruptiveness counts
        assertFalse(summary.isInterruptive());
    }

    @Test
    public void testGroupAlertSummaryLightsNonGroupChild() {
        NotificationRecord nonGroup = getLightsNotificationRecord(null, GROUP_ALERT_SUMMARY);

        mService.buzzBeepBlinkLocked(nonGroup);

        verifyLights();
        assertTrue(nonGroup.isInterruptive());
    }

    @Test
    public void testGroupAlertChildNoLightsSummary() {
        NotificationRecord summary = getLightsNotificationRecord("a", GROUP_ALERT_CHILDREN);
        summary.getNotification().flags |= Notification.FLAG_GROUP_SUMMARY;

        mService.buzzBeepBlinkLocked(summary);

        verifyNeverLights();
        assertFalse(summary.isInterruptive());
    }

    @Test
    public void testGroupAlertChildLightsChild() {
        NotificationRecord child = getLightsNotificationRecord("a", GROUP_ALERT_CHILDREN);

        mService.buzzBeepBlinkLocked(child);

        verifyLights();
        assertTrue(child.isInterruptive());
    }

    @Test
    public void testGroupAlertChildLightsNonGroupSummary() {
        NotificationRecord nonGroup = getLightsNotificationRecord(null, GROUP_ALERT_CHILDREN);

        mService.buzzBeepBlinkLocked(nonGroup);

        verifyLights();
        assertTrue(nonGroup.isInterruptive());
    }

    @Test
    public void testGroupAlertAllLightsGroup() {
        NotificationRecord group = getLightsNotificationRecord("a", GROUP_ALERT_ALL);

        mService.buzzBeepBlinkLocked(group);

        verifyLights();
        assertTrue(group.isInterruptive());
    }

    static class VibrateRepeatMatcher implements ArgumentMatcher<VibrationEffect> {
        private final int mRepeatIndex;