Fix CFv2 initial scrolling bugs.

app/src/main/java/androidx/recyclerview/widget/ConversationLayoutManager.kt

@@ -0,0 +1,76 @@
+/*
+ * Copyright 2023 Signal Messenger, LLC
+ * SPDX-License-Identifier: AGPL-3.0-only
+ */
+
+package androidx.recyclerview.widget
+
+import android.content.Context
+import org.signal.core.util.logging.Log
+
+/**
+ * Variation of a vertical, reversed [LinearLayoutManager] that makes specific assumptions in how it will
+ * be used by Conversation view to support easier scrolling to the initial start position.
+ *
+ * Primarily, it assumes that an initial scroll to position call will always happen and that the implementation
+ * of [LinearLayoutManager] remains unchanged with respect to how it assigns [mPendingScrollPosition] and
+ * [mPendingScrollPositionOffset] in [LinearLayoutManager.scrollToPositionWithOffset] and how it always clears
+ * the pending state variables in every call to [LinearLayoutManager.onLayoutCompleted].
+ *
+ * The assumptions are necessary to force the requested scroll position/layout to occur even if the request
+ * happens prior to the data source populating the recycler view/adapter.
+ */
+class ConversationLayoutManager(context: Context) : LinearLayoutManager(context, RecyclerView.VERTICAL, true) {
+
+  private var afterScroll: (() -> Unit)? = null
+
+  override fun supportsPredictiveItemAnimations(): Boolean {
+    return false
+  }
+
+  /**
+   * Scroll to the desired position and be notified when the layout manager has completed the request
+   * via [afterScroll] callback.
+   */
+  fun scrollToPositionWithOffset(position: Int, offset: Int, afterScroll: () -> Unit) {
+    this.afterScroll = afterScroll
+    super.scrollToPositionWithOffset(position, offset)
+  }
+
+  /**
+   * If a scroll to position request is made and a layout pass occurs prior to the list being populated with via the data source,
+   * the base implementation clears the request as if it was never made.
+   *
+   * This override will capture the pending scroll position and offset, determine if the scroll request was satisfied, and
+   * re-request the scroll to position to force another attempt if not satisfied.
+   *
+   * A pending scroll request will be re-requested if the pending scroll position is outside the bounds of the current known size of
+   * items in the list.
+   */
+  override fun onLayoutCompleted(state: RecyclerView.State?) {
+    val pendingScrollPosition = mPendingScrollPosition
+    val pendingScrollOffset = mPendingScrollPositionOffset
+
+    val reRequestPendingPosition = pendingScrollPosition >= (state?.mItemCount ?: 0)
+
+    // Base implementation always clears mPendingScrollPosition+mPendingScrollPositionOffset
+    super.onLayoutCompleted(state)
+
+    // Re-request scroll to position request if necessary thus forcing mPendingScrollPosition+mPendingScrollPositionOffset to be re-assigned
+    if (reRequestPendingPosition) {
+      Log.d(TAG, "Re-requesting pending scroll position: $pendingScrollPosition offset: $pendingScrollOffset")
+      if (pendingScrollOffset != INVALID_OFFSET) {
+        scrollToPositionWithOffset(pendingScrollPosition, pendingScrollOffset)
+      } else {
+        scrollToPosition(pendingScrollPosition)
+      }
+    } else {
+      afterScroll?.invoke()
+      afterScroll = null
+    }
+  }
+
+  companion object {
+    private val TAG = Log.tag(ConversationLayoutManager::class.java)
+  }
+}

app/src/main/java/org/thoughtcrime/securesms/conversation/MarkReadHelper.java

@@ -8,6 +8,7 @@ package org.thoughtcrime.securesms.conversation;
 import android.content.Context;
 
 import androidx.annotation.NonNull;
+import androidx.annotation.Nullable;
 import androidx.lifecycle.Lifecycle;
 import androidx.lifecycle.LifecycleOwner;
 import androidx.recyclerview.widget.LinearLayoutManager;
