From 057ffdbaaf71a725cccf89930566c9e67a67f93b Mon Sep 17 00:00:00 2001 From: Cody Henthorne Date: Mon, 10 Jun 2024 14:54:02 -0400 Subject: [PATCH] Fix conversation memory leak. --- .../v2/items/V2ConversationItemShapeTest.kt | 3 +- .../conversation/v2/ConversationAdapterV2.kt | 58 +------------------ .../v2/items/V2ConversationContext.kt | 2 + .../V2ConversationItemTextOnlyViewHolder.kt | 10 ++-- .../securesms/recipients/LiveRecipient.java | 7 +++ 5 files changed, 17 insertions(+), 63 deletions(-) diff --git a/app/src/androidTest/java/org/thoughtcrime/securesms/conversation/v2/items/V2ConversationItemShapeTest.kt b/app/src/androidTest/java/org/thoughtcrime/securesms/conversation/v2/items/V2ConversationItemShapeTest.kt index 8641f619d9..f135dab675 100644 --- a/app/src/androidTest/java/org/thoughtcrime/securesms/conversation/v2/items/V2ConversationItemShapeTest.kt +++ b/app/src/androidTest/java/org/thoughtcrime/securesms/conversation/v2/items/V2ConversationItemShapeTest.kt @@ -7,6 +7,7 @@ package org.thoughtcrime.securesms.conversation.v2.items import android.net.Uri import android.view.View +import androidx.lifecycle.LifecycleOwner import androidx.lifecycle.Observer import com.bumptech.glide.RequestManager import io.mockk.mockk @@ -203,8 +204,8 @@ class V2ConversationItemShapeTest { private val colorizer = Colorizer() + override val lifecycleOwner: LifecycleOwner = mockk(relaxed = true) override val displayMode: ConversationItemDisplayMode = ConversationItemDisplayMode.Standard - override val clickListener: ConversationAdapter.ItemClickListener = FakeConversationItemClickListener override val selectedItems: Set = emptySet() override val isMessageRequestAccepted: Boolean = true diff --git a/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/ConversationAdapterV2.kt b/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/ConversationAdapterV2.kt index 08f13e10a5..8e9fd1fe1c 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/ConversationAdapterV2.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/ConversationAdapterV2.kt @@ -66,7 +66,7 @@ import java.util.Locale import java.util.Optional class ConversationAdapterV2( - private val lifecycleOwner: LifecycleOwner, + override val lifecycleOwner: LifecycleOwner, override val requestManager: RequestManager, override val clickListener: ItemClickListener, private var hasWallpaper: Boolean, @@ -356,34 +356,6 @@ class ConversationAdapterV2( } } - private inner class OutgoingTextOnlyViewHolder(itemView: View) : ConversationViewHolder(itemView) { - override fun bind(model: OutgoingTextOnly) { - bindable.setEventListener(clickListener) - - if (bindPayloadsIfAvailable()) { - return - } - - bindable.bind( - lifecycleOwner, - model.conversationMessage, - previousMessage, - nextMessage, - requestManager, - Locale.getDefault(), - _selected, - model.conversationMessage.threadRecipient, - searchQuery, - false, - hasWallpaper && displayMode.displayWallpaper(), - isMessageRequestAccepted, - model.conversationMessage == inlineContent, - colorizer, - displayMode - ) - } - } - private inner class OutgoingMediaViewHolder(itemView: View) : ConversationViewHolder(itemView) { override fun bind(model: OutgoingMedia) { bindable.setEventListener(clickListener) @@ -413,34 +385,6 @@ class ConversationAdapterV2( } } - private inner class IncomingTextOnlyViewHolder(itemView: View) : ConversationViewHolder(itemView) { - override fun bind(model: IncomingTextOnly) { - bindable.setEventListener(clickListener) - - if (bindPayloadsIfAvailable()) { - return - } - - bindable.bind( - lifecycleOwner, - model.conversationMessage, - previousMessage, - nextMessage, - requestManager, - Locale.getDefault(), - _selected, - model.conversationMessage.threadRecipient, - searchQuery, - false, - hasWallpaper && displayMode.displayWallpaper(), - isMessageRequestAccepted, - model.conversationMessage == inlineContent, - colorizer, - displayMode - ) - } - } - private inner class IncomingMediaViewHolder(itemView: View) : ConversationViewHolder(itemView) { override fun bind(model: IncomingMedia) { bindable.setEventListener(clickListener) diff --git a/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/items/V2ConversationContext.kt b/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/items/V2ConversationContext.kt index e58f6c9792..7fd75828c4 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/items/V2ConversationContext.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/items/V2ConversationContext.kt @@ -5,6 +5,7 @@ package org.thoughtcrime.securesms.conversation.v2.items +import androidx.lifecycle.LifecycleOwner import com.bumptech.glide.RequestManager import org.thoughtcrime.securesms.conversation.ConversationAdapter import org.thoughtcrime.securesms.conversation.ConversationItemDisplayMode @@ -17,6 +18,7 @@ import org.thoughtcrime.securesms.database.model.MessageRecord * visible to an inner class. */ interface V2ConversationContext { + val lifecycleOwner: LifecycleOwner val requestManager: RequestManager val displayMode: ConversationItemDisplayMode val clickListener: ConversationAdapter.ItemClickListener diff --git a/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/items/V2ConversationItemTextOnlyViewHolder.kt b/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/items/V2ConversationItemTextOnlyViewHolder.kt index 29f7f5bb44..ad00ce1fee 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/items/V2ConversationItemTextOnlyViewHolder.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/items/V2ConversationItemTextOnlyViewHolder.kt @@ -24,6 +24,7 @@ import android.view.View import android.view.ViewGroup import androidx.core.content.ContextCompat import androidx.core.view.updateLayoutParams +import androidx.lifecycle.Observer import androidx.recyclerview.widget.RecyclerView import org.signal.core.util.StringUtil import org.signal.core.util.dp @@ -44,7 +45,6 @@ import org.thoughtcrime.securesms.database.model.MmsMessageRecord import org.thoughtcrime.securesms.dependencies.AppDependencies import org.thoughtcrime.securesms.keyvalue.SignalStore import org.thoughtcrime.securesms.recipients.Recipient -import org.thoughtcrime.securesms.recipients.RecipientForeverObserver import org.thoughtcrime.securesms.recipients.RecipientId import org.thoughtcrime.securesms.util.InterceptableLongClickCopyLinkSpan import org.thoughtcrime.securesms.util.LongClickMovementMethod @@ -70,7 +70,7 @@ open class V2ConversationItemTextOnlyViewHolder>( private val binding: V2ConversationItemTextOnlyBindingBridge, private val conversationContext: V2ConversationContext, footerDelegate: V2FooterPositionDelegate = V2FooterPositionDelegate(binding) -) : V2ConversationItemViewHolder(binding.root, conversationContext), Multiselectable, InteractiveConversationElement, RecipientForeverObserver { +) : V2ConversationItemViewHolder(binding.root, conversationContext), Multiselectable, InteractiveConversationElement, Observer { companion object { private val STYLE_FACTORY = SearchUtil.StyleFactory { arrayOf(BackgroundColorSpan(Color.YELLOW), ForegroundColorSpan(Color.BLACK)) } @@ -210,12 +210,12 @@ open class V2ConversationItemTextOnlyViewHolder>( check(model is ConversationMessageElement) if (this::conversationMessage.isInitialized) { - conversationMessage.messageRecord.fromRecipient.live().removeForeverObserver(this) + conversationMessage.messageRecord.fromRecipient.live().removeObserver(this) } conversationMessage = model.conversationMessage if (conversationMessage.threadRecipient.isGroup) { - conversationMessage.messageRecord.fromRecipient.live().observeForever(this) + conversationMessage.messageRecord.fromRecipient.live().observe(conversationContext.lifecycleOwner, this) } shape = shapeDelegate.setMessageShape( @@ -790,7 +790,7 @@ open class V2ConversationItemTextOnlyViewHolder>( } } - override fun onRecipientChanged(recipient: Recipient) { + override fun onChanged(recipient: Recipient) { presentSender() } } diff --git a/app/src/main/java/org/thoughtcrime/securesms/recipients/LiveRecipient.java b/app/src/main/java/org/thoughtcrime/securesms/recipients/LiveRecipient.java index fba94fb12f..80330fbbff 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/recipients/LiveRecipient.java +++ b/app/src/main/java/org/thoughtcrime/securesms/recipients/LiveRecipient.java @@ -102,6 +102,13 @@ public final class LiveRecipient { ThreadUtil.postToMain(() -> observableLiveData.observe(owner, observer)); } + /** + * Removes observer of this data. + */ + public void removeObserver(@NonNull Observer observer) { + ThreadUtil.runOnMain(() -> observableLiveData.removeObserver(observer)); + } + /** * Removes all observers of this data registered for the given LifecycleOwner. */