Merge "Be more strict about triggering notification lights" into pi-dev

am: f00bfbe46e

Change-Id: I6c999a970a309f7888d6157c7a17f5d0ca05e32c

2 files changed, 236 insertions, 10 deletions

diff --git a/services/core/java/com/android/server/notification/NotificationManagerService.java b/services/core/java/com/android/server/notification/NotificationManagerService.java
index cafc6fce6901..c1112f7ec2c9 100644
--- a/services/core/java/com/android/server/notification/NotificationManagerService.java
+++ b/services/core/java/com/android/server/notification/NotificationManagerService.java
@@ -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;
@@ -1195,7 +1197,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();
@@ -1457,6 +1460,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.
@@ -3828,6 +3833,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);
@@ -4825,8 +4831,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) {
@@ -4837,7 +4842,19 @@ public class NotificationManagerService extends SystemService {
             updateLightsLocked();
         }
         if (buzz || beep || blink) {
-            record.setInterruptive(true);
+            // 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)
@@ -4847,11 +4864,49 @@ 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;
         }
 
diff --git a/services/tests/uiservicestests/src/com/android/server/notification/BuzzBeepBlinkTest.java b/services/tests/uiservicestests/src/com/android/server/notification/BuzzBeepBlinkTest.java
index 78099996a1a0..bdba3d5cd677 100644
--- a/services/tests/uiservicestests/src/com/android/server/notification/BuzzBeepBlinkTest.java
+++ b/services/tests/uiservicestests/src/com/android/server/notification/BuzzBeepBlinkTest.java
@@ -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;