Check for concurrent cache updates.

briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentItem.java

@@ -135,10 +135,6 @@ public class AttachmentItem implements Parcelable {
 		return toHexString(instanceId);
 	}
 
-	boolean hasSize() {
-		return width != 0 && height != 0;
-	}
-
 	@Override
 	public int describeContents() {
 		return 0;
@@ -156,6 +152,10 @@ public class AttachmentItem implements Parcelable {
 		dest.writeString(state.name());
 	}
 
+	/**
+	 * This is used to identity if two items are the same,
+	 * irrespective of their state or size.
+	 */
 	@Override
 	public boolean equals(@Nullable Object o) {
 		return o instanceof AttachmentItem &&
@@ -164,4 +164,8 @@ public class AttachmentItem implements Parcelable {
 				);
 	}
 
+	@Override
+	public int hashCode() {
+		return header.getMessageId().hashCode();
+	}
 }

briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentRetriever.java

@@ -49,6 +49,13 @@ public interface AttachmentRetriever {
 	 * Loads an {@link AttachmentItem}
 	 * that arrived via an {@link AttachmentReceivedEvent}
 	 * and notifies the associated {@link LiveData}.
+	 *
+	 * Note that you need to call {@link #getAttachmentItems(PrivateMessageHeader)}
+	 * first to get the LiveData.
+	 *
+	 * It is possible that no LiveData is available,
+	 * because the message of the AttachmentItem did not arrive, yet.
+	 * In this case, the load wil be deferred until the message arrives.
 	 */
 	@DatabaseExecutor
 	void loadAttachmentItem(MessageId attachmentId);

briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentRetrieverImpl.java

@@ -15,8 +15,8 @@ import java.io.BufferedInputStream;
 import java.io.InputStream;
 import java.util.ArrayList;
 import java.util.List;
-import java.util.Map;
 import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentMap;
 import java.util.concurrent.Executor;
 import java.util.logging.Logger;
 
@@ -50,9 +50,9 @@ class AttachmentRetrieverImpl implements AttachmentRetriever {
 	private final int minWidth, maxWidth;
 	private final int minHeight, maxHeight;
 
-	private final Map<MessageId, MutableLiveData<AttachmentItem>>
+	private final ConcurrentMap<MessageId, MutableLiveData<AttachmentItem>>
 			itemsWithSize = new ConcurrentHashMap<>();
-	private final Map<MessageId, MutableLiveData<AttachmentItem>>
+	private final ConcurrentMap<MessageId, MutableLiveData<AttachmentItem>>
 			itemsWithoutSize = new ConcurrentHashMap<>();
 
 	@Inject
@@ -99,15 +99,25 @@ class AttachmentRetrieverImpl implements AttachmentRetriever {
 			if (liveData == null) {
 				AttachmentItem item = new AttachmentItem(h,
 						defaultSize, defaultSize, LOADING);
-				final MutableLiveData<AttachmentItem> finalLiveData =
+				liveData = new MutableLiveData<>(item);
-						new MutableLiveData<>(item);
+				// add new LiveData to cache, checking for concurrent updates
-				// kick-off loading of attachment, will post to live data
+				MutableLiveData<AttachmentItem> oldLiveData;
-				dbExecutor.execute(
+				if (needsSize) {
-						() -> loadAttachmentItem(h, needsSize, finalLiveData));
+					oldLiveData = itemsWithSize.putIfAbsent(h.getMessageId(),
-				// add new LiveData to cache
+							liveData);
-				liveData = finalLiveData;
+				} else {
-				if (needsSize) itemsWithSize.put(h.getMessageId(), liveData);
+					oldLiveData = itemsWithoutSize.putIfAbsent(h.getMessageId(),
-				else itemsWithoutSize.put(h.getMessageId(), liveData);
+							liveData);
+				}
+				if (oldLiveData == null) {
+					// kick-off loading of attachment, will post to live data
+					MutableLiveData<AttachmentItem> finalLiveData = liveData;
+					dbExecutor.execute(() ->
+							loadAttachmentItem(h, needsSize, finalLiveData));
+				} else {
+					// Concurrent cache update - use the existing live data
+					liveData = oldLiveData;
+				}
 			}
 			items.add(liveData);
 		}
@@ -118,12 +128,15 @@ class AttachmentRetrieverImpl implements AttachmentRetriever {
 	@DatabaseExecutor
 	public void cacheAttachmentItemWithSize(MessageId conversationMessageId,
 			AttachmentHeader h) throws DbException {
+		// If a live data is already cached we don't need to do anything
+		if (itemsWithSize.containsKey(h.getMessageId())) return;
 		try {
 			Attachment a = messagingManager.getAttachment(h);
 			AttachmentItem item = createAttachmentItem(a, true);
 			MutableLiveData<AttachmentItem> liveData =
 					new MutableLiveData<>(item);
-			itemsWithSize.put(h.getMessageId(), liveData);
+			// If a live data was concurrently cached, don't replace it
+			itemsWithSize.putIfAbsent(h.getMessageId(), liveData);
 		} catch (NoSuchMessageException e) {
 			LOG.info("Attachment not received yet");
 		}

briar-android/src/main/java/org/briarproject/briar/android/attachment/UnavailableItem.java

@@ -1,36 +0,0 @@
-package org.briarproject.briar.android.attachment;
-
-import org.briarproject.bramble.api.nullsafety.NotNullByDefault;
-import org.briarproject.bramble.api.sync.MessageId;
-import org.briarproject.briar.api.messaging.AttachmentHeader;
-
-import javax.annotation.concurrent.Immutable;
-
-@Immutable
-@NotNullByDefault
-class UnavailableItem {
-
-	private final MessageId conversationMessageId;
-	private final AttachmentHeader header;
-	private final boolean needsSize;
-
-	UnavailableItem(MessageId conversationMessageId,
-			AttachmentHeader header, boolean needsSize) {
-		this.conversationMessageId = conversationMessageId;
-		this.header = header;
-		this.needsSize = needsSize;
-	}
-
-	MessageId getConversationMessageId() {
-		return conversationMessageId;
-	}
-
-	AttachmentHeader getHeader() {
-		return header;
-	}
-
-	boolean needsSize() {
-		return needsSize;
-	}
-
-}

briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationActivity.java

@@ -640,16 +640,14 @@ public class ConversationActivity extends BriarActivity
 				&& adapter.isScrolledToBottom(layoutManager);
 	}
 
+	@UiThread
 	private void updateMessageAttachment(MessageId m, AttachmentItem item) {
-		runOnUiThreadUnlessDestroyed(() -> {
+		Pair<Integer, ConversationMessageItem> pair = adapter.getMessageItem(m);
-			Pair<Integer, ConversationMessageItem> pair =
+		if (pair != null && pair.getSecond().updateAttachments(item)) {
-					adapter.getMessageItem(m);
+			boolean scroll = shouldScrollWhenUpdatingMessage();
-			if (pair != null && pair.getSecond().updateAttachments(item)) {
+			adapter.notifyItemChanged(pair.getFirst());
-				boolean scroll = shouldScrollWhenUpdatingMessage();
+			if (scroll) scrollToBottom();
-				adapter.notifyItemChanged(pair.getFirst());
+		}
-				if (scroll) scrollToBottom();
-			}
-		});
 	}
 
 	@Override

briar-android/src/main/java/org/briarproject/briar/android/conversation/ImageAdapter.java

@@ -59,7 +59,7 @@ class ImageAdapter extends Adapter<ImageViewHolder> {
 		// get item
 		requireNonNull(conversationItem);
 		AttachmentItem item = items.get(position);
-		// set onClick listener, if not missing or error
+		// set onClick listener
 		imageViewHolder.itemView.setOnClickListener(v ->
 				listener.onAttachmentClicked(v, conversationItem, item)
 		);