Change notification interruption calculation

Updates that change notification text will only be counted if the
user sees the update, so apps that are silently keeping their
notification data fresh will not be punished.

Test: runtest systemui-notification
Change-Id: I3d494417e92296ad9a1742db2ab949132ebac18f
Fixes: 78643290

diff --git a/services/core/java/com/android/server/notification/NotificationManagerService.java b/services/core/java/com/android/server/notification/NotificationManagerService.java
index e2e6f6d..8f7fa1b 100644 (file)
--- a/services/core/java/com/android/server/notification/NotificationManagerService.java
+++ b/services/core/java/com/android/server/notification/NotificationManagerService.java
@@ -821,6 +821,7 @@ public class NotificationManagerService extends SystemService {
                         }
                     }
                     r.setVisibility(true, nv.rank, nv.count);
+                    maybeRecordInterruptionLocked(r);
                     nv.recycle();
                 }
                 // Note that we might receive this event after notifications
@@ -1928,11 +1929,12 @@ public class NotificationManagerService extends SystemService {
 
     @GuardedBy("mNotificationLock")
     protected void maybeRecordInterruptionLocked(NotificationRecord r) {
-        if (r.isInterruptive()) {
+        if (r.isInterruptive() && !r.hasRecordedInterruption()) {
             mAppUsageStats.reportInterruptiveNotification(r.sbn.getPackageName(),
                     r.getChannel().getId(),
                     getRealUserId(r.sbn.getUserId()));
             logRecentLocked(r);
+            r.setRecordedInterruption(true);
         }
     }
 
@@ -2669,6 +2671,7 @@ public class NotificationManagerService extends SystemService {
                                 if (DBG) Slog.d(TAG, "Marking notification as seen " + keys[i]);
                                 reportSeen(r);
                                 r.setSeen();
+                                maybeRecordInterruptionLocked(r);
                             }
                         }
                     }
