Skip to content

Commit 45f7066

Browse files
authored
Tab manager crash fix (#4878)
Task/Issue URL: https://app.asana.com/0/1207418217763355/1208000508832396/f ### Description This is the 2nd attempt at fixing the crash. I wasn’t able to reproduce it, but the theory is that the adapter list gets modified while it’s being diffed, which results in an inconsistency and `IndexOutOfBoundsException` being thrown. The fix for this is to create a local copy of the diffed lists, which should resolve race condition issue. I also added index range checks to be extra careful even though they *should* be redundant. ### Steps to test this PR I wasn’t able to reproduce the crash. Smoke-testing the drag & drop feature should be enough.
1 parent 03d2add commit 45f7066

File tree

2 files changed

+22
-17
lines changed

2 files changed

+22
-17
lines changed

app/src/main/java/com/duckduckgo/app/browser/tabpreview/TabEntityDiffCallback.kt

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,11 @@ import android.os.Bundle
2020
import androidx.recyclerview.widget.DiffUtil
2121
import com.duckduckgo.app.tabs.model.TabEntity
2222

23-
class TabEntityDiffCallback(
24-
private val oldList: List<TabEntity>,
25-
private val newList: List<TabEntity>,
26-
) : DiffUtil.Callback() {
23+
class TabEntityDiffCallback(old: List<TabEntity>, new: List<TabEntity>) : DiffUtil.Callback() {
24+
25+
// keep a local copy of the lists to avoid any changes to the lists during the diffing process
26+
private val oldList = old.toList()
27+
private val newList = new.toList()
2728

2829
private fun areItemsTheSame(
2930
oldItem: TabEntity,
@@ -76,15 +77,27 @@ class TabEntityDiffCallback(
7677
}
7778

7879
override fun areItemsTheSame(oldItemPosition: Int, newItemPosition: Int): Boolean {
79-
return areItemsTheSame(oldList[oldItemPosition], newList[newItemPosition])
80+
return if (oldItemPosition in oldList.indices && newItemPosition in newList.indices) {
81+
areItemsTheSame(oldList[oldItemPosition], newList[newItemPosition])
82+
} else {
83+
false
84+
}
8085
}
8186

8287
override fun areContentsTheSame(oldItemPosition: Int, newItemPosition: Int): Boolean {
83-
return areContentsTheSame(oldList[oldItemPosition], newList[newItemPosition])
88+
return if (oldItemPosition in oldList.indices && newItemPosition in newList.indices) {
89+
areContentsTheSame(oldList[oldItemPosition], newList[newItemPosition])
90+
} else {
91+
false
92+
}
8493
}
8594

8695
override fun getChangePayload(oldItemPosition: Int, newItemPosition: Int): Any {
87-
return getChangePayload(oldList[oldItemPosition], newList[newItemPosition])
96+
return if (oldItemPosition in oldList.indices && newItemPosition in newList.indices) {
97+
getChangePayload(oldList[oldItemPosition], newList[newItemPosition])
98+
} else {
99+
Bundle()
100+
}
88101
}
89102

90103
companion object {

app/src/main/java/com/duckduckgo/app/tabs/ui/TabSwitcherAdapter.kt

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -261,13 +261,7 @@ class TabSwitcherAdapter(
261261
}
262262
}
263263

264-
fun updateData(data: List<TabEntity>?) {
265-
if (data != null) {
266-
submitList(data)
267-
}
268-
}
269-
270-
private fun submitList(updatedList: List<TabEntity>) {
264+
fun updateData(updatedList: List<TabEntity>) {
271265
val diffResult = DiffUtil.calculateDiff(TabEntityDiffCallback(list, updatedList))
272266

273267
list.clear()
@@ -292,9 +286,7 @@ class TabSwitcherAdapter(
292286

293287
fun onTabMoved(from: Int, to: Int) {
294288
val swapped = list.swap(from, to)
295-
list.clear()
296-
list.addAll(swapped)
297-
notifyItemMoved(from, to)
289+
updateData(swapped)
298290
}
299291

300292
@SuppressLint("NotifyDataSetChanged")

0 commit comments

Comments
 (0)