Refactor attachment code to reduce mutable state.

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

@@ -1,22 +1,28 @@
 package org.briarproject.briar.android.attachment;
 
+import android.arch.lifecycle.LiveData;
+import android.arch.lifecycle.MutableLiveData;
 import android.content.ContentResolver;
 import android.net.Uri;
-import android.support.annotation.Nullable;
 
 import org.briarproject.bramble.api.db.DbException;
 import org.briarproject.bramble.api.lifecycle.IoExecutor;
 import org.briarproject.bramble.api.nullsafety.NotNullByDefault;
 import org.briarproject.bramble.api.sync.GroupId;
+import org.briarproject.briar.api.messaging.Attachment;
 import org.briarproject.briar.api.messaging.AttachmentHeader;
 import org.briarproject.briar.api.messaging.MessagingManager;
 import org.jsoup.UnsupportedMimeTypeException;
 
 import java.io.IOException;
 import java.io.InputStream;
+import java.util.ArrayList;
 import java.util.Collection;
+import java.util.List;
 import java.util.logging.Logger;
 
+import javax.annotation.Nullable;
+
 import static java.util.logging.Level.WARNING;
 import static java.util.logging.Logger.getLogger;
 import static org.briarproject.bramble.util.IoUtils.tryToClose;
@@ -33,56 +39,64 @@ class AttachmentCreationTask {
 
 	private final MessagingManager messagingManager;
 	private final ContentResolver contentResolver;
+	private final AttachmentRetriever retriever;
 	private final GroupId groupId;
 	private final Collection<Uri> uris;
 	private final boolean needsSize;
-	@Nullable
+	private final MutableLiveData<AttachmentResult> result;
-	private volatile AttachmentCreator attachmentCreator;
 
 	private volatile boolean canceled = false;
 
 	AttachmentCreationTask(MessagingManager messagingManager,
-			ContentResolver contentResolver,
+			ContentResolver contentResolver, AttachmentRetriever retriever,
-			AttachmentCreator attachmentCreator, GroupId groupId,
+			GroupId groupId, Collection<Uri> uris, boolean needsSize) {
-			Collection<Uri> uris, boolean needsSize) {
 		this.messagingManager = messagingManager;
 		this.contentResolver = contentResolver;
+		this.retriever = retriever;
 		this.groupId = groupId;
 		this.uris = uris;
 		this.needsSize = needsSize;
-		this.attachmentCreator = attachmentCreator;
+		result = new MutableLiveData<>();
 	}
 
-	public void cancel() {
+	LiveData<AttachmentResult> getResult() {
+		return result;
+	}
+
+	void cancel() {
 		canceled = true;
-		attachmentCreator = null;
 	}
 
 	@IoExecutor
 	void storeAttachments() {
-		for (Uri uri: uris) processUri(uri);
+		List<AttachmentItemResult> results = new ArrayList<>();
-		AttachmentCreator attachmentCreator = this.attachmentCreator;
+		for (Uri uri : uris) {
-		if (!canceled && attachmentCreator != null)
+			if (canceled) break;
-			attachmentCreator.onAttachmentCreationFinished();
+			results.add(processUri(uri));
-		this.attachmentCreator = null;
+			result.postValue(new AttachmentResult(new ArrayList<>(results),
+					false, false));
+		}
+		result.postValue(new AttachmentResult(new ArrayList<>(results), true,
+				!canceled));
 	}
 
 	@IoExecutor
-	private void processUri(Uri uri) {
+	private AttachmentItemResult processUri(Uri uri) {
-		if (canceled) return;
+		AttachmentHeader header = null;
 		try {
-			AttachmentHeader h = storeAttachment(uri);
+			header = storeAttachment(uri);
-			AttachmentCreator attachmentCreator = this.attachmentCreator;
+			Attachment a = retriever.getMessageAttachment(header);
-			if (attachmentCreator != null) {
+			AttachmentItem item =
-				attachmentCreator.onAttachmentHeaderReceived(uri, h, needsSize);
+					retriever.getAttachmentItem(header, a, needsSize);
-			}
+			if (item.hasError()) throw new IOException();
+			retriever.cachePut(item);
+			return new AttachmentItemResult(uri, item);
 		} catch (DbException | IOException e) {
 			logException(LOG, WARNING, e);
-			AttachmentCreator attachmentCreator = this.attachmentCreator;
+			// If the attachment was already stored, delete it
-			if (attachmentCreator != null) {
+			tryToRemove(header);
-				attachmentCreator.onAttachmentError(uri, e);
-			}
 			canceled = true;
+			return new AttachmentItemResult(uri, e);
 		}
 	}
 
@@ -113,4 +127,11 @@ class AttachmentCreationTask {
 		return false;
 	}
 
+	private void tryToRemove(@Nullable AttachmentHeader h) {
+		try {
+			if (h != null) messagingManager.removeAttachment(h);
+		} catch (DbException e1) {
+			logException(LOG, WARNING, e1);
+		}
+	}
 }

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

@@ -3,7 +3,7 @@ package org.briarproject.briar.android.attachment;
 
 import android.app.Application;
 import android.arch.lifecycle.LiveData;
-import android.arch.lifecycle.MutableLiveData;
+import android.arch.lifecycle.Observer;
 import android.net.Uri;
 import android.support.annotation.Nullable;
 import android.support.annotation.UiThread;
@@ -12,27 +12,20 @@ import org.briarproject.bramble.api.db.DbException;
 import org.briarproject.bramble.api.lifecycle.IoExecutor;
 import org.briarproject.bramble.api.nullsafety.NotNullByDefault;
 import org.briarproject.bramble.api.sync.GroupId;
-import org.briarproject.bramble.api.sync.MessageId;
-import org.briarproject.briar.R;
-import org.briarproject.briar.api.messaging.Attachment;
 import org.briarproject.briar.api.messaging.AttachmentHeader;
-import org.briarproject.briar.api.messaging.FileTooBigException;
 import org.briarproject.briar.api.messaging.MessagingManager;
-import org.jsoup.UnsupportedMimeTypeException;
 
-import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.List;
-import java.util.concurrent.CopyOnWriteArrayList;
 import java.util.concurrent.Executor;
 import java.util.logging.Logger;
 
+import static java.util.Collections.emptyList;
+import static java.util.Objects.requireNonNull;
 import static java.util.logging.Level.WARNING;
 import static java.util.logging.Logger.getLogger;
 import static org.briarproject.bramble.util.LogUtils.logException;
-import static org.briarproject.briar.android.util.UiUtils.observeForeverOnce;
-import static org.briarproject.briar.api.messaging.MessagingConstants.MAX_IMAGE_SIZE;
 
 @NotNullByDefault
 public class AttachmentCreator {
@@ -45,12 +38,6 @@ public class AttachmentCreator {
 	private final MessagingManager messagingManager;
 	private final AttachmentRetriever retriever;
 
-	private final CopyOnWriteArrayList<Uri> uris = new CopyOnWriteArrayList<>();
-	private final CopyOnWriteArrayList<AttachmentItemResult> itemResults =
-			new CopyOnWriteArrayList<>();
-
-	private final MutableLiveData<AttachmentResult> result =
-			new MutableLiveData<>();
 	@Nullable
 	private AttachmentCreationTask task;
 
@@ -62,20 +49,19 @@ public class AttachmentCreator {
 		this.retriever = retriever;
 	}
 
+	/**
+	 * Starts a background task to create attachments from the given URIs and
+	 * returns a LiveData to monitor the progress of the task.
+	 */
 	@UiThread
-	public LiveData<AttachmentResult> storeAttachments(
+	public LiveData<AttachmentResult> storeAttachments(GroupId groupId,
-			LiveData<GroupId> groupId, Collection<Uri> newUris) {
+			Collection<Uri> uris) {
-		if (task != null || !uris.isEmpty())
+		if (task != null) throw new IllegalStateException();
-			throw new IllegalStateException();
+		boolean needsSize = uris.size() == 1;
-		uris.addAll(newUris);
+		task = new AttachmentCreationTask(messagingManager,
-		observeForeverOnce(groupId, id -> {
+				app.getContentResolver(), retriever, groupId, uris, needsSize);
-			if (id == null) throw new IllegalStateException();
+		ioExecutor.execute(() -> task.storeAttachments());
-			boolean needsSize = uris.size() == 1;
+		return task.getResult();
-			task = new AttachmentCreationTask(messagingManager,
-					app.getContentResolver(), this, id, uris, needsSize);
-			ioExecutor.execute(() -> task.storeAttachments());
-		});
-		return result;
 	}
 
 	/**
@@ -85,112 +71,70 @@ public class AttachmentCreator {
 	 */
 	@UiThread
 	public LiveData<AttachmentResult> getLiveAttachments() {
-		if (task == null || uris.isEmpty())
+		if (task == null) throw new IllegalStateException();
-			throw new IllegalStateException();
 		// A task is already running. It will update the result LiveData.
 		// So nothing more to do here.
-		return result;
+		return task.getResult();
-	}
-
-	@IoExecutor
-	void onAttachmentHeaderReceived(Uri uri, AttachmentHeader h,
-			boolean needsSize) {
-		// get and cache AttachmentItem for ImagePreview
-		try {
-			Attachment a = retriever.getMessageAttachment(h);
-			AttachmentItem item = retriever.getAttachmentItem(h, a, needsSize);
-			if (item.hasError()) throw new IOException();
-			AttachmentItemResult itemResult =
-					new AttachmentItemResult(uri, item);
-			itemResults.add(itemResult);
-			result.postValue(getResult(false));
-		} catch (IOException | DbException e) {
-			logException(LOG, WARNING, e);
-			onAttachmentError(uri, e);
-		}
-	}
-
-	@IoExecutor
-	void onAttachmentError(Uri uri, Exception e) {
-		// get error message
-		String errorMsg;
-		if (e instanceof UnsupportedMimeTypeException) {
-			String mimeType = ((UnsupportedMimeTypeException) e).getMimeType();
-			errorMsg = app.getString(
-					R.string.image_attach_error_invalid_mime_type, mimeType);
-		} else if (e instanceof FileTooBigException) {
-			int mb = MAX_IMAGE_SIZE / 1024 / 1024;
-			errorMsg = app.getString(R.string.image_attach_error_too_big, mb);
-		} else {
-			errorMsg = null; // generic error
-		}
-		AttachmentItemResult itemResult =
-				new AttachmentItemResult(uri, errorMsg);
-		itemResults.add(itemResult);
-		result.postValue(getResult(false));
-		// expect to receive a cancel from the UI
-	}
-
-	@IoExecutor
-	void onAttachmentCreationFinished() {
-		result.postValue(getResult(true));
 	}
 
+	/**
+	 * Returns the headers of any attachments created by
+	 * {@link #storeAttachments(GroupId, Collection)}.
+	 */
 	@UiThread
 	public List<AttachmentHeader> getAttachmentHeadersForSending() {
-		List<AttachmentHeader> headers = new ArrayList<>(itemResults.size());
+		if (task == null) return emptyList();
-		for (AttachmentItemResult itemResult : itemResults) {
+		AttachmentResult result = task.getResult().getValue();
-			// check if we are trying to send attachment items with errors
+		if (result == null) return emptyList();
-			if (itemResult.getItem() == null) throw new IllegalStateException();
+		List<AttachmentHeader> headers = new ArrayList<>();
-			headers.add(itemResult.getItem().getHeader());
+		for (AttachmentItemResult itemResult : result.getItemResults()) {
+			AttachmentItem item = itemResult.getItem();
+			if (item != null) headers.add(item.getHeader());
 		}
 		return headers;
 	}
 
 	/**
-	 * Marks the attachments as sent and adds the items to the cache for display
+	 * Informs the AttachmentCreator that the attachments created by
-	 *
+	 * {@link #storeAttachments(GroupId, Collection)} will be sent.
-	 * @param id The MessageId of the sent message.
 	 */
 	@UiThread
-	public void onAttachmentsSent(MessageId id) {
+	public void onAttachmentsSent() {
-		List<AttachmentItem> items = new ArrayList<>(itemResults.size());
+		task = null;
-		for (AttachmentItemResult itemResult : itemResults) {
-			// check if we are trying to send attachment items with errors
-			if (itemResult.getItem() == null) throw new IllegalStateException();
-			items.add(itemResult.getItem());
-		}
-		retriever.cachePut(id, items);
-		resetState();
 	}
 
 	/**
-	 * Needs to be called when created attachments will not be sent anymore.
+	 * Cancels the task started by
+	 * {@link #storeAttachments(GroupId, Collection)} and deletes any
+	 * created attachments, unless {@link #onAttachmentsSent()} has
+	 * been called.
 	 */
 	@UiThread
 	public void cancel() {
-		if (task == null) throw new AssertionError();
+		if (task == null) return; // Already sent or cancelled
 		task.cancel();
-		deleteUnsentAttachments();
+		// Observe the task until it finishes (which may already have
-		resetState();
+		// happened) and delete any created attachments
-	}
+		LiveData<AttachmentResult> taskResult = task.getResult();
-
+		taskResult.observeForever(new Observer<AttachmentResult>() {
-	@UiThread
+			@Override
-	private void resetState() {
+			public void onChanged(@Nullable AttachmentResult result) {
+				requireNonNull(result);
+				if (result.isFinished()) {
+					deleteUnsentAttachments(result.getItemResults());
+					taskResult.removeObserver(this);
+				}
+			}
+		});
 		task = null;
-		uris.clear();
-		itemResults.clear();
-		result.setValue(null);
 	}
 
-	@UiThread
+	private void deleteUnsentAttachments(
-	public void deleteUnsentAttachments() {
+			Collection<AttachmentItemResult> itemResults) {
-		// Make a copy for the IoExecutor as we clear the itemResults soon
 		List<AttachmentHeader> headers = new ArrayList<>(itemResults.size());
 		for (AttachmentItemResult itemResult : itemResults) {
-			// check if we are trying to send attachment items with errors
+			AttachmentItem item = itemResult.getItem();
-			if (itemResult.getItem() != null)
+			if (item != null) headers.add(item.getHeader());
-				headers.add(itemResult.getItem().getHeader());
 		}
 		ioExecutor.execute(() -> {
 			for (AttachmentHeader header : headers) {
@@ -202,17 +146,4 @@ public class AttachmentCreator {
 			}
 		});
 	}
-
-	private AttachmentResult getResult(boolean finished) {
-		// Make a copy of the list,
-		// because our copy will continue to change in the background.
-		// (As it's a CopyOnWriteArrayList,
-		//  the code that receives the result can safely do simple things
-		//  like iterating over the list,
-		//  but anything that involves calling more than one list method
-		//  is still unsafe.)
-		Collection<AttachmentItemResult> items = new ArrayList<>(itemResults);
-		return new AttachmentResult(items, finished);
-	}
-
 }

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

@@ -15,18 +15,18 @@ public class AttachmentItemResult {
 	@Nullable
 	private final AttachmentItem item;
 	@Nullable
-	private final String errorMsg;
+	private final Exception exception;
 
 	AttachmentItemResult(Uri uri, AttachmentItem item) {
 		this.uri = uri;
 		this.item = item;
-		this.errorMsg = null;
+		this.exception = null;
 	}
 
-	AttachmentItemResult(Uri uri, @Nullable String errorMsg) {
+	AttachmentItemResult(Uri uri, Exception exception) {
 		this.uri = uri;
 		this.item = null;
-		this.errorMsg = errorMsg;
+		this.exception = exception;
 	}
 
 	public Uri getUri() {
@@ -43,8 +43,8 @@ public class AttachmentItemResult {
 	}
 
 	@Nullable
-	public String getErrorMsg() {
+	public Exception getException() {
-		return errorMsg;
+		return exception;
 	}
 
 }

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

@@ -12,11 +12,13 @@ public class AttachmentResult {
 
 	private final Collection<AttachmentItemResult> itemResults;
 	private final boolean finished;
+	private final boolean success;
 
-	public AttachmentResult(Collection<AttachmentItemResult> itemResults,
+	AttachmentResult(Collection<AttachmentItemResult> itemResults,
-			boolean finished) {
+			boolean finished, boolean success) {
 		this.itemResults = itemResults;
 		this.finished = finished;
+		this.success = success;
 	}
 
 	public Collection<AttachmentItemResult> getItemResults() {
@@ -27,4 +29,7 @@ public class AttachmentResult {
 		return finished;
 	}
 
+	public boolean isSuccess() {
+		return success;
+	}
 }

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

@@ -56,7 +56,7 @@ public class AttachmentRetriever {
 	private final int minWidth, maxWidth;
 	private final int minHeight, maxHeight;
 
-	private final Map<MessageId, List<AttachmentItem>> attachmentCache =
+	private final Map<MessageId, AttachmentItem> attachmentCache =
 			new ConcurrentHashMap<>();
 
 	@VisibleForTesting
@@ -94,13 +94,13 @@ public class AttachmentRetriever {
 		});
 	}
 
-	public void cachePut(MessageId messageId, List<AttachmentItem> attachments) {
+	public void cachePut(AttachmentItem item) {
-		attachmentCache.put(messageId, attachments);
+		attachmentCache.put(item.getMessageId(), item);
 	}
 
 	@Nullable
-	public List<AttachmentItem> cacheGet(MessageId messageId) {
+	public AttachmentItem cacheGet(MessageId attachmentId) {
-		return attachmentCache.get(messageId);
+		return attachmentCache.get(attachmentId);
 	}
 
 	@DatabaseExecutor
@@ -117,7 +117,6 @@ public class AttachmentRetriever {
 		return attachments;
 	}
 
-	@DatabaseExecutor
 	Attachment getMessageAttachment(AttachmentHeader h) throws DbException {
 		return messagingManager.getAttachment(h.getMessageId());
 	}

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

@@ -448,14 +448,16 @@ public class ConversationActivity extends BriarActivity
 		}
 		// If the message has a single image, load its size - for multiple
 		// images we use a grid so the size is fixed
-		if (h.getAttachmentHeaders().size() == 1) {
+		List<AttachmentHeader> headers = h.getAttachmentHeaders();
-			List<AttachmentItem> items = attachmentRetriever.cacheGet(id);
+		if (headers.size() == 1) {
-			if (items == null) {
+			MessageId attachmentId = headers.get(0).getMessageId();
+			AttachmentItem item = attachmentRetriever.cacheGet(attachmentId);
+			if (item == null) {
 				LOG.info("Eagerly loading image size for latest message");
-				items = attachmentRetriever.getAttachmentItems(
+				item = attachmentRetriever.getAttachmentItems(
-						attachmentRetriever.getMessageAttachments(
+						attachmentRetriever.getMessageAttachments(headers))
-								h.getAttachmentHeaders()));
+						.get(0);
-				attachmentRetriever.cachePut(id, items);
+				attachmentRetriever.cachePut(item);
 			}
 		}
 	}
@@ -553,7 +555,9 @@ public class ConversationActivity extends BriarActivity
 	private void displayMessageAttachments(MessageId m,
 			List<AttachmentItem> items) {
 		runOnUiThreadUnlessDestroyed(() -> {
-			attachmentRetriever.cachePut(m, items);
+			for (AttachmentItem item : items) {
+				attachmentRetriever.cachePut(item);
+			}
 			Pair<Integer, ConversationMessageItem> pair =
 					adapter.getMessageItem(m);
 			if (pair != null) {
@@ -905,12 +909,17 @@ public class ConversationActivity extends BriarActivity
 	@Override
 	public List<AttachmentItem> getAttachmentItems(MessageId m,
 			List<AttachmentHeader> headers) {
-		List<AttachmentItem> attachments = attachmentRetriever.cacheGet(m);
+		List<AttachmentItem> items = new ArrayList<>(headers.size());
-		if (attachments == null) {
+		for (AttachmentHeader header : headers) {
-			loadMessageAttachments(m, headers);
+			AttachmentItem item =
-			return emptyList();
+					attachmentRetriever.cacheGet(header.getMessageId());
+			if (item == null) {
+				loadMessageAttachments(m, headers);
+				return emptyList();
+			}
+			items.add(item);
 		}
-		return attachments;
+		return items;
 	}
 
 }

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

@@ -4,6 +4,7 @@ import android.app.Application;
 import android.arch.lifecycle.AndroidViewModel;
 import android.arch.lifecycle.LiveData;
 import android.arch.lifecycle.MutableLiveData;
+import android.arch.lifecycle.Observer;
 import android.arch.lifecycle.Transformations;
 import android.net.Uri;
 import android.support.annotation.Nullable;
@@ -124,7 +125,7 @@ public class ConversationViewModel extends AndroidViewModel
 	@Override
 	protected void onCleared() {
 		super.onCleared();
-		attachmentCreator.deleteUnsentAttachments();
+		attachmentCreator.cancel();
 	}
 
 	/**
@@ -200,12 +201,23 @@ public class ConversationViewModel extends AndroidViewModel
 	@UiThread
 	public LiveData<AttachmentResult> storeAttachments(Collection<Uri> uris,
 			boolean restart) {
-		if (restart) {
+		MutableLiveData<AttachmentResult> delegate = new MutableLiveData<>();
-			return attachmentCreator.getLiveAttachments();
+		// messagingGroupId is loaded with the contact
-		} else {
+		observeForeverOnce(messagingGroupId, groupId -> {
-			// messagingGroupId is loaded with the contact
+			requireNonNull(groupId);
-			return attachmentCreator.storeAttachments(messagingGroupId, uris);
+			LiveData<AttachmentResult> result;
-		}
+			if (restart) result = attachmentCreator.getLiveAttachments();
+			else result = attachmentCreator.storeAttachments(groupId, uris);
+			result.observeForever(new Observer<AttachmentResult>() {
+				@Override
+				public void onChanged(@Nullable AttachmentResult value) {
+					requireNonNull(value);
+					if (value.isFinished()) result.removeObserver(this);
+					delegate.setValue(value);
+				}
+			});
+		});
+		return delegate;
 	}
 
 	@Override
@@ -294,7 +306,7 @@ public class ConversationViewModel extends AndroidViewModel
 	}
 
 	private void storeMessage(PrivateMessage m) {
-		attachmentCreator.onAttachmentsSent(m.getMessage().getId());
+		attachmentCreator.onAttachmentsSent();
 		dbExecutor.execute(() -> {
 			try {
 				long start = now();

briar-android/src/main/java/org/briarproject/briar/android/view/TextAttachmentController.java

@@ -22,6 +22,8 @@ import org.briarproject.briar.android.attachment.AttachmentItemResult;
 import org.briarproject.briar.android.attachment.AttachmentManager;
 import org.briarproject.briar.android.attachment.AttachmentResult;
 import org.briarproject.briar.android.view.ImagePreview.ImagePreviewListener;
+import org.briarproject.briar.api.messaging.FileTooBigException;
+import org.jsoup.UnsupportedMimeTypeException;
 
 import java.util.ArrayList;
 import java.util.Collection;
@@ -41,9 +43,11 @@ import static android.support.v4.content.ContextCompat.getColor;
 import static android.support.v4.view.AbsSavedState.EMPTY_STATE;
 import static android.view.View.GONE;
 import static android.widget.Toast.LENGTH_LONG;
+import static java.util.Objects.requireNonNull;
 import static org.briarproject.briar.android.util.UiUtils.resolveColorAttribute;
 import static org.briarproject.briar.api.messaging.MessagingConstants.IMAGE_MIME_TYPES;
 import static org.briarproject.briar.api.messaging.MessagingConstants.MAX_ATTACHMENTS_PER_MESSAGE;
+import static org.briarproject.briar.api.messaging.MessagingConstants.MAX_IMAGE_SIZE;
 import static uk.co.samuelwall.materialtaptargetprompt.MaterialTapTargetPrompt.STATE_DISMISSED;
 import static uk.co.samuelwall.materialtaptargetprompt.MaterialTapTargetPrompt.STATE_FINISHED;
 
@@ -186,18 +190,16 @@ public class TextAttachmentController extends TextSendController
 		result.observe(attachmentListener, new Observer<AttachmentResult>() {
 			@Override
 			public void onChanged(@Nullable AttachmentResult attachmentResult) {
-				if (attachmentResult == null) {
+				requireNonNull(attachmentResult);
-					// The fresh LiveData was deliberately set to null.
+				boolean finished = attachmentResult.isFinished();
-					// This means that we can stop observing it.
+				boolean success = attachmentResult.isSuccess();
+				if (finished) {
 					result.removeObserver(this);
-				} else {
+					if (!success) return;
-					boolean noError = onNewAttachmentItemResults(
-							attachmentResult.getItemResults());
-					if (noError && attachmentResult.isFinished()) {
-						onAllAttachmentsCreated();
-						result.removeObserver(this);
-					}
 				}
+				boolean noError = onNewAttachmentItemResults(
+						attachmentResult.getItemResults());
+				if (noError && success) onAllAttachmentsCreated();
 			}
 		});
 	}
@@ -207,7 +209,7 @@ public class TextAttachmentController extends TextSendController
 		if (!loadingUris) throw new AssertionError();
 		for (AttachmentItemResult result : itemResults) {
 			if (result.hasError()) {
-				onError(result.getErrorMsg());
+				onError(requireNonNull(result.getException()));
 				return false;
 			} else {
 				imagePreview.loadPreviewImage(result);
@@ -253,12 +255,20 @@ public class TextAttachmentController extends TextSendController
 	}
 
 	@UiThread
-	private void onError(@Nullable String errorMsg) {
+	private void onError(Exception e) {
-		if (errorMsg == null) {
+		String errorMsg;
-			errorMsg = imagePreview.getContext()
+		Context ctx = imagePreview.getContext();
-					.getString(R.string.image_attach_error);
+		if (e instanceof UnsupportedMimeTypeException) {
+			String mimeType = ((UnsupportedMimeTypeException) e).getMimeType();
+			errorMsg = ctx.getString(
+					R.string.image_attach_error_invalid_mime_type, mimeType);
+		} else if (e instanceof FileTooBigException) {
+			int mb = MAX_IMAGE_SIZE / 1024 / 1024;
+			errorMsg = ctx.getString(R.string.image_attach_error_too_big, mb);
+		} else {
+			errorMsg = ctx.getString(R.string.image_attach_error);
 		}
-		Toast.makeText(textInput.getContext(), errorMsg, LENGTH_LONG).show();
+		Toast.makeText(ctx, errorMsg, LENGTH_LONG).show();
 		onCancel();
 	}