From 53d122ed5592cfc7d81597ab21cc1b5959b64f80 Mon Sep 17 00:00:00 2001 From: Greyson Parrelli Date: Mon, 11 May 2020 11:47:30 -0400 Subject: [PATCH] Fix jumping to last seen position. --- .../thoughtcrime/securesms/MainNavigator.java | 4 +- .../conversation/ConversationActivity.java | 7 --- .../conversation/ConversationAdapter.java | 36 ++++++--------- .../conversation/ConversationData.java | 11 ++++- .../conversation/ConversationFragment.java | 45 +++++++------------ .../conversation/ConversationRepository.java | 24 ++++++---- .../conversation/ConversationViewModel.java | 19 ++++---- .../ConversationListFragment.java | 10 ++--- .../securesms/database/MmsSmsDatabase.java | 12 +++++ .../notifications/NotificationItem.java | 2 +- 10 files changed, 81 insertions(+), 89 deletions(-) diff --git a/app/src/main/java/org/thoughtcrime/securesms/MainNavigator.java b/app/src/main/java/org/thoughtcrime/securesms/MainNavigator.java index 38552b5592..7e4893aa62 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/MainNavigator.java +++ b/app/src/main/java/org/thoughtcrime/securesms/MainNavigator.java @@ -55,8 +55,8 @@ public class MainNavigator { return false; } - public void goToConversation(@NonNull RecipientId recipientId, long threadId, int distributionType, long lastSeen, int startingPosition) { - Intent intent = ConversationActivity.buildIntent(activity, recipientId, threadId, distributionType, lastSeen, startingPosition); + public void goToConversation(@NonNull RecipientId recipientId, long threadId, int distributionType, int startingPosition) { + Intent intent = ConversationActivity.buildIntent(activity, recipientId, threadId, distributionType, startingPosition); activity.startActivity(intent); activity.overridePendingTransition(R.anim.slide_from_end, R.anim.fade_scale_out); diff --git a/app/src/main/java/org/thoughtcrime/securesms/conversation/ConversationActivity.java b/app/src/main/java/org/thoughtcrime/securesms/conversation/ConversationActivity.java index b0d624ede0..4644d77782 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/conversation/ConversationActivity.java +++ b/app/src/main/java/org/thoughtcrime/securesms/conversation/ConversationActivity.java @@ -281,12 +281,10 @@ public class ConversationActivity extends PassphraseRequiredActionBarActivity public static final String RECIPIENT_EXTRA = "recipient_id"; public static final String THREAD_ID_EXTRA = "thread_id"; - public static final String IS_ARCHIVED_EXTRA = "is_archived"; public static final String TEXT_EXTRA = "draft_text"; public static final String MEDIA_EXTRA = "media_list"; public static final String STICKER_EXTRA = "sticker_extra"; public static final String DISTRIBUTION_TYPE_EXTRA = "distribution_type"; - public static final String LAST_SEEN_EXTRA = "last_seen"; public static final String STARTING_POSITION_EXTRA = "starting_position"; private static final int PICK_GALLERY = 1; @@ -342,12 +340,10 @@ public class ConversationActivity extends PassphraseRequiredActionBarActivity private LiveRecipient recipient; private long threadId; private int distributionType; - private boolean archived; private boolean isSecureText; private boolean isDefaultSms = true; private boolean isMmsEnabled = true; private boolean isSecurityInitialized = false; - private boolean shouldDisplayMessageRequestUi = true; private final IdentityRecordList identityRecords = new IdentityRecordList(); private final DynamicTheme dynamicTheme = new DynamicDarkToolbarTheme(); @@ -357,14 +353,12 @@ public class ConversationActivity extends PassphraseRequiredActionBarActivity @NonNull RecipientId recipientId, long threadId, int distributionType, - long lastSeen, int startingPosition) { Intent intent = new Intent(context, ConversationActivity.class); intent.putExtra(ConversationActivity.RECIPIENT_EXTRA, recipientId); intent.putExtra(ConversationActivity.THREAD_ID_EXTRA, threadId); intent.putExtra(ConversationActivity.DISTRIBUTION_TYPE_EXTRA, distributionType); - intent.putExtra(ConversationActivity.LAST_SEEN_EXTRA, lastSeen); intent.putExtra(ConversationActivity.STARTING_POSITION_EXTRA, startingPosition); return intent; @@ -1739,7 +1733,6 @@ public class ConversationActivity extends PassphraseRequiredActionBarActivity recipient = Recipient.live(getIntent().getParcelableExtra(RECIPIENT_EXTRA)); threadId = getIntent().getLongExtra(THREAD_ID_EXTRA, -1); - archived = getIntent().getBooleanExtra(IS_ARCHIVED_EXTRA, false); distributionType = getIntent().getIntExtra(DISTRIBUTION_TYPE_EXTRA, ThreadDatabase.DistributionTypes.DEFAULT); glideRequests = GlideApp.with(this); diff --git a/app/src/main/java/org/thoughtcrime/securesms/conversation/ConversationAdapter.java b/app/src/main/java/org/thoughtcrime/securesms/conversation/ConversationAdapter.java index ebe143fd69..a68f4e922c 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/conversation/ConversationAdapter.java +++ b/app/src/main/java/org/thoughtcrime/securesms/conversation/ConversationAdapter.java @@ -24,6 +24,7 @@ import android.widget.TextView; import androidx.annotation.AnyThread; import androidx.annotation.LayoutRes; +import androidx.annotation.MainThread; import androidx.annotation.NonNull; import androidx.annotation.Nullable; import androidx.paging.PagedList; @@ -241,16 +242,17 @@ public class ConversationAdapter @Override public void submitList(@Nullable PagedList pagedList) { cleanFastRecords(); - super.submitList(pagedList); - notifyDataSetChanged(); + super.submitList(pagedList, this::notifyDataSetChanged); } @Override protected @Nullable MessageRecord getItem(int position) { + position = hasHeader() ? position - 1 : position; + if (position < fastRecords.size()) { return fastRecords.get(position); } else { - int correctedPosition = position - fastRecords.size() - (hasHeader() ? 1 : 0); + int correctedPosition = position - fastRecords.size(); return super.getItem(correctedPosition); } } @@ -302,31 +304,19 @@ public class ConversationAdapter } /** - * Given a timestamp, this will return the position in the adapter of the message with the - * nearest received timestamp, or -1 if none is found. + * The presence of a header may throw off the position you'd like to jump to. This will return + * an adjusted message position based on adapter state. */ - int findLastSeenPosition(long lastSeen) { - if (lastSeen <= 0) { - return -1; - } - - int count = getItemCount() - (hasFooter() ? 1 : 0); - - for (int i = (hasHeader() ? 1 : 0); i < count; i++) { - MessageRecord messageRecord = getItem(i); - - if (messageRecord == null || messageRecord.isOutgoing() || messageRecord.getDateReceived() <= lastSeen) { - return i; - } - } - - return -1; + @MainThread + int getAdapterPositionForMessagePosition(int messagePosition) { + return hasHeader() ? messagePosition + 1 : messagePosition; } /** * Finds the received timestamp for the item at the requested adapter position. Will return 0 if * the position doesn't refer to an incoming message. */ + @MainThread long getReceivedTimestamp(int position) { if (isHeaderPosition(position)) return 0; if (isFooterPosition(position)) return 0; @@ -385,8 +375,9 @@ public class ConversationAdapter * Adds a record to a memory cache to allow it to be rendered immediately, as opposed to waiting * for a database change. */ + @MainThread void addFastRecord(MessageRecord record) { - fastRecords.add(record); + fastRecords.add(0, record); notifyDataSetChanged(); } @@ -426,6 +417,7 @@ public class ConversationAdapter } } + @MainThread private void cleanFastRecords() { synchronized (releasedFastRecords) { Iterator recordIterator = fastRecords.iterator(); diff --git a/app/src/main/java/org/thoughtcrime/securesms/conversation/ConversationData.java b/app/src/main/java/org/thoughtcrime/securesms/conversation/ConversationData.java index b598e84759..539d36ed1c 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/conversation/ConversationData.java +++ b/app/src/main/java/org/thoughtcrime/securesms/conversation/ConversationData.java @@ -5,18 +5,21 @@ package org.thoughtcrime.securesms.conversation; */ final class ConversationData { private final long lastSeen; + private final int lastSeenPosition; private final boolean hasSent; private final boolean isMessageRequestAccepted; private final boolean hasPreMessageRequestMessages; private final int jumpToPosition; ConversationData(long lastSeen, + int lastSeenPosition, boolean hasSent, boolean isMessageRequestAccepted, boolean hasPreMessageRequestMessages, int jumpToPosition) { this.lastSeen = lastSeen; + this.lastSeenPosition = lastSeenPosition; this.hasSent = hasSent; this.isMessageRequestAccepted = isMessageRequestAccepted; this.hasPreMessageRequestMessages = hasPreMessageRequestMessages; @@ -24,8 +27,12 @@ final class ConversationData { } long getLastSeen() { - return lastSeen; - } + return lastSeen; + } + + int getLastSeenPosition() { + return lastSeenPosition; + } boolean hasSent() { return hasSent; diff --git a/app/src/main/java/org/thoughtcrime/securesms/conversation/ConversationFragment.java b/app/src/main/java/org/thoughtcrime/securesms/conversation/ConversationFragment.java index 6b814a38c9..a50af2305b 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/conversation/ConversationFragment.java +++ b/app/src/main/java/org/thoughtcrime/securesms/conversation/ConversationFragment.java @@ -292,7 +292,7 @@ public class ConversationFragment extends Fragment { } public void moveToLastSeen() { - if (conversationViewModel.getLastSeen() <= 0) { + if (conversationViewModel.getLastSeenPosition() <= 0) { Log.i(TAG, "No need to move to last seen."); return; } @@ -302,7 +302,7 @@ public class ConversationFragment extends Fragment { return; } - int position = getListAdapter().findLastSeenPosition(conversationViewModel.getLastSeen()); + int position = getListAdapter().getAdapterPositionForMessagePosition(conversationViewModel.getLastSeenPosition()); scrollToLastSeenPosition(position); } @@ -391,14 +391,13 @@ public class ConversationFragment extends Fragment { private void initializeResources() { long oldThreadId = threadId; - long lastSeen = this.getActivity().getIntent().getLongExtra(ConversationActivity.LAST_SEEN_EXTRA, -1); - int startingPosition = this.getActivity().getIntent().getIntExtra(ConversationActivity.STARTING_POSITION_EXTRA, -1); + int startingPosition = this.getActivity().getIntent().getIntExtra(ConversationActivity.STARTING_POSITION_EXTRA, -1); this.recipient = Recipient.live(getActivity().getIntent().getParcelableExtra(ConversationActivity.RECIPIENT_EXTRA)); this.threadId = this.getActivity().getIntent().getLongExtra(ConversationActivity.THREAD_ID_EXTRA, -1); this.unknownSenderView = new UnknownSenderView(getActivity(), recipient.get(), threadId); - conversationViewModel.onConversationDataAvailable(threadId, lastSeen, startingPosition); + conversationViewModel.onConversationDataAvailable(threadId, startingPosition); OnScrollListener scrollListener = new ConversationScrollListener(getActivity()); list.addOnScrollListener(scrollListener); @@ -538,6 +537,7 @@ public class ConversationFragment extends Fragment { if (this.threadId != threadId) { this.threadId = threadId; messageRequestViewModel.setConversationInfo(recipient.getId(), threadId); + conversationViewModel.onConversationDataAvailable(threadId, -1); initializeListAdapter(); } } @@ -551,8 +551,6 @@ public class ConversationFragment extends Fragment { } public void setLastSeen(long lastSeen) { - conversationViewModel.onLastSeenChanged(lastSeen); - if (lastSeenDecoration != null) { list.removeItemDecoration(lastSeenDecoration); } @@ -864,9 +862,7 @@ public class ConversationFragment extends Fragment { adapter.setFooterView(null); } - if (conversationViewModel.getLastSeen() == -1) { - setLastSeen(conversation.getLastSeen()); - } + setLastSeen(conversation.getLastSeen()); if (FeatureFlags.messageRequests() && !conversation.hasPreMessageRequestMessages()) { clearHeaderIfNotTyping(adapter); @@ -880,34 +876,25 @@ public class ConversationFragment extends Fragment { listener.onCursorChanged(); - list.post(() -> { + int lastSeenPosition = adapter.getAdapterPositionForMessagePosition(conversation.getLastSeenPosition()); - int lastSeenPosition = adapter.findLastSeenPosition(conversationViewModel.getLastSeen()); - - if (isTypingIndicatorShowing()) { - lastSeenPosition = Math.max(lastSeenPosition - 1, 0); - } - - if (conversation.shouldJumpToMessage()) { - scrollToStartingPosition(conversation.getJumpToPosition()); - } else if (conversation.isMessageRequestAccepted()) { - scrollToLastSeenPosition(lastSeenPosition); - } - - if (lastSeenPosition <= 0) { - setLastSeen(0); - } - }); + if (conversation.shouldJumpToMessage()) { + scrollToStartingPosition(conversation.getJumpToPosition()); + } else if (conversation.isMessageRequestAccepted()) { + scrollToLastSeenPosition(lastSeenPosition); + } else if (FeatureFlags.messageRequests()) { + list.post(() -> getListLayoutManager().scrollToPosition(adapter.getItemCount() - 1)); + } } - private void scrollToStartingPosition(final int startingPosition) { + private void scrollToStartingPosition(int startingPosition) { list.post(() -> { list.getLayoutManager().scrollToPosition(startingPosition); getListAdapter().pulseHighlightItem(startingPosition); }); } - private void scrollToLastSeenPosition(final int lastSeenPosition) { + private void scrollToLastSeenPosition(int lastSeenPosition) { if (lastSeenPosition > 0) { list.post(() -> getListLayoutManager().scrollToPositionWithOffset(lastSeenPosition, list.getHeight())); } diff --git a/app/src/main/java/org/thoughtcrime/securesms/conversation/ConversationRepository.java b/app/src/main/java/org/thoughtcrime/securesms/conversation/ConversationRepository.java index a0475af943..ad2e29bf5d 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/conversation/ConversationRepository.java +++ b/app/src/main/java/org/thoughtcrime/securesms/conversation/ConversationRepository.java @@ -24,28 +24,34 @@ class ConversationRepository { this.executor = SignalExecutors.BOUNDED; } - LiveData getConversationData(long threadId, long lastSeen, int jumpToPosition) { + LiveData getConversationData(long threadId, int jumpToPosition) { MutableLiveData liveData = new MutableLiveData<>(); executor.execute(() -> { - liveData.postValue(getConversationDataInternal(threadId, lastSeen, jumpToPosition)); + liveData.postValue(getConversationDataInternal(threadId, jumpToPosition)); }); return liveData; } - private @NonNull ConversationData getConversationDataInternal(long threadId, long lastSeen, int jumpToPosition) { + private @NonNull ConversationData getConversationDataInternal(long threadId, int jumpToPosition) { Pair lastSeenAndHasSent = DatabaseFactory.getThreadDatabase(context).getLastSeenAndHasSent(threadId); - boolean hasSent = lastSeenAndHasSent.second(); - - if (lastSeen == -1) { - lastSeen = lastSeenAndHasSent.first(); - } + long lastSeen = lastSeenAndHasSent.first(); + boolean hasSent = lastSeenAndHasSent.second(); + int lastSeenPosition = 0; boolean isMessageRequestAccepted = RecipientUtil.isMessageRequestAccepted(context, threadId); boolean hasPreMessageRequestMessages = RecipientUtil.isPreMessageRequestThread(context, threadId); - return new ConversationData(lastSeen, hasSent, isMessageRequestAccepted, hasPreMessageRequestMessages, jumpToPosition); + if (lastSeen > 0) { + lastSeenPosition = DatabaseFactory.getMmsSmsDatabase(context).getMessagePositionForLastSeen(threadId, lastSeen); + } + + if (lastSeenPosition <= 0) { + lastSeen = 0; + } + + return new ConversationData(lastSeen, lastSeenPosition, hasSent, isMessageRequestAccepted, hasPreMessageRequestMessages, jumpToPosition); } } diff --git a/app/src/main/java/org/thoughtcrime/securesms/conversation/ConversationViewModel.java b/app/src/main/java/org/thoughtcrime/securesms/conversation/ConversationViewModel.java index adec08da63..afb9d86b7d 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/conversation/ConversationViewModel.java +++ b/app/src/main/java/org/thoughtcrime/securesms/conversation/ConversationViewModel.java @@ -31,10 +31,9 @@ class ConversationViewModel extends ViewModel { private final MutableLiveData> recentMedia; private final MutableLiveData threadId; private final LiveData> messages; - private final LiveData conversationMetadata; + private final LiveData conversationMetadata; private int jumpToPosition; - private long lastSeen; private ConversationViewModel() { this.context = ApplicationDependencies.getApplication(); @@ -56,7 +55,7 @@ class ConversationViewModel extends ViewModel { }); conversationMetadata = Transformations.switchMap(threadId, thread -> { - LiveData data = conversationRepository.getConversationData(thread, lastSeen, jumpToPosition); + LiveData data = conversationRepository.getConversationData(thread, jumpToPosition); jumpToPosition = -1; return data; }); @@ -66,17 +65,13 @@ class ConversationViewModel extends ViewModel { mediaRepository.getMediaInBucket(context, Media.ALL_MEDIA_BUCKET_ID, recentMedia::postValue); } - void onConversationDataAvailable(long threadId, long lastSeen, int startingPosition) { - this.lastSeen = lastSeen; + void onConversationDataAvailable(long threadId, int startingPosition) { + Log.d(TAG, "[onConversationDataAvailable] threadId: " + threadId + ", startingPosition: " + startingPosition); this.jumpToPosition = startingPosition; this.threadId.setValue(threadId); } - void onLastSeenChanged(long lastSeen) { - this.lastSeen = lastSeen; - } - @NonNull LiveData> getRecentMedia() { return recentMedia; } @@ -90,7 +85,11 @@ class ConversationViewModel extends ViewModel { } long getLastSeen() { - return lastSeen; + return conversationMetadata.getValue() != null ? conversationMetadata.getValue().getLastSeen() : 0; + } + + int getLastSeenPosition() { + return conversationMetadata.getValue() != null ? conversationMetadata.getValue().getLastSeenPosition() : 0; } static class Factory extends ViewModelProvider.NewInstanceFactory { diff --git a/app/src/main/java/org/thoughtcrime/securesms/conversationlist/ConversationListFragment.java b/app/src/main/java/org/thoughtcrime/securesms/conversationlist/ConversationListFragment.java index e214f20b98..ad6b0f8d73 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/conversationlist/ConversationListFragment.java +++ b/app/src/main/java/org/thoughtcrime/securesms/conversationlist/ConversationListFragment.java @@ -347,7 +347,6 @@ public class ConversationListFragment extends MainFragment implements LoaderMana getNavigator().goToConversation(threadRecord.getRecipient().getId(), threadRecord.getThreadId(), threadRecord.getDistributionType(), - threadRecord.getLastSeen(), -1); } @@ -360,7 +359,6 @@ public class ConversationListFragment extends MainFragment implements LoaderMana getNavigator().goToConversation(contact.getId(), threadId, ThreadDatabase.DistributionTypes.DEFAULT, - -1, -1); }); } @@ -375,7 +373,6 @@ public class ConversationListFragment extends MainFragment implements LoaderMana getNavigator().goToConversation(message.conversationRecipient.getId(), message.threadId, ThreadDatabase.DistributionTypes.DEFAULT, - -1, startingPosition); }); } @@ -691,8 +688,8 @@ public class ConversationListFragment extends MainFragment implements LoaderMana actionMode.setTitle(String.valueOf(defaultAdapter.getBatchSelections().size())); } - private void handleCreateConversation(long threadId, Recipient recipient, int distributionType, long lastSeen) { - getNavigator().goToConversation(recipient.getId(), threadId, distributionType, lastSeen, -1); + private void handleCreateConversation(long threadId, Recipient recipient, int distributionType) { + getNavigator().goToConversation(recipient.getId(), threadId, distributionType, -1); } @Override @@ -726,8 +723,7 @@ public class ConversationListFragment extends MainFragment implements LoaderMana @Override public void onItemClick(ConversationListItem item) { if (actionMode == null) { - handleCreateConversation(item.getThreadId(), item.getRecipient(), - item.getDistributionType(), item.getLastSeen()); + handleCreateConversation(item.getThreadId(), item.getRecipient(), item.getDistributionType()); } else { ConversationListAdapter adapter = (ConversationListAdapter)list.getAdapter(); adapter.toggleThreadInBatchSet(item.getThreadId()); diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/MmsSmsDatabase.java b/app/src/main/java/org/thoughtcrime/securesms/database/MmsSmsDatabase.java index f86e3d1745..d8b10193f5 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/database/MmsSmsDatabase.java +++ b/app/src/main/java/org/thoughtcrime/securesms/database/MmsSmsDatabase.java @@ -125,6 +125,18 @@ public class MmsSmsDatabase extends Database { return new Pair<>(id, latestQuit); } + public int getMessagePositionForLastSeen(long threadId, long lastSeen) { + String[] projection = new String[] { "COUNT(*)" }; + String selection = MmsSmsColumns.THREAD_ID + " = " + threadId + " AND " + MmsSmsColumns.NORMALIZED_DATE_RECEIVED + " > " + lastSeen; + + try (Cursor cursor = queryTables(projection, selection, null, null)) { + if (cursor != null && cursor.moveToNext()) { + return cursor.getInt(0); + } + } + return 0; + } + public @Nullable MessageRecord getMessageFor(long timestamp, RecipientId author) { MmsSmsDatabase db = DatabaseFactory.getMmsSmsDatabase(context); diff --git a/app/src/main/java/org/thoughtcrime/securesms/notifications/NotificationItem.java b/app/src/main/java/org/thoughtcrime/securesms/notifications/NotificationItem.java index 74d65f00bf..49fd09880b 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/notifications/NotificationItem.java +++ b/app/src/main/java/org/thoughtcrime/securesms/notifications/NotificationItem.java @@ -80,7 +80,7 @@ public class NotificationItem { public PendingIntent getPendingIntent(Context context) { Recipient recipient = threadRecipient != null ? threadRecipient : conversationRecipient; int startingPosition = jumpToMessage ? getStartingPosition(context, threadId, messageReceivedTimestamp) : -1; - Intent intent = ConversationActivity.buildIntent(context, recipient.getId(), threadId, 0, -1, startingPosition); + Intent intent = ConversationActivity.buildIntent(context, recipient.getId(), threadId, 0, startingPosition); makeIntentUniqueToPreventMerging(intent);