Skip to content

Tab manager crash fix #4865

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Aug 6, 2024
Merged

Tab manager crash fix #4865

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
079881f
Add a check for invalid index before deletion
0nko Aug 5, 2024
bb60e26
Add a check before scrolling
0nko Aug 6, 2024
8bfcfbd
Possibly avoid a race condition by always creating a new diff callbac…
0nko Aug 6, 2024
d1588f5
Make the getTab() call safer
0nko Aug 6, 2024
df36fcb
Make the VM properties immutable
0nko Aug 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion app/src/main/java/com/duckduckgo/app/browser/tabpreview/TabEntityDiffCallback.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import com.duckduckgo.app.tabs.model.TabEntity

class TabEntityDiffCallback(
private val oldList: List<TabEntity>,
var newList: List<TabEntity>,
private val newList: List<TabEntity>,
) : DiffUtil.Callback() {

private fun areItemsTheSame(
Expand Down
12 changes: 4 additions & 8 deletions app/src/main/java/com/duckduckgo/app/tabs/ui/TabGridItemDecorator.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,14 +53,10 @@ class TabGridItemDecorator(
val child = recyclerView.getChildAt(i)

val positionInAdapter = recyclerView.getChildAdapterPosition(child)
if (positionInAdapter < 0) {
continue
}

val tab = adapter.getTab(positionInAdapter)

if (tab.tabId == selectedTabId) {
drawSelectedTabDecoration(child, canvas)
adapter.getTab(positionInAdapter)?.let { tab ->
if (tab.tabId == selectedTabId) {
drawSelectedTabDecoration(child, canvas)
}
}
}

Expand Down
9 changes: 6 additions & 3 deletions app/src/main/java/com/duckduckgo/app/tabs/ui/TabSwitcherActivity.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,9 @@ class TabSwitcherActivity : DuckDuckGoActivity(), TabSwitcherListener, Coroutine

private fun scrollToShowCurrentTab() {
val index = tabsAdapter.adapterPositionForTab(selectedTabId)
tabsRecycler.post { tabsRecycler.scrollToPosition(index) }
if (index != -1) {
tabsRecycler.post { tabsRecycler.scrollToPosition(index) }
}
}

private fun processCommand(command: Command) {
Expand Down Expand Up @@ -297,8 +299,9 @@ class TabSwitcherActivity : DuckDuckGoActivity(), TabSwitcherListener, Coroutine
}

override fun onTabDeleted(position: Int, deletedBySwipe: Boolean) {
val tab = tabsAdapter.getTab(position)
launch { viewModel.onMarkTabAsDeletable(tab, deletedBySwipe) }
tabsAdapter.getTab(position)?.let { tab ->
launch { viewModel.onMarkTabAsDeletable(tab, deletedBySwipe) }
}
}

override fun onTabMoved(from: Int, to: Int) {
Expand Down
6 changes: 2 additions & 4 deletions app/src/main/java/com/duckduckgo/app/tabs/ui/TabSwitcherAdapter.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@ class TabSwitcherAdapter(
) : Adapter<TabViewHolder>() {

private val list = mutableListOf<TabEntity>()
private val diffCallback = TabEntityDiffCallback(list, listOf())

private var isDragging: Boolean = false

Expand Down Expand Up @@ -201,15 +200,14 @@ class TabSwitcherAdapter(
}

private fun submitList(updatedList: List<TabEntity>) {
diffCallback.newList = updatedList
val diffResult = DiffUtil.calculateDiff(diffCallback)
val diffResult = DiffUtil.calculateDiff(TabEntityDiffCallback(list, updatedList))

list.clear()
list.addAll(updatedList)
diffResult.dispatchUpdatesTo(this)
}

fun getTab(position: Int): TabEntity = list[position]
fun getTab(position: Int): TabEntity? = list.getOrNull(position)

fun adapterPositionForTab(tabId: String?): Int {
if (tabId == null) return -1
Expand Down
4 changes: 2 additions & 2 deletions app/src/main/java/com/duckduckgo/app/tabs/ui/TabSwitcherViewModel.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,9 @@ class TabSwitcherViewModel @Inject constructor(
const val REINSTALL_VARIANT = "ru"
}

var tabs: LiveData<List<TabEntity>> = tabRepository.liveTabs
val tabs: LiveData<List<TabEntity>> = tabRepository.liveTabs
val activeTab = tabRepository.liveSelectedTab
var deletableTabs: LiveData<List<TabEntity>> = tabRepository.flowDeletableTabs.asLiveData(
val deletableTabs: LiveData<List<TabEntity>> = tabRepository.flowDeletableTabs.asLiveData(
context = viewModelScope.coroutineContext,
)

Expand Down