From 5dba1067d60ce6fe6fd52e29f67a3fb710edd412 Mon Sep 17 00:00:00 2001 From: Cody Henthorne Date: Tue, 24 May 2022 16:09:55 -0400 Subject: [PATCH] Fix conversation list memory leak. --- .../BindableConversationListItem.java | 4 +- .../ConversationListAdapter.java | 9 +- .../ConversationListFragment.java | 29 ++++- .../ConversationListItem.java | 113 +++++++++--------- .../ConversationListItemAction.java | 4 +- .../ConversationListItemInboxZero.java | 63 ---------- .../ConversationListSearchAdapter.java | 39 +++--- 7 files changed, 119 insertions(+), 142 deletions(-) delete mode 100644 app/src/main/java/org/thoughtcrime/securesms/conversationlist/ConversationListItemInboxZero.java diff --git a/app/src/main/java/org/thoughtcrime/securesms/BindableConversationListItem.java b/app/src/main/java/org/thoughtcrime/securesms/BindableConversationListItem.java index 6a487fff4..5b11c1fa2 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/BindableConversationListItem.java +++ b/app/src/main/java/org/thoughtcrime/securesms/BindableConversationListItem.java @@ -1,6 +1,7 @@ package org.thoughtcrime.securesms; import androidx.annotation.NonNull; +import androidx.lifecycle.LifecycleOwner; import org.thoughtcrime.securesms.conversationlist.model.ConversationSet; import org.thoughtcrime.securesms.database.model.ThreadRecord; @@ -11,7 +12,8 @@ import java.util.Set; public interface BindableConversationListItem extends Unbindable { - void bind(@NonNull ThreadRecord thread, + void bind(@NonNull LifecycleOwner lifecycleOwner, + @NonNull ThreadRecord thread, @NonNull GlideRequests glideRequests, @NonNull Locale locale, @NonNull Set typingThreads, @NonNull ConversationSet selectedConversations); diff --git a/app/src/main/java/org/thoughtcrime/securesms/conversationlist/ConversationListAdapter.java b/app/src/main/java/org/thoughtcrime/securesms/conversationlist/ConversationListAdapter.java index 14527df5d..86928a48d 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/conversationlist/ConversationListAdapter.java +++ b/app/src/main/java/org/thoughtcrime/securesms/conversationlist/ConversationListAdapter.java @@ -8,6 +8,7 @@ import android.widget.TextView; import androidx.annotation.NonNull; import androidx.annotation.Nullable; +import androidx.lifecycle.LifecycleOwner; import androidx.recyclerview.widget.DiffUtil; import androidx.recyclerview.widget.ListAdapter; import androidx.recyclerview.widget.RecyclerView; @@ -39,6 +40,7 @@ class ConversationListAdapter extends ListAdapter -{ +public final class ConversationListItem extends ConstraintLayout implements BindableConversationListItem, Unbindable { @SuppressWarnings("unused") private final static String TAG = Log.tag(ConversationListItem.class); - private final static Typeface BOLD_TYPEFACE = Typeface.create("sans-serif-medium", Typeface.NORMAL); - private final static Typeface LIGHT_TYPEFACE = Typeface.create("sans-serif", Typeface.NORMAL); + private final static Typeface BOLD_TYPEFACE = Typeface.create("sans-serif-medium", Typeface.NORMAL); + private final static Typeface LIGHT_TYPEFACE = Typeface.create("sans-serif", Typeface.NORMAL); - private final Rect conversationAvatarTouchDelegateBounds = new Rect(); - private final Rect newConversationAvatarTouchDelegateBounds = new Rect(); + private final Rect conversationAvatarTouchDelegateBounds = new Rect(); + private final Rect newConversationAvatarTouchDelegateBounds = new Rect(); + private final Observer recipientObserver = this::onRecipientChanged; + private final Observer displayBodyObserver = this::onDisplayBodyChanged; private Set typingThreads; private LiveRecipient recipient; @@ -174,7 +171,7 @@ public final class ConversationListItem extends ConstraintLayout newConversationAvatarTouchDelegateBounds.right = right; } - newConversationAvatarTouchDelegateBounds.top = top; + newConversationAvatarTouchDelegateBounds.top = top; newConversationAvatarTouchDelegateBounds.bottom = bottom; TouchDelegate currentDelegate = getTouchDelegate(); @@ -187,25 +184,24 @@ public final class ConversationListItem extends ConstraintLayout } @Override - public void bind(@NonNull ThreadRecord thread, + public void bind(@NonNull LifecycleOwner lifecycleOwner, + @NonNull ThreadRecord thread, @NonNull GlideRequests glideRequests, @NonNull Locale locale, @NonNull Set typingThreads, @NonNull ConversationSet selectedConversations) { - bindThread(thread, glideRequests, locale, typingThreads, selectedConversations, null); + bindThread(lifecycleOwner, thread, glideRequests, locale, typingThreads, selectedConversations, null); } - public void bindThread(@NonNull ThreadRecord thread, + public void bindThread(@NonNull LifecycleOwner lifecycleOwner, + @NonNull ThreadRecord thread, @NonNull GlideRequests glideRequests, @NonNull Locale locale, @NonNull Set typingThreads, @NonNull ConversationSet selectedConversations, @Nullable String highlightSubstring) { - observeRecipient(thread.getRecipient().live()); - observeDisplayBody(null); - this.threadId = thread.getThreadId(); this.glideRequests = glideRequests; this.unreadCount = thread.getUnreadCount(); @@ -214,6 +210,9 @@ public final class ConversationListItem extends ConstraintLayout this.locale = locale; this.highlightSubstring = highlightSubstring; + observeRecipient(lifecycleOwner, thread.getRecipient().live()); + observeDisplayBody(null, null); + if (highlightSubstring != null) { String name = recipient.get().isSelf() ? getContext().getString(R.string.note_to_self) : recipient.get().getDisplayName(getContext()); @@ -227,7 +226,7 @@ public final class ConversationListItem extends ConstraintLayout LiveData displayBody = getThreadDisplayBody(getContext(), thread, glideRequests, thumbSize, thumbTarget); setSubjectViewText(displayBody.getValue()); - observeDisplayBody(displayBody); + observeDisplayBody(lifecycleOwner, displayBody); if (thread.getDate() > 0) { CharSequence date = DateUtils.getBriefRelativeTimeSpanString(getContext(), locale, thread.getDate()); @@ -259,19 +258,20 @@ public final class ConversationListItem extends ConstraintLayout } } - public void bindContact(@NonNull Recipient contact, - @NonNull GlideRequests glideRequests, - @NonNull Locale locale, - @Nullable String highlightSubstring) + public void bindContact(@NonNull LifecycleOwner lifecycleOwner, + @NonNull Recipient contact, + @NonNull GlideRequests glideRequests, + @NonNull Locale locale, + @Nullable String highlightSubstring) { - observeRecipient(contact.live()); - observeDisplayBody(null); - setSubjectViewText(null); - this.glideRequests = glideRequests; this.locale = locale; this.highlightSubstring = highlightSubstring; + observeRecipient(lifecycleOwner, contact.live()); + observeDisplayBody(null, null); + setSubjectViewText(null); + fromView.setText(contact, SearchUtil.getHighlightedSpan(locale, SpanUtil::getMediumBoldSpan, new SpannableString(contact.getDisplayName(getContext())), highlightSubstring, SearchUtil.MATCH_ALL), true, null); setSubjectViewText(SearchUtil.getHighlightedSpan(locale, SpanUtil::getBoldSpan, contact.getE164().orElse(""), highlightSubstring, SearchUtil.MATCH_ALL)); dateView.setText(""); @@ -285,19 +285,20 @@ public final class ConversationListItem extends ConstraintLayout contactPhotoImage.setAvatar(glideRequests, recipient.get(), !batchMode); } - public void bindMessage(@NonNull MessageResult messageResult, - @NonNull GlideRequests glideRequests, - @NonNull Locale locale, - @Nullable String highlightSubstring) + public void bindMessage(@NonNull LifecycleOwner lifecycleOwner, + @NonNull MessageResult messageResult, + @NonNull GlideRequests glideRequests, + @NonNull Locale locale, + @Nullable String highlightSubstring) { - observeRecipient(messageResult.getConversationRecipient().live()); - observeDisplayBody(null); - setSubjectViewText(null); - this.glideRequests = glideRequests; this.locale = locale; this.highlightSubstring = highlightSubstring; + observeRecipient(lifecycleOwner, messageResult.getConversationRecipient().live()); + observeDisplayBody(null, null); + setSubjectViewText(null); + fromView.setText(recipient.get(), false); setSubjectViewText(SearchUtil.getHighlightedSpan(locale, SpanUtil::getBoldSpan, messageResult.getBodySnippet(), highlightSubstring, SearchUtil.MATCH_ALL)); dateView.setText(DateUtils.getBriefRelativeTimeSpanString(getContext(), locale, messageResult.getReceivedTimestampMs())); @@ -314,12 +315,12 @@ public final class ConversationListItem extends ConstraintLayout @Override public void unbind() { if (this.recipient != null) { - observeRecipient(null); + observeRecipient(null, null); setSelectedConversations(new ConversationSet()); contactPhotoImage.setAvatar(glideRequests, null, !batchMode); } - observeDisplayBody(null); + observeDisplayBody(null, null); } @Override @@ -332,7 +333,7 @@ public final class ConversationListItem extends ConstraintLayout if (recipient != null) { contactPhotoImage.setAvatar(glideRequests, recipient.get(), !batchMode); } - + if (batchMode && selected) { checkContainer.setVisibility(VISIBLE); uncheckedView.setVisibility(GONE); @@ -383,31 +384,31 @@ public final class ConversationListItem extends ConstraintLayout return lastSeen; } - private void observeRecipient(@Nullable LiveRecipient newRecipient) { + private void observeRecipient(@Nullable LifecycleOwner lifecycleOwner, @Nullable LiveRecipient newRecipient) { if (this.recipient != null) { - this.recipient.removeForeverObserver(this); + this.recipient.getLiveData().removeObserver(recipientObserver); } this.recipient = newRecipient; - if (this.recipient != null) { - this.recipient.observeForever(this); + if (lifecycleOwner != null && this.recipient != null) { + this.recipient.getLiveData().observe(lifecycleOwner, recipientObserver); } } - private void observeDisplayBody(@Nullable LiveData displayBody) { + private void observeDisplayBody(@Nullable LifecycleOwner lifecycleOwner, @Nullable LiveData displayBody) { if (displayBody == null && glideRequests != null) { glideRequests.clear(thumbTarget); } if (this.displayBody != null) { - this.displayBody.removeObserver(this); + this.displayBody.removeObserver(displayBodyObserver); } this.displayBody = displayBody; - if (this.displayBody != null) { - this.displayBody.observeForever(this); + if (lifecycleOwner != null && this.displayBody != null) { + this.displayBody.observe(lifecycleOwner, displayBodyObserver); } } @@ -424,7 +425,7 @@ public final class ConversationListItem extends ConstraintLayout if (MmsSmsColumns.Types.isBadDecryptType(thread.getType())) { deliveryStatusIndicator.setNone(); alertView.setFailed(); - } else if (!thread.isOutgoing() || + } else if (!thread.isOutgoing() || thread.isOutgoingAudioCall() || thread.isOutgoingVideoCall() || thread.isVerificationStatusChange()) @@ -470,8 +471,7 @@ public final class ConversationListItem extends ConstraintLayout unreadIndicator.setVisibility(View.VISIBLE); } - @Override - public void onRecipientChanged(@NonNull Recipient recipient) { + private void onRecipientChanged(@NonNull Recipient recipient) { if (this.recipient == null || !this.recipient.getId().equals(recipient.getId())) { Log.w(TAG, "Bad change! Local recipient doesn't match. Ignoring. Local: " + (this.recipient == null ? "null" : this.recipient.getId()) + ", Changed: " + recipient.getId()); return; @@ -531,7 +531,7 @@ public final class ConversationListItem extends ConstraintLayout } else if (SmsDatabase.Types.isJoinedType(thread.getType())) { return emphasisAdded(recipientToStringAsync(thread.getRecipient().getId(), r -> new SpannableString(context.getString(R.string.ThreadRecord_s_is_on_signal, r.getDisplayName(context))))); } else if (SmsDatabase.Types.isExpirationTimerUpdate(thread.getType())) { - int seconds = (int)(thread.getExpiresIn() / 1000); + int seconds = (int) (thread.getExpiresIn() / 1000); if (seconds <= 0) { return emphasisAdded(context, context.getString(R.string.ThreadRecord_disappearing_messages_disabled), R.drawable.ic_update_timer_disabled_16, defaultTint); } @@ -649,7 +649,9 @@ public final class ConversationListItem extends ConstraintLayout return new SpannableString(builder); } - /** After a short delay, if the main data hasn't shown yet, then a loading message is displayed. */ + /** + * After a short delay, if the main data hasn't shown yet, then a loading message is displayed. + */ private static @NonNull LiveData whileLoadingShow(@NonNull String loading, @NonNull LiveData string) { return LiveDataUtil.until(string, LiveDataUtil.delay(250, new SpannableString(loading))); } @@ -682,9 +684,9 @@ public final class ConversationListItem extends ConstraintLayout return Transformations.map(description, sequence -> { SpannableString spannable = new SpannableString(sequence); spannable.setSpan(new StyleSpan(Typeface.ITALIC), - 0, - sequence.length(), - Spannable.SPAN_EXCLUSIVE_EXCLUSIVE); + 0, + sequence.length(), + Spannable.SPAN_EXCLUSIVE_EXCLUSIVE); return spannable; }); } @@ -699,8 +701,7 @@ public final class ConversationListItem extends ConstraintLayout } } - @Override - public void onChanged(SpannableString spannableString) { + private void onDisplayBodyChanged(SpannableString spannableString) { setSubjectViewText(spannableString); if (typingThreads != null) { diff --git a/app/src/main/java/org/thoughtcrime/securesms/conversationlist/ConversationListItemAction.java b/app/src/main/java/org/thoughtcrime/securesms/conversationlist/ConversationListItemAction.java index 9f4df2a9e..a3b01a760 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/conversationlist/ConversationListItemAction.java +++ b/app/src/main/java/org/thoughtcrime/securesms/conversationlist/ConversationListItemAction.java @@ -6,6 +6,7 @@ import android.widget.FrameLayout; import android.widget.TextView; import androidx.annotation.NonNull; +import androidx.lifecycle.LifecycleOwner; import org.thoughtcrime.securesms.BindableConversationListItem; import org.thoughtcrime.securesms.R; @@ -39,7 +40,8 @@ public class ConversationListItemAction extends FrameLayout implements BindableC } @Override - public void bind(@NonNull ThreadRecord thread, + public void bind(@NonNull LifecycleOwner lifecycleOwner, + @NonNull ThreadRecord thread, @NonNull GlideRequests glideRequests, @NonNull Locale locale, @NonNull Set typingThreads, diff --git a/app/src/main/java/org/thoughtcrime/securesms/conversationlist/ConversationListItemInboxZero.java b/app/src/main/java/org/thoughtcrime/securesms/conversationlist/ConversationListItemInboxZero.java deleted file mode 100644 index aec9e9e84..000000000 --- a/app/src/main/java/org/thoughtcrime/securesms/conversationlist/ConversationListItemInboxZero.java +++ /dev/null @@ -1,63 +0,0 @@ -package org.thoughtcrime.securesms.conversationlist; - - -import android.content.Context; -import android.os.Build; -import android.util.AttributeSet; -import android.widget.LinearLayout; - -import androidx.annotation.NonNull; -import androidx.annotation.Nullable; -import androidx.annotation.RequiresApi; - -import org.thoughtcrime.securesms.BindableConversationListItem; -import org.thoughtcrime.securesms.conversationlist.model.ConversationSet; -import org.thoughtcrime.securesms.database.model.ThreadRecord; -import org.thoughtcrime.securesms.mms.GlideRequests; - -import java.util.Locale; -import java.util.Set; - -public class ConversationListItemInboxZero extends LinearLayout implements BindableConversationListItem { - public ConversationListItemInboxZero(Context context) { - super(context); - } - - public ConversationListItemInboxZero(Context context, @Nullable AttributeSet attrs) { - super(context, attrs); - } - - public ConversationListItemInboxZero(Context context, @Nullable AttributeSet attrs, int defStyleAttr) { - super(context, attrs, defStyleAttr); - } - - @RequiresApi(api = Build.VERSION_CODES.LOLLIPOP) - public ConversationListItemInboxZero(Context context, AttributeSet attrs, int defStyleAttr, int defStyleRes) { - super(context, attrs, defStyleAttr, defStyleRes); - } - - @Override - public void unbind() { - - } - - @Override - public void bind(@NonNull ThreadRecord thread, - @NonNull GlideRequests glideRequests, - @NonNull Locale locale, - @NonNull Set typingThreads, - @NonNull ConversationSet selectedConversations) - { - - } - - @Override - public void setSelectedConversations(@NonNull ConversationSet conversations) { - - } - - @Override - public void updateTypingIndicator(@NonNull Set typingThreads) { - - } -} diff --git a/app/src/main/java/org/thoughtcrime/securesms/conversationlist/ConversationListSearchAdapter.java b/app/src/main/java/org/thoughtcrime/securesms/conversationlist/ConversationListSearchAdapter.java index 9acf853a5..6424bb9cc 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/conversationlist/ConversationListSearchAdapter.java +++ b/app/src/main/java/org/thoughtcrime/securesms/conversationlist/ConversationListSearchAdapter.java @@ -7,6 +7,7 @@ import android.widget.TextView; import androidx.annotation.NonNull; import androidx.annotation.Nullable; +import androidx.lifecycle.LifecycleOwner; import androidx.recyclerview.widget.RecyclerView; import org.thoughtcrime.securesms.R; @@ -28,20 +29,23 @@ class ConversationListSearchAdapter extends RecyclerView.Adapter eventListener.onConversationClicked(conversationResult)); } - void bind(@NonNull Recipient contactResult, + void bind(@NonNull LifecycleOwner lifecycleOwner, + @NonNull Recipient contactResult, @NonNull GlideRequests glideRequests, @NonNull EventListener eventListener, @NonNull Locale locale, @Nullable String query) { - root.bindContact(contactResult, glideRequests, locale, query); + root.bindContact(lifecycleOwner, contactResult, glideRequests, locale, query); root.setOnClickListener(view -> eventListener.onContactClicked(contactResult)); } - void bind(@NonNull MessageResult messageResult, + void bind(@NonNull LifecycleOwner lifecycleOwner, + @NonNull MessageResult messageResult, @NonNull GlideRequests glideRequests, @NonNull EventListener eventListener, @NonNull Locale locale, @Nullable String query) { - root.bindMessage(messageResult, glideRequests, locale, query); + root.bindMessage(lifecycleOwner, messageResult, glideRequests, locale, query); root.setOnClickListener(view -> eventListener.onMessageClicked(messageResult)); }