Skip to content

Commit 2157b22

Browse files
0nkokarlenDimla
authored andcommitted
Tab manager crash fix (#4865)
Task/Issue URL: https://app.asana.com/0/1202552961248957/1207981775109546/f Possibly also: https://app.asana.com/0/1202552961248957/1207981775109549 ### Description This PR adds bounds checks for the tab getter in the RecyclerView adapter. The first crash is caused by a rapid double tap on the tab close button, which results in an invalid index being returned. The second crash is possibly a different manifestation of the same problem but due to a race condition it happens elsewhere. It should be resolved by [8bfcfbd](8bfcfbd). ### Steps to test this PR _Duble tap_ - [x] Go to the tab manager - [x] Add a few tabs - [x] Tap on a tab close button very quickly - [x] Notice the app doesn’t crash
1 parent e0c4a87 commit 2157b22

File tree

5 files changed

+15
-18
lines changed

5 files changed

+15
-18
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ import com.duckduckgo.app.tabs.model.TabEntity
2222

2323
class TabEntityDiffCallback(
2424
private val oldList: List<TabEntity>,
25-
var newList: List<TabEntity>,
25+
private val newList: List<TabEntity>,
2626
) : DiffUtil.Callback() {
2727

2828
private fun areItemsTheSame(

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

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -53,14 +53,10 @@ class TabGridItemDecorator(
5353
val child = recyclerView.getChildAt(i)
5454

5555
val positionInAdapter = recyclerView.getChildAdapterPosition(child)
56-
if (positionInAdapter < 0) {
57-
continue
58-
}
59-
60-
val tab = adapter.getTab(positionInAdapter)
61-
62-
if (tab.tabId == selectedTabId) {
63-
drawSelectedTabDecoration(child, canvas)
56+
adapter.getTab(positionInAdapter)?.let { tab ->
57+
if (tab.tabId == selectedTabId) {
58+
drawSelectedTabDecoration(child, canvas)
59+
}
6460
}
6561
}
6662

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

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -220,7 +220,9 @@ class TabSwitcherActivity : DuckDuckGoActivity(), TabSwitcherListener, Coroutine
220220

221221
private fun scrollToShowCurrentTab() {
222222
val index = tabsAdapter.adapterPositionForTab(selectedTabId)
223-
tabsRecycler.post { tabsRecycler.scrollToPosition(index) }
223+
if (index != -1) {
224+
tabsRecycler.post { tabsRecycler.scrollToPosition(index) }
225+
}
224226
}
225227

226228
private fun processCommand(command: Command) {
@@ -297,8 +299,9 @@ class TabSwitcherActivity : DuckDuckGoActivity(), TabSwitcherListener, Coroutine
297299
}
298300

299301
override fun onTabDeleted(position: Int, deletedBySwipe: Boolean) {
300-
val tab = tabsAdapter.getTab(position)
301-
launch { viewModel.onMarkTabAsDeletable(tab, deletedBySwipe) }
302+
tabsAdapter.getTab(position)?.let { tab ->
303+
launch { viewModel.onMarkTabAsDeletable(tab, deletedBySwipe) }
304+
}
302305
}
303306

304307
override fun onTabMoved(from: Int, to: Int) {

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

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,6 @@ class TabSwitcherAdapter(
5454
) : Adapter<TabViewHolder>() {
5555

5656
private val list = mutableListOf<TabEntity>()
57-
private val diffCallback = TabEntityDiffCallback(list, listOf())
5857

5958
private var isDragging: Boolean = false
6059

@@ -201,15 +200,14 @@ class TabSwitcherAdapter(
201200
}
202201

203202
private fun submitList(updatedList: List<TabEntity>) {
204-
diffCallback.newList = updatedList
205-
val diffResult = DiffUtil.calculateDiff(diffCallback)
203+
val diffResult = DiffUtil.calculateDiff(TabEntityDiffCallback(list, updatedList))
206204

207205
list.clear()
208206
list.addAll(updatedList)
209207
diffResult.dispatchUpdatesTo(this)
210208
}
211209

212-
fun getTab(position: Int): TabEntity = list[position]
210+
fun getTab(position: Int): TabEntity? = list.getOrNull(position)
213211

214212
fun adapterPositionForTab(tabId: String?): Int {
215213
if (tabId == null) return -1

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,9 +55,9 @@ class TabSwitcherViewModel @Inject constructor(
5555
const val REINSTALL_VARIANT = "ru"
5656
}
5757

58-
var tabs: LiveData<List<TabEntity>> = tabRepository.liveTabs
58+
val tabs: LiveData<List<TabEntity>> = tabRepository.liveTabs
5959
val activeTab = tabRepository.liveSelectedTab
60-
var deletableTabs: LiveData<List<TabEntity>> = tabRepository.flowDeletableTabs.asLiveData(
60+
val deletableTabs: LiveData<List<TabEntity>> = tabRepository.flowDeletableTabs.asLiveData(
6161
context = viewModelScope.coroutineContext,
6262
)
6363

0 commit comments

Comments
 (0)