From 3f4fb9dcfcff42685d9523d1a758dddf8493428c Mon Sep 17 00:00:00 2001 From: MohitMaliFtechiz Date: Tue, 10 Dec 2024 12:37:41 +0530 Subject: [PATCH] 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. --- .../kiwixmobile/webserver/KiwixServer.kt | 65 ++++++++++--------- .../kiwixmobile/webserver/WebServerHelper.kt | 6 +- .../kiwixmobile/webserver/ZimHostFragment.kt | 5 +- .../webserver/wifi_hotspot/HotspotService.kt | 30 +++++---- .../core/main/CoreReaderFragment.kt | 51 ++++++++------- 5 files changed, 86 insertions(+), 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 index 6f0daf93f..94ebff4bd 100644 --- 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): 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): 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 index 6261b91de..862266d57 100644 --- 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, restartServer: Boolean ): ServerStatus? { @@ -64,7 +64,7 @@ class WebServerHelper @Inject constructor( } } - private fun startAndroidWebServer( + private suspend fun startAndroidWebServer( selectedBooksPath: ArrayList, restartServer: Boolean ): ServerStatus? { @@ -78,7 +78,7 @@ class WebServerHelper @Inject constructor( return serverStatus } - private fun startKiwixServer(selectedBooksPath: ArrayList): ServerStatus { + private suspend fun startKiwixServer(selectedBooksPath: ArrayList): 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 index 93a47a74d..f7b4743e2 100644 --- 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 index da64d0844..49c34b8c7 100644 --- 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 index 5c88bec3d..9a1142926 100644 --- 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.