DO NOT MERGE: Delay sending of attachments.

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

@@ -135,6 +135,10 @@ public class AttachmentItem implements Parcelable {
 		return toHexString(instanceId);
 	}
 
+	boolean hasSize() {
+		return width != 0 && height != 0;
+	}
+
 	@Override
 	public int describeContents() {
 		return 0;
@@ -152,10 +156,6 @@ 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,8 +164,4 @@ 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,13 +49,6 @@ 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 ConcurrentMap<MessageId, MutableLiveData<AttachmentItem>>
+	private final Map<MessageId, MutableLiveData<AttachmentItem>>
 			itemsWithSize = new ConcurrentHashMap<>();
-	private final ConcurrentMap<MessageId, MutableLiveData<AttachmentItem>>
+	private final Map<MessageId, MutableLiveData<AttachmentItem>>
 			itemsWithoutSize = new ConcurrentHashMap<>();
 
 	@Inject
@@ -99,25 +99,15 @@ class AttachmentRetrieverImpl implements AttachmentRetriever {
 			if (liveData == null) {
 				AttachmentItem item = new AttachmentItem(h,
 						defaultSize, defaultSize, LOADING);
-				liveData = new MutableLiveData<>(item);
+				final MutableLiveData<AttachmentItem> finalLiveData =
-				// add new LiveData to cache, checking for concurrent updates
+						new MutableLiveData<>(item);
-				MutableLiveData<AttachmentItem> oldLiveData;
+				// kick-off loading of attachment, will post to live data
-				if (needsSize) {
+				dbExecutor.execute(
-					oldLiveData = itemsWithSize.putIfAbsent(h.getMessageId(),
+						() -> loadAttachmentItem(h, needsSize, finalLiveData));
-							liveData);
+				// add new LiveData to cache
-				} else {
+				liveData = finalLiveData;
-					oldLiveData = itemsWithoutSize.putIfAbsent(h.getMessageId(),
+				if (needsSize) itemsWithSize.put(h.getMessageId(), liveData);
-							liveData);
+				else itemsWithoutSize.put(h.getMessageId(), 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);
 		}
@@ -128,15 +118,12 @@ 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);
-			// If a live data was concurrently cached, don't replace it
+			itemsWithSize.put(h.getMessageId(), liveData);
-			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

@@ -0,0 +1,36 @@
+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,14 +640,16 @@ public class ConversationActivity extends BriarActivity
 				&& adapter.isScrolledToBottom(layoutManager);
 	}
 
-	@UiThread
 	private void updateMessageAttachment(MessageId m, AttachmentItem item) {
-		Pair<Integer, ConversationMessageItem> pair = adapter.getMessageItem(m);
+		runOnUiThreadUnlessDestroyed(() -> {
-		if (pair != null && pair.getSecond().updateAttachments(item)) {
+			Pair<Integer, ConversationMessageItem> pair =
-			boolean scroll = shouldScrollWhenUpdatingMessage();
+					adapter.getMessageItem(m);
-			adapter.notifyItemChanged(pair.getFirst());
+			if (pair != null && pair.getSecond().updateAttachments(item)) {
-			if (scroll) scrollToBottom();
+				boolean scroll = shouldScrollWhenUpdatingMessage();
-		}
+				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
+		// set onClick listener, if not missing or error
 		imageViewHolder.itemView.setOnClickListener(v ->
 				listener.onAttachmentClicked(v, conversationItem, item)
 		);

briar-core/src/main/java/org/briarproject/briar/messaging/MessagingManagerImpl.java

@@ -25,6 +25,7 @@ import org.briarproject.bramble.api.sync.Message;
 import org.briarproject.bramble.api.sync.MessageId;
 import org.briarproject.bramble.api.sync.MessageStatus;
 import org.briarproject.bramble.api.sync.validation.IncomingMessageHook;
+import org.briarproject.bramble.api.system.Scheduler;
 import org.briarproject.bramble.api.versioning.ClientVersioningManager;
 import org.briarproject.bramble.api.versioning.ClientVersioningManager.ClientVersioningHook;
 import org.briarproject.briar.api.client.MessageTracker;
@@ -52,14 +53,20 @@ import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.concurrent.ScheduledExecutorService;
+import java.util.logging.Logger;
 
 import javax.annotation.concurrent.Immutable;
 import javax.inject.Inject;
 
 import static java.util.Collections.emptyList;
+import static java.util.concurrent.TimeUnit.SECONDS;
+import static java.util.logging.Level.WARNING;
+import static java.util.logging.Logger.getLogger;
 import static org.briarproject.bramble.api.sync.SyncConstants.MAX_MESSAGE_BODY_LENGTH;
 import static org.briarproject.bramble.api.sync.validation.MessageState.DELIVERED;
 import static org.briarproject.bramble.util.IoUtils.copyAndClose;
+import static org.briarproject.bramble.util.LogUtils.logException;
 import static org.briarproject.briar.client.MessageTrackerConstants.MSG_KEY_READ;
 import static org.briarproject.briar.messaging.MessageTypes.ATTACHMENT;
 import static org.briarproject.briar.messaging.MessageTypes.PRIVATE_MESSAGE;
@@ -78,6 +85,10 @@ class MessagingManagerImpl implements MessagingManager, IncomingMessageHook,
 		ConversationClient, OpenDatabaseHook, ContactHook,
 		ClientVersioningHook {
 
+	private static final Logger LOG =
+			getLogger(MessagingManagerImpl.class.getName());
+
+	private final ScheduledExecutorService scheduler;
 	private final DatabaseComponent db;
 	private final ClientHelper clientHelper;
 	private final MetadataParser metadataParser;
@@ -86,10 +97,12 @@ class MessagingManagerImpl implements MessagingManager, IncomingMessageHook,
 	private final ContactGroupFactory contactGroupFactory;
 
 	@Inject
-	MessagingManagerImpl(DatabaseComponent db, ClientHelper clientHelper,
+	MessagingManagerImpl(@Scheduler ScheduledExecutorService scheduler,
+			DatabaseComponent db, ClientHelper clientHelper,
 			ClientVersioningManager clientVersioningManager,
 			MetadataParser metadataParser, MessageTracker messageTracker,
 			ContactGroupFactory contactGroupFactory) {
+		this.scheduler = scheduler;
 		this.db = db;
 		this.clientHelper = clientHelper;
 		this.metadataParser = metadataParser;
@@ -245,9 +258,20 @@ class MessagingManagerImpl implements MessagingManager, IncomingMessageHook,
 				meta.put(MSG_KEY_ATTACHMENT_HEADERS, headers);
 			}
 			// Mark attachments as shared and permanent now we're ready to send
+			// FIXME: Revert
+			int i = 15;
 			for (AttachmentHeader a : m.getAttachmentHeaders()) {
-				db.setMessageShared(txn, a.getMessageId());
+				scheduler.schedule(() -> {
-				db.setMessagePermanent(txn, a.getMessageId());
+					try {
+						db.transaction(false, txn1 -> {
+							db.setMessageShared(txn1, a.getMessageId());
+							db.setMessagePermanent(txn1, a.getMessageId());
+						});
+					} catch (DbException e) {
+						logException(LOG, WARNING, e);
+					}
+				}, i, SECONDS);
+				i *= 2;
 			}
 			clientHelper.addLocalMessage(txn, m.getMessage(), meta, true,
 					false);