From 23a4b15e08bfe57be3242790d1a92db8dd4b9980 Mon Sep 17 00:00:00 2001 From: Ben Komalo Date: Fri, 22 Jul 2011 11:41:51 -0700 Subject: [PATCH] Fix notification issues. - use title as ticker text - also properly play notification sounds on new messages. Before, if you left a notification unread, and a new message comes in, no sound would be played. Since fixing that, it introduces another issue where on initial sync, tons of new messages come in (and the sync could take > 1 min). We throttle them with a 15 second interval. The notification is always updated to reflect the most up to date information, but sounds will never be played closer than 15 seconds together. Bug: 5020191 Bug: 5067059 Change-Id: I5ca474fd3b210ee856035bd78bd72931da80fe40 --- .../android/email/NotificationController.java | 26 +++++++++++++++---- .../email/NotificationControllerTest.java | 16 ++++++------ 2 files changed, 29 insertions(+), 13 deletions(-) diff --git a/src/com/android/email/NotificationController.java b/src/com/android/email/NotificationController.java index 2d48530db..80bc6b1c4 100644 --- a/src/com/android/email/NotificationController.java +++ b/src/com/android/email/NotificationController.java @@ -98,6 +98,20 @@ public class NotificationController { */ private long mSuspendAccountId = Account.NO_ACCOUNT; + /** + * Timestamp indicating when the last message notification sound was played. + * Used for throttling. + */ + private long mLastMessageNotifyTime; + + /** + * Minimum interval between notification sounds. + * Since a long sync (either on account setup or after a long period of being offline) can cause + * several notifications consecutively, it can be pretty overwhelming to get a barrage of + * notification sounds. Throttle them using this value. + */ + private static final long MIN_SOUND_INTERVAL = 15 * 1000; // 15 seconds + /** Constructor */ @VisibleForTesting NotificationController(Context context, Clock clock) { @@ -381,7 +395,7 @@ public class NotificationController { */ @VisibleForTesting Notification createNewMessageNotification(long accountId, long mailboxId, long messageId, - int unseenMessageCount, int unreadCount, boolean enableAudio) { + int unseenMessageCount, int unreadCount) { final Account account = Account.restoreAccountWithId(mContext, accountId); if (account == null) { return null; @@ -413,8 +427,12 @@ public class NotificationController { intent = Welcome.createOpenMessageIntent(mContext, accountId, mailboxId, messageId); } - Notification notification = createAccountNotification(account, null, title, text, + long now = mClock.getTime(); + boolean enableAudio = (now - mLastMessageNotifyTime) > MIN_SOUND_INTERVAL; + Notification notification = createAccountNotification( + account, title.toString(), title, text, intent, largeIcon, number, enableAudio); + mLastMessageNotifyTime = now; return notification; } @@ -653,8 +671,6 @@ public class NotificationController { } else if (newMessageCount != oldMessageCount || (newMessageId != 0 && newMessageId != oldMessageId)) { // Either the count or last message has changed; update the notification - boolean playAudio = (oldMessageCount == 0); // play audio on first notification - Integer unreadCount = Utility.getFirstRowInt( mContext, ContentUris.withAppendedId(Mailbox.CONTENT_URI, mMailboxId), new String[] { MailboxColumns.UNREAD_COUNT }, @@ -666,7 +682,7 @@ public class NotificationController { Notification n = sInstance.createNewMessageNotification( mAccountId, mMailboxId, newMessageId, - newMessageCount, unreadCount, playAudio); + newMessageCount, unreadCount); if (n != null) { // Make the notification visible sInstance.mNotificationManager.notify( diff --git a/tests/src/com/android/email/NotificationControllerTest.java b/tests/src/com/android/email/NotificationControllerTest.java index adb05eab2..ea5169fe3 100644 --- a/tests/src/com/android/email/NotificationControllerTest.java +++ b/tests/src/com/android/email/NotificationControllerTest.java @@ -16,17 +16,17 @@ package com.android.email; -import com.android.email.provider.ProviderTestUtils; -import com.android.emailcommon.provider.Account; -import com.android.emailcommon.provider.EmailContent.Message; -import com.android.emailcommon.provider.Mailbox; - import android.app.Notification; import android.content.Context; import android.media.AudioManager; import android.net.Uri; import android.test.AndroidTestCase; +import com.android.email.provider.ProviderTestUtils; +import com.android.emailcommon.provider.Account; +import com.android.emailcommon.provider.EmailContent.Message; +import com.android.emailcommon.provider.Mailbox; + /** * Test for {@link NotificationController}. * @@ -213,7 +213,7 @@ public class NotificationControllerTest extends AndroidTestCase { Mailbox b1 = ProviderTestUtils.setupMailbox("inbox", a1.mId, true, c, Mailbox.TYPE_INBOX); Message m1 = ProviderTestUtils.setupMessage("message", a1.mId, b1.mId, true, true, c); - n = mTarget.createNewMessageNotification(a1.mId, b1.mId, m1.mId, 1, 1, true); + n = mTarget.createNewMessageNotification(a1.mId, b1.mId, m1.mId, 1, 1); assertEquals(R.drawable.stat_notify_email_generic, n.icon); assertEquals(mMockClock.mTime, n.when); @@ -223,7 +223,7 @@ public class NotificationControllerTest extends AndroidTestCase { // TODO Check content -- how? // Case 2: 1 account, 2 unseen message - n = mTarget.createNewMessageNotification(a1.mId, b1.mId, m1.mId, 2, 2, true); + n = mTarget.createNewMessageNotification(a1.mId, b1.mId, m1.mId, 2, 2); assertEquals(R.drawable.stat_notify_email_generic, n.icon); assertEquals(mMockClock.mTime, n.when); @@ -247,7 +247,7 @@ public class NotificationControllerTest extends AndroidTestCase { m1.save(c); // This shouldn't crash. - n = mTarget.createNewMessageNotification(a1.mId, b1.mId, m1.mId, 1, 1, true); + n = mTarget.createNewMessageNotification(a1.mId, b1.mId, m1.mId, 1, 1); // Minimum test for the result assertEquals(R.drawable.stat_notify_email_generic, n.icon);