fix(message-list): onSaveInstance crash (#10583)

Author: coreycb

legacy/ui/legacy/src/main/java/com/fsck/k9/ui/messagelist/BaseMessageListFragment.kt

@@ -3,7 +3,6 @@ package com.fsck.k9.ui.messagelist
 import android.app.SearchManager
 import android.content.Context
 import android.content.Intent
-import android.os.Build
 import android.os.Bundle
 import android.os.SystemClock
 import android.view.LayoutInflater
@@ -38,7 +37,6 @@ import androidx.fragment.app.setFragmentResultListener
 import androidx.lifecycle.Lifecycle
 import androidx.lifecycle.Observer
 import androidx.lifecycle.lifecycleScope
-import androidx.lifecycle.repeatOnLifecycle
 import androidx.recyclerview.widget.RecyclerView
 import androidx.swiperefreshlayout.widget.SwipeRefreshLayout
 import app.k9mail.core.android.common.contact.ContactRepository
@@ -81,9 +79,7 @@ import kotlin.time.Clock
 import kotlin.time.ExperimentalTime
 import kotlinx.collections.immutable.persistentSetOf
 import kotlinx.coroutines.Dispatchers
-import kotlinx.coroutines.flow.Flow
 import kotlinx.coroutines.flow.StateFlow
-import kotlinx.coroutines.flow.collectLatest
 import kotlinx.coroutines.flow.distinctUntilChanged
 import kotlinx.coroutines.flow.drop
 import kotlinx.coroutines.flow.launchIn
@@ -106,8 +102,8 @@ import net.thunderbird.core.featureflag.FeatureFlagProvider
 import net.thunderbird.core.featureflag.FeatureFlagResult
 import net.thunderbird.core.logging.Logger
 import net.thunderbird.core.outcome.Outcome
-import net.thunderbird.core.preference.GeneralSettings
 import net.thunderbird.core.preference.GeneralSettingsManager
+import net.thunderbird.core.preference.display.visualSettings.message.list.DisplayMessageListSettings
 import net.thunderbird.core.preference.interaction.InteractionSettings
 import net.thunderbird.core.ui.theme.api.FeatureThemeProvider
 import net.thunderbird.feature.account.AccountId
@@ -273,6 +269,8 @@ abstract class BaseMessageListFragment :
     private var messageListSwipeCallback: MessageListSwipeCallback? = null
     private val interactionSettings: InteractionSettings
         get() = generalSettingsManager.getConfig().interaction
+    private val messageListSettings: DisplayMessageListSettings
+        get() = generalSettingsManager.getConfig().display.visualSettings.messageListSettings
 
     /**
      * Set this to `true` when the fragment should be considered active. When active, the fragment adds its actions to
@@ -287,10 +285,6 @@ abstract class BaseMessageListFragment :
             maybeHideFloatingActionButton()
         }
 
-    private lateinit var messageListAppearance: MessageListAppearance
-    private var pendingMessageListInfo: MessageListInfo? = null
-    private var pendingAdapterDependentFunctionExecution = mutableListOf<() -> Unit>()
-
     fun isSearchViewCollapsed(): Boolean {
         return searchView?.isIconified != false
     }
@@ -322,6 +316,12 @@ abstract class BaseMessageListFragment :
             return
         }
 
+        viewModel.getMessageListLiveData().observe(this) { messageListInfo: MessageListInfo ->
+            setMessageList(messageListInfo)
+        }
+
+        adapter = createMessageListAdapter()
+
         generalSettingsManager.getSettingsFlow()
             /**
              * Skips the first emitted item from the settings flow,
@@ -351,18 +351,6 @@ abstract class BaseMessageListFragment :
         initialSearchViewIconified = savedInstanceState.getBoolean(STATE_SEARCH_VIEW_ICONIFIED, true)
         val messageReferenceString = savedInstanceState.getString(STATE_ACTIVE_MESSAGE)
         activeMessage = MessageReference.parse(messageReferenceString)
-
-        messageListAppearance = requireNotNull(
-            if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.TIRAMISU) {
-                savedInstanceState.getParcelable(STATE_MESSAGE_LIST_APPEARANCE, MessageListAppearance::class.java)
-            } else {
-                @Suppress("DEPRECATION")
-                savedInstanceState.getParcelable(STATE_MESSAGE_LIST_APPEARANCE)
-            },
-        ) {
-            "Could not restore MessageListAppearance. Missing parcelable extra '$STATE_MESSAGE_LIST_APPEARANCE'. " +
-                "Extras: $savedInstanceState"
-        }
     }
 
     private fun restoreSelectedMessages(savedInstanceState: Bundle) {
@@ -416,7 +404,7 @@ abstract class BaseMessageListFragment :
             layoutInflater = layoutInflater,
             contactsPictureLoader = ContactPicture.getContactPictureLoader(),
             listItemListener = this,
-            appearance = ::messageListAppearance,
+            appearance = messageListAppearance,
             relativeDateTimeFormatter = RelativeDateTimeFormatter(requireContext(), clock),
             themeProvider = featureThemeProvider,
             featureFlagProvider = featureFlagProvider,
@@ -446,27 +434,6 @@ abstract class BaseMessageListFragment :
     }
 
     override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
-        lifecycleScope.launch {
-            viewLifecycleOwner.repeatOnLifecycle(state = Lifecycle.State.CREATED) {
-                fetchMessageListAppearance()
-                    .distinctUntilChanged()
-                    .collectLatest { appearance ->
-                        messageListAppearance = appearance
-                        if (recyclerView == null) {
-                            initializeRecyclerView(requireView())
-                        }
-                    }
-            }
-        }
-
-        viewModel.getMessageListLiveData().observe(viewLifecycleOwner) { messageListInfo: MessageListInfo ->
-            if (::adapter.isInitialized) {
-                setMessageList(messageListInfo)
-            } else {
-                pendingMessageListInfo = messageListInfo
-            }
-        }
-
         val menuHost: MenuHost = requireActivity()
 
         menuHost.addMenuProvider(
@@ -499,17 +466,6 @@ abstract class BaseMessageListFragment :
         }
     }
 
-    protected open suspend fun fetchMessageListAppearance(): Flow<MessageListAppearance> {
-        return generalSettingsManager
-            .getConfigFlow()
-            .map { config ->
-                if (showingThreadedList != config.display.inboxSettings.isThreadedViewEnabled) {
-                    showingThreadedList = config.display.inboxSettings.isThreadedViewEnabled
-                }
-                createMessageListAppearance(config)
-            }
-    }
-
     private fun initializeErrorLayout(view: View) {
         val errorMessageView = view.findViewById<MaterialTextView>(R.id.message_list_error_message)
         errorMessageView.text = getString(error!!.errorText)
@@ -518,6 +474,7 @@ abstract class BaseMessageListFragment :
     private fun initializeMessageListLayout(view: View) {
         initializeSwipeRefreshLayout(view)
         initializeFloatingActionButton(view)
+        initializeRecyclerView(view)
         initializeRecentChangesSnackbar()
 
         // This needs to be done before loading the message list below
@@ -602,12 +559,6 @@ abstract class BaseMessageListFragment :
     }
 
     private fun initializeRecyclerView(view: View) {
-        adapter = createMessageListAdapter()
-        pendingMessageListInfo?.let { messageListInfo ->
-            setMessageList(messageListInfo)
-            pendingMessageListInfo = null
-        }
-
         val recyclerView = view.findViewById<RecyclerView>(R.id.message_list)
 
         if (!isShowFloatingActionButton) {
@@ -658,11 +609,6 @@ abstract class BaseMessageListFragment :
 
         this.recyclerView = recyclerView
         this.itemTouchHelper = itemTouchHelper
-        if (pendingAdapterDependentFunctionExecution.isNotEmpty()) {
-            logger.debug(logTag) { "Executing pending adapter dependent functions" }
-            pendingAdapterDependentFunctionExecution.forEach { it() }
-            pendingAdapterDependentFunctionExecution.clear()
-        }
     }
 
     private fun requireCoordinatorLayout(): CoordinatorLayout {
@@ -761,17 +707,7 @@ abstract class BaseMessageListFragment :
         viewModel.loadMessageList(config, forceUpdate)
     }
 
-    private fun executeOnlyAfterAdapterIsReady(function: () -> Unit) {
-        if (::adapter.isInitialized.not()) {
-            pendingAdapterDependentFunctionExecution.add {
-                function()
-            }
-        } else {
-            function()
-        }
-    }
-
-    fun folderLoading(folderId: Long, loading: Boolean) = executeOnlyAfterAdapterIsReady {
+    fun folderLoading(folderId: Long, loading: Boolean) {
         currentFolder?.let {
             if (it.databaseId == folderId) {
                 it.loading = loading
@@ -897,7 +833,6 @@ abstract class BaseMessageListFragment :
         itemTouchHelper = null
         swipeRefreshLayout = null
         floatingActionButton = null
-        pendingAdapterDependentFunctionExecution.clear()
 
         if (isNewMessagesView && !requireActivity().isChangingConfigurations) {
             account?.id?.let { messagingController.clearNewMessages(it) }
@@ -924,25 +859,24 @@ abstract class BaseMessageListFragment :
         if (activeMessage != null) {
             outState.putString(STATE_ACTIVE_MESSAGE, activeMessage!!.toIdentityString())
         }
-        outState.putParcelable(STATE_MESSAGE_LIST_APPEARANCE, messageListAppearance)
     }
 
-    protected fun createMessageListAppearance(config: GeneralSettings): MessageListAppearance {
-        val displaySettings = config.display
-        val inboxSettings = displaySettings.inboxSettings
-        val messageListSettings = displaySettings.visualSettings.messageListSettings
-        return MessageListAppearance(
+    private val messageListAppearance: MessageListAppearance
+        get() = MessageListAppearance(
             fontSizes = K9.fontSizes,
             previewLines = messageListSettings.previewLines,
-            stars = !isOutbox && displaySettings.inboxSettings.isShowMessageListStars,
-            senderAboveSubject = inboxSettings.isMessageListSenderAboveSubject,
+            stars = !isOutbox && generalSettingsManager.getConfig().display.inboxSettings.isShowMessageListStars,
+            senderAboveSubject = generalSettingsManager
+                .getConfig()
+                .display
+                .inboxSettings
+                .isMessageListSenderAboveSubject,
             showContactPicture = messageListSettings.isShowContactPicture,
             showingThreadedList = showingThreadedList,
             backGroundAsReadIndicator = messageListSettings.isUseBackgroundAsUnreadIndicator,
             showAccountIndicator = isShowAccountIndicator,
             density = messageListSettings.uiDensity,
         )
-    }
 
     private fun getFolderInfoHolder(account: LegacyAccount, folderId: Long): FolderInfoHolder {
         val localStore = localStoreProvider.getInstanceByLegacyAccount(account)
@@ -2786,6 +2720,5 @@ abstract class BaseMessageListFragment :
         protected const val STATE_REMOTE_SEARCH_PERFORMED = "remoteSearchPerformed"
         protected const val STATE_SEARCH_VIEW_QUERY = "searchViewQuery"
         protected const val STATE_SEARCH_VIEW_ICONIFIED = "searchViewIconified"
-        protected const val STATE_MESSAGE_LIST_APPEARANCE = "messageListAppearance"
     }
 }

legacy/ui/legacy/src/main/java/com/fsck/k9/ui/messagelist/MessageListAdapter.kt

@@ -44,7 +44,7 @@ class MessageListAdapter internal constructor(
     private val layoutInflater: LayoutInflater,
     private val contactsPictureLoader: ContactPictureLoader,
     private val listItemListener: MessageListItemActionListener,
-    private val appearance: () -> MessageListAppearance,
+    private val appearance: MessageListAppearance,
     private val relativeDateTimeFormatter: RelativeDateTimeFormatter,
     private val themeProvider: FeatureThemeProvider,
     private val featureFlagProvider: FeatureFlagProvider,
@@ -350,7 +350,7 @@ class MessageListAdapter internal constructor(
     private fun calculateSelectionCount(): Int {
         return when {
             selected.isEmpty() -> 0
-            !appearance().showingThreadedList -> selected.size
+            !appearance.showingThreadedList -> selected.size
             else ->
                 viewItems
                     .asSequence()

legacy/ui/legacy/src/main/java/com/fsck/k9/ui/messagelist/MessageListAppearance.kt

@@ -1,15 +1,10 @@
 package com.fsck.k9.ui.messagelist
 
-import android.os.Parcelable
 import com.fsck.k9.FontSizes
-import kotlinx.parcelize.IgnoredOnParcel
-import kotlinx.parcelize.Parcelize
 import net.thunderbird.core.preference.display.visualSettings.message.list.UiDensity
 
-@Parcelize
 data class MessageListAppearance(
-    @IgnoredOnParcel
-    val fontSizes: FontSizes = FontSizes(),
+    val fontSizes: FontSizes,
     val previewLines: Int,
     val stars: Boolean,
     val senderAboveSubject: Boolean,
@@ -21,4 +16,4 @@ data class MessageListAppearance(
      */
     val showAccountIndicator: Boolean,
     val density: UiDensity,
-) : Parcelable
+)

