Fixed: Application crash caused by "Input dispatching timed out" whil…

…e retrieving storageDeviceList information in the online library. * Moved the storage device retrieval to the IO thread. * Updated the StorageDeviceUtils.getWritableStorage method to a suspend function to ensure execution on the IO thread. * Refactored CopyMoveFileHandler, OnlineLibraryFragment, KiwixPrefsFragment, and LocalLibraryFragment to adapt to this change. * Optimized storage device retrieval by caching it in the main activity and reusing it across fragments to enhance performance, especially with large SD cards. * Refactored the CopyMoveFileHandlerTest. * Improved the showing of storage device list.
kiwix · Jan 6, 2025 · 9618ea5 · 9618ea5
1 parent 359a1fc
commit 9618ea5
Show file tree

Hide file tree

Showing 8 changed files with 110 additions and 88 deletions.
diff --git a/app/src/main/java/org/kiwix/kiwixmobile/main/KiwixMainActivity.kt b/app/src/main/java/org/kiwix/kiwixmobile/main/KiwixMainActivity.kt
@@ -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
@@ -115,6 +116,7 @@ class KiwixMainActivity : CoreMainActivity() {
     NavController.OnDestinationChangedListener { _, _, _ ->
       actionMode?.finish()
     }
+  private val storageDeviceList = arrayListOf<StorageDevice>()
 
   override fun onCreate(savedInstanceState: Bundle?) {
     cachedComponent.inject(this)
@@ -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 {
@@ -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) {

diff --git a/app/src/main/java/org/kiwix/kiwixmobile/nav/destination/library/CopyMoveFileHandler.kt b/app/src/main/java/org/kiwix/kiwixmobile/nav/destination/library/CopyMoveFileHandler.kt
@@ -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
@@ -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
@@ -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) {
@@ -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()
       }
@@ -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 {
@@ -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()
+      }
     }
   }
 
@@ -538,6 +542,9 @@ class CopyMoveFileHandler @Inject constructor(
     }
   }
 
+  suspend fun getStorageDeviceList() =
+    (activity as KiwixMainActivity).getStorageDeviceList()
+
   fun dispose() {
     fileSystemDisposable?.dispose()
     setFileCopyMoveCallback(null)

diff --git a/app/src/main/java/org/kiwix/kiwixmobile/nav/destination/library/LocalLibraryFragment.kt b/app/src/main/java/org/kiwix/kiwixmobile/nav/destination/library/LocalLibraryFragment.kt
@@ -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
@@ -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
@@ -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(
@@ -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

diff --git a/app/src/main/java/org/kiwix/kiwixmobile/nav/destination/library/OnlineLibraryFragment.kt b/app/src/main/java/org/kiwix/kiwixmobile/nav/destination/library/OnlineLibraryFragment.kt
@@ -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
@@ -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
@@ -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
@@ -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.
@@ -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())
+                      }
+                    }
                   )
                 }
               )
@@ -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)) {
@@ -615,4 +617,7 @@ class OnlineLibraryFragment : BaseFragment(), FragmentActivityExtensions {
       )
     }
   }
+
+  private suspend fun getStorageDeviceList() =
+    (activity as KiwixMainActivity).getStorageDeviceList()
 }
diff --git a/app/src/main/java/org/kiwix/kiwixmobile/settings/KiwixPrefsFragment.kt b/app/src/main/java/org/kiwix/kiwixmobile/settings/KiwixPrefsFragment.kt
@@ -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
@@ -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?) {
@@ -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)
+      }
     }
   }
 
@@ -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"