Merge pull request #4129 from kiwix/Fixes#4127

Fixed: Input dispatching timed out in `CoreReaderFragment.toggleBookmark`.
kiwix · Dec 10, 2024 · 3e197cf · 3e197cf
2 parents 696087e + 3f4fb9d
commit 3e197cf
Show file tree

Hide file tree

Showing 5 changed files with 86 additions and 71 deletions.
diff --git a/app/src/main/java/org/kiwix/kiwixmobile/webserver/KiwixServer.kt b/app/src/main/java/org/kiwix/kiwixmobile/webserver/KiwixServer.kt
@@ -19,6 +19,8 @@
 package org.kiwix.kiwixmobile.webserver
 
 import android.content.Context
+import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.withContext
 import org.kiwix.kiwixmobile.core.utils.files.Log
 import org.kiwix.kiwixmobile.core.reader.ZimReaderContainer
 import org.kiwix.kiwixmobile.core.utils.files.FileUtils.getDemoFilePathForCustomApp
@@ -43,41 +45,42 @@ class KiwixServer @Inject constructor(
     private val zimReaderContainer: ZimReaderContainer
   ) {
     @Suppress("NestedBlockDepth")
-    fun createKiwixServer(selectedBooksPath: ArrayList<String>): KiwixServer {
-      val kiwixLibrary = Library()
-      selectedBooksPath.forEach { path ->
-        try {
-          val book = Book().apply {
-            // Determine whether to create an Archive from an asset or a file path
-            val archive = if (path == getDemoFilePathForCustomApp(context)) {
-              // For custom apps using a demo file, create an Archive with FileDescriptor
-              val assetFileDescriptor =
-                zimReaderContainer.zimReaderSource?.assetFileDescriptorList?.get(0)
-              val startOffset = assetFileDescriptor?.startOffset ?: 0L
-              val size = assetFileDescriptor?.length ?: 0L
-              Archive(
-                assetFileDescriptor?.parcelFileDescriptor?.fileDescriptor,
-                startOffset,
-                size
-              )
-            } else {
-              // For regular files, create an Archive from the file path
-              Archive(path)
+    suspend fun createKiwixServer(selectedBooksPath: ArrayList<String>): KiwixServer =
+      withContext(Dispatchers.IO) {
+        val kiwixLibrary = Library()
+        selectedBooksPath.forEach { path ->
+          try {
+            val book = Book().apply {
+              // Determine whether to create an Archive from an asset or a file path
+              val archive = if (path == getDemoFilePathForCustomApp(context)) {
+                // For custom apps using a demo file, create an Archive with FileDescriptor
+                val assetFileDescriptor =
+                  zimReaderContainer.zimReaderSource?.assetFileDescriptorList?.get(0)
+                val startOffset = assetFileDescriptor?.startOffset ?: 0L
+                val size = assetFileDescriptor?.length ?: 0L
+                Archive(
+                  assetFileDescriptor?.parcelFileDescriptor?.fileDescriptor,
+                  startOffset,
+                  size
+                )
+              } else {
+                // For regular files, create an Archive from the file path
+                Archive(path)
+              }
+              update(archive)
             }
-            update(archive)
+            kiwixLibrary.addBook(book)
+          } catch (ignore: Exception) {
+            // Catch the other exceptions as well. e.g. while hosting the split zim files.
+            // we have an issue with split zim files, see #3827
+            Log.v(
+              TAG,
+              "Couldn't add book with path:{ $path }.\n Original Exception = $ignore"
+            )
           }
-          kiwixLibrary.addBook(book)
-        } catch (ignore: Exception) {
-          // Catch the other exceptions as well. e.g. while hosting the split zim files.
-          // we have an issue with split zim files, see #3827
-          Log.v(
-            TAG,
-            "Couldn't add book with path:{ $path }.\n Original Exception = $ignore"
-          )
         }
+        return@withContext KiwixServer(kiwixLibrary, Server(kiwixLibrary))
       }
-      return KiwixServer(kiwixLibrary, Server(kiwixLibrary))
-    }
   }
 
   fun startServer(port: Int): Boolean {

diff --git a/app/src/main/java/org/kiwix/kiwixmobile/webserver/WebServerHelper.kt b/app/src/main/java/org/kiwix/kiwixmobile/webserver/WebServerHelper.kt
@@ -45,7 +45,7 @@ class WebServerHelper @Inject constructor(
   private var isServerStarted = false
   private var validIpAddressDisposable: Disposable? = null
 
-  fun startServerHelper(
+  suspend fun startServerHelper(
     selectedBooksPath: ArrayList<String>,
     restartServer: Boolean
   ): ServerStatus? {
@@ -64,7 +64,7 @@ class WebServerHelper @Inject constructor(
     }
   }
 
-  private fun startAndroidWebServer(
+  private suspend fun startAndroidWebServer(
     selectedBooksPath: ArrayList<String>,
     restartServer: Boolean
   ): ServerStatus? {
@@ -78,7 +78,7 @@ class WebServerHelper @Inject constructor(
     return serverStatus
   }
 
-  private fun startKiwixServer(selectedBooksPath: ArrayList<String>): ServerStatus {
+  private suspend fun startKiwixServer(selectedBooksPath: ArrayList<String>): ServerStatus {
     var errorMessage: Int? = null
     ServerUtils.port = DEFAULT_PORT
     kiwixServer = kiwixServerFactory.createKiwixServer(selectedBooksPath).also {

diff --git a/app/src/main/java/org/kiwix/kiwixmobile/webserver/ZimHostFragment.kt b/app/src/main/java/org/kiwix/kiwixmobile/webserver/ZimHostFragment.kt
@@ -42,8 +42,8 @@ import androidx.appcompat.widget.Toolbar
 import androidx.core.app.ActivityCompat
 import androidx.core.content.ContextCompat
 import androidx.fragment.app.FragmentActivity
-import org.kiwix.kiwixmobile.core.R
 import org.kiwix.kiwixmobile.R.layout
+import org.kiwix.kiwixmobile.core.R
 import org.kiwix.kiwixmobile.core.base.BaseActivity
 import org.kiwix.kiwixmobile.core.base.BaseFragment
 import org.kiwix.kiwixmobile.core.extensions.ActivityExtensions.hasNotificationPermission
@@ -443,6 +443,7 @@ class ZimHostFragment : BaseFragment(), ZimHostCallbacks, ZimHostContract.View {
     Intent(requireActivity(), HotspotService::class.java).setAction(action)
 
   override fun onServerStarted(ip: String) {
+    dialog?.dismiss() // Dismiss dialog when server started.
     this.ip = ip
     layoutServerStarted()
   }
@@ -452,6 +453,7 @@ class ZimHostFragment : BaseFragment(), ZimHostCallbacks, ZimHostContract.View {
   }
 
   override fun onServerFailedToStart(errorMessage: Int?) {
+    dialog?.dismiss() // Dismiss dialog if there is some error in starting the server.
     errorMessage?.let {
       toast(errorMessage)
     }
@@ -514,7 +516,6 @@ class ZimHostFragment : BaseFragment(), ZimHostCallbacks, ZimHostContract.View {
   }
 
   override fun onIpAddressValid() {
-    dialog?.dismiss()
     startWifiHotspot(false)
   }
 

diff --git a/app/src/main/java/org/kiwix/kiwixmobile/webserver/wifi_hotspot/HotspotService.kt b/app/src/main/java/org/kiwix/kiwixmobile/webserver/wifi_hotspot/HotspotService.kt
@@ -22,6 +22,10 @@ import android.content.Intent
 import android.os.Binder
 import android.os.IBinder
 import android.widget.Toast
+import kotlinx.coroutines.CoroutineScope
+import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.launch
+import kotlinx.coroutines.withContext
 import org.kiwix.kiwixmobile.KiwixApp
 import org.kiwix.kiwixmobile.core.R
 import org.kiwix.kiwixmobile.core.extensions.registerReceiver
@@ -75,18 +79,22 @@ class HotspotService :
       ACTION_START_SERVER -> {
         val restartServer = intent.getBooleanExtra(RESTART_SERVER, false)
         intent.getStringArrayListExtra(ZimHostFragment.SELECTED_ZIM_PATHS_KEY)?.let {
-          val serverStatus = webServerHelper?.startServerHelper(it, restartServer)
-          if (serverStatus?.isServerStarted == true) {
-            zimHostCallbacks?.onServerStarted(getSocketAddress())
-            startForegroundNotificationHelper()
-            if (!restartServer) {
-              Toast.makeText(
-                this, R.string.server_started_successfully_toast_message,
-                Toast.LENGTH_SHORT
-              ).show()
+          CoroutineScope(Dispatchers.Main).launch {
+            val serverStatus = withContext(Dispatchers.IO) {
+              webServerHelper?.startServerHelper(it, restartServer)
+            }
+            if (serverStatus?.isServerStarted == true) {
+              zimHostCallbacks?.onServerStarted(getSocketAddress())
+              startForegroundNotificationHelper()
+              if (!restartServer) {
+                Toast.makeText(
+                  this@HotspotService, R.string.server_started_successfully_toast_message,
+                  Toast.LENGTH_SHORT
+                ).show()
+              }
+            } else {
+              onServerFailedToStart(serverStatus?.errorMessage)
             }
-          } else {
-            onServerFailedToStart(serverStatus?.errorMessage)
           }
         } ?: kotlin.run { onServerFailedToStart(R.string.no_books_selected_toast_message) }
       }

diff --git a/core/src/main/java/org/kiwix/kiwixmobile/core/main/CoreReaderFragment.kt b/core/src/main/java/org/kiwix/kiwixmobile/core/main/CoreReaderFragment.kt
@@ -1214,6 +1214,7 @@ abstract class CoreReaderFragment :
   override fun onDestroyView() {
     super.onDestroyView()
     try {
+      coreReaderLifeCycleScope?.cancel()
       readerLifeCycleScope?.cancel()
       readerLifeCycleScope = null
     } catch (ignore: Exception) {
@@ -1803,7 +1804,7 @@ abstract class CoreReaderFragment :
     when (requestCode) {
       REQUEST_STORAGE_PERMISSION -> {
         if (hasPermission(Manifest.permission.READ_EXTERNAL_STORAGE)) {
-          lifecycleScope.launch {
+          coreReaderLifeCycleScope?.launch {
             zimReaderSource?.let { openZimFile(it) }
           }
         } else {
@@ -1899,32 +1900,34 @@ abstract class CoreReaderFragment :
   @Suppress("NestedBlockDepth")
   private fun toggleBookmark() {
     try {
-      getCurrentWebView()?.url?.let { articleUrl ->
-        zimReaderContainer?.zimFileReader?.let { zimFileReader ->
-          val libKiwixBook = Book().apply {
-            update(zimFileReader.jniKiwixReader)
-          }
-          if (isBookmarked) {
-            repositoryActions?.deleteBookmark(libKiwixBook.id, articleUrl)
-            snackBarRoot?.snack(R.string.bookmark_removed)
-          } else {
-            getCurrentWebView()?.title?.let {
-              repositoryActions?.saveBookmark(
-                LibkiwixBookmarkItem(it, articleUrl, zimFileReader, libKiwixBook)
-              )
-              snackBarRoot?.snack(
-                stringId = R.string.bookmark_added,
-                actionStringId = R.string.open,
-                actionClick = {
-                  goToBookmarks()
-                  Unit
-                }
-              )
+      lifecycleScope.launch {
+        getCurrentWebView()?.url?.let { articleUrl ->
+          zimReaderContainer?.zimFileReader?.let { zimFileReader ->
+            val libKiwixBook = Book().apply {
+              update(zimFileReader.jniKiwixReader)
+            }
+            if (isBookmarked) {
+              repositoryActions?.deleteBookmark(libKiwixBook.id, articleUrl)
+              snackBarRoot?.snack(R.string.bookmark_removed)
+            } else {
+              getCurrentWebView()?.title?.let {
+                repositoryActions?.saveBookmark(
+                  LibkiwixBookmarkItem(it, articleUrl, zimFileReader, libKiwixBook)
+                )
+                snackBarRoot?.snack(
+                  stringId = R.string.bookmark_added,
+                  actionStringId = R.string.open,
+                  actionClick = {
+                    goToBookmarks()
+                    Unit
+                  }
+                )
+              }
             }
           }
+        } ?: kotlin.run {
+          requireActivity().toast(R.string.unable_to_add_to_bookmarks, Toast.LENGTH_SHORT)
         }
-      } ?: kotlin.run {
-        requireActivity().toast(R.string.unable_to_add_to_bookmarks, Toast.LENGTH_SHORT)
       }
     } catch (ignore: Exception) {
       // Catch the exception while saving the bookmarks for splitted zim files.