Use coroutine dispatcher instead of WorkerThread

2023-05-24 17:58:16 +02:00
parent 3d77083aa7
commit fe85a7cc92
8 changed files with 48 additions and 70 deletions
--- a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/NotificationBitmapLoader.kt
+++ b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/NotificationBitmapLoader.kt
@@ -19,7 +19,6 @@ package io.element.android.libraries.push.impl.notifications
 import android.content.Context
 import android.graphics.Bitmap
 import android.os.Build
-import androidx.annotation.WorkerThread
 import androidx.core.graphics.drawable.IconCompat
 import androidx.core.graphics.drawable.toBitmap
 import coil.imageLoader
@@ -27,7 +26,6 @@ import coil.request.ImageRequest
 import coil.transform.CircleCropTransformation
 import io.element.android.libraries.di.ApplicationContext
 import io.element.android.libraries.matrix.api.media.MediaResolver
-import kotlinx.coroutines.runBlocking
 import timber.log.Timber
 import javax.inject.Inject

@@ -39,24 +37,20 @@ class NotificationBitmapLoader @Inject constructor(
     * Get icon of a room.
     * @param path mxc url
     */
-    @WorkerThread
-    fun getRoomBitmap(path: String?): Bitmap? {
+    suspend fun getRoomBitmap(path: String?): Bitmap? {
        if (path == null) {
            return null
        }
        return loadRoomBitmap(path)
    }

-    @WorkerThread
-    private fun loadRoomBitmap(path: String): Bitmap? {
+    private suspend fun loadRoomBitmap(path: String): Bitmap? {
        return try {
            val imageRequest = ImageRequest.Builder(context)
                .data(MediaResolver.Meta(path, MediaResolver.Kind.Thumbnail(1024)))
                .build()
-            runBlocking {
-                val result = context.imageLoader.execute(imageRequest)
-                result.drawable?.toBitmap()
-            }
+            val result = context.imageLoader.execute(imageRequest)
+            result.drawable?.toBitmap()
        } catch (e: Throwable) {
            Timber.e(e, "Unable to load room bitmap")
            null
@@ -68,8 +62,7 @@ class NotificationBitmapLoader @Inject constructor(
     * Before Android P, this does nothing because the icon won't be used
     * @param path mxc url
     */
-    @WorkerThread
-    fun getUserIcon(path: String?): IconCompat? {
+    suspend fun getUserIcon(path: String?): IconCompat? {
        if (path == null || Build.VERSION.SDK_INT < Build.VERSION_CODES.P) {
            return null
        }
@@ -77,17 +70,14 @@ class NotificationBitmapLoader @Inject constructor(
        return loadUserIcon(path)
    }

-    @WorkerThread
-    private fun loadUserIcon(path: String): IconCompat? {
+    private suspend fun loadUserIcon(path: String): IconCompat? {
        return try {
            val imageRequest = ImageRequest.Builder(context)
                .data(MediaResolver.Meta(path, MediaResolver.Kind.Thumbnail(1024)))
                .transformations(CircleCropTransformation())
                .build()
-            val bitmap = runBlocking {
-                val result = context.imageLoader.execute(imageRequest)
-                result.drawable?.toBitmap()
-            }
+            val result = context.imageLoader.execute(imageRequest)
+            val bitmap = result.drawable?.toBitmap()
            return bitmap?.let { IconCompat.createWithBitmap(it) }
        } catch (e: Throwable) {
            Timber.e(e, "Unable to load user bitmap")
--- a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/NotificationDrawerManager.kt
+++ b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/NotificationDrawerManager.kt
@@ -16,11 +16,9 @@

 package io.element.android.libraries.push.impl.notifications

-import android.os.Handler
-import android.os.HandlerThread
-import androidx.annotation.WorkerThread
 import io.element.android.libraries.androidutils.throttler.FirstThrottler
 import io.element.android.libraries.core.cache.CircularCache
+import io.element.android.libraries.core.coroutine.CoroutineDispatchers
 import io.element.android.libraries.core.data.tryOrNull
 import io.element.android.libraries.core.meta.BuildMeta
 import io.element.android.libraries.di.AppScope
@@ -37,8 +35,9 @@ import io.element.android.libraries.push.impl.notifications.model.shouldIgnoreMe
 import io.element.android.services.appnavstate.api.AppNavigationState
 import io.element.android.services.appnavstate.api.AppNavigationStateService
 import kotlinx.coroutines.CoroutineScope
+import kotlinx.coroutines.delay
 import kotlinx.coroutines.launch
-import kotlinx.coroutines.runBlocking
+import kotlinx.coroutines.withContext
 import timber.log.Timber
 import javax.inject.Inject

@@ -56,13 +55,10 @@ class NotificationDrawerManager @Inject constructor(
    private val filteredEventDetector: FilteredEventDetector,
    private val appNavigationStateService: AppNavigationStateService,
    private val coroutineScope: CoroutineScope,
+    private val dispatchers: CoroutineDispatchers,
    private val buildMeta: BuildMeta,
    private val matrixAuthenticationService: MatrixAuthenticationService,
 ) {
-
-    private val handlerThread: HandlerThread = HandlerThread("NotificationDrawerManager", Thread.MIN_PRIORITY)
-    private var backgroundHandler: Handler
-
    /**
     * Lazily initializes the NotificationState as we rely on having a current session in order to fetch the persisted queue of events.
     */
@@ -74,8 +70,6 @@ class NotificationDrawerManager @Inject constructor(
    private var useCompleteNotificationFormat = true

    init {
-        handlerThread.start()
-        backgroundHandler = Handler(handlerThread.looper)
        // Observe application state
        coroutineScope.launch {
            appNavigationStateService.appNavigationStateFlow
@@ -193,30 +187,25 @@ class NotificationDrawerManager @Inject constructor(
        notificationState.updateQueuedEvents(this) { queuedEvents, _ ->
            action(queuedEvents)
        }
-        refreshNotificationDrawer()
+        coroutineScope.refreshNotificationDrawer()
    }

-    private fun refreshNotificationDrawer() {
+    private fun CoroutineScope.refreshNotificationDrawer() = launch {
        // Implement last throttler
        val canHandle = firstThrottler.canHandle()
        Timber.v("refreshNotificationDrawer(), delay: ${canHandle.waitMillis()} ms")
-        backgroundHandler.removeCallbacksAndMessages(null)
-
-        backgroundHandler.postDelayed(
-            {
-                try {
-                    refreshNotificationDrawerBg()
-                } catch (throwable: Throwable) {
-                    // It can happen if for instance session has been destroyed. It's a bit ugly to try catch like this, but it's safer
-                    Timber.w(throwable, "refreshNotificationDrawerBg failure")
-                }
-            },
-            canHandle.waitMillis()
-        )
+        withContext(dispatchers.io) {
+            delay(canHandle.waitMillis())
+            try {
+                refreshNotificationDrawerBg()
+            } catch (throwable: Throwable) {
+                // It can happen if for instance session has been destroyed. It's a bit ugly to try catch like this, but it's safer
+                Timber.w(throwable, "refreshNotificationDrawerBg failure")
+            }
+        }
    }

-    @WorkerThread
-    private fun refreshNotificationDrawerBg() {
+    private suspend fun refreshNotificationDrawerBg() {
        Timber.v("refreshNotificationDrawerBg()")
        val eventsToRender = notificationState.updateQueuedEvents(this) { queuedEvents, renderedEvents ->
            notifiableEventProcessor.process(queuedEvents.rawEvents(), currentAppNavigationState, renderedEvents).also {
@@ -239,8 +228,7 @@ class NotificationDrawerManager @Inject constructor(
        }
    }

-    @WorkerThread
-    private fun renderEvents(eventsToRender: List<ProcessedEvent<NotifiableEvent>>) {
+    private suspend fun renderEvents(eventsToRender: List<ProcessedEvent<NotifiableEvent>>) {
        // Group by sessionId
        val eventsForSessions = eventsToRender.groupBy {
            it.event.sessionId
@@ -250,18 +238,16 @@ class NotificationDrawerManager @Inject constructor(
            val currentUser = tryOrNull(
                onError = { Timber.e(it, "Unable to retrieve info for user ${sessionId.value}") },
                operation = {
-                    runBlocking {
-                        val client = matrixAuthenticationService.restoreSession(sessionId).getOrNull()
+                    val client = matrixAuthenticationService.restoreSession(sessionId).getOrNull()

-                        // myUserDisplayName cannot be empty else NotificationCompat.MessagingStyle() will crash
-                        val myUserDisplayName = client?.loadUserDisplayName()?.getOrNull() ?: sessionId.value
-                        val userAvatarUrl = client?.loadUserAvatarURLString()?.getOrNull()
-                        MatrixUser(
-                            userId = sessionId,
-                            displayName = myUserDisplayName,
-                            avatarUrl = userAvatarUrl
-                        )
-                    }
+                    // myUserDisplayName cannot be empty else NotificationCompat.MessagingStyle() will crash
+                    val myUserDisplayName = client?.loadUserDisplayName()?.getOrNull() ?: sessionId.value
+                    val userAvatarUrl = client?.loadUserAvatarURLString()?.getOrNull()
+                    MatrixUser(
+                        userId = sessionId,
+                        displayName = myUserDisplayName,
+                        avatarUrl = userAvatarUrl
+                    )
                }
            ) ?: MatrixUser(
                userId = sessionId,
--- a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/NotificationFactory.kt
+++ b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/NotificationFactory.kt
@@ -34,7 +34,7 @@ class NotificationFactory @Inject constructor(
    private val summaryGroupMessageCreator: SummaryGroupMessageCreator
 ) {

-    fun Map<RoomId, ProcessedMessageEvents>.toNotifications(
+    suspend fun Map<RoomId, ProcessedMessageEvents>.toNotifications(
        currentUser: MatrixUser,
    ): List<RoomNotification> {
        return map { (roomId, events) ->
--- a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/NotificationRenderer.kt
+++ b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/NotificationRenderer.kt
@@ -16,7 +16,6 @@

 package io.element.android.libraries.push.impl.notifications

-import androidx.annotation.WorkerThread
 import io.element.android.libraries.matrix.api.core.RoomId
 import io.element.android.libraries.matrix.api.user.MatrixUser
 import io.element.android.libraries.push.impl.notifications.model.InviteNotifiableEvent
@@ -32,8 +31,7 @@ class NotificationRenderer @Inject constructor(
    private val notificationFactory: NotificationFactory,
 ) {

-    @WorkerThread
-    fun render(
+    suspend fun render(
        currentUser: MatrixUser,
        useCompleteNotificationFormat: Boolean,
        eventsToProcess: List<ProcessedEvent<NotifiableEvent>>
--- a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/RoomGroupMessageCreator.kt
+++ b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/RoomGroupMessageCreator.kt
@@ -37,7 +37,7 @@ class RoomGroupMessageCreator @Inject constructor(
    private val notificationFactory: NotificationFactory
 ) {

-    fun createRoomMessage(
+    suspend fun createRoomMessage(
        currentUser: MatrixUser,
        events: List<NotifiableMessageEvent>,
        roomId: RoomId,
@@ -98,7 +98,7 @@ class RoomGroupMessageCreator @Inject constructor(
        )
    }

-    private fun NotificationCompat.MessagingStyle.addMessagesFromEvents(events: List<NotifiableMessageEvent>) {
+    private suspend fun NotificationCompat.MessagingStyle.addMessagesFromEvents(events: List<NotifiableMessageEvent>) {
        events.forEach { event ->
            val senderPerson = if (event.outGoingMessage) {
                null
@@ -171,7 +171,7 @@ class RoomGroupMessageCreator @Inject constructor(
        }
    }

-    private fun getRoomBitmap(events: List<NotifiableMessageEvent>): Bitmap? {
+    private suspend fun getRoomBitmap(events: List<NotifiableMessageEvent>): Bitmap? {
        // Use the last event (most recent?)
        return events.lastOrNull()
            ?.roomAvatarPath
--- a/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/notifications/NotificationFactoryTest.kt
+++ b/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/notifications/NotificationFactoryTest.kt
@@ -28,6 +28,7 @@ import io.element.android.libraries.push.impl.notifications.fake.FakeSummaryGrou
 import io.element.android.libraries.push.impl.notifications.fixtures.aNotifiableMessageEvent
 import io.element.android.libraries.push.impl.notifications.fixtures.aSimpleNotifiableEvent
 import io.element.android.libraries.push.impl.notifications.fixtures.anInviteNotifiableEvent
+import kotlinx.coroutines.test.runTest
 import org.junit.Test

 private val MY_AVATAR_URL: String? = null
@@ -196,6 +197,8 @@ class NotificationFactoryTest {
    }
 }

-fun <T> testWith(receiver: T, block: T.() -> Unit) {
-    receiver.block()
+fun <T> testWith(receiver: T, block: suspend T.() -> Unit) {
+    runTest {
+        receiver.block()
+    }
 }
--- a/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/notifications/fake/FakeNotificationFactory.kt
+++ b/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/notifications/fake/FakeNotificationFactory.kt
@@ -22,6 +22,7 @@ import io.element.android.libraries.push.impl.notifications.NotificationFactory
 import io.element.android.libraries.push.impl.notifications.OneShotNotification
 import io.element.android.libraries.push.impl.notifications.RoomNotification
 import io.element.android.libraries.push.impl.notifications.SummaryNotification
+import io.mockk.coEvery
 import io.mockk.every
 import io.mockk.mockk

@@ -38,7 +39,7 @@ class FakeNotificationFactory {
        summaryNotification: SummaryNotification
    ) {
        with(instance) {
-            every { groupedEvents.roomEvents.toNotifications(matrixUser) } returns roomNotifications
+            coEvery { groupedEvents.roomEvents.toNotifications(matrixUser) } returns roomNotifications
            every { groupedEvents.invitationEvents.toNotifications() } returns invitationNotifications
            every { groupedEvents.simpleEvents.toNotifications() } returns simpleNotifications

--- a/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/notifications/fake/FakeRoomGroupMessageCreator.kt
+++ b/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/notifications/fake/FakeRoomGroupMessageCreator.kt
@@ -21,7 +21,7 @@ import io.element.android.libraries.matrix.api.user.MatrixUser
 import io.element.android.libraries.push.impl.notifications.RoomGroupMessageCreator
 import io.element.android.libraries.push.impl.notifications.RoomNotification
 import io.element.android.libraries.push.impl.notifications.model.NotifiableMessageEvent
-import io.mockk.every
+import io.mockk.coEvery
 import io.mockk.mockk

 class FakeRoomGroupMessageCreator {
@@ -34,7 +34,7 @@ class FakeRoomGroupMessageCreator {
        roomId: RoomId,
    ): RoomNotification.Message {
        val mockMessage = mockk<RoomNotification.Message>()
-        every {
+        coEvery {
            instance.createRoomMessage(
                currentUser = matrixUser,
                events = events,