@@ -4440,7 +4443,7 @@ public class NotificationManagerService extends SystemService {
                     if (index < 0) {
                         mNotificationList.add(r);
                         mUsageStats.registerPostedByApp(r);
-                        r.setInterruptive(isVisuallyInterruptive(null, r));
+                        r.setInterruptive(true);
                     } else {
                         old = mNotificationList.get(index);
                         mNotificationList.set(index, r);
@@ -4449,7 +4452,7 @@ public class NotificationManagerService extends SystemService {
                         notification.flags |=
                                 old.getNotification().flags & FLAG_FOREGROUND_SERVICE;
                         r.isUpdate = true;
-                        r.setInterruptive(isVisuallyInterruptive(old, r));
+                        r.setTextChanged(isVisuallyInterruptive(old, r));
                     }
 
                     mNotificationsByKey.put(n.getKey(), r);

diff --git a/services/core/java/com/android/server/notification/NotificationRecord.java b/services/core/java/com/android/server/notification/NotificationRecord.java
index 2aec3ea..0f3f44e 100644 (file)
--- a/services/core/java/com/android/server/notification/NotificationRecord.java
+++ b/services/core/java/com/android/server/notification/NotificationRecord.java
@@ -158,6 +158,8 @@ public final class NotificationRecord {
     private final NotificationStats mStats;
     private int mUserSentiment;
     private boolean mIsInterruptive;
+    private boolean mTextChanged;
+    private boolean mRecordedInterruption;
     private int mNumberOfSmartRepliesAdded;
     private boolean mHasSeenSmartReplies;
     /**
@@ -839,6 +841,9 @@ public final class NotificationRecord {
     /** Mark the notification as seen by the user. */
     public void setSeen() {
         mStats.setSeen();
+        if (mTextChanged) {
+            mIsInterruptive = true;
+        }
     }
 
     public void setAuthoritativeRank(int authoritativeRank) {
@@ -935,6 +940,18 @@ public final class NotificationRecord {
         mIsInterruptive = interruptive;
     }
 
+    public void setTextChanged(boolean textChanged) {
+        mTextChanged = textChanged;
+    }
+
+    public void setRecordedInterruption(boolean recorded) {
+        mRecordedInterruption = recorded;
+    }
+
+    public boolean hasRecordedInterruption() {
+        return mRecordedInterruption;
+    }
+
     public boolean isInterruptive() {
         return mIsInterruptive;
     }

diff --git a/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java b/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java
index 3dde7f1..41b4718 100644 (file)
--- a/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java
+++ b/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java
@@ -91,10 +91,12 @@ import android.os.Build;
 import android.os.Bundle;
 import android.os.IBinder;
 import android.os.Process;
+import android.os.RemoteException;
 import android.os.UserHandle;
 import android.provider.MediaStore;
 import android.provider.Settings.Secure;
 import android.service.notification.Adjustment;
+import android.service.notification.INotificationListener;
 import android.service.notification.NotificationListenerService;
 import android.service.notification.NotificationStats;
 import android.service.notification.NotifyingApp;
@@ -160,6 +162,8 @@ public class NotificationManagerServiceTest extends UiServiceTestCase {
     @Mock
     private NotificationUsageStats mUsageStats;
     @Mock
+    private UsageStatsManagerInternal mAppUsageStats;
+    @Mock
     private AudioManager mAudioManager;
     @Mock
     ActivityManager mActivityManager;
@@ -276,7 +280,7 @@ public class NotificationManagerServiceTest extends UiServiceTestCase {
                     mPackageManager, mPackageManagerClient, mockLightsManager,
                     mListeners, mAssistants, mConditionProviders,
                     mCompanionMgr, mSnoozeHelper, mUsageStats, mPolicyFile, mActivityManager,
-                    mGroupHelper, mAm, mock(UsageStatsManagerInternal.class),
+                    mGroupHelper, mAm, mAppUsageStats,
                     mock(DevicePolicyManagerInternal.class));
         } catch (SecurityException e) {
             if (!e.getMessage().contains("Permission Denial: not allowed to send broadcast")) {
@@ -3019,4 +3023,46 @@ public class NotificationManagerServiceTest extends UiServiceTestCase {
 
         assertEquals(true, mService.canUseManagedServices("d"));
     }
+
+    @Test
+    public void testOnNotificationVisibilityChanged_triggersInterruptionUsageStat() {
+        final NotificationRecord r = generateNotificationRecord(
+                mTestNotificationChannel, 1, null, true);
+        r.setTextChanged(true);
+        mService.addNotification(r);
+
+        mService.mNotificationDelegate.onNotificationVisibilityChanged(new NotificationVisibility[]
+                {NotificationVisibility.obtain(r.getKey(), 1, 1, true)},
+                new NotificationVisibility[]{});
+
+        verify(mAppUsageStats).reportInterruptiveNotification(anyString(), anyString(), anyInt());
+    }
+
+    @Test
+    public void testSetNotificationsShownFromListener_triggersInterruptionUsageStat()
+            throws RemoteException {
+        final NotificationRecord r = generateNotificationRecord(
+                mTestNotificationChannel, 1, null, true);
+        r.setTextChanged(true);
+        mService.addNotification(r);
+
+        mBinderService.setNotificationsShownFromListener(null, new String[] {r.getKey()});
+
+        verify(mAppUsageStats).reportInterruptiveNotification(anyString(), anyString(), anyInt());
+    }
+
+    @Test
+    public void testMybeRecordInterruptionLocked_doesNotRecordTwice()
+            throws RemoteException {
+        final NotificationRecord r = generateNotificationRecord(
+                mTestNotificationChannel, 1, null, true);
+        r.setInterruptive(true);
+        mService.addNotification(r);
+
+        mService.maybeRecordInterruptionLocked(r);
+        mService.maybeRecordInterruptionLocked(r);
+
+        verify(mAppUsageStats, times(1)).reportInterruptiveNotification(
+                anyString(), anyString(), anyInt());
+    }
 }

diff --git a/services/tests/uiservicestests/src/com/android/server/notification/NotificationRecordTest.java b/services/tests/uiservicestests/src/com/android/server/notification/NotificationRecordTest.java
index e3289ab..c1868e7 100644 (file)
--- a/services/tests/uiservicestests/src/com/android/server/notification/NotificationRecordTest.java
+++ b/services/tests/uiservicestests/src/com/android/server/notification/NotificationRecordTest.java
@@ -602,4 +602,45 @@ public class NotificationRecordTest extends UiServiceTestCase {
         record.setIsAppImportanceLocked(false);
         assertEquals(false, record.getIsAppImportanceLocked());
     }
+
+    @Test
+    public void testIsInterruptive_textChanged_notSeen() {
+        StatusBarNotification sbn = getNotification(false /*preO */, false /* noisy */,
+                false /* defaultSound */, false /* buzzy */, false /* defaultBuzz */,
+                false /* lights */, false /* defaultLights */, null /* group */);
+        NotificationRecord record = new NotificationRecord(mMockContext, sbn, channel);
+
+        assertEquals(false, record.isInterruptive());
+
+        record.setTextChanged(true);
+        assertEquals(false, record.isInterruptive());
+    }
+
+    @Test
+    public void testIsInterruptive_textChanged_seen() {
+        StatusBarNotification sbn = getNotification(false /*preO */, false /* noisy */,
+                false /* defaultSound */, false /* buzzy */, false /* defaultBuzz */,
+                false /* lights */, false /* defaultLights */, null /* group */);
+        NotificationRecord record = new NotificationRecord(mMockContext, sbn, channel);
+
+        assertEquals(false, record.isInterruptive());
+
+        record.setTextChanged(true);
+        record.setSeen();
+        assertEquals(true, record.isInterruptive());
+    }
+
+    @Test
+    public void testIsInterruptive_textNotChanged_seen() {
+        StatusBarNotification sbn = getNotification(false /*preO */, false /* noisy */,
+                false /* defaultSound */, false /* buzzy */, false /* defaultBuzz */,
+                false /* lights */, false /* defaultLights */, null /* group */);
+        NotificationRecord record = new NotificationRecord(mMockContext, sbn, channel);
+
+        assertEquals(false, record.isInterruptive());
+
+        record.setTextChanged(false);
+        record.setSeen();
+        assertEquals(false, record.isInterruptive());
+    }
 }