Merge pull request #4129 from kiwix/Fixes#4127

Fixed: Input dispatching timed out in `CoreReaderFragment.toggleBookmark`.
This commit is contained in:
Kelson 2024-12-10 17:15:23 +01:00 committed by GitHub
commit 3e197cfcd2
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 86 additions and 71 deletions

View File

@ -19,6 +19,8 @@
package org.kiwix.kiwixmobile.webserver package org.kiwix.kiwixmobile.webserver
import android.content.Context 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.utils.files.Log
import org.kiwix.kiwixmobile.core.reader.ZimReaderContainer import org.kiwix.kiwixmobile.core.reader.ZimReaderContainer
import org.kiwix.kiwixmobile.core.utils.files.FileUtils.getDemoFilePathForCustomApp import org.kiwix.kiwixmobile.core.utils.files.FileUtils.getDemoFilePathForCustomApp
@ -43,41 +45,42 @@ class KiwixServer @Inject constructor(
private val zimReaderContainer: ZimReaderContainer private val zimReaderContainer: ZimReaderContainer
) { ) {
@Suppress("NestedBlockDepth") @Suppress("NestedBlockDepth")
fun createKiwixServer(selectedBooksPath: ArrayList<String>): KiwixServer { suspend fun createKiwixServer(selectedBooksPath: ArrayList<String>): KiwixServer =
val kiwixLibrary = Library() withContext(Dispatchers.IO) {
selectedBooksPath.forEach { path -> val kiwixLibrary = Library()
try { selectedBooksPath.forEach { path ->
val book = Book().apply { try {
// Determine whether to create an Archive from an asset or a file path val book = Book().apply {
val archive = if (path == getDemoFilePathForCustomApp(context)) { // Determine whether to create an Archive from an asset or a file path
// For custom apps using a demo file, create an Archive with FileDescriptor val archive = if (path == getDemoFilePathForCustomApp(context)) {
val assetFileDescriptor = // For custom apps using a demo file, create an Archive with FileDescriptor
zimReaderContainer.zimReaderSource?.assetFileDescriptorList?.get(0) val assetFileDescriptor =
val startOffset = assetFileDescriptor?.startOffset ?: 0L zimReaderContainer.zimReaderSource?.assetFileDescriptorList?.get(0)
val size = assetFileDescriptor?.length ?: 0L val startOffset = assetFileDescriptor?.startOffset ?: 0L
Archive( val size = assetFileDescriptor?.length ?: 0L
assetFileDescriptor?.parcelFileDescriptor?.fileDescriptor, Archive(
startOffset, assetFileDescriptor?.parcelFileDescriptor?.fileDescriptor,
size startOffset,
) size
} else { )
// For regular files, create an Archive from the file path } else {
Archive(path) // 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 { fun startServer(port: Int): Boolean {

View File

@ -45,7 +45,7 @@ class WebServerHelper @Inject constructor(
private var isServerStarted = false private var isServerStarted = false
private var validIpAddressDisposable: Disposable? = null private var validIpAddressDisposable: Disposable? = null
fun startServerHelper( suspend fun startServerHelper(
selectedBooksPath: ArrayList<String>, selectedBooksPath: ArrayList<String>,
restartServer: Boolean restartServer: Boolean
): ServerStatus? { ): ServerStatus? {
@ -64,7 +64,7 @@ class WebServerHelper @Inject constructor(
} }
} }
private fun startAndroidWebServer( private suspend fun startAndroidWebServer(
selectedBooksPath: ArrayList<String>, selectedBooksPath: ArrayList<String>,
restartServer: Boolean restartServer: Boolean
): ServerStatus? { ): ServerStatus? {
@ -78,7 +78,7 @@ class WebServerHelper @Inject constructor(
return serverStatus return serverStatus
} }
private fun startKiwixServer(selectedBooksPath: ArrayList<String>): ServerStatus { private suspend fun startKiwixServer(selectedBooksPath: ArrayList<String>): ServerStatus {
var errorMessage: Int? = null var errorMessage: Int? = null
ServerUtils.port = DEFAULT_PORT ServerUtils.port = DEFAULT_PORT
kiwixServer = kiwixServerFactory.createKiwixServer(selectedBooksPath).also { kiwixServer = kiwixServerFactory.createKiwixServer(selectedBooksPath).also {

View File

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

View File

@ -22,6 +22,10 @@ import android.content.Intent
import android.os.Binder import android.os.Binder
import android.os.IBinder import android.os.IBinder
import android.widget.Toast 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.KiwixApp
import org.kiwix.kiwixmobile.core.R import org.kiwix.kiwixmobile.core.R
import org.kiwix.kiwixmobile.core.extensions.registerReceiver import org.kiwix.kiwixmobile.core.extensions.registerReceiver
@ -75,18 +79,22 @@ class HotspotService :
ACTION_START_SERVER -> { ACTION_START_SERVER -> {
val restartServer = intent.getBooleanExtra(RESTART_SERVER, false) val restartServer = intent.getBooleanExtra(RESTART_SERVER, false)
intent.getStringArrayListExtra(ZimHostFragment.SELECTED_ZIM_PATHS_KEY)?.let { intent.getStringArrayListExtra(ZimHostFragment.SELECTED_ZIM_PATHS_KEY)?.let {
val serverStatus = webServerHelper?.startServerHelper(it, restartServer) CoroutineScope(Dispatchers.Main).launch {
if (serverStatus?.isServerStarted == true) { val serverStatus = withContext(Dispatchers.IO) {
zimHostCallbacks?.onServerStarted(getSocketAddress()) webServerHelper?.startServerHelper(it, restartServer)
startForegroundNotificationHelper() }
if (!restartServer) { if (serverStatus?.isServerStarted == true) {
Toast.makeText( zimHostCallbacks?.onServerStarted(getSocketAddress())
this, R.string.server_started_successfully_toast_message, startForegroundNotificationHelper()
Toast.LENGTH_SHORT if (!restartServer) {
).show() 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) } } ?: kotlin.run { onServerFailedToStart(R.string.no_books_selected_toast_message) }
} }

View File

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