Fix configuration change crash with PdfNavigatorFragment (#398)

readium/adapters/pdfium/pdfium-document/src/main/java/org/readium/adapters/pdfium/document/PdfiumDocument.kt

@@ -19,7 +19,9 @@ import org.readium.r2.shared.InternalReadiumApi
 import org.readium.r2.shared.extensions.md5
 import org.readium.r2.shared.extensions.tryOrNull
 import org.readium.r2.shared.resource.Resource
-import org.readium.r2.shared.util.getOrThrow
+import org.readium.r2.shared.resource.ResourceTry
+import org.readium.r2.shared.resource.mapCatching
+import org.readium.r2.shared.util.Try
 import org.readium.r2.shared.util.pdf.PdfDocument
 import org.readium.r2.shared.util.pdf.PdfDocumentFactory
 import org.readium.r2.shared.util.use
@@ -84,24 +86,25 @@ public class PdfiumDocumentFactory(context: Context) : PdfDocumentFactory<Pdfium
 
     private val core by lazy { PdfiumCore(context.applicationContext) }
 
-    override suspend fun open(file: File, password: String?): PdfiumDocument =
-        core.fromFile(file, password)
-
-    override suspend fun open(resource: Resource, password: String?): PdfiumDocument {
+    override suspend fun open(resource: Resource, password: String?): ResourceTry<PdfiumDocument> {
         // First try to open the resource as a file on the FS for performance improvement, as
         // PDFium requires the whole PDF document to be loaded in memory when using raw bytes.
         return resource.openAsFile(password)
             ?: resource.openBytes(password)
     }
 
-    private suspend fun Resource.openAsFile(password: String?): PdfiumDocument? =
+    private suspend fun Resource.openAsFile(password: String?): ResourceTry<PdfiumDocument>? =
         tryOrNull {
-            source?.toFile()?.let { open(it, password) }
+            source?.toFile()?.let { file ->
+                withContext(Dispatchers.IO) {
+                    Try.success(core.fromFile(file, password))
+                }
+            }
         }
 
-    private suspend fun Resource.openBytes(password: String?): PdfiumDocument =
+    private suspend fun Resource.openBytes(password: String?): ResourceTry<PdfiumDocument> =
         use {
-            core.fromBytes(read().getOrThrow(), password)
+            read().mapCatching { core.fromBytes(it, password) }
         }
 
     private fun PdfiumCore.fromFile(file: File, password: String?): PdfiumDocument =

readium/adapters/pdfium/pdfium-navigator/src/main/java/org/readium/adapters/pdfium/navigator/PdfiumDocumentFragment.kt

@@ -14,47 +14,39 @@ import android.view.ViewGroup
 import androidx.lifecycle.lifecycleScope
 import com.github.barteksc.pdfviewer.PDFView
 import kotlin.math.roundToInt
-import kotlinx.coroutines.launch
+import kotlinx.coroutines.flow.MutableStateFlow
+import kotlinx.coroutines.flow.StateFlow
+import kotlinx.coroutines.flow.asStateFlow
 import org.readium.adapters.pdfium.document.PdfiumDocumentFactory
 import org.readium.r2.navigator.pdf.PdfDocumentFragment
 import org.readium.r2.navigator.preferences.Axis
 import org.readium.r2.navigator.preferences.Fit
 import org.readium.r2.navigator.preferences.ReadingProgression
 import org.readium.r2.shared.ExperimentalReadiumApi
-import org.readium.r2.shared.publication.Link
 import org.readium.r2.shared.publication.Publication
 import org.readium.r2.shared.resource.Resource
+import org.readium.r2.shared.util.SingleJob
+import org.readium.r2.shared.util.Url
+import org.readium.r2.shared.util.getOrElse
 import timber.log.Timber
 
 @ExperimentalReadiumApi
 public class PdfiumDocumentFragment internal constructor(
     private val publication: Publication,
-    private val link: Link,
+    private val href: Url,
     private val initialPageIndex: Int,
-    settings: PdfiumSettings,
-    private val appListener: Listener?,
-    private val navigatorListener: PdfDocumentFragment.Listener?
+    initialSettings: PdfiumSettings,
+    private val listener: Listener?
 ) : PdfDocumentFragment<PdfiumSettings>() {
 
-    public interface Listener {
-        /** Called when configuring [PDFView]. */
-        public fun onConfigurePdfView(configurator: PDFView.Configurator) {}
+    internal interface Listener {
+        fun onResourceLoadFailed(href: Url, error: Resource.Exception)
+        fun onConfigurePdfView(configurator: PDFView.Configurator)
+        fun onTap(point: PointF): Boolean
     }
 
-    override var settings: PdfiumSettings = settings
-        set(value) {
-            if (field == value) return
-
-            val page = pageIndex
-            field = value
-            reloadDocumentAtPage(page)
-        }
-
     private lateinit var pdfView: PDFView
 
-    private var isReloading: Boolean = false
-    private var hasToReload: Int? = null
-
     override fun onCreateView(
         inflater: LayoutInflater,
         container: ViewGroup?,
@@ -65,86 +57,70 @@ public class PdfiumDocumentFragment internal constructor(
 
     override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
         super.onViewCreated(view, savedInstanceState)
-        reloadDocumentAtPage(pageIndex)
-    }
 
-    private fun reloadDocumentAtPage(pageIndex: Int) {
-        if (isReloading) {
-            hasToReload = pageIndex
-            return
-        }
+        resetJob = SingleJob(viewLifecycleOwner.lifecycleScope)
+        reset(pageIndex = initialPageIndex)
+    }
 
-        isReloading = true
+    private lateinit var resetJob: SingleJob
 
+    private fun reset(pageIndex: Int = _pageIndex.value) {
+        if (view == null) return
         val context = context?.applicationContext ?: return
 
-        viewLifecycleOwner.lifecycleScope.launch {
-            try {
-                val document = PdfiumDocumentFactory(context)
-                    // PDFium crashes when reusing the same PdfDocument, so we must not cache it.
+        resetJob.launch {
+            val document = PdfiumDocumentFactory(context)
+                // PDFium crashes when reusing the same PdfDocument, so we must not cache it.
 //                    .cachedIn(publication)
-                    .open(publication.get(link), null)
-
-                pageCount = document.pageCount
-                val page = convertPageIndexToView(pageIndex)
-
-                pdfView.recycle()
-                pdfView
-                    .fromSource { _, _, _ -> document.document }
-                    .apply {
-                        if (isPagesOrderReversed) {
-                            // AndroidPdfViewer doesn't support RTL. A workaround is to provide
-                            // the explicit page list in the right order.
-                            pages(*((pageCount - 1) downTo 0).toList().toIntArray())
-                        }
-                    }
-                    .swipeHorizontal(settings.scrollAxis == Axis.HORIZONTAL)
-                    .spacing(settings.pageSpacing.roundToInt())
-                    // Customization of [PDFView] is done before setting the listeners,
-                    // to avoid overriding them in reading apps, which would break the
-                    // navigator.
-                    .apply { appListener?.onConfigurePdfView(this) }
-                    .defaultPage(page)
-                    .onRender { _, _, _ ->
-                        if (settings.fit == Fit.WIDTH) {
-                            pdfView.fitToWidth()
-                            // Using `fitToWidth` often breaks the use of `defaultPage`, so we
-                            // need to jump manually to the target page.
-                            pdfView.jumpTo(page, false)
-                        }
-                    }
-                    .onLoad {
-                        val hasToReloadNow = hasToReload
-                        if (hasToReloadNow != null) {
-                            reloadDocumentAtPage(pageIndex)
-                        } else {
-                            isReloading = false
-                        }
-                    }
-                    .onPageChange { index, _ ->
-                        navigatorListener?.onPageChanged(convertPageIndexFromView(index))
+                .open(publication.get(href), null)
+                .getOrElse { error ->
+                    Timber.e(error)
+                    listener?.onResourceLoadFailed(href, error)
+                    return@launch
+                }
+
+            pageCount = document.pageCount
+            val page = convertPageIndexToView(pageIndex)
+
+            pdfView.recycle()
+            pdfView
+                .fromSource { _, _, _ -> document.document }
+                .apply {
+                    if (isPagesOrderReversed) {
+                        // AndroidPdfViewer doesn't support RTL. A workaround is to provide
+                        // the explicit page list in the right order.
+                        pages(*((pageCount - 1) downTo 0).toList().toIntArray())
                     }
-                    .onTap { event ->
-                        navigatorListener?.onTap(PointF(event.x, event.y))
-                            ?: false
+                }
+                .swipeHorizontal(settings.scrollAxis == Axis.HORIZONTAL)
+                .spacing(settings.pageSpacing.roundToInt())
+                // Customization of [PDFView] is done before setting the listeners,
+                // to avoid overriding them in reading apps, which would break the
+                // navigator.
+                .apply { listener?.onConfigurePdfView(this) }
+                .defaultPage(page)
+                .onRender { _, _, _ ->
+                    if (settings.fit == Fit.WIDTH) {
+                        pdfView.fitToWidth()
+                        // Using `fitToWidth` often breaks the use of `defaultPage`, so we
+                        // need to jump manually to the target page.
+                        pdfView.jumpTo(page, false)
                     }
-                    .load()
-            } catch (e: Exception) {
-                val error = Resource.Exception.wrap(e)
-                Timber.e(error)
-                navigatorListener?.onResourceLoadFailed(link, error)
-            }
+                }
+                .onPageChange { index, _ ->
+                    _pageIndex.value = convertPageIndexFromView(index)
+                }
+                .onTap { event ->
+                    listener?.onTap(PointF(event.x, event.y)) ?: false
+                }
+                .load()
         }
     }
 
-    override val pageIndex: Int get() = viewPageIndex ?: initialPageIndex
+    private var pageCount = 0
 
-    private val viewPageIndex: Int? get() =
-        if (pdfView.isRecycled) {
-            null
-        } else {
-            convertPageIndexFromView(pdfView.currentPage)
-        }
+    private val _pageIndex = MutableStateFlow(initialPageIndex)
+    override val pageIndex: StateFlow<Int> = _pageIndex.asStateFlow()
 
     override fun goToPageIndex(index: Int, animated: Boolean): Boolean {
         if (!isValidPageIndex(index)) {
@@ -154,8 +130,6 @@ public class PdfiumDocumentFragment internal constructor(
         return true
     }
 
-    private var pageCount = 0
-
     private fun isValidPageIndex(pageIndex: Int): Boolean {
         val validRange = 0 until pageCount
         return validRange.contains(pageIndex)
@@ -182,6 +156,16 @@ public class PdfiumDocumentFragment internal constructor(
      * right-to-left reading progressions.
      */
     private val isPagesOrderReversed: Boolean get() =
-        settings.scrollAxis == Axis.HORIZONTAL &&
-            settings.readingProgression == ReadingProgression.RTL
+        settings.scrollAxis == Axis.HORIZONTAL && settings.readingProgression == ReadingProgression.RTL
+
+    private var settings: PdfiumSettings = initialSettings
+
+    override fun applySettings(settings: PdfiumSettings) {
+        if (this.settings == settings) {
+            return
+        }
+
+        this.settings = settings
+        reset()
+    }
 }

readium/adapters/pdfium/pdfium-navigator/src/main/java/org/readium/adapters/pdfium/navigator/PdfiumEngineProvider.kt

@@ -6,13 +6,20 @@
 
 package org.readium.adapters.pdfium.navigator
 
+import android.graphics.PointF
+import com.github.barteksc.pdfviewer.PDFView
 import org.readium.r2.navigator.SimplePresentation
 import org.readium.r2.navigator.VisualNavigator
+import org.readium.r2.navigator.input.TapEvent
 import org.readium.r2.navigator.pdf.PdfDocumentFragmentInput
 import org.readium.r2.navigator.pdf.PdfEngineProvider
+import org.readium.r2.navigator.util.SingleFragmentFactory
+import org.readium.r2.navigator.util.createFragmentFactory
 import org.readium.r2.shared.ExperimentalReadiumApi
 import org.readium.r2.shared.publication.Metadata
 import org.readium.r2.shared.publication.Publication
+import org.readium.r2.shared.resource.Resource
+import org.readium.r2.shared.util.Url
 
 /**
  * Main component to use the PDF navigator with the PDFium adapter.
@@ -22,19 +29,39 @@ import org.readium.r2.shared.publication.Publication
  */
 @ExperimentalReadiumApi
 public class PdfiumEngineProvider(
-    private val listener: PdfiumDocumentFragment.Listener? = null,
-    private val defaults: PdfiumDefaults = PdfiumDefaults()
+    private val defaults: PdfiumDefaults = PdfiumDefaults(),
+    private val listener: Listener? = null
 ) : PdfEngineProvider<PdfiumSettings, PdfiumPreferences, PdfiumPreferencesEditor> {
 
-    override suspend fun createDocumentFragment(input: PdfDocumentFragmentInput<PdfiumSettings>): PdfiumDocumentFragment =
-        PdfiumDocumentFragment(
-            publication = input.publication,
-            link = input.link,
-            initialPageIndex = input.initialPageIndex,
-            settings = input.settings,
-            appListener = listener,
-            navigatorListener = input.listener
-        )
+    public interface Listener : PdfEngineProvider.Listener {
+
+        /** Called when configuring [PDFView]. */
+        public fun onConfigurePdfView(configurator: PDFView.Configurator) {}
+    }
+
+    override fun createDocumentFragmentFactory(
+        input: PdfDocumentFragmentInput<PdfiumSettings>
+    ): SingleFragmentFactory<PdfiumDocumentFragment> =
+        createFragmentFactory {
+            PdfiumDocumentFragment(
+                publication = input.publication,
+                href = input.href,
+                initialPageIndex = input.pageIndex,
+                initialSettings = input.settings,
+                listener = object : PdfiumDocumentFragment.Listener {
+                    override fun onResourceLoadFailed(href: Url, error: Resource.Exception) {
+                        input.navigatorListener?.onResourceLoadFailed(href, error)
+                    }
+
+                    override fun onConfigurePdfView(configurator: PDFView.Configurator) {
+                        listener?.onConfigurePdfView(configurator)
+                    }
+
+                    override fun onTap(point: PointF): Boolean =
+                        input.inputListener?.onTap(TapEvent(point)) ?: false
+                }
+            )
+        }
 
     override fun computeSettings(metadata: Metadata, preferences: PdfiumPreferences): PdfiumSettings {
         val settingsPolicy = PdfiumSettingsResolver(metadata, defaults)

readium/adapters/pdfium/pdfium-navigator/src/main/java/org/readium/adapters/pdfium/navigator/PdfiumNavigator.kt

@@ -0,0 +1,17 @@
+/*
+ * Copyright 2023 Readium Foundation. All rights reserved.
+ * Use of this source code is governed by the BSD-style license
+ * available in the top-level LICENSE file of the project.
+ */
+
+package org.readium.adapters.pdfium.navigator
+
+import org.readium.r2.navigator.pdf.PdfNavigatorFactory
+import org.readium.r2.navigator.pdf.PdfNavigatorFragment
+import org.readium.r2.shared.ExperimentalReadiumApi
+
+@ExperimentalReadiumApi
+public typealias PdfiumNavigatorFragment = PdfNavigatorFragment<PdfiumSettings, PdfiumPreferences>
+
+@ExperimentalReadiumApi
+public typealias PdfiumNavigatorFactory = PdfNavigatorFactory<PdfiumSettings, PdfiumPreferences, PdfiumPreferencesEditor>