moving the notifable queue adding to the queue itself and making onNotifiableEventReceived not synchronised for use within the synchronized batching

- makes the refresh function private as all interactions now come through via update

vector/src/gplay/java/im/vector/app/gplay/push/fcm/VectorFirebaseMessagingService.kt

@@ -201,8 +201,7 @@ class VectorFirebaseMessagingService : FirebaseMessagingService() {
             resolvedEvent
                     ?.also { Timber.tag(loggerTag.value).d("Fast lane: notify drawer") }
                     ?.let {
-                        notificationDrawerManager.onNotifiableEventReceived(it)
+                        notificationDrawerManager.updateEvents { it.onNotifiableEventReceived(resolvedEvent) }
-                        notificationDrawerManager.refreshNotificationDrawer()
                     }
         }
     }

vector/src/main/java/im/vector/app/features/notifications/NotificationBroadcastReceiver.kt

@@ -145,8 +145,7 @@ class NotificationBroadcastReceiver : BroadcastReceiver() {
                 canBeReplaced = false
         )
 
-        notificationDrawerManager.onNotifiableEventReceived(notifiableMessageEvent)
+        notificationDrawerManager.updateEvents { it.onNotifiableEventReceived(notifiableMessageEvent) }
-        notificationDrawerManager.refreshNotificationDrawer()
 
         /*
         // TODO Error cannot be managed the same way than in Riot

vector/src/main/java/im/vector/app/features/notifications/NotificationDrawerManager.kt

@@ -93,7 +93,7 @@ class NotificationDrawerManager @Inject constructor(private val context: Context
     #refreshNotificationDrawer() is called.
     Events might be grouped and there might not be one notification per event!
      */
