From 61d3fe90551eb9c451241616a2ad3b3ea0349e5a Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Tue, 5 Nov 2019 09:41:01 -0300 Subject: [PATCH 1/2] [android] fix IllegalStateException when creating attachments Injecting the non-singleton AttachmentCreator keeps an instance around that gets re-used with a different ViewModel. When backing out without sending or cancelling the attachments, we don't reset the state which leads us into an illegal state. --- .../android/attachment/AttachmentCreatorImpl.java | 15 +++++++++++---- .../conversation/ConversationViewModel.java | 4 +++- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentCreatorImpl.java b/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentCreatorImpl.java index 60f498fdd..4eb79a02f 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentCreatorImpl.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentCreatorImpl.java @@ -76,8 +76,12 @@ class AttachmentCreatorImpl implements AttachmentCreator { @UiThread public LiveData storeAttachments( LiveData groupId, Collection newUris) { - if (task != null || result != null || !uris.isEmpty()) + if (task != null || result != null || !uris.isEmpty()) { + if (task != null) LOG.warning("Task already exists!"); + if (result != null) LOG.warning("Result already exists!"); + if (!uris.isEmpty()) LOG.warning("Uris available: " + uris); throw new IllegalStateException(); + } MutableLiveData result = new MutableLiveData<>(); this.result = result; uris.addAll(newUris); @@ -96,8 +100,12 @@ class AttachmentCreatorImpl implements AttachmentCreator { @UiThread public LiveData getLiveAttachments() { MutableLiveData result = this.result; - if (task == null || result == null || uris.isEmpty()) + if (task == null || result == null || uris.isEmpty()) { + if (task == null) LOG.warning("No Task!"); + if (result == null) LOG.warning("No Result!"); + if (uris.isEmpty()) LOG.warning("Uris empty!"); throw new IllegalStateException(); + } // A task is already running. It will update the result LiveData. // So nothing more to do here. return result; @@ -174,8 +182,7 @@ class AttachmentCreatorImpl implements AttachmentCreator { @Override @UiThread public void cancel() { - if (task == null) throw new AssertionError(); - task.cancel(); + if (task != null) task.cancel(); deleteUnsentAttachments(); resetState(); } diff --git a/briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationViewModel.java b/briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationViewModel.java index 5becffce2..8e65db26c 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationViewModel.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationViewModel.java @@ -133,7 +133,7 @@ public class ConversationViewModel extends AndroidViewModel @Override protected void onCleared() { super.onCleared(); - attachmentCreator.deleteUnsentAttachments(); + attachmentCreator.cancel(); // also deletes unsent attachments eventBus.removeListener(this); } @@ -274,6 +274,7 @@ public class ConversationViewModel extends AndroidViewModel settingsManager.mergeSettings(settings, SETTINGS_NAMESPACE); } + @UiThread private void createMessage(GroupId groupId, @Nullable String text, List headers, long timestamp, boolean hasImageSupport) { @@ -292,6 +293,7 @@ public class ConversationViewModel extends AndroidViewModel } } + @UiThread private void storeMessage(PrivateMessage m) { attachmentCreator.onAttachmentsSent(m.getMessage().getId()); dbExecutor.execute(() -> { From cf8241e79c1f58a37bfbc0032272f308604ac26b Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Thu, 23 Jan 2020 10:24:19 -0300 Subject: [PATCH 2/2] Fix IllegalStateException in RecyclerView when backing out very quickly after adding image attachments for preview before sending --- .../java/org/briarproject/briar/android/view/ImagePreview.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/briar-android/src/main/java/org/briarproject/briar/android/view/ImagePreview.java b/briar-android/src/main/java/org/briarproject/briar/android/view/ImagePreview.java index cb379b3d3..661ddaefa 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/view/ImagePreview.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/view/ImagePreview.java @@ -79,7 +79,7 @@ public class ImagePreview extends ConstraintLayout { ((ImagePreviewAdapter) imageList.getAdapter()); int pos = requireNonNull(adapter).loadItemPreview(result); if (pos != NO_POSITION) { - imageList.smoothScrollToPosition(pos); + imageList.scrollToPosition(pos); } }