Skip to content

Commit

Permalink
Merge pull request #4164 from kiwix/Fixes#4163
Browse files Browse the repository at this point in the history 
Fixed: Application crash caused by "Input dispatching timed out" while retrieving storageDeviceList information in the online library.
  • Loading branch information
@kelson42
kelson42 authored Jan 6, 2025
2 parents 3c4614e + 7f6f0d4 commit 1e5bc58
Show file tree
Hide file tree
Showing 8 changed files with 110 additions and 88 deletions.
21 changes: 20 additions & 1 deletion app/src/main/java/org/kiwix/kiwixmobile/main/KiwixMainActivity.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ import androidx.navigation.fragment.NavHostFragment
import androidx.navigation.ui.NavigationUI
import androidx.navigation.ui.setupWithNavController
import com.google.android.material.navigation.NavigationView
import eu.mhutti1.utils.storage.StorageDevice
import eu.mhutti1.utils.storage.StorageDeviceUtils
import kotlinx.coroutines.launch
import org.kiwix.kiwixmobile.BuildConfig
Expand Down Expand Up @@ -115,6 +116,7 @@ class KiwixMainActivity : CoreMainActivity() {
NavController.OnDestinationChangedListener { _, _, _ ->
actionMode?.finish()
}
private val storageDeviceList = arrayListOf<StorageDevice>()

override fun onCreate(savedInstanceState: Bundle?) {
cachedComponent.inject(this)
Expand Down Expand Up @@ -142,7 +144,7 @@ class KiwixMainActivity : CoreMainActivity() {

private suspend fun migrateInternalToPublicAppDirectory() {
if (!sharedPreferenceUtil.prefIsAppDirectoryMigrated) {
val storagePath = StorageDeviceUtils.getWritableStorage(this)
val storagePath = getStorageDeviceList()
.getOrNull(sharedPreferenceUtil.storagePosition)
?.name
storagePath?.let {
Expand All @@ -152,6 +154,23 @@ class KiwixMainActivity : CoreMainActivity() {
}
}

/**
* Fetches the storage device list once in the main activity and reuses it across all fragments.
* This is necessary because retrieving the storage device list, especially on devices with large SD cards,
* is a resource-intensive operation. Performing this operation repeatedly in fragments can negatively
* affect the user experience, as it takes time and can block the UI.
*
* If a fragment is destroyed and we need to retrieve the device list again, performing the operation
* repeatedly leads to inefficiency. To optimize this, we fetch the storage device list once and reuse
* it in all fragments, thereby reducing redundant processing and improving performance.
*/
suspend fun getStorageDeviceList(): List<StorageDevice> {
if (storageDeviceList.isEmpty()) {
storageDeviceList.addAll(StorageDeviceUtils.getWritableStorage(this))
}
return storageDeviceList
}

override fun onConfigurationChanged(newConfig: Configuration) {
super.onConfigurationChanged(newConfig)
if (::activityKiwixMainBinding.isInitialized) {
Expand Down
57 changes: 32 additions & 25 deletions app/src/main/java/org/kiwix/kiwixmobile/nav/destination/library/CopyMoveFileHandler.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ import androidx.documentfile.provider.DocumentFile
import androidx.fragment.app.FragmentManager
import eu.mhutti1.utils.storage.STORAGE_SELECT_STORAGE_TITLE_TEXTVIEW_SIZE
import eu.mhutti1.utils.storage.StorageDevice
import eu.mhutti1.utils.storage.StorageDeviceUtils
import eu.mhutti1.utils.storage.StorageSelectDialog
import io.reactivex.android.schedulers.AndroidSchedulers
import io.reactivex.disposables.Disposable
Expand All @@ -53,6 +52,7 @@ import org.kiwix.kiwixmobile.core.utils.INTERNAL_SELECT_POSITION
import org.kiwix.kiwixmobile.core.utils.SharedPreferenceUtil
import org.kiwix.kiwixmobile.core.utils.dialog.AlertDialogShower
import org.kiwix.kiwixmobile.core.utils.dialog.KiwixDialog
import org.kiwix.kiwixmobile.main.KiwixMainActivity
import org.kiwix.kiwixmobile.zimManager.Fat32Checker
import org.kiwix.kiwixmobile.zimManager.Fat32Checker.Companion.FOUR_GIGABYTES_IN_KILOBYTES
import org.kiwix.kiwixmobile.zimManager.Fat32Checker.FileSystemState.CannotWrite4GbFile
Expand Down Expand Up @@ -83,9 +83,6 @@ class CopyMoveFileHandler @Inject constructor(
var shouldValidateZimFile: Boolean = false
private var fileSystemDisposable: Disposable? = null
private lateinit var fragmentManager: FragmentManager
val storageDeviceList by lazy {
StorageDeviceUtils.getWritableStorage(activity)
}

private val copyMoveTitle: String by lazy {
if (isMoveOperation) {
Expand All @@ -111,18 +108,23 @@ class CopyMoveFileHandler @Inject constructor(
this.shouldValidateZimFile = shouldValidateZimFile
this.fragmentManager = fragmentManager
setSelectedFileAndUri(uri, documentFile)
if (sharedPreferenceUtil.shouldShowStorageSelectionDialog && storageDeviceList.size > 1) {
if (getStorageDeviceList().isEmpty()) {
showPreparingCopyMoveDialog()
}
if (sharedPreferenceUtil.shouldShowStorageSelectionDialog && getStorageDeviceList().size > 1) {
// Show dialog to select storage if more than one storage device is available, and user
// have not configured the storage yet.
hidePreparingCopyMoveDialog()
showCopyMoveDialog(true)
} else {
if (storageDeviceList.size == 1) {
if (getStorageDeviceList().size == 1) {
// If only internal storage is currently available, set shouldShowStorageSelectionDialog
// to true. This allows the storage configuration dialog to be shown again if the
// user removes an external storage device (like an SD card) and then reinserts it.
// This ensures they are prompted to configure storage settings upon SD card reinsertion.
sharedPreferenceUtil.shouldShowStorageSelectionDialog = true
}
hidePreparingCopyMoveDialog()
if (validateZimFileCanCopyOrMove()) {
showCopyMoveDialog()
}
Expand All @@ -142,17 +144,15 @@ class CopyMoveFileHandler @Inject constructor(
lifecycleScope = coroutineScope
}

fun showStorageSelectDialog() = StorageSelectDialog()
.apply {
onSelectAction = ::copyMoveZIMFileInSelectedStorage
titleSize = STORAGE_SELECT_STORAGE_TITLE_TEXTVIEW_SIZE
setStorageDeviceList(storageDeviceList)
setShouldShowCheckboxSelected(false)
}
.show(
fragmentManager,
activity.getString(R.string.choose_storage_to_copy_move_zim_file)
)
fun showStorageSelectDialog(storageDeviceList: List<StorageDevice>) =
StorageSelectDialog()
.apply {
onSelectAction = ::copyMoveZIMFileInSelectedStorage
titleSize = STORAGE_SELECT_STORAGE_TITLE_TEXTVIEW_SIZE
setStorageDeviceList(storageDeviceList)
setShouldShowCheckboxSelected(false)
}
.show(fragmentManager, activity.getString(R.string.choose_storage_to_copy_move_zim_file))

fun copyMoveZIMFileInSelectedStorage(storageDevice: StorageDevice) {
lifecycleScope?.launch {
Expand Down Expand Up @@ -259,19 +259,23 @@ class CopyMoveFileHandler @Inject constructor(

fun performCopyOperation(showStorageSelectionDialog: Boolean = false) {
isMoveOperation = false
if (showStorageSelectionDialog) {
showStorageSelectDialog()
} else {
copyZimFileToPublicAppDirectory()
lifecycleScope?.launch {
if (showStorageSelectionDialog) {
showStorageSelectDialog(getStorageDeviceList())
} else {
copyZimFileToPublicAppDirectory()
}
}
}

fun performMoveOperation(showStorageSelectionDialog: Boolean = false) {
isMoveOperation = true
if (showStorageSelectionDialog) {
showStorageSelectDialog()
} else {
moveZimFileToPublicAppDirectory()
lifecycleScope?.launch {
if (showStorageSelectionDialog) {
showStorageSelectDialog(getStorageDeviceList())
} else {
moveZimFileToPublicAppDirectory()
}
}
}

Expand Down Expand Up @@ -538,6 +542,9 @@ class CopyMoveFileHandler @Inject constructor(
}
}

suspend fun getStorageDeviceList() =
(activity as KiwixMainActivity).getStorageDeviceList()

fun dispose() {
fileSystemDisposable?.dispose()
setFileCopyMoveCallback(null)
Expand Down
26 changes: 14 additions & 12 deletions app/src/main/java/org/kiwix/kiwixmobile/nav/destination/library/LocalLibraryFragment.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@ import androidx.recyclerview.widget.RecyclerView
import com.google.android.material.bottomnavigation.BottomNavigationView
import eu.mhutti1.utils.storage.Bytes
import eu.mhutti1.utils.storage.StorageDevice
import eu.mhutti1.utils.storage.StorageDeviceUtils
import eu.mhutti1.utils.storage.StorageSelectDialog
import io.reactivex.android.schedulers.AndroidSchedulers
import io.reactivex.disposables.CompositeDisposable
Expand Down Expand Up @@ -99,6 +98,7 @@ import org.kiwix.kiwixmobile.core.zim_manager.fileselect_view.adapter.BooksOnDis
import org.kiwix.kiwixmobile.core.zim_manager.fileselect_view.adapter.BooksOnDiskListItem
import org.kiwix.kiwixmobile.core.zim_manager.fileselect_view.adapter.BooksOnDiskListItem.BookOnDisk
import org.kiwix.kiwixmobile.databinding.FragmentDestinationLibraryBinding
import org.kiwix.kiwixmobile.main.KiwixMainActivity
import org.kiwix.kiwixmobile.zimManager.MAX_PROGRESS
import org.kiwix.kiwixmobile.zimManager.ZimManageViewModel
import org.kiwix.kiwixmobile.zimManager.ZimManageViewModel.FileSelectActions
Expand Down Expand Up @@ -135,9 +135,6 @@ class LocalLibraryFragment : BaseFragment(), CopyMoveFileHandler.FileCopyMoveCal
private val zimManageViewModel by lazy {
requireActivity().viewModel<ZimManageViewModel>(viewModelFactory)
}
private val storageDeviceList by lazy {
StorageDeviceUtils.getWritableStorage(requireActivity())
}

private var storagePermissionLauncher: ActivityResultLauncher<Array<String>>? =
registerForActivityResult(
Expand Down Expand Up @@ -665,17 +662,22 @@ class LocalLibraryFragment : BaseFragment(), CopyMoveFileHandler.FileCopyMoveCal
message,
requireActivity().findViewById(R.id.bottom_nav_view),
string.download_change_storage,
::showStorageSelectDialog
{
lifecycleScope.launch {
showStorageSelectDialog((requireActivity() as KiwixMainActivity).getStorageDeviceList())
}
}
)
}

private fun showStorageSelectDialog() = StorageSelectDialog()
.apply {
onSelectAction = ::storeDeviceInPreferences
setStorageDeviceList(storageDeviceList)
setShouldShowCheckboxSelected(true)
}
.show(parentFragmentManager, getString(string.pref_storage))
private fun showStorageSelectDialog(storageDeviceList: List<StorageDevice>) =
StorageSelectDialog()
.apply {
onSelectAction = ::storeDeviceInPreferences
setStorageDeviceList(storageDeviceList)
setShouldShowCheckboxSelected(true)
}
.show(parentFragmentManager, getString(string.pref_storage))

private fun storeDeviceInPreferences(
storageDevice: StorageDevice
Expand Down
35 changes: 20 additions & 15 deletions app/src/main/java/org/kiwix/kiwixmobile/nav/destination/library/OnlineLibraryFragment.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@ import androidx.recyclerview.widget.LinearLayoutManager
import androidx.recyclerview.widget.RecyclerView
import eu.mhutti1.utils.storage.STORAGE_SELECT_STORAGE_TITLE_TEXTVIEW_SIZE
import eu.mhutti1.utils.storage.StorageDevice
import eu.mhutti1.utils.storage.StorageDeviceUtils
import eu.mhutti1.utils.storage.StorageSelectDialog
import kotlinx.coroutines.launch
import org.kiwix.kiwixmobile.R
Expand Down Expand Up @@ -93,6 +92,7 @@ import org.kiwix.kiwixmobile.core.utils.dialog.KiwixDialog
import org.kiwix.kiwixmobile.core.utils.dialog.KiwixDialog.YesNoDialog.WifiOnly
import org.kiwix.kiwixmobile.core.zim_manager.NetworkState
import org.kiwix.kiwixmobile.databinding.FragmentDestinationDownloadBinding
import org.kiwix.kiwixmobile.main.KiwixMainActivity
import org.kiwix.kiwixmobile.zimManager.ZimManageViewModel
import org.kiwix.kiwixmobile.zimManager.libraryView.AvailableSpaceCalculator
import org.kiwix.kiwixmobile.zimManager.libraryView.adapter.LibraryAdapter
Expand All @@ -116,9 +116,6 @@ class OnlineLibraryFragment : BaseFragment(), FragmentActivityExtensions {
private val zimManageViewModel by lazy {
requireActivity().viewModel<ZimManageViewModel>(viewModelFactory)
}
private val storageDeviceList by lazy {
StorageDeviceUtils.getWritableStorage(requireActivity())
}

@VisibleForTesting
fun getOnlineLibraryList() = libraryAdapter.items
Expand Down Expand Up @@ -550,8 +547,8 @@ class OnlineLibraryFragment : BaseFragment(), FragmentActivityExtensions {

else -> if (sharedPreferenceUtil.showStorageOption) {
// Show the storage selection dialog for configuration if there is an SD card available.
if (storageDeviceList.size > 1) {
showStorageSelectDialog()
if (getStorageDeviceList().size > 1) {
showStorageSelectDialog(getStorageDeviceList())
} else {
// If only internal storage is available, proceed with the ZIM file download directly.
// Displaying a configuration dialog is unnecessary in this case.
Expand All @@ -575,7 +572,11 @@ class OnlineLibraryFragment : BaseFragment(), FragmentActivityExtensions {
""".trimIndent(),
requireActivity().findViewById(R.id.bottom_nav_view),
string.download_change_storage,
::showStorageSelectDialog
{
lifecycleScope.launch {
showStorageSelectDialog(getStorageDeviceList())
}
}
)
}
)
Expand All @@ -588,14 +589,15 @@ class OnlineLibraryFragment : BaseFragment(), FragmentActivityExtensions {
}
}

private fun showStorageSelectDialog() = StorageSelectDialog()
.apply {
onSelectAction = ::storeDeviceInPreferences
titleSize = STORAGE_SELECT_STORAGE_TITLE_TEXTVIEW_SIZE
setStorageDeviceList(storageDeviceList)
setShouldShowCheckboxSelected(false)
}
.show(parentFragmentManager, getString(string.choose_storage_to_download_book))
private fun showStorageSelectDialog(storageDeviceList: List<StorageDevice>) =
StorageSelectDialog()
.apply {
onSelectAction = ::storeDeviceInPreferences
titleSize = STORAGE_SELECT_STORAGE_TITLE_TEXTVIEW_SIZE
setStorageDeviceList(storageDeviceList)
setShouldShowCheckboxSelected(false)
}
.show(parentFragmentManager, getString(string.choose_storage_to_download_book))

private fun clickOnBookItem() {
if (!requireActivity().isManageExternalStoragePermissionGranted(sharedPreferenceUtil)) {
Expand All @@ -615,4 +617,7 @@ class OnlineLibraryFragment : BaseFragment(), FragmentActivityExtensions {
)
}
}

private suspend fun getStorageDeviceList() =
(activity as KiwixMainActivity).getStorageDeviceList()
}
34 changes: 8 additions & 26 deletions app/src/main/java/org/kiwix/kiwixmobile/settings/KiwixPrefsFragment.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,6 @@ import androidx.lifecycle.lifecycleScope
import androidx.preference.Preference
import androidx.preference.PreferenceCategory
import eu.mhutti1.utils.storage.StorageDevice
import eu.mhutti1.utils.storage.StorageDeviceUtils
import io.reactivex.Flowable
import io.reactivex.android.schedulers.AndroidSchedulers
import io.reactivex.disposables.Disposable
import io.reactivex.schedulers.Schedulers
import kotlinx.coroutines.launch
import org.kiwix.kiwixmobile.core.R
import org.kiwix.kiwixmobile.core.extensions.getFreeSpace
Expand All @@ -44,11 +39,11 @@ import org.kiwix.kiwixmobile.core.settings.StorageRadioButtonPreference
import org.kiwix.kiwixmobile.core.utils.SharedPreferenceUtil
import org.kiwix.kiwixmobile.core.utils.SharedPreferenceUtil.Companion.PREF_EXTERNAL_STORAGE
import org.kiwix.kiwixmobile.core.utils.SharedPreferenceUtil.Companion.PREF_INTERNAL_STORAGE
import org.kiwix.kiwixmobile.main.KiwixMainActivity

const val PREF_STORAGE_PROGRESSBAR = "storage_progressbar"

class KiwixPrefsFragment : CorePrefsFragment() {
private var storageDisposable: Disposable? = null
private var storageDeviceList: List<StorageDevice> = listOf()

override fun onCreatePreferences(savedInstanceState: Bundle?, rootKey: String?) {
Expand All @@ -65,20 +60,13 @@ class KiwixPrefsFragment : CorePrefsFragment() {
return@setStorage
}
showHideProgressBarWhileFetchingStorageInfo(true)
storageDisposable =
Flowable.fromCallable { StorageDeviceUtils.getWritableStorage(requireActivity()) }
.subscribeOn(Schedulers.io())
.observeOn(AndroidSchedulers.mainThread())
.subscribe(
{ storageList ->
storageDeviceList = storageList
showHideProgressBarWhileFetchingStorageInfo(false)
showInternalStoragePreferece()
showExternalPreferenceIfAvailable()
setUpStoragePreference(it)
},
Throwable::printStackTrace
)
lifecycleScope.launch {
storageDeviceList = (requireActivity() as KiwixMainActivity).getStorageDeviceList()
showHideProgressBarWhileFetchingStorageInfo(false)
showInternalStoragePreferece()
showExternalPreferenceIfAvailable()
setUpStoragePreference(it)
}
}
}

Expand Down Expand Up @@ -157,12 +145,6 @@ class KiwixPrefsFragment : CorePrefsFragment() {
preferenceCategory?.isVisible = true
}

override fun onDestroyView() {
storageDisposable?.dispose()
storageDisposable = null
super.onDestroyView()
}

companion object {
const val PREF_MANAGE_EXTERNAL_STORAGE_PERMISSION =
"pref_manage_external_storage"
Expand Down
Loading

0 comments on commit 1e5bc58

Please sign in to comment.