Merge pull request #852 from vector-im/feature/bma/moreLogAndFixes

More log and various fixes

Author: bmarty

app/src/main/kotlin/io/element/android/x/MainActivity.kt

@@ -52,8 +52,7 @@ class MainActivity : NodeComponentActivity() {
         Timber.tag(loggerTag.value).w("onCreate, with savedInstanceState: ${savedInstanceState != null}")
         installSplashScreen()
         super.onCreate(savedInstanceState)
-        appBindings = bindings<AppBindings>()
-        appBindings.matrixClientsHolder().restore(savedInstanceState)
+        appBindings = bindings()
         WindowCompat.setDecorFitsSystemWindows(window, false)
         setContent {
             MainContent(appBindings)
@@ -125,9 +124,4 @@ class MainActivity : NodeComponentActivity() {
         super.onDestroy()
         Timber.tag(loggerTag.value).w("onDestroy")
     }
-
-    override fun onSaveInstanceState(outState: Bundle) {
-        super.onSaveInstanceState(outState)
-        bindings<AppBindings>().matrixClientsHolder().onSaveInstanceState(outState)
-    }
 }

app/src/main/kotlin/io/element/android/x/di/AppBindings.kt

@@ -17,13 +17,11 @@
 package io.element.android.x.di
 
 import com.squareup.anvil.annotations.ContributesTo
-import io.element.android.appnav.di.MatrixClientsHolder
 import io.element.android.libraries.designsystem.utils.SnackbarDispatcher
 import io.element.android.libraries.di.AppScope
 
 @ContributesTo(AppScope::class)
 interface AppBindings {
-    fun matrixClientsHolder(): MatrixClientsHolder
     fun mainDaggerComponentOwner(): MainDaggerComponentsOwner
     fun snackbarDispatcher(): SnackbarDispatcher
 }

appnav/src/main/kotlin/io/element/android/appnav/LoggedInFlowNode.kt

@@ -154,8 +154,6 @@ class LoggedInFlowNode @AssistedInject constructor(
                 syncService.stopSync()
             },
             onDestroy = {
-                val imageLoaderFactory = bindings<MatrixUIBindings>().notLoggedInImageLoaderFactory()
-                Coil.setImageLoader(imageLoaderFactory)
                 plugins<LifecycleCallback>().forEach { it.onFlowReleased(id, inputs.matrixClient) }
                 appNavigationStateService.onLeavingSpace(id)
                 appNavigationStateService.onLeavingSession(id)

appnav/src/main/kotlin/io/element/android/appnav/NotLoggedInFlowNode.kt

@@ -19,6 +19,7 @@ package io.element.android.appnav
 import android.os.Parcelable
 import androidx.compose.runtime.Composable
 import androidx.compose.ui.Modifier
+import coil.Coil
 import com.bumble.appyx.core.composable.Children
 import com.bumble.appyx.core.lifecycle.subscribe
 import com.bumble.appyx.core.modality.BuildContext
@@ -34,15 +35,16 @@ import io.element.android.features.onboarding.api.OnBoardingEntryPoint
 import io.element.android.libraries.architecture.BackstackNode
 import io.element.android.libraries.architecture.animation.rememberDefaultTransitionHandler
 import io.element.android.libraries.di.AppScope
+import io.element.android.libraries.matrix.ui.media.NotLoggedInImageLoaderFactory
 import kotlinx.parcelize.Parcelize
-import timber.log.Timber
 
 @ContributesNode(AppScope::class)
 class NotLoggedInFlowNode @AssistedInject constructor(
     @Assisted buildContext: BuildContext,
     @Assisted plugins: List<Plugin>,
     private val onBoardingEntryPoint: OnBoardingEntryPoint,
     private val loginEntryPoint: LoginEntryPoint,
+    private val notLoggedInImageLoaderFactory: NotLoggedInImageLoaderFactory,
 ) : BackstackNode<NotLoggedInFlowNode.NavTarget>(
     backstack = BackStack(
         initialElement = NavTarget.OnBoarding,
@@ -51,10 +53,12 @@ class NotLoggedInFlowNode @AssistedInject constructor(
     buildContext = buildContext,
     plugins = plugins,
 ) {
-    init {
+    override fun onBuilt() {
+        super.onBuilt()
         lifecycle.subscribe(
-            onCreate = { Timber.v("OnCreate") },
-            onDestroy = { Timber.v("OnDestroy") }
+            onCreate = {
+                Coil.setImageLoader(notLoggedInImageLoaderFactory)
+            },
         )
     }
 

appnav/src/main/kotlin/io/element/android/appnav/RootFlowNode.kt

@@ -30,6 +30,7 @@ import com.bumble.appyx.core.node.Node
 import com.bumble.appyx.core.node.node
 import com.bumble.appyx.core.plugin.Plugin
 import com.bumble.appyx.core.plugin.plugins
+import com.bumble.appyx.core.state.MutableSavedStateMap
 import com.bumble.appyx.navmodel.backstack.BackStack
 import com.bumble.appyx.navmodel.backstack.operation.pop
 import com.bumble.appyx.navmodel.backstack.operation.push
@@ -90,10 +91,16 @@ class RootFlowNode @AssistedInject constructor(
     ) {
 
     override fun onBuilt() {
+        matrixClientsHolder.restore(buildContext.savedStateMap)
         super.onBuilt()
         observeLoggedInState()
     }
 
+    override fun onSaveInstanceState(state: MutableSavedStateMap) {
+        super.onSaveInstanceState(state)
+        matrixClientsHolder.save(state)
+    }
+
     private fun observeLoggedInState() {
         combine(
             cacheService.onClearedCacheEventFlow(),

appnav/src/main/kotlin/io/element/android/appnav/di/MatrixClientsHolder.kt

@@ -16,21 +16,18 @@
 
 package io.element.android.appnav.di
 
-import android.os.Bundle
-import io.element.android.libraries.di.AppScope
-import io.element.android.libraries.di.SingleIn
+import com.bumble.appyx.core.state.MutableSavedStateMap
+import com.bumble.appyx.core.state.SavedStateMap
 import io.element.android.libraries.matrix.api.MatrixClient
 import io.element.android.libraries.matrix.api.auth.MatrixAuthenticationService
 import io.element.android.libraries.matrix.api.core.SessionId
-import io.element.android.libraries.matrix.api.core.UserId
 import kotlinx.coroutines.runBlocking
 import timber.log.Timber
 import java.util.concurrent.ConcurrentHashMap
 import javax.inject.Inject
 
 private const val SAVE_INSTANCE_KEY = "io.element.android.x.di.MatrixClientsHolder.SaveInstanceKey"
 
-@SingleIn(AppScope::class)
 class MatrixClientsHolder @Inject constructor(private val authenticationService: MatrixAuthenticationService) {
 
     private val sessionIdsToMatrixClient = ConcurrentHashMap<SessionId, MatrixClient>()
@@ -55,16 +52,20 @@ class MatrixClientsHolder @Inject constructor(private val authenticationService:
         return sessionIdsToMatrixClient[sessionId]
     }
 
-    @Suppress("DEPRECATION")
-    fun restore(savedInstanceState: Bundle?) {
-        if (savedInstanceState == null || sessionIdsToMatrixClient.isNotEmpty()) return
-        val userIds = savedInstanceState.getSerializable(SAVE_INSTANCE_KEY) as? Array<UserId>
-        if (userIds.isNullOrEmpty()) return
+    @Suppress("UNCHECKED_CAST")
+    fun restore(state: SavedStateMap?) {
+        Timber.d("Restore state")
+        if (state == null || sessionIdsToMatrixClient.isNotEmpty()) return Unit.also {
+            Timber.w("Restore with non-empty map")
+        }
+        val sessionIds = state[SAVE_INSTANCE_KEY] as? Array<SessionId>
+        Timber.d("Restore matrix session keys = ${sessionIds?.map { it.value }}")
+        if (sessionIds.isNullOrEmpty()) return
         // Not ideal but should only happens in case of process recreation. This ensure we restore all the active sessions before restoring the node graphs.
         runBlocking {
-            userIds.forEach { userId ->
-                Timber.v("Restore matrix session: $userId")
-                authenticationService.restoreSession(userId)
+            sessionIds.forEach { sessionId ->
+                Timber.d("Restore matrix session: $sessionId")
+                authenticationService.restoreSession(sessionId)
                     .onSuccess { matrixClient ->
                         add(matrixClient)
                     }
@@ -75,9 +76,9 @@ class MatrixClientsHolder @Inject constructor(private val authenticationService:
         }
     }
 
-    fun onSaveInstanceState(outState: Bundle) {
+    fun save(state: MutableSavedStateMap) {
         val sessionKeys = sessionIdsToMatrixClient.keys.toTypedArray()
-        Timber.v("Save matrix session keys = $sessionKeys")
-        outState.putSerializable(SAVE_INSTANCE_KEY, sessionKeys)
+        Timber.d("Save matrix session keys = ${sessionKeys.map { it.value }}")
+        state[SAVE_INSTANCE_KEY] = sessionKeys
     }
 }

libraries/architecture/src/main/kotlin/io/element/android/libraries/architecture/BackstackNode.kt

@@ -19,6 +19,7 @@ package io.element.android.libraries.architecture
 import androidx.compose.runtime.Stable
 import com.bumble.appyx.core.children.ChildEntry
 import com.bumble.appyx.core.modality.BuildContext
+import com.bumble.appyx.core.node.Node
 import com.bumble.appyx.core.node.ParentNode
 import com.bumble.appyx.core.plugin.Plugin
 import com.bumble.appyx.navmodel.backstack.BackStack
@@ -39,4 +40,15 @@ abstract class BackstackNode<NavTarget : Any>(
     buildContext = buildContext,
     plugins = plugins,
     childKeepMode = childKeepMode,
-)
+) {
+    override fun onBuilt() {
+        super.onBuilt()
+        lifecycle.logLifecycle(this::class.java.simpleName)
+        whenChildAttached<Node> { _, child ->
+            // BackstackNode will be logged by their parent.
+            if (child !is BackstackNode<*>) {
+                child.lifecycle.logLifecycle(child::class.java.simpleName)
+            }
+        }
+    }
+}

libraries/architecture/src/main/kotlin/io/element/android/libraries/architecture/LifecycleExt.kt

@@ -0,0 +1,30 @@
+/*
+ * 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.libraries.architecture
+
+import androidx.lifecycle.Lifecycle
+import com.bumble.appyx.core.lifecycle.subscribe
+import timber.log.Timber
+
+fun Lifecycle.logLifecycle(name: String) {
+    subscribe(
+        onCreate = { Timber.tag("Lifecycle").d("onCreate $name") },
+        onPause = { Timber.tag("Lifecycle").d("onPause $name") },
+        onResume = { Timber.tag("Lifecycle").d("onResume $name") },
+        onDestroy = { Timber.tag("Lifecycle").d("onDestroy $name") },
+    )
+}

libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/room/RoomSummaryDataSource.kt

@@ -38,12 +38,13 @@ interface RoomSummaryDataSource {
 
 suspend fun RoomSummaryDataSource.awaitAllRoomsAreLoaded(timeout: Duration = Duration.INFINITE) {
     try {
+        Timber.d("awaitAllRoomsAreLoaded: wait")
         withTimeout(timeout) {
             allRoomsLoadingState().firstOrNull {
                 it is RoomSummaryDataSource.LoadingState.Loaded
             }
         }
     } catch (timeoutException: TimeoutCancellationException) {
-        Timber.v("AwaitAllRooms: no response after $timeout")
+        Timber.d("awaitAllRoomsAreLoaded: no response after $timeout")
     }
 }

libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/RoomListExtensions.kt

@@ -62,7 +62,7 @@ fun RoomListService.roomOrNull(roomId: String): RoomListItem? {
     return try {
         room(roomId)
     } catch (exception: RoomListException) {
-        Timber.e(exception, "Failed finding room with id=$roomId")
+        Timber.d(exception, "Failed finding room with id=$roomId.")
         return null
     }
 }