Fixed: Input dispatching timed out in CoreReaderFragment.toggleBookmark.

* Moved the `Book()` object creation to `lifeCycleScope` instead of running it on the main thread. Earlier, it was blocking the UI, especially with large ZIM files. Now, by using `lifeCycleScope`, it won't block the main thread. * Improved the KiwixServer creation process. Server creation was previously happening on the main thread, which could cause the same issue we are facing with adding bookmarks. Because we are creating the same book object while creating the server, which is required for `server` creation. * Updated the behavior of the "Starting server" dialog. Before, the dialog disappeared after validating the IP address, even though server creation was still in progress. Now, the server creation process is moved to the IO thread. For large ZIM files, server creation can take some time, so the dialog will only dismiss once the server is successfully created or if an error occurs. This ensures users are aware that server creation is in progress.
kiwix · Dec 10, 2024 · 3f4fb9d · 3f4fb9d
1 parent 696087e
commit 3f4fb9d
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.