Task/anr fix zimfile reader

app/src/main/java/org/kiwix/kiwixmobile/nav/destination/reader/KiwixReaderFragment.kt

@@ -33,7 +33,11 @@ import android.view.ViewGroup
 import androidx.appcompat.app.AppCompatActivity
 import androidx.core.net.toFile
 import androidx.drawerlayout.widget.DrawerLayout
+import androidx.lifecycle.Lifecycle
+import androidx.lifecycle.lifecycleScope
+import androidx.lifecycle.repeatOnLifecycle
 import com.google.android.material.bottomnavigation.BottomNavigationView
+import kotlinx.coroutines.launch
 import org.kiwix.kiwixmobile.R
 import org.kiwix.kiwixmobile.cachedComponent
 import org.kiwix.kiwixmobile.core.R.anim
@@ -71,44 +75,52 @@ class KiwixReaderFragment : CoreReaderFragment() {
 
   override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
     super.onViewCreated(view, savedInstanceState)
-
-    val activity = activity as CoreMainActivity
-    noOpenBookButton?.setOnClickListener {
-      activity.navigate(
-        KiwixReaderFragmentDirections.actionNavigationReaderToNavigationLibrary()
-      )
+    lifecycleScope.launch {
+      viewLifecycleOwner.repeatOnLifecycle(Lifecycle.State.STARTED) {
+        val activity = activity as CoreMainActivity
+        noOpenBookButton?.setOnClickListener {
+          activity.navigate(
+            KiwixReaderFragmentDirections.actionNavigationReaderToNavigationLibrary()
+          )
+        }
+        activity.supportActionBar?.setDisplayHomeAsUpEnabled(true)
+        toolbar?.let(activity::setupDrawerToggle)
+        setFragmentContainerBottomMarginToSizeOfNavBar()
+        openPageInBookFromNavigationArguments()
+      }
     }
-    activity.supportActionBar?.setDisplayHomeAsUpEnabled(true)
-    toolbar?.let(activity::setupDrawerToggle)
-    setFragmentContainerBottomMarginToSizeOfNavBar()
-    openPageInBookFromNavigationArguments()
   }
 
-  private fun openPageInBookFromNavigationArguments() {
+  @Suppress("TooGenericExceptionCaught")
+  private suspend fun openPageInBookFromNavigationArguments() {
     val args = KiwixReaderFragmentArgs.fromBundle(requireArguments())
-
-    if (args.pageUrl.isNotEmpty()) {
-      if (args.zimFileUri.isNotEmpty()) {
-        tryOpeningZimFile(args.zimFileUri)
-      } else {
-        // Set up bookmarks for the current book when opening bookmarks from the Bookmark screen.
-        // This is necessary because we are not opening the ZIM file again; the bookmark is
-        // inside the currently opened book. Bookmarks are set up when opening the ZIM file.
-        // See https://github.com/kiwix/kiwix-android/issues/3541
-        zimReaderContainer?.zimFileReader?.let(::setUpBookmarks)
-      }
-      loadUrlWithCurrentWebview(args.pageUrl)
-    } else {
-      if (args.zimFileUri.isNotEmpty()) {
-        tryOpeningZimFile(args.zimFileUri)
+    try {
+      if (args.pageUrl.isNotEmpty()) {
+        if (args.zimFileUri.isNotEmpty()) {
+          tryOpeningZimFile(args.zimFileUri)
+        } else {
+          // Set up bookmarks for the current book when opening bookmarks from the Bookmark screen.
+          // This is necessary because we are not opening the ZIM file again; the bookmark is
+          // inside the currently opened book. Bookmarks are set up when opening the ZIM file.
+          // See https://github.com/kiwix/kiwix-android/issues/3541
+          zimReaderContainer?.zimFileReader?.let(::setUpBookmarks)
+        }
+        loadUrlWithCurrentWebview(args.pageUrl)
       } else {
-        manageExternalLaunchAndRestoringViewState()
+        if (args.zimFileUri.isNotEmpty()) {
+          tryOpeningZimFile(args.zimFileUri)
+        } else {
+          manageExternalLaunchAndRestoringViewState()
+        }
       }
+    } catch (error: Exception) {
+      Log.e("KiwixReader", "Error opening ZIM file", error)
+    } finally {
+      requireArguments().clear()
     }
-    requireArguments().clear()
   }
 
-  private fun tryOpeningZimFile(zimFileUri: String) {
+  private suspend fun tryOpeningZimFile(zimFileUri: String) {
     val filePath = FileUtils.getLocalFilePathByUri(
       requireActivity().applicationContext, Uri.parse(zimFileUri)
     )
@@ -155,14 +167,16 @@ class KiwixReaderFragment : CoreReaderFragment() {
         contentFrame?.visibility = View.VISIBLE
       }
       mainMenu?.showWebViewOptions(true)
-      if (webViewList.isEmpty()) {
-        exitBook()
-      } else {
-        // Reset the top margin of web views to 0 to remove any previously set margin
-        // This ensures that the web views are displayed without any additional
-        // top margin for kiwix main app.
-        setTopMarginToWebViews(0)
-        selectTab(currentWebViewIndex)
+      lifecycleScope.launch {
+        if (webViewList.isEmpty()) {
+          exitBook()
+        } else {
+          // Reset the top margin of web views to 0 to remove any previously set margin
+          // This ensures that the web views are displayed without any additional
+          // top margin for kiwix main app.
+          setTopMarginToWebViews(0)
+          selectTab(currentWebViewIndex)
+        }
       }
     }
   }
@@ -199,17 +213,19 @@ class KiwixReaderFragment : CoreReaderFragment() {
 
   override fun onResume() {
     super.onResume()
-    if (zimReaderContainer?.zimFile == null &&
-      zimReaderContainer?.zimFileReader?.assetFileDescriptorList?.isEmpty() == true
-    ) {
-      exitBook()
-    }
-    if (isFullScreenVideo || isInFullScreenMode()) {
-      hideNavBar()
+    lifecycleScope.launch {
+      if (zimReaderContainer?.zimFile == null &&
+        zimReaderContainer?.zimFileReader?.assetFileDescriptorList?.isEmpty() == true
+      ) {
+        exitBook()
+      }
+      if (isFullScreenVideo || isInFullScreenMode()) {
+        hideNavBar()
+      }
     }
   }
 
-  override fun restoreViewStateOnInvalidJSON() {
+  override suspend fun restoreViewStateOnInvalidJSON() {
     Log.d(TAG_KIWIX, "Kiwix normal start, no zimFile loaded last time  -> display home page")
     exitBook()
   }
@@ -222,20 +238,22 @@ class KiwixReaderFragment : CoreReaderFragment() {
     val settings = requireActivity().getSharedPreferences(SharedPreferenceUtil.PREF_KIWIX_MOBILE, 0)
     val zimFile = settings.getString(TAG_CURRENT_FILE, null)
 
-    if (zimFile != null && File(zimFile).isFileExist()) {
-      if (zimReaderContainer?.zimFile == null) {
-        openZimFile(File(zimFile))
-        Log.d(
-          TAG_KIWIX,
-          "Kiwix normal start, Opened last used zimFile: -> $zimFile"
-        )
+    lifecycleScope.launch {
+      if (zimFile != null && File(zimFile).isFileExist()) {
+        if (zimReaderContainer?.zimFile == null) {
+          openZimFile(File(zimFile))
+          Log.d(
+            TAG_KIWIX,
+            "Kiwix normal start, Opened last used zimFile: -> $zimFile"
+          )
+        } else {
+          zimReaderContainer?.zimFileReader?.let(::setUpBookmarks)
+        }
       } else {
-        zimReaderContainer?.zimFileReader?.let(::setUpBookmarks)
+        getCurrentWebView()?.snack(R.string.zim_not_opened)
+        exitBook() // hide the options for zim file to avoid unexpected UI behavior
+        return@launch // book not found so don't need to restore the tabs for this file
       }
-    } else {
-      getCurrentWebView()?.snack(R.string.zim_not_opened)
-      exitBook() // hide the options for zim file to avoid unexpected UI behavior
-      return // book not found so don't need to restore the tabs for this file
     }
     restoreTabs(zimArticles, zimPositions, currentTab)
   }
@@ -304,18 +322,22 @@ class KiwixReaderFragment : CoreReaderFragment() {
       when (it.scheme) {
         "file" -> {
           Handler(Looper.getMainLooper()).postDelayed({
-            openZimFile(it.toFile()).also {
-              // if used once then clear it to avoid affecting any other functionality
-              // of the application.
-              requireActivity().intent.action = null
+            lifecycleScope.launch {
+              openZimFile(it.toFile()).also {
+                // if used once then clear it to avoid affecting any other functionality
+                // of the application.
+                requireActivity().intent.action = null
+              }
             }
           }, 300)
         }
 
         "content" -> {
           Handler(Looper.getMainLooper()).postDelayed({
             getZimFileFromUri(it)?.let { zimFile ->
-              openZimFile(zimFile)
+              lifecycleScope.launch {
+                openZimFile(zimFile)
+              }
             }.also {
               requireActivity().intent.action = null
             }

app/src/main/java/org/kiwix/kiwixmobile/zimManager/ZimManageViewModel.kt

@@ -23,6 +23,7 @@ import android.net.ConnectivityManager
 import androidx.annotation.VisibleForTesting
 import androidx.lifecycle.MutableLiveData
 import androidx.lifecycle.ViewModel
+import androidx.lifecycle.viewModelScope
 import io.reactivex.Flowable
 import io.reactivex.disposables.CompositeDisposable
 import io.reactivex.disposables.Disposable
@@ -164,7 +165,7 @@ class ZimManageViewModel @Inject constructor(
       when (it) {
         is RequestNavigateTo -> OpenFileWithNavigation(it.bookOnDisk)
         is RequestMultiSelection -> startMultiSelectionAndSelectBook(it.bookOnDisk)
-        RequestDeleteMultiSelection -> DeleteFiles(selectionsFromState())
+        RequestDeleteMultiSelection -> DeleteFiles(selectionsFromState(), viewModelScope)
         RequestShareMultiSelection -> ShareFiles(selectionsFromState())
         MultiModeFinished -> noSideEffectAndClearSelectionState()
         is RequestSelect -> noSideEffectSelectBook(it.bookOnDisk)

app/src/main/java/org/kiwix/kiwixmobile/zimManager/fileselectView/effects/DeleteFiles.kt

@@ -19,6 +19,8 @@
 package org.kiwix.kiwixmobile.zimManager.fileselectView.effects
 
 import androidx.appcompat.app.AppCompatActivity
+import kotlinx.coroutines.CoroutineScope
+import kotlinx.coroutines.launch
 import org.kiwix.kiwixmobile.R
 import org.kiwix.kiwixmobile.cachedComponent
 import org.kiwix.kiwixmobile.core.base.BaseActivity
@@ -33,7 +35,10 @@ import org.kiwix.kiwixmobile.core.utils.files.FileUtils
 import org.kiwix.kiwixmobile.core.zim_manager.fileselect_view.adapter.BooksOnDiskListItem.BookOnDisk
 import javax.inject.Inject
 
-data class DeleteFiles(private val booksOnDiskListItems: List<BookOnDisk>) :
+data class DeleteFiles(
+  private val booksOnDiskListItems: List<BookOnDisk>,
+  private val coroutineScope: CoroutineScope
+) :
   SideEffect<Unit> {
 
   @Inject lateinit var dialogShower: DialogShower
@@ -42,21 +47,22 @@ data class DeleteFiles(private val booksOnDiskListItems: List<BookOnDisk>) :
 
   override fun invokeWith(activity: AppCompatActivity) {
     (activity as BaseActivity).cachedComponent.inject(this)
-
     val name = booksOnDiskListItems.joinToString(separator = "\n") { it.book.title }
 
     dialogShower.show(DeleteZims(name), {
-      activity.toast(
-        if (booksOnDiskListItems.deleteAll()) {
-          R.string.delete_zims_toast
-        } else {
-          R.string.delete_zim_failed
-        }
-      )
+      coroutineScope.launch {
+        activity.toast(
+          if (booksOnDiskListItems.deleteAll()) {
+            R.string.delete_zims_toast
+          } else {
+            R.string.delete_zim_failed
+          }
+        )
+      }
     })
   }
 
-  private fun List<BookOnDisk>.deleteAll(): Boolean {
+  private suspend fun List<BookOnDisk>.deleteAll(): Boolean {
     return fold(true) { acc, book ->
       acc && deleteSpecificZimFile(book).also {
         if (it && book.file.canonicalPath == zimReaderContainer.zimCanonicalPath) {

app/src/test/java/org/kiwix/kiwixmobile/zimManager/ZimManageViewModelTest.kt

@@ -22,6 +22,7 @@ import android.app.Application
 import android.net.ConnectivityManager
 import android.net.NetworkCapabilities
 import android.net.NetworkCapabilities.TRANSPORT_WIFI
+import androidx.lifecycle.viewModelScope
 import com.jraska.livedata.test
 import io.mockk.clearAllMocks
 import io.mockk.every
@@ -470,7 +471,7 @@ class ZimManageViewModelTest {
         FileSelectListState(listOf(selectedBook, bookOnDisk()), NORMAL)
       viewModel.sideEffects.test()
         .also { viewModel.fileSelectActions.offer(RequestDeleteMultiSelection) }
-        .assertValues(DeleteFiles(listOf(selectedBook)))
+        .assertValues(DeleteFiles(listOf(selectedBook), viewModel.viewModelScope))
     }
 
     @Test