legacy/ui/legacy/src/main/java/com/fsck/k9/ui/messagelist/MessageListFragment.kt

@@ -7,17 +7,14 @@ import androidx.core.os.bundleOf
 import androidx.lifecycle.Lifecycle
 import androidx.lifecycle.lifecycleScope
 import androidx.lifecycle.repeatOnLifecycle
-import kotlinx.coroutines.flow.Flow
 import kotlinx.coroutines.flow.SharingStarted
 import kotlinx.coroutines.flow.StateFlow
 import kotlinx.coroutines.flow.map
-import kotlinx.coroutines.flow.mapNotNull
 import kotlinx.coroutines.flow.stateIn
 import kotlinx.coroutines.launch
 import net.thunderbird.core.common.action.SwipeActions
 import net.thunderbird.feature.account.AccountId
 import net.thunderbird.feature.account.AccountIdFactory
-import net.thunderbird.feature.mail.message.list.preferences.MessageListPreferences
 import net.thunderbird.feature.mail.message.list.ui.MessageListContract
 import net.thunderbird.feature.search.legacy.LocalMessageSearch
 import net.thunderbird.feature.search.legacy.serialization.LocalMessageSearchSerializer
@@ -58,21 +55,6 @@ class MessageListFragment : BaseMessageListFragment() {
         }
     }
 
