Merge pull request #1484 from AL-Session/SES1901_LastSentMsgANR

SES1901- Last sent msg ANR fix

app/src/main/java/org/thoughtcrime/securesms/conversation/v2/ConversationActivityV2.kt

@@ -298,6 +298,13 @@ class ConversationActivityV2 : PassphraseRequiredActionBarActivity(), InputBarDe
     private val reverseMessageList = false
 
     private val adapter by lazy {
+
+        // To prevent repeated attachment download jobs being spawned for any that fail we'll keep
+        // track of the attachment Ids we've attempted to download. Without this guard mechanism
+        // then when the retry limit for a failed job is reached another job is immediately spawned
+        // to download the same attachment (endlessly).
+        val alreadyAttemptedAttachmentDownloads = mutableSetOf<Long>()
+
         val cursor = mmsSmsDb.getConversation(viewModel.threadId, reverseMessageList)
         val adapter = ConversationAdapter(
             this,
@@ -325,9 +332,14 @@ class ConversationActivityV2 : PassphraseRequiredActionBarActivity(), InputBarDe
                 }
             },
             onAttachmentNeedsDownload = { attachmentId, mmsId ->
-                // Start download (on IO thread)
+
-                lifecycleScope.launch(Dispatchers.IO) {
+                alreadyAttemptedAttachmentDownloads.takeUnless {
-                    JobQueue.shared.add(AttachmentDownloadJob(attachmentId, mmsId))
+                    attachmentId in alreadyAttemptedAttachmentDownloads
+                }.let {
+                    alreadyAttemptedAttachmentDownloads += attachmentId
+                    lifecycleScope.launch(Dispatchers.IO) {
+                        JobQueue.shared.add(AttachmentDownloadJob(attachmentId, mmsId))
+                    }
                 }
             },
             glide = glide,
@@ -335,8 +347,8 @@ class ConversationActivityV2 : PassphraseRequiredActionBarActivity(), InputBarDe
         )
         adapter.visibleMessageViewDelegate = this
 
-        // Register an AdapterDataObserver to scroll us to the bottom of the RecyclerView if we're
+        // Register an AdapterDataObserver to scroll us to the bottom of the RecyclerView for if
-        // already near the the bottom and the data changes.
+        // we're already near the the bottom and the data changes.
         adapter.registerAdapterDataObserver(ConversationAdapterDataObserver(binding?.conversationRecyclerView!!, adapter))
 
         adapter
@@ -360,6 +372,7 @@ class ConversationActivityV2 : PassphraseRequiredActionBarActivity(), InputBarDe
     private var currentLastVisibleRecyclerViewIndex:  Int = RecyclerView.NO_POSITION
     private var recyclerScrollState: Int = RecyclerView.SCROLL_STATE_IDLE
 
+
     // region Settings
     companion object {
         // Extras
@@ -375,6 +388,7 @@ class ConversationActivityV2 : PassphraseRequiredActionBarActivity(), InputBarDe
         const val PICK_FROM_LIBRARY = 12
         const val INVITE_CONTACTS = 124
 
+        var lastSentMessageId = -1L;
     }
     // endregion
 
@@ -501,6 +515,9 @@ class ConversationActivityV2 : PassphraseRequiredActionBarActivity(), InputBarDe
         viewModel.run {
             binding?.toolbarContent?.update(recipient ?: return, openGroup, expirationConfiguration)
         }
+
+        // Update our last sent message Id on startup / resume (resume is called after onCreate)
+        lastSentMessageId = mmsSmsDb.getLastOutgoingMessage(viewModel.threadId)
     }
 
     override fun onPause() {
@@ -2209,6 +2226,11 @@ class ConversationActivityV2 : PassphraseRequiredActionBarActivity(), InputBarDe
                 // to the bottom of long messages as required by Jira SES-789 / GitHub 1364).
                 recyclerView.scrollToPosition(adapter.itemCount)
             }
+
+            // Update our cached last sent message to ensure we have accurate details.
+            // Note: This `onChanged` method is not triggered when scrolling so should minimally
+            // affect performance.
+            lastSentMessageId = mmsSmsDb.getLastOutgoingMessage(viewModel.threadId)
         }
     }
 

app/src/main/java/org/thoughtcrime/securesms/conversation/v2/ConversationAdapter.kt