@@ -39,9 +40,10 @@ public class MarkReadHelper {
 
   private final ConversationId conversationId;
   private final Context        context;
-  private final LifecycleOwner     lifecycleOwner;
-  private final Debouncer          debouncer = new Debouncer(DEBOUNCE_TIMEOUT);
-  private       long               latestTimestamp;
+  private final LifecycleOwner lifecycleOwner;
+  private final Debouncer      debouncer         = new Debouncer(DEBOUNCE_TIMEOUT);
+  private       long           latestTimestamp;
+  private       boolean        ignoreViewReveals = false;
 
   public MarkReadHelper(@NonNull ConversationId conversationId, @NonNull Context context, @NonNull LifecycleOwner lifecycleOwner) {
     this.conversationId = conversationId;
@@ -50,7 +52,7 @@ public class MarkReadHelper {
   }
 
   public void onViewsRevealed(long timestamp) {
-    if (timestamp <= latestTimestamp || lifecycleOwner.getLifecycle().getCurrentState() != Lifecycle.State.RESUMED) {
+    if (timestamp <= latestTimestamp || lifecycleOwner.getLifecycle().getCurrentState() != Lifecycle.State.RESUMED || ignoreViewReveals) {
       return;
     }
 
@@ -69,11 +71,33 @@ public class MarkReadHelper {
     });
   }
 
+  /**
+   * Prevent calls to {@link #onViewsRevealed(long)} from causing messages to be marked read.
+   * <p>
+   * This is particularly useful when the conversation could move around after views are
+   * displayed (e.g., initial scrolling to start position).
+   */
+  public void ignoreViewReveals() {
+    ignoreViewReveals = true;
+  }
+
+  /**
+   * Stop preventing calls to {@link #onViewsRevealed(long)} from not marking messages as read.
+   *
+   * @param timestamp Timestamp of most recent reveal messages, same as usually provided to {@code onViewsRevealed}
+   */
+  public void stopIgnoringViewReveals(@Nullable Long timestamp) {
+    this.ignoreViewReveals = false;
+    if (timestamp != null) {
+      onViewsRevealed(timestamp);
+    }
+  }
+
   /**
    * Given the adapter and manager, figure out the timestamp to mark read up to.
    *
-   * @param conversationAdapter   The conversation thread's adapter
-   * @param layoutManager         The conversation thread's layout manager
+   * @param conversationAdapter The conversation thread's adapter
+   * @param layoutManager       The conversation thread's layout manager
    * @return A Present(Long) if there's a timestamp to proceed with, or Empty if this request should be ignored.
    */
   @SuppressWarnings("resource")

app/src/main/java/org/thoughtcrime/securesms/conversation/v2/ConversationFragment.kt

@@ -60,6 +60,7 @@ import androidx.lifecycle.DefaultLifecycleObserver
 import androidx.lifecycle.Lifecycle
 import androidx.lifecycle.LifecycleOwner
 import androidx.lifecycle.Observer
+import androidx.recyclerview.widget.ConversationLayoutManager
 import androidx.recyclerview.widget.LinearLayoutManager
 import androidx.recyclerview.widget.RecyclerView
 import com.google.android.material.dialog.MaterialAlertDialogBuilder
@@ -118,7 +119,6 @@ import org.thoughtcrime.securesms.components.location.SignalPlace
 import org.thoughtcrime.securesms.components.mention.MentionAnnotation
 import org.thoughtcrime.securesms.components.menu.ActionItem
 import org.thoughtcrime.securesms.components.menu.SignalBottomActionBar
-import org.thoughtcrime.securesms.components.recyclerview.SmoothScrollingLinearLayoutManager
 import org.thoughtcrime.securesms.components.settings.app.subscription.donate.DonateToSignalFragment
 import org.thoughtcrime.securesms.components.settings.app.subscription.donate.DonateToSignalType
 import org.thoughtcrime.securesms.components.settings.conversation.ConversationSettingsActivity
@@ -442,7 +442,7 @@ class ConversationFragment :
   private val textDraftSaveDebouncer = Debouncer(500)
   private val recentEmojis: RecentEmojiPageModel by lazy { RecentEmojiPageModel(ApplicationDependencies.getApplication(), TextSecurePreferences.RECENT_STORAGE_KEY) }
 
-  private lateinit var layoutManager: LinearLayoutManager
+  private lateinit var layoutManager: ConversationLayoutManager
   private lateinit var markReadHelper: MarkReadHelper
   private lateinit var giphyMp4ProjectionRecycler: GiphyMp4ProjectionRecycler
   private lateinit var addToContactsLauncher: ActivityResultLauncher<Intent>
@@ -519,6 +519,7 @@ class ConversationFragment :
     FullscreenHelper(requireActivity()).showSystemUI()
 
     markReadHelper = MarkReadHelper(ConversationId.forConversation(args.threadId), requireContext(), viewLifecycleOwner)
+    markReadHelper.ignoreViewReveals()
 
     initializeConversationThreadUi()
 
@@ -778,7 +779,6 @@ class ConversationFragment :
             binding.conversationItemRecycler.doAfterNextLayout {
               SignalLocalMetrics.ConversationOpen.onRenderFinished()
               doAfterFirstRender()
-              animationsAllowed = true
             }
           }
         }
@@ -1395,7 +1395,7 @@ class ConversationFragment :
   private fun getVoiceNoteMediaController() = requireListener<VoiceNoteMediaControllerOwner>().voiceNoteMediaController
 
   private fun initializeConversationThreadUi() {
-    layoutManager = SmoothScrollingLinearLayoutManager(requireContext(), true)
+    layoutManager = ConversationLayoutManager(requireContext())
     binding.conversationItemRecycler.setHasFixedSize(false)
     binding.conversationItemRecycler.layoutManager = layoutManager
     binding.conversationItemRecycler.addOnScrollListener(ScrollListener())
@@ -2105,15 +2105,43 @@ class ConversationFragment :
   //region Scroll Handling
 
   private fun moveToStartPosition(meta: ConversationData) {
-    scrollToPositionDelegate.requestScrollPosition(
-      position = meta.getStartPosition(),
-      smooth = true,
-      scrollStrategy = if (meta.shouldJumpToMessage()) {
-        jumpAndPulseScrollStrategy
-      } else {
-        ScrollToPositionDelegate.DefaultScrollStrategy
+    if (meta.getStartPosition() == 0) {
+      layoutManager.scrollToPositionWithOffset(0, 0) {
+        animationsAllowed = true
+        markReadHelper.stopIgnoringViewReveals(MarkReadHelper.getLatestTimestamp(adapter, layoutManager).orNull())
       }
-    )
+    } else {
+      binding.toolbar.viewTreeObserver.addOnGlobalLayoutListener(StartPositionScroller(meta))
+    }
+  }
+
+  /** Helper to scroll the conversation to the correct position and offset based on toolbar height and the type of position */
+  private inner class StartPositionScroller(private val meta: ConversationData) : ViewTreeObserver.OnGlobalLayoutListener {
+
+    override fun onGlobalLayout() {
+      val rect = Rect()
+      binding.toolbar.getGlobalVisibleRect(rect)
+      val toolbarOffset = rect.bottom
+      binding.toolbar.viewTreeObserver.removeOnGlobalLayoutListener(this)
+
+      val offset = when {
+        meta.getStartPosition() == 0 -> 0
+        meta.shouldJumpToMessage() -> (binding.conversationItemRecycler.height - toolbarOffset) / 4
+        meta.shouldScrollToLastSeen() -> binding.conversationItemRecycler.height - toolbarOffset
+        else -> binding.conversationItemRecycler.height
+      }
+
+      Log.d(TAG, "Scrolling to start position ${meta.getStartPosition()}")
+      layoutManager.scrollToPositionWithOffset(meta.getStartPosition(), offset) {
+        animationsAllowed = true
+        markReadHelper.stopIgnoringViewReveals(MarkReadHelper.getLatestTimestamp(adapter, layoutManager).orNull())
+        if (meta.shouldJumpToMessage()) {
+          binding.conversationItemRecycler.post {
+            adapter.pulseAtPosition(meta.getStartPosition())
+          }
+        }
+      }
+    }
   }
 
   /**