Persist state of VoiceMessagePresenter in memory (#1795)

Allows [VoiceMessagePresenter] instances to keep their progress and download states while going in and out of the timeline viewport.

This is implemented by caching each instance of a TimelineItem presenter inside the RoomScope. TimelineItem presenters can move some of their state outside of the `present()` function so that such state will survive scrollings of the timeline.

Author: Marco Romano

features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/di/LocalTimelineItemPresenterFactories.kt

@@ -0,0 +1,26 @@
+/*
+ * Copyright (c) 2023 New Vector Ltd
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package io.element.android.features.messages.impl.timeline.di
+
+import androidx.compose.runtime.staticCompositionLocalOf
+
+/**
+ * Provides a [TimelineItemPresenterFactories] to the composition.
+ */
+val LocalTimelineItemPresenterFactories = staticCompositionLocalOf {
+    TimelineItemPresenterFactories(emptyMap())
+}

features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/di/TimelineItemPresenterFactories.kt

@@ -18,13 +18,13 @@ package io.element.android.features.messages.impl.timeline.di
 
 import androidx.compose.runtime.Composable
 import androidx.compose.runtime.remember
-import androidx.compose.runtime.staticCompositionLocalOf
 import com.squareup.anvil.annotations.ContributesTo
 import dagger.Module
 import dagger.multibindings.Multibinds
 import io.element.android.features.messages.impl.timeline.model.event.TimelineItemEventContent
 import io.element.android.libraries.architecture.Presenter
 import io.element.android.libraries.di.RoomScope
+import io.element.android.libraries.di.SingleIn
 import javax.inject.Inject
 
 /**
@@ -40,38 +40,60 @@ interface TimelineItemPresenterFactoriesModule {
 }
 
 /**
- * Wrapper around the [TimelineItemPresenterFactory] map multi binding.
+ * Room level caching layer for the [TimelineItemPresenterFactory] instances.
  *
- * Its only purpose is to provide a nicer type name than:
- * `@JvmSuppressWildcards Map<Class<out TimelineItemEventContent>, TimelineItemPresenterFactory<*, *>>`.
- *
- * A typealias would have been better but typealiases on Dagger types which use @JvmSuppressWildcards
- * currently make Dagger crash.
- *
- * Request this type from Dagger to access the [TimelineItemPresenterFactory] map multibinding.
+ * It will cache the presenter instances in the room scope, so that they can be
+ * reused across recompositions of the timeline items that happen whenever an item
+ * goes out of the [LazyColumn] viewport.
  */
-data class TimelineItemPresenterFactories @Inject constructor(
-    val factories: @JvmSuppressWildcards Map<Class<out TimelineItemEventContent>, TimelineItemPresenterFactory<*, *>>,
-)
+@SingleIn(RoomScope::class)
+class TimelineItemPresenterFactories @Inject constructor(
+    private val factories: @JvmSuppressWildcards Map<Class<out TimelineItemEventContent>, TimelineItemPresenterFactory<*, *>>,
+) {
+    private val presenters: MutableMap<TimelineItemEventContent, Presenter<*>> = mutableMapOf()
 
-/**
- * Provides a [TimelineItemPresenterFactories] to the composition.
- */
-val LocalTimelineItemPresenterFactories = staticCompositionLocalOf {
-    TimelineItemPresenterFactories(emptyMap())
+    /**
+     * Creates and caches a presenter for the given content.
+     *
+     * Will throw if the presenter is not found in the [TimelineItemPresenterFactory] map multi binding.
+     *
+     * @param C The [TimelineItemEventContent] subtype handled by this TimelineItem presenter.
+     * @param S The state type produced by this timeline item presenter.
+     * @param content The [TimelineItemEventContent] instance to create a presenter for.
+     * @param contentClass The class of [content].
+     * @return An instance of a TimelineItem presenter that will be cached in the room scope.
+     */
+    @Composable
+    fun <C : TimelineItemEventContent, S : Any> rememberPresenter(
+        content: C,
+        contentClass: Class<C>,
+    ): Presenter<S> = remember(content) {
+        presenters[content]?.let {
+            @Suppress("UNCHECKED_CAST")
+            it as Presenter<S>
+        } ?: factories.getValue(contentClass).let {
+            @Suppress("UNCHECKED_CAST")
+            (it as TimelineItemPresenterFactory<C, S>).create(content).apply {
+                presenters[content] = this
+            }
+        }
+    }
 }
 
 /**
- * Creates and remembers a presenter for the given content.
+ * Creates and caches a presenter for the given content.
  *
  * Will throw if the presenter is not found in the [TimelineItemPresenterFactory] map multi binding.
+ *
+ * @param C The [TimelineItemEventContent] subtype handled by this TimelineItem presenter.
+ * @param S The state type produced by this timeline item presenter.
+ * @param content The [TimelineItemEventContent] instance to create a presenter for.
+ * @return An instance of a TimelineItem presenter that will be cached in the room scope.
  */
 @Composable
-inline fun <reified C : TimelineItemEventContent, reified S : Any> TimelineItemPresenterFactories.rememberPresenter(
+inline fun <reified C : TimelineItemEventContent, S : Any> TimelineItemPresenterFactories.rememberPresenter(
     content: C
-): Presenter<S> = remember(content) {
-    factories.getValue(C::class.java).let {
-        @Suppress("UNCHECKED_CAST")
-        (it as TimelineItemPresenterFactory<C, S>).create(content)
-    }
-}
+): Presenter<S> = rememberPresenter(
+    content = content,
+    contentClass = C::class.java
+)

features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/voicemessages/timeline/VoiceMessagePresenter.kt

@@ -22,7 +22,6 @@ import androidx.compose.runtime.derivedStateOf
 import androidx.compose.runtime.getValue
 import androidx.compose.runtime.mutableStateOf
 import androidx.compose.runtime.remember
-import androidx.compose.runtime.rememberCoroutineScope
 import com.squareup.anvil.annotations.ContributesTo
 import dagger.Binds
 import dagger.Module
@@ -40,6 +39,7 @@ import io.element.android.libraries.architecture.runUpdatingState
 import io.element.android.libraries.di.RoomScope
 import io.element.android.libraries.ui.utils.time.formatShort
 import io.element.android.services.analytics.api.AnalyticsService
+import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.launch
 import kotlin.time.Duration.Companion.milliseconds
 
@@ -55,6 +55,7 @@ interface VoiceMessagePresenterModule {
 class VoiceMessagePresenter @AssistedInject constructor(
     voiceMessagePlayerFactory: VoiceMessagePlayer.Factory,
     private val analyticsService: AnalyticsService,
+    private val scope: CoroutineScope,
     @Assisted private val content: TimelineItemVoiceContent,
 ) : Presenter<VoiceMessageState> {
 
@@ -70,13 +71,13 @@ class VoiceMessagePresenter @AssistedInject constructor(
         body = content.body,
     )
 
+    private val play = mutableStateOf<Async<Unit>>(Async.Uninitialized)
+    private var progressCache: Float = 0f
+
     @Composable
     override fun present(): VoiceMessageState {
 
-        val scope = rememberCoroutineScope()
-
         val playerState by player.state.collectAsState(VoiceMessagePlayer.State(isPlaying = false, isMyMedia = false, currentPosition = 0L))
-        val play = remember { mutableStateOf<Async<Unit>>(Async.Uninitialized) }
 
         val button by remember {
             derivedStateOf {
@@ -90,7 +91,12 @@ class VoiceMessagePresenter @AssistedInject constructor(
             }
         }
         val progress by remember {
-            derivedStateOf { if (playerState.isMyMedia) playerState.currentPosition / content.duration.toMillis().toFloat() else 0f }
+            derivedStateOf {
+                if (playerState.isMyMedia) {
+                    progressCache = playerState.currentPosition / content.duration.toMillis().toFloat()
+                }
+                progressCache
+            }
         }
         val time by remember {
             derivedStateOf {

features/messages/impl/src/test/kotlin/io/element/android/features/messages/voicemessages/timeline/VoiceMessagePresenterTest.kt

@@ -31,6 +31,7 @@ import io.element.android.features.messages.impl.voicemessages.timeline.VoiceMes
 import io.element.android.libraries.mediaplayer.test.FakeMediaPlayer
 import io.element.android.services.analytics.api.AnalyticsService
 import io.element.android.services.analytics.test.FakeAnalyticsService
+import kotlinx.coroutines.test.TestScope
 import kotlinx.coroutines.test.runTest
 import org.junit.Test
 
@@ -201,7 +202,7 @@ class VoiceMessagePresenterTest {
     }
 }
 
-fun createVoiceMessagePresenter(
+fun TestScope.createVoiceMessagePresenter(
     voiceMessageMediaRepo: VoiceMessageMediaRepo = FakeVoiceMessageMediaRepo(),
     analyticsService: AnalyticsService = FakeAnalyticsService(),
     content: TimelineItemVoiceContent = aTimelineItemVoiceContent(),
@@ -217,5 +218,6 @@ fun createVoiceMessagePresenter(
         )
     },
     analyticsService = analyticsService,
+    scope = this,
     content = content,
 )