Skip to content

Commit

Permalink
Refactor notification thumbnails to reduce chances for ANR.
Browse files Browse the repository at this point in the history
  • Loading branch information
cody-signal authored and greyson-signal committed Aug 24, 2022
1 parent a9fc562 commit 1e499fd
Show file tree
Hide file tree
Showing 13 changed files with 213 additions and 72 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
import org.thoughtcrime.securesms.database.model.MessageId;
import org.thoughtcrime.securesms.database.model.MessageRecord;
import org.thoughtcrime.securesms.dependencies.ApplicationDependencies;
import org.thoughtcrime.securesms.notifications.v2.MessageNotifierV2;
import org.thoughtcrime.securesms.notifications.v2.DefaultMessageNotifier;
import org.thoughtcrime.securesms.recipients.Recipient;
import org.thoughtcrime.securesms.recipients.RecipientId;
import org.signal.core.util.CursorUtil;
Expand Down Expand Up @@ -251,9 +251,9 @@ public long getConversationSnippetType(long threadId) throws NoSuchMessageExcept
}
}

public Cursor getMessagesForNotificationState(Collection<MessageNotifierV2.StickyThread> stickyThreads) {
public Cursor getMessagesForNotificationState(Collection<DefaultMessageNotifier.StickyThread> stickyThreads) {
StringBuilder stickyQuery = new StringBuilder();
for (MessageNotifierV2.StickyThread stickyThread : stickyThreads) {
for (DefaultMessageNotifier.StickyThread stickyThread : stickyThreads) {
if (stickyQuery.length() > 0) {
stickyQuery.append(" OR ");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
import org.signal.core.util.logging.Log;
import org.thoughtcrime.securesms.database.SignalDatabase;
import org.thoughtcrime.securesms.dependencies.ApplicationDependencies;
import org.thoughtcrime.securesms.notifications.v2.MessageNotifierV2;
import org.thoughtcrime.securesms.notifications.v2.DefaultMessageNotifier;
import org.thoughtcrime.securesms.notifications.v2.ConversationId;
import org.thoughtcrime.securesms.recipients.RecipientId;
import org.thoughtcrime.securesms.util.BubbleUtil;
Expand Down Expand Up @@ -47,7 +47,7 @@ public static void cancelAllMessageNotifications(@NonNull Context context) {

/**
* Cancels all Message-Based notifications. Specifically, this is any notification that is not the
* summary notification assigned to the {@link MessageNotifierV2#NOTIFICATION_GROUP} group.
* summary notification assigned to the {@link DefaultMessageNotifier#NOTIFICATION_GROUP} group.
*
* We utilize our wrapped cancellation methods and a counter to make sure that we do not lose
* bubble notifications that do not have unread messages in them.
Expand Down Expand Up @@ -113,7 +113,7 @@ public static void cancelMessageSummaryIfSoleNotification(@NonNull Context conte
@RequiresApi(23)
private static boolean isSingleThreadNotification(@NonNull StatusBarNotification statusBarNotification) {
return statusBarNotification.getId() != NotificationIds.MESSAGE_SUMMARY &&
Objects.equals(statusBarNotification.getNotification().getGroup(), MessageNotifierV2.NOTIFICATION_GROUP);
Objects.equals(statusBarNotification.getNotification().getGroup(), DefaultMessageNotifier.NOTIFICATION_GROUP);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@

import org.signal.core.util.ExceptionUtil;
import org.signal.core.util.concurrent.SignalExecutors;
import org.thoughtcrime.securesms.notifications.v2.MessageNotifierV2;
import org.thoughtcrime.securesms.notifications.v2.DefaultMessageNotifier;
import org.thoughtcrime.securesms.notifications.v2.ConversationId;
import org.thoughtcrime.securesms.recipients.Recipient;
import org.thoughtcrime.securesms.util.BubbleUtil;
Expand All @@ -23,13 +23,13 @@
*/
public class OptimizedMessageNotifier implements MessageNotifier {

private final LeakyBucketLimiter limiter;
private final MessageNotifierV2 messageNotifierV2;
private final LeakyBucketLimiter limiter;
private final DefaultMessageNotifier defaultMessageNotifier;

@MainThread
public OptimizedMessageNotifier(@NonNull Application context) {
this.limiter = new LeakyBucketLimiter(5, 1000, new Handler(SignalExecutors.getAndStartHandlerThread("signal-notifier").getLooper()));
this.messageNotifierV2 = new MessageNotifierV2(context);
this.limiter = new LeakyBucketLimiter(5, 1000, new Handler(SignalExecutors.getAndStartHandlerThread("signal-notifier").getLooper()));
this.defaultMessageNotifier = new DefaultMessageNotifier(context);
}

@Override
Expand Down Expand Up @@ -119,6 +119,6 @@ private void runOnLimiter(@NonNull Runnable runnable) {
}

private MessageNotifier getNotifier() {
return messageNotifierV2;
return defaultMessageNotifier;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
import org.thoughtcrime.securesms.database.model.StoryType;
import org.thoughtcrime.securesms.dependencies.ApplicationDependencies;
import org.thoughtcrime.securesms.mms.OutgoingMediaMessage;
import org.thoughtcrime.securesms.notifications.v2.MessageNotifierV2;
import org.thoughtcrime.securesms.notifications.v2.DefaultMessageNotifier;
import org.thoughtcrime.securesms.notifications.v2.ConversationId;
import org.thoughtcrime.securesms.recipients.Recipient;
import org.thoughtcrime.securesms.recipients.RecipientId;
Expand Down Expand Up @@ -67,7 +67,7 @@ public void onReceive(final Context context, Intent intent) {

final RecipientId recipientId = intent.getParcelableExtra(RECIPIENT_EXTRA);
final ReplyMethod replyMethod = (ReplyMethod) intent.getSerializableExtra(REPLY_METHOD);
final CharSequence responseText = remoteInput.getCharSequence(MessageNotifierV2.EXTRA_REMOTE_REPLY);
final CharSequence responseText = remoteInput.getCharSequence(DefaultMessageNotifier.EXTRA_REMOTE_REPLY);
final long groupStoryId = intent.getLongExtra(GROUP_STORY_ID_EXTRA, Long.MIN_VALUE);

if (recipientId == null) throw new AssertionError("No recipientId specified");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,14 +44,14 @@ import kotlin.math.max
/**
* MessageNotifier implementation using the new system for creating and showing notifications.
*/
class MessageNotifierV2(context: Application) : MessageNotifier {
class DefaultMessageNotifier(context: Application) : MessageNotifier {
@Volatile private var visibleThread: ConversationId? = null
@Volatile private var lastDesktopActivityTimestamp: Long = -1
@Volatile private var lastAudibleNotification: Long = -1
@Volatile private var lastScheduledReminder: Long = 0
@Volatile private var previousLockedStatus: Boolean = KeyCachingService.isLocked(context)
@Volatile private var previousPrivacyPreference: NotificationPrivacyPreference = SignalStore.settings().messageNotificationsPrivacy
@Volatile private var previousState: NotificationStateV2 = NotificationStateV2.EMPTY
@Volatile private var previousState: NotificationState = NotificationState.EMPTY

private val threadReminders: MutableMap<ConversationId, Reminder> = ConcurrentHashMap()
private val stickyThreads: MutableMap<ConversationId, StickyThread> = mutableMapOf()
Expand Down Expand Up @@ -132,7 +132,7 @@ class MessageNotifierV2(context: Application) : MessageNotifier {
val notificationProfile: NotificationProfile? = NotificationProfiles.getActiveProfile(SignalDatabase.notificationProfiles.getProfiles())

Log.internal().i(TAG, "sticky thread: $stickyThreads active profile: ${notificationProfile?.id ?: "none" }")
var state: NotificationStateV2 = NotificationStateProvider.constructNotificationState(stickyThreads, notificationProfile)
var state: NotificationState = NotificationStateProvider.constructNotificationState(stickyThreads, notificationProfile)
Log.internal().i(TAG, "state: $state")

if (state.muteFilteredMessages.isNotEmpty()) {
Expand Down Expand Up @@ -208,13 +208,14 @@ class MessageNotifierV2(context: Application) : MessageNotifier {
lastAudibleNotification = System.currentTimeMillis()

updateReminderTimestamps(context, alertOverrides, threadsThatAlerted)
NotificationThumbnails.removeAllExcept(state.notificationItems)

ServiceUtil.getNotificationManager(context).cancelOrphanedNotifications(context, state, stickyThreads.map { it.value.notificationId }.toSet())
updateBadge(context, state.messageCount)

val smsIds: MutableList<Long> = mutableListOf()
val mmsIds: MutableList<Long> = mutableListOf()
for (item: NotificationItemV2 in state.notificationItems) {
for (item: NotificationItem in state.notificationItems) {
if (item.isMms) {
mmsIds.add(item.id)
} else {
Expand Down Expand Up @@ -301,7 +302,7 @@ class MessageNotifierV2(context: Application) : MessageNotifier {
}

companion object {
val TAG: String = Log.tag(MessageNotifierV2::class.java)
val TAG: String = Log.tag(DefaultMessageNotifier::class.java)

private val REMINDER_TIMEOUT: Long = TimeUnit.MINUTES.toMillis(2)
val MIN_AUDIBLE_PERIOD_MILLIS = TimeUnit.SECONDS.toMillis(2)
Expand Down Expand Up @@ -339,12 +340,12 @@ private fun NotificationManager.getDisplayedNotificationIds(): Result<Set<Int>>
return try {
Result.success(activeNotifications.filter { it.isMessageNotification() }.map { it.id }.toSet())
} catch (e: Throwable) {
Log.w(MessageNotifierV2.TAG, e)
Log.w(DefaultMessageNotifier.TAG, e)
Result.failure(e)
}
}

private fun NotificationManager.cancelOrphanedNotifications(context: Context, state: NotificationStateV2, stickyNotifications: Set<Int>) {
private fun NotificationManager.cancelOrphanedNotifications(context: Context, state: NotificationState, stickyNotifications: Set<Int>) {
if (Build.VERSION.SDK_INT < 24) {
return
}
Expand All @@ -354,13 +355,13 @@ private fun NotificationManager.cancelOrphanedNotifications(context: Context, st
.map { it.id }
.filterNot { state.notificationIds.contains(it) }
.forEach { id ->
Log.d(MessageNotifierV2.TAG, "Cancelling orphaned notification: $id")
Log.d(DefaultMessageNotifier.TAG, "Cancelling orphaned notification: $id")
NotificationCancellationHelper.cancel(context, id)
}

NotificationCancellationHelper.cancelMessageSummaryIfSoleNotification(context)
} catch (e: Throwable) {
Log.w(MessageNotifierV2.TAG, e)
Log.w(DefaultMessageNotifier.TAG, e)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ sealed class NotificationBuilder(protected val context: Context) {
abstract fun setOnlyAlertOnce(onlyAlertOnce: Boolean)
abstract fun setGroupSummary(isGroupSummary: Boolean)
abstract fun setSubText(subText: String)
abstract fun addMarkAsReadActionActual(state: NotificationStateV2)
abstract fun addMarkAsReadActionActual(state: NotificationState)
abstract fun setPriority(priority: Int)
abstract fun setAlarms(recipient: Recipient?)
abstract fun setTicker(ticker: CharSequence?)
Expand All @@ -79,7 +79,7 @@ sealed class NotificationBuilder(protected val context: Context) {
protected abstract fun setWhen(timestamp: Long)
protected abstract fun addActions(replyMethod: ReplyMethod, conversation: NotificationConversation)
protected abstract fun addMessagesActual(conversation: NotificationConversation, includeShortcut: Boolean)
protected abstract fun addMessagesActual(state: NotificationStateV2)
protected abstract fun addMessagesActual(state: NotificationState)
protected abstract fun setBubbleMetadataActual(conversation: NotificationConversation, bubbleState: BubbleUtil.BubbleState)
protected abstract fun setLights(@ColorInt color: Int, onTime: Int, offTime: Int)

Expand Down Expand Up @@ -107,7 +107,7 @@ sealed class NotificationBuilder(protected val context: Context) {
}
}

fun setWhen(notificationItem: NotificationItemV2?) {
fun setWhen(notificationItem: NotificationItem?) {
if (notificationItem != null && notificationItem.timestamp != 0L) {
setWhen(notificationItem.timestamp)
}
Expand All @@ -126,7 +126,7 @@ sealed class NotificationBuilder(protected val context: Context) {
}
}

fun addMarkAsReadAction(state: NotificationStateV2) {
fun addMarkAsReadAction(state: NotificationState) {
if (privacy.isDisplayMessage && isNotLocked) {
addMarkAsReadActionActual(state)
}
Expand All @@ -136,7 +136,7 @@ sealed class NotificationBuilder(protected val context: Context) {
addMessagesActual(conversation, privacy.isDisplayContact)
}

fun addMessages(state: NotificationStateV2) {
fun addMessages(state: NotificationState) {
if (privacy.isDisplayNothing) {
return
}
Expand Down Expand Up @@ -207,7 +207,7 @@ sealed class NotificationBuilder(protected val context: Context) {
val label: String = context.getString(replyMethod.toLongDescription())
val replyAction: NotificationCompat.Action = if (Build.VERSION.SDK_INT >= 24) {
NotificationCompat.Action.Builder(R.drawable.ic_reply_white_36dp, actionName, remoteReply)
.addRemoteInput(RemoteInput.Builder(MessageNotifierV2.EXTRA_REMOTE_REPLY).setLabel(label).build())
.addRemoteInput(RemoteInput.Builder(DefaultMessageNotifier.EXTRA_REMOTE_REPLY).setLabel(label).build())
.setSemanticAction(NotificationCompat.Action.SEMANTIC_ACTION_REPLY)
.setShowsUserInterface(false)
.build()
Expand All @@ -216,7 +216,7 @@ sealed class NotificationBuilder(protected val context: Context) {
}

val wearableReplyAction = NotificationCompat.Action.Builder(R.drawable.ic_reply, actionName, remoteReply)
.addRemoteInput(RemoteInput.Builder(MessageNotifierV2.EXTRA_REMOTE_REPLY).setLabel(label).build())
.addRemoteInput(RemoteInput.Builder(DefaultMessageNotifier.EXTRA_REMOTE_REPLY).setLabel(label).build())
.build()

builder.addAction(replyAction)
Expand All @@ -226,7 +226,7 @@ sealed class NotificationBuilder(protected val context: Context) {
builder.extend(extender)
}

override fun addMarkAsReadActionActual(state: NotificationStateV2) {
override fun addMarkAsReadActionActual(state: NotificationState) {
val markAllAsReadAction = NotificationCompat.Action(R.drawable.check, context.getString(R.string.MessageNotifier_mark_all_as_read), state.getMarkAsReadIntent(context))
builder.addAction(markAllAsReadAction)
builder.extend(NotificationCompat.WearableExtender().addAction(markAllAsReadAction))
Expand Down Expand Up @@ -286,14 +286,14 @@ sealed class NotificationBuilder(protected val context: Context) {
builder.setStyle(messagingStyle)
}

override fun addMessagesActual(state: NotificationStateV2) {
override fun addMessagesActual(state: NotificationState) {
if (Build.VERSION.SDK_INT >= 24) {
return
}

val style: NotificationCompat.InboxStyle = NotificationCompat.InboxStyle()

for (notificationItem: NotificationItemV2 in state.notificationItems) {
for (notificationItem: NotificationItem in state.notificationItems) {
val line: CharSequence? = notificationItem.getInboxLine(context)
if (line != null) {
style.addLine(line)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,9 @@ import java.lang.NullPointerException
data class NotificationConversation(
val recipient: Recipient,
val thread: ConversationId,
val notificationItems: List<NotificationItemV2>
val notificationItems: List<NotificationItem>
) {
val mostRecentNotification: NotificationItemV2 = notificationItems.last()
val mostRecentNotification: NotificationItem = notificationItems.last()
val notificationId: Int = NotificationIds.getNotificationIdForThread(thread)
val sortKey: Long = Long.MAX_VALUE - mostRecentNotification.timestamp
val messageCount: Int = notificationItems.size
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,18 +37,18 @@ import org.thoughtcrime.securesms.util.TextSecurePreferences
*/
object NotificationFactory {

val TAG = Log.tag(NotificationFactory::class.java)
val TAG: String = Log.tag(NotificationFactory::class.java)

fun notify(
context: Context,
state: NotificationStateV2,
state: NotificationState,
visibleThread: ConversationId?,
targetThread: ConversationId?,
defaultBubbleState: BubbleUtil.BubbleState,
lastAudibleNotification: Long,
notificationConfigurationChanged: Boolean,
alertOverrides: Set<ConversationId>,
previousState: NotificationStateV2
previousState: NotificationState
): Set<ConversationId> {
if (state.isEmpty) {
Log.d(TAG, "State is empty, bailing")
Expand Down Expand Up @@ -85,7 +85,7 @@ object NotificationFactory {

private fun notify19(
context: Context,
state: NotificationStateV2,
state: NotificationState,
visibleThread: ConversationId?,
targetThread: ConversationId?,
defaultBubbleState: BubbleUtil.BubbleState,
Expand Down Expand Up @@ -127,15 +127,15 @@ object NotificationFactory {
@TargetApi(24)
private fun notify24(
context: Context,
state: NotificationStateV2,
state: NotificationState,
visibleThread: ConversationId?,
targetThread: ConversationId?,
defaultBubbleState: BubbleUtil.BubbleState,
lastAudibleNotification: Long,
notificationConfigurationChanged: Boolean,
alertOverrides: Set<ConversationId>,
nonVisibleThreadCount: Int,
previousState: NotificationStateV2
previousState: NotificationState
): Set<ConversationId> {
val threadsThatNewlyAlerted: MutableSet<ConversationId> = mutableSetOf()

Expand Down Expand Up @@ -186,7 +186,7 @@ object NotificationFactory {
setSmallIcon(R.drawable.ic_notification)
setColor(ContextCompat.getColor(context, R.color.core_ultramarine))
setCategory(NotificationCompat.CATEGORY_MESSAGE)
setGroup(MessageNotifierV2.NOTIFICATION_GROUP)
setGroup(DefaultMessageNotifier.NOTIFICATION_GROUP)
setGroupAlertBehavior(NotificationCompat.GROUP_ALERT_CHILDREN)
setChannelId(conversation.getChannelId(context))
setContentTitle(conversation.getContentTitle(context))
Expand Down Expand Up @@ -224,7 +224,7 @@ object NotificationFactory {
NotificationManagerCompat.from(context).safelyNotify(context, conversation.recipient, notificationId, builder.build())
}

private fun notifySummary(context: Context, state: NotificationStateV2) {
private fun notifySummary(context: Context, state: NotificationState) {
if (state.messageCount == 0) {
return
}
Expand All @@ -235,7 +235,7 @@ object NotificationFactory {
setSmallIcon(R.drawable.ic_notification)
setColor(ContextCompat.getColor(context, R.color.core_ultramarine))
setCategory(NotificationCompat.CATEGORY_MESSAGE)
setGroup(MessageNotifierV2.NOTIFICATION_GROUP)
setGroup(DefaultMessageNotifier.NOTIFICATION_GROUP)
setGroupAlertBehavior(NotificationCompat.GROUP_ALERT_CHILDREN)
setChannelId(NotificationChannels.getMessagesChannel(context))
setContentTitle(context.getString(R.string.app_name))
Expand Down Expand Up @@ -263,7 +263,7 @@ object NotificationFactory {
private fun notifyInThread(context: Context, recipient: Recipient, lastAudibleNotification: Long) {
if (!SignalStore.settings().isMessageNotificationsInChatSoundsEnabled ||
ServiceUtil.getAudioManager(context).ringerMode != AudioManager.RINGER_MODE_NORMAL ||
(System.currentTimeMillis() - lastAudibleNotification) < MessageNotifierV2.MIN_AUDIBLE_PERIOD_MILLIS
(System.currentTimeMillis() - lastAudibleNotification) < DefaultMessageNotifier.MIN_AUDIBLE_PERIOD_MILLIS
) {
return
}
Expand Down Expand Up @@ -378,7 +378,7 @@ object NotificationFactory {
setSmallIcon(R.drawable.ic_notification)
setColor(ContextCompat.getColor(context, R.color.core_ultramarine))
setCategory(NotificationCompat.CATEGORY_MESSAGE)
setGroup(MessageNotifierV2.NOTIFICATION_GROUP)
setGroup(DefaultMessageNotifier.NOTIFICATION_GROUP)
setChannelId(conversation.getChannelId(context))
setContentTitle(conversation.getContentTitle(context))
setLargeIcon(conversation.getContactLargeIcon(context).toLargeBitmap(context))
Expand Down

0 comments on commit 1e499fd

Please sign in to comment.