@@ -59,7 +59,6 @@ class ConversationAdapter(
     private val contactCache = SparseArray<Contact>(100)
     private val contactLoadedCache = SparseBooleanArray(100)
     private val lastSeen = AtomicLong(originalLastSeen)
-    private var lastSentMessageId: Long = -1L
 
     init {
         lifecycleCoroutineScope.launch(IO) {
@@ -139,8 +138,7 @@ class ConversationAdapter(
                         senderId,
                         lastSeen.get(),
                         visibleMessageViewDelegate,
-                        onAttachmentNeedsDownload,
+                        onAttachmentNeedsDownload
-                        lastSentMessageId
                 )
 
                 if (!message.isDeleted) {
@@ -216,8 +214,7 @@ class ConversationAdapter(
         if (cursorHasContent) {
             val thisThreadId = cursor.getLong(4) // Column index 4 is "thread_id"
             if (thisThreadId != -1L) {
-                val thisUsersSessionId = TextSecurePreferences.getLocalNumber(context)
+                return messageDB.getLastOutgoingMessage(thisThreadId)
-                return messageDB.getLastSentMessageFromSender(thisThreadId, thisUsersSessionId)
             }
         }
         return -1L
@@ -243,11 +240,6 @@ class ConversationAdapter(
         toDeselect.iterator().forEach { (pos, record) ->
             onDeselect(record, pos)
         }
-
-        // This value gets updated here ONLY when the cursor changes, and the value is then passed
-        // through to `VisibleMessageView.bind` each time we bind via `onBindItemViewHolder`, above.
-        // If there are no messages then lastSentMessageId is assigned the value -1L.
-        if (cursor != null) { lastSentMessageId = getLastSentMessageId(cursor) }
     }
 
     fun findLastSeenItemPosition(lastSeenTimestamp: Long): Int? {

app/src/main/java/org/thoughtcrime/securesms/conversation/v2/messages/VisibleMessageView.kt

@@ -134,8 +134,7 @@ class VisibleMessageView : LinearLayout {
         senderSessionID: String,
         lastSeen: Long,
         delegate: VisibleMessageViewDelegate? = null,
-        onAttachmentNeedsDownload: (Long, Long) -> Unit,
+        onAttachmentNeedsDownload: (Long, Long) -> Unit
-        lastSentMessageId: Long
     ) {
         replyDisabled = message.isOpenGroupInvitation
         val threadID = message.threadId
@@ -303,8 +302,7 @@ class VisibleMessageView : LinearLayout {
 
         // --- If we got here then we know the message is outgoing ---
 
-        val thisUsersSessionId = TextSecurePreferences.getLocalNumber(context)
+        val lastSentMessageId = ConversationActivityV2.lastSentMessageId;
-        val lastSentMessageId = mmsSmsDb.getLastSentMessageFromSender(message.threadId, thisUsersSessionId)
         val isLastSentMessage = lastSentMessageId == message.id
 
         // ----- Case ii.) Message is outgoing but NOT scheduled to disappear -----

app/src/main/java/org/thoughtcrime/securesms/database/MmsSmsColumns.java

@@ -9,7 +9,11 @@ public interface MmsSmsColumns {
   public static final String THREAD_ID                = "thread_id";
   public static final String READ                     = "read";
   public static final String BODY                     = "body";
+
+  // This is the address of the message recipient, which may be a single user, a group, or a community!
+  // It is NOT the address of the sender of any given message!
   public static final String ADDRESS                  = "address";
+
   public static final String ADDRESS_DEVICE_ID        = "address_device_id";
   public static final String DELIVERY_RECEIPT_COUNT   = "delivery_receipt_count";
   public static final String READ_RECEIPT_COUNT       = "read_receipt_count";

app/src/main/java/org/thoughtcrime/securesms/database/MmsSmsDatabase.java

@@ -295,15 +295,7 @@ public class MmsSmsDatabase extends Database {
     return identifiedMessages;
   }
 
-  public long getLastSentMessageFromSender(long threadId, String serializedAuthor) {
+  public long getLastOutgoingMessage(long threadId) {
-
-    // Early exit
-    boolean isOwnNumber = Util.isOwnNumber(context, serializedAuthor);
-    if (!isOwnNumber) {
-      Log.i(TAG, "Asked to find last sent message but sender isn't us - returning null.");
-      return -1;
-    }
-
     String order = MmsSmsColumns.NORMALIZED_DATE_SENT + " DESC";
     String selection = MmsSmsColumns.THREAD_ID + " = " + threadId;
 
@@ -312,7 +304,9 @@ public class MmsSmsDatabase extends Database {
       try (MmsSmsDatabase.Reader reader = readerFor(cursor)) {
         MessageRecord messageRecord;
         while ((messageRecord = reader.getNext()) != null) {
-          if (messageRecord.isOutgoing()) { return messageRecord.id; }
+          // Note: We rely on the message order to get us the most recent outgoing message - so we
+          // take the first outgoing message we find as the last outgoing message.
+          if (messageRecord.isOutgoing()) return messageRecord.id;
         }
       }
     }