Skip to content

Fix 3 crashes #760

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
RankoR wants to merge 3 commits into
base: develop
Choose a base branch
from
Open

Fix 3 crashes #760

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e8797d2
Fix production crash in WebView width & height checks
RankoR Mar 1, 2026
5473d55
Early-return instead of unwrapping nullable value to fix crashes
RankoR Mar 1, 2026
e7884b5
Prevent race condition when container reads continue while zip file i…
RankoR Mar 1, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions ...fixedlayout/src/main/kotlin/org/readium/navigator/web/fixedlayout/spread/SpreadWebView.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import org.readium.navigator.web.internals.webview.RelaxedWebView
import org.readium.navigator.web.internals.webview.WebView
import org.readium.navigator.web.internals.webview.WebViewScrollController
import org.readium.navigator.web.internals.webview.WebViewState
import org.readium.navigator.web.internals.webview.invokeOnWebViewUpToDate
import org.readium.r2.shared.ExperimentalReadiumApi
import org.readium.r2.shared.util.AbsoluteUrl

Expand Down Expand Up @@ -96,8 +97,7 @@ internal fun SpreadWebView(
documentStateApi?.let { documentStateApi ->
documentStateApi.listener = DelegatingDocumentApiListener(
onDocumentLoadedAndSizedDelegate = {
webView.requestLayout()
webView.setNextLayoutListener {
webView.invokeOnWebViewUpToDate {
val scrollController = WebViewScrollController(webView)
scrollController.moveToProgression(
progression = progression,
Expand Down
2 changes: 1 addition & 1 deletion ...ternals/src/main/kotlin/org/readium/navigator/web/internals/pager/RenditionScrollState.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ public class RenditionScrollState(
return
}

val scrollController = pageStates[index].scrollController.value!!
val scrollController = pageStates[index].scrollController.value ?: return
val scrolled = scrollController.scrollToMax(orientation)
if (scrolled > 0) {
dispatchRawDelta(scrolled.toFloat())
Expand Down
11 changes: 8 additions & 3 deletions ...b/internals/src/main/kotlin/org/readium/navigator/web/internals/webview/RelaxedWebView.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,9 @@ public class RelaxedWebView(context: Context) : WebView(context) {
@Deprecated("Deprecated in Java")
override fun onLayout(changed: Boolean, l: Int, t: Int, r: Int, b: Int) {
super.onLayout(changed, l, t, r, b)
nextLayoutListener.invoke()
val listener = nextLayoutListener
nextLayoutListener = {}
listener.invoke()
}

private var hasActionMode: Boolean = false
Expand Down Expand Up @@ -140,8 +141,12 @@ private class CallbackDecorator(
* has received data up-to-date at the moment when the call occurs or newer.
*/
public fun RelaxedWebView.invokeOnWebViewUpToDate(block: WebView.() -> Unit) {
requestLayout()
setNextLayoutListener {
if (verticalScrollExtent > 0 && horizontalScrollExtent > 0) {
invokeOnReadyToBeDrawn(block)
} else {
requestLayout()
setNextLayoutListener {
invokeOnWebViewUpToDate(block)
Copy link
Contributor

@stevenzeck stevenzeck Mar 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This being recursive, if the webview is detached or something similar, won't this will loop?

Copy link
Contributor Author

@RankoR RankoR Mar 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, this will loop. Do you have any suggestions better than limiting max recursion?

Copy link
Member

@qnga qnga Mar 18, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you could use coroutine cancellation with a scope got from the composition.

}
}
}
48 changes: 32 additions & 16 deletions readium/shared/src/main/java/org/readium/r2/shared/util/zip/FileZipContainer.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ import kotlinx.coroutines.DelicateCoroutinesApi
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.GlobalScope
import kotlinx.coroutines.launch
import kotlinx.coroutines.sync.Mutex
import kotlinx.coroutines.sync.withLock
import kotlinx.coroutines.withContext
import org.readium.r2.shared.InternalReadiumApi
import org.readium.r2.shared.extensions.readFully
Expand All @@ -42,6 +44,8 @@ internal class FileZipContainer(
file: File,
) : Container<Resource> {

private val mutex = Mutex()

private inner class Entry(private val url: Url, private val entry: ZipEntry) :
Resource {

Expand Down Expand Up @@ -76,20 +80,30 @@ internal class FileZipContainer(
}

override suspend fun read(range: LongRange?): Try<ByteArray, ReadError> =
try {
withContext(Dispatchers.IO) {
val bytes =
if (range == null) {
readFully()
} else {
readRange(range)
}
Try.success(bytes)
mutex.withLock {
try {
withContext(Dispatchers.IO) {
val bytes =
if (range == null) {
readFully()
} else {
readRange(range)
}
Try.success(bytes)
}
} catch (e: ZipException) {
Try.failure(ReadError.Decoding(e))
} catch (e: IOException) {
Try.failure(ReadError.Access(FileSystemError.IO(e)))
} catch (e: IllegalStateException) {
Try.failure(
ReadError.Access(
FileSystemError.IO(
IOException("Zip file closed.", e)
)
)
)
}
} catch (e: ZipException) {
Try.failure(ReadError.Decoding(e))
} catch (e: IOException) {
Try.failure(ReadError.Access(FileSystemError.IO(e)))
}

private suspend fun readFully(): ByteArray =
Expand Down Expand Up @@ -156,9 +170,11 @@ internal class FileZipContainer(
@OptIn(DelicateCoroutinesApi::class)
override fun close() {
GlobalScope.launch {
tryOrLog {
withContext(Dispatchers.IO) {
archive.close()
mutex.withLock {
tryOrLog {
withContext(Dispatchers.IO) {
archive.close()
}
}
}
}
Expand Down