-    override suspend fun fetchMessageListAppearance(): Flow<MessageListAppearance> = viewModel
-        .state
-        .mapNotNull { state -> state.preferences?.toMessageListAppearance() }
-
-    private fun MessageListPreferences.toMessageListAppearance(): MessageListAppearance = MessageListAppearance(
-        previewLines = excerptLines,
-        stars = showFavouriteButton,
-        senderAboveSubject = senderAboveSubject,
-        showContactPicture = showMessageAvatar,
-        showingThreadedList = groupConversations,
-        backGroundAsReadIndicator = colorizeBackgroundWhenRead,
-        showAccountIndicator = isShowAccountIndicator,
-        density = density,
-    )
-
     companion object Factory : BaseMessageListFragment.Factory {
         override fun newInstance(
             search: LocalMessageSearch,

legacy/ui/legacy/src/main/java/com/fsck/k9/ui/messagelist/item/ComposableMessageViewHolder.kt

@@ -19,7 +19,7 @@ class ComposableMessageViewHolder(
     private val onLongClick: (MessageListItem) -> Unit,
     private val onAvatarClick: (MessageListItem) -> Unit,
     private val onFavouriteClick: (MessageListItem) -> Unit,
-    private val appearance: () -> MessageListAppearance,
+    private val appearance: MessageListAppearance,
     private val contactRepository: ContactRepository,
     private val avatarMonogramCreator: AvatarMonogramCreator,
 ) : MessageListViewHolder(composeView) {
@@ -41,7 +41,7 @@ class ComposableMessageViewHolder(
                     onLongClick = { onLongClick(item) },
                     onAvatarClick = { onAvatarClick(item) },
                     onFavouriteClick = { onFavouriteClick(item) },
-                    appearance = appearance(),
+                    appearance = appearance,
                 )
             }
         }
@@ -58,7 +58,7 @@ class ComposableMessageViewHolder(
             onLongClick: (MessageListItem) -> Unit,
             onFavouriteClick: (MessageListItem) -> Unit,
             onAvatarClick: (MessageListItem) -> Unit,
-            appearance: () -> MessageListAppearance,
+            appearance: MessageListAppearance,
         ): ComposableMessageViewHolder {
             val composeView = ComposeView(context)