-    fun onNotifiableEventReceived(notifiableEvent: NotifiableEvent) {
+    fun NotificationEventQueue.onNotifiableEventReceived(notifiableEvent: NotifiableEvent) {
         if (!vectorPreferences.areNotificationEnabledForDevice()) {
             Timber.i("Notification are disabled for this device")
             return
@@ -105,46 +105,8 @@ class NotificationDrawerManager @Inject constructor(private val context: Context
         } else {
             Timber.d("onNotifiableEventReceived(): is push: ${notifiableEvent.canBeReplaced}")
         }
-        synchronized(queuedEvents) {
-            val existing = queuedEvents.findExistingById(notifiableEvent)
-            val edited = queuedEvents.findEdited(notifiableEvent)
-            when {
-                existing != null                               -> {
-                    if (existing.canBeReplaced) {
-                        // Use the event coming from the event stream as it may contains more info than
-                        // the fcm one (like type/content/clear text) (e.g when an encrypted message from
-                        // FCM should be update with clear text after a sync)
-                        // In this case the message has already been notified, and might have done some noise
-                        // So we want the notification to be updated even if it has already been displayed
-                        // Use setOnlyAlertOnce to ensure update notification does not interfere with sound
-                        // from first notify invocation as outlined in:
-                        // https://developer.android.com/training/notify-user/build-notification#Updating
-                        queuedEvents.replace(replace = existing, with = notifiableEvent)
-                    } else {
-                        // keep the existing one, do not replace
-                    }
-                }
-                edited != null                                 -> {
-                    // Replace the existing notification with the new content
-                    queuedEvents.replace(replace = edited, with = notifiableEvent)
-                }
-                seenEventIds.contains(notifiableEvent.eventId) -> {
-                    // we've already seen the event, lets skip
-                    Timber.d("onNotifiableEventReceived(): skipping event, already seen")
-                }
-                else                                           -> {
-                    seenEventIds.put(notifiableEvent.eventId)
-                    queuedEvents.add(notifiableEvent)
-                }
-            }
-        }
-    }
 
-    fun updateEvents(action: (NotificationEventQueue) -> Unit) {
+        add(notifiableEvent, seenEventIds)
-        synchronized(queuedEvents) {
-            action(queuedEvents)
-        }
-        refreshNotificationDrawer()
     }
 
     /**
@@ -168,9 +130,27 @@ class NotificationDrawerManager @Inject constructor(private val context: Context
         }
     }
 
+    fun notificationStyleChanged() {
+        updateEvents {
+            val newSettings = vectorPreferences.useCompleteNotificationFormat()
+            if (newSettings != useCompleteNotificationFormat) {
+                // Settings has changed, remove all current notifications
+                notificationDisplayer.cancelAllNotifications()
+                useCompleteNotificationFormat = newSettings
+            }
+        }
+    }
+
+    fun updateEvents(action: NotificationDrawerManager.(NotificationEventQueue) -> Unit) {
+        synchronized(queuedEvents) {
+            action(this, queuedEvents)
+        }
+        refreshNotificationDrawer()
+    }
+
     private var firstThrottler = FirstThrottler(200)
 
-    fun refreshNotificationDrawer() {
+    private fun refreshNotificationDrawer() {
         // Implement last throttler
         val canHandle = firstThrottler.canHandle()
         Timber.v("refreshNotificationDrawer(), delay: ${canHandle.waitMillis()} ms")
@@ -191,14 +171,6 @@ class NotificationDrawerManager @Inject constructor(private val context: Context
     @WorkerThread
     private fun refreshNotificationDrawerBg() {
         Timber.v("refreshNotificationDrawerBg()")
-
-        val newSettings = vectorPreferences.useCompleteNotificationFormat()
-        if (newSettings != useCompleteNotificationFormat) {
-            // Settings has changed, remove all current notifications
-            notificationDisplayer.cancelAllNotifications()
-            useCompleteNotificationFormat = newSettings
-        }
-
         val eventsToRender = synchronized(queuedEvents) {
             notifiableEventProcessor.process(queuedEvents.rawEvents(), currentRoomId, renderedEvents).also {
                 queuedEvents.clearAndAdd(it.onlyKeptEvents())

vector/src/main/java/im/vector/app/features/notifications/NotificationEventQueue.kt

@@ -82,10 +82,41 @@ class NotificationEventQueue(
         queue.clear()
     }
 
-    fun add(notifiableEvent: NotifiableEvent) {
+    fun add(notifiableEvent: NotifiableEvent, seenEventIds: CircularCache<String>) {
-        queue.add(notifiableEvent)
+        val existing = findExistingById(notifiableEvent)
+        val edited = findEdited(notifiableEvent)
+        when {
+            existing != null                               -> {
+                if (existing.canBeReplaced) {
+                    // Use the event coming from the event stream as it may contains more info than
+                    // the fcm one (like type/content/clear text) (e.g when an encrypted message from
+                    // FCM should be update with clear text after a sync)
+                    // In this case the message has already been notified, and might have done some noise
+                    // So we want the notification to be updated even if it has already been displayed
+                    // Use setOnlyAlertOnce to ensure update notification does not interfere with sound
+                    // from first notify invocation as outlined in:
+                    // https://developer.android.com/training/notify-user/build-notification#Updating
+                    replace(replace = existing, with = notifiableEvent)
+                } else {
+                    // keep the existing one, do not replace
+                }
+            }
+            edited != null                                 -> {
+                // Replace the existing notification with the new content
+                replace(replace = edited, with = notifiableEvent)
+            }
+            seenEventIds.contains(notifiableEvent.eventId) -> {
+                // we've already seen the event, lets skip
+                Timber.d("onNotifiableEventReceived(): skipping event, already seen")
+            }
+            else                                           -> {
+                seenEventIds.put(notifiableEvent.eventId)
+                queue.add(notifiableEvent)
+            }
+        }
     }
 
+
     fun clearMemberShipNotificationForRoom(roomId: String) {
         Timber.v("clearMemberShipOfRoom $roomId")
         queue.removeAll { it is InviteNotifiableEvent && it.roomId == roomId }

vector/src/main/java/im/vector/app/features/notifications/PushRuleTriggerListener.kt

@@ -34,7 +34,7 @@ class PushRuleTriggerListener @Inject constructor(
 
     override fun onEvents(pushEvents: PushEvents) {
         session?.let { session ->
-            pushEvents.matchedEvents.mapNotNull { (event, pushRule) ->
+            val notifiableEvents = pushEvents.matchedEvents.mapNotNull { (event, pushRule) ->
                 Timber.v("Push rule match for event ${event.eventId}")
                 val action = pushRule.getActions().toNotificationAction()
                 if (action.shouldNotify) {
@@ -43,14 +43,15 @@ class PushRuleTriggerListener @Inject constructor(
                     Timber.v("Matched push rule is set to not notify")
                     null
                 }
-            }.forEach { notificationDrawerManager.onNotifiableEventReceived(it) }
+            }
 
             notificationDrawerManager.updateEvents { queuedEvents ->
+                notifiableEvents.forEach { notifiableEvent ->
+                    queuedEvents.onNotifiableEventReceived(notifiableEvent)
+                }
                 queuedEvents.syncRoomEvents(roomsLeft = pushEvents.roomsLeft, roomsJoined = pushEvents.roomsJoined)
                 queuedEvents.markRedacted(pushEvents.redactedEventIds)
             }
-
-            notificationDrawerManager.refreshNotificationDrawer()
         } ?: Timber.e("Called without active session")
     }
 

vector/src/main/java/im/vector/app/features/settings/VectorSettingsPinFragment.kt

@@ -55,7 +55,7 @@ class VectorSettingsPinFragment @Inject constructor(
 
         useCompleteNotificationPref.setOnPreferenceChangeListener { _, _ ->
             // Refresh the drawer for an immediate effect of this change
-            notificationDrawerManager.refreshNotificationDrawer()
+            notificationDrawerManager.notificationStyleChanged()
             true
         }
     }