From b895ce936e082d5ce783cde48f549078d3212b34 Mon Sep 17 00:00:00 2001 From: Konrad Pozniak Date: Wed, 19 Jun 2024 16:51:12 +0200 Subject: [PATCH] improve MediaPreviewAdapter / fix IndexOutOfBoundsException (#4514) Found this crash in the Google Play reports: ``` Exception java.lang.IndexOutOfBoundsException: Index -1 out of bounds for length 0 at jdk.internal.util.Preconditions.outOfBounds (Preconditions.java:64) at jdk.internal.util.Preconditions.outOfBoundsCheckIndex (Preconditions.java:70) at jdk.internal.util.Preconditions.checkIndex (Preconditions.java:266) at java.util.Objects.checkIndex (Objects.java:359) at java.util.ArrayList.get (ArrayList.java:434) at java.util.Collections$UnmodifiableList.get (Collections.java:1394) at com.keylesspalace.tusky.components.compose.MediaPreviewAdapter.onMediaClick (MediaPreviewAdapter.java:45) at com.keylesspalace.tusky.components.compose.MediaPreviewAdapter.access$onMediaClick (MediaPreviewAdapter.java:32) at com.keylesspalace.tusky.components.compose.MediaPreviewAdapter$PreviewViewHolder._init_$lambda$0 (MediaPreviewAdapter.java:144) at android.view.View.performClick (View.java:7535) at android.view.View.performClickInternal (View.java:7512) at android.view.View.-$$Nest$mperformClickInternal at android.view.View$PerformClick.run (View.java:29488) at android.os.Handler.handleCallback (Handler.java:984) at android.os.Handler.dispatchMessage (Handler.java:104) at android.os.Looper.loopOnce (Looper.java:238) at android.os.Looper.loop (Looper.java:357) at android.app.ActivityThread.main (ActivityThread.java:8118) at java.lang.reflect.Method.invoke at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run (RuntimeInit.java:548) at com.android.internal.os.ZygoteInit.main (ZygoteInit.java:957) ``` Can't reproduce, but seems to be some kind of race condition where the view is clicked at the same time as it is being removed from the `RecyclerView`. Not using the index in the click listener should resolve the problem. Also refactored to `ListAdapter` to not deal with the `AsyncListDiffer` manually. --- .../components/compose/MediaPreviewAdapter.kt | 49 +++++++------------ 1 file changed, 18 insertions(+), 31 deletions(-) diff --git a/app/src/main/java/com/keylesspalace/tusky/components/compose/MediaPreviewAdapter.kt b/app/src/main/java/com/keylesspalace/tusky/components/compose/MediaPreviewAdapter.kt index bfc814293..5837405af 100644 --- a/app/src/main/java/com/keylesspalace/tusky/components/compose/MediaPreviewAdapter.kt +++ b/app/src/main/java/com/keylesspalace/tusky/components/compose/MediaPreviewAdapter.kt @@ -21,8 +21,8 @@ import android.view.ViewGroup import android.widget.ImageView import android.widget.PopupMenu import androidx.constraintlayout.widget.ConstraintLayout -import androidx.recyclerview.widget.AsyncListDiffer import androidx.recyclerview.widget.DiffUtil +import androidx.recyclerview.widget.ListAdapter import androidx.recyclerview.widget.RecyclerView import com.bumptech.glide.Glide import com.bumptech.glide.load.engine.DiskCacheStrategy @@ -35,14 +35,21 @@ class MediaPreviewAdapter( private val onAddFocus: (ComposeActivity.QueuedMedia) -> Unit, private val onEditImage: (ComposeActivity.QueuedMedia) -> Unit, private val onRemove: (ComposeActivity.QueuedMedia) -> Unit -) : RecyclerView.Adapter() { +) : ListAdapter( + object : DiffUtil.ItemCallback() { + override fun areItemsTheSame( + oldItem: ComposeActivity.QueuedMedia, + newItem: ComposeActivity.QueuedMedia + ) = oldItem.localId == newItem.localId - fun submitList(list: List) { - this.differ.submitList(list) + override fun areContentsTheSame( + oldItem: ComposeActivity.QueuedMedia, + newItem: ComposeActivity.QueuedMedia + ) = oldItem == newItem } +) { - private fun onMediaClick(position: Int, view: View) { - val item = differ.currentList[position] + private fun onMediaClick(item: ComposeActivity.QueuedMedia, view: View) { val popup = PopupMenu(view.context, view) val addCaptionId = 1 val addFocusId = 2 @@ -73,14 +80,12 @@ class MediaPreviewAdapter( private val thumbnailViewSize = context.resources.getDimensionPixelSize(R.dimen.compose_media_preview_size) - override fun getItemCount(): Int = differ.currentList.size - override fun onCreateViewHolder(parent: ViewGroup, viewType: Int): PreviewViewHolder { return PreviewViewHolder(ProgressImageView(parent.context)) } override fun onBindViewHolder(holder: PreviewViewHolder, position: Int) { - val item = differ.currentList[position] + val item = getItem(position) holder.progressImageView.setChecked(!item.description.isNullOrEmpty()) holder.progressImageView.setProgress(item.uploadPercent) if (item.type == ComposeActivity.QueuedMedia.Type.AUDIO) { @@ -107,28 +112,13 @@ class MediaPreviewAdapter( } glide.into(imageView) + + holder.progressImageView.setOnClickListener { + onMediaClick(item, holder.progressImageView) + } } } - private val differ = AsyncListDiffer( - this, - object : DiffUtil.ItemCallback() { - override fun areItemsTheSame( - oldItem: ComposeActivity.QueuedMedia, - newItem: ComposeActivity.QueuedMedia - ): Boolean { - return oldItem.localId == newItem.localId - } - - override fun areContentsTheSame( - oldItem: ComposeActivity.QueuedMedia, - newItem: ComposeActivity.QueuedMedia - ): Boolean { - return oldItem == newItem - } - } - ) - inner class PreviewViewHolder(val progressImageView: ProgressImageView) : RecyclerView.ViewHolder(progressImageView) { init { @@ -140,9 +130,6 @@ class MediaPreviewAdapter( layoutParams.setMargins(margin, 0, margin, marginBottom) progressImageView.layoutParams = layoutParams progressImageView.scaleType = ImageView.ScaleType.CENTER_CROP - progressImageView.setOnClickListener { - onMediaClick(bindingAdapterPosition, progressImageView) - } } } }