Do not crash when fail to read from the additional content cursor

Bug: 354546194
Test: atest IntentResolver-tests-unit
Flag: EXEMPT bugfix
Change-Id: I5123fe6c1cabbbae7479d2cea8bd9e4b8ff0b4a8

2 files changed, 128 insertions, 34 deletions

diff --git a/java/src/com/android/intentresolver/contentpreview/payloadtoggle/domain/interactor/CursorPreviewsInteractor.kt b/java/src/com/android/intentresolver/contentpreview/payloadtoggle/domain/interactor/CursorPreviewsInteractor.kt
index a475263c..7d658209 100644
--- a/java/src/com/android/intentresolver/contentpreview/payloadtoggle/domain/interactor/CursorPreviewsInteractor.kt
+++ b/java/src/com/android/intentresolver/contentpreview/payloadtoggle/domain/interactor/CursorPreviewsInteractor.kt
@@ -20,6 +20,7 @@ package com.android.intentresolver.contentpreview.payloadtoggle.domain.interacto
 
 import android.net.Uri
 import android.service.chooser.AdditionalContentContract.CursorExtraKeys.POSITION
+import android.util.Log
 import com.android.intentresolver.contentpreview.UriMetadataReader
 import com.android.intentresolver.contentpreview.payloadtoggle.domain.model.CursorRow
 import com.android.intentresolver.contentpreview.payloadtoggle.domain.model.LoadDirection
@@ -51,6 +52,8 @@ import kotlinx.coroutines.flow.filterNotNull
 import kotlinx.coroutines.flow.first
 import kotlinx.coroutines.flow.mapLatest
 
+private const val TAG = "CursorPreviewsIntr"
+
 /** Queries data from a remote cursor, and caches it locally for presentation in Shareousel. */
 class CursorPreviewsInteractor
 @Inject
@@ -273,8 +276,7 @@ constructor(
         pagedCursor
             .getPageRows(pageNum) // TODO: what do we do if the load fails?
             ?.filter { it.uri !in state.merged }
-            ?.toPage(this, unclaimedRecords)
-            ?: this
+            ?.toPage(this, unclaimedRecords) ?: this
 
     private suspend fun <M : MutablePreviewMap> Sequence<CursorRow>.toPage(
         destination: M,
@@ -288,26 +290,32 @@ constructor(
     private fun createPreviewModel(
         row: CursorRow,
         unclaimedRecords: MutableUnclaimedMap,
-    ): PreviewModel = uriMetadataReader.getMetadata(row.uri).let { metadata ->
-            val size =
-                row.previewSize
-                    ?: metadata.previewUri?.let { uriMetadataReader.readPreviewSize(it) }
-            PreviewModel(
-                uri = row.uri,
-                previewUri = metadata.previewUri,
-                mimeType = metadata.mimeType,
-                aspectRatio = size.aspectRatioOrDefault(1f),
-                order = row.position,
-            )
-        }.also { updated ->
-            if (unclaimedRecords.remove(row.uri) != null) {
-                // unclaimedRecords contains initially shared (and thus selected) items with unknown
-                // cursor position. Update selection records when any of those items is encountered
-                // in the cursor to maintain proper selection order should other items also be
-                // selected.
-                selectionInteractor.updateSelection(updated)
+    ): PreviewModel =
+        uriMetadataReader
+            .getMetadata(row.uri)
+            .let { metadata ->
+                val size =
+                    row.previewSize
+                        ?: metadata.previewUri?.let { uriMetadataReader.readPreviewSize(it) }
+                PreviewModel(
+                    uri = row.uri,
+                    previewUri = metadata.previewUri,
+                    mimeType = metadata.mimeType,
+                    aspectRatio = size.aspectRatioOrDefault(1f),
+                    order = row.position,
+                )
+            }
+            .also { updated ->
+                if (unclaimedRecords.remove(row.uri) != null) {
+                    // unclaimedRecords contains initially shared (and thus selected) items with
+                    // unknown
+                    // cursor position. Update selection records when any of those items is
+                    // encountered
+                    // in the cursor to maintain proper selection order should other items also be
+                    // selected.
+                    selectionInteractor.updateSelection(updated)
+                }
             }
-        }
 
     private fun <M : MutablePreviewMap> M.putAllUnclaimedRight(unclaimed: UnclaimedMap): M =
         putAllUnclaimedWhere(unclaimed) { it >= focusedItemIdx }
@@ -343,7 +351,28 @@ private fun <M : MutablePreviewMap> M.putAllUnclaimedWhere(
         .toMap(this)
 
 private fun PagedCursor<CursorRow?>.getPageRows(pageNum: Int): Sequence<CursorRow>? =
-    get(pageNum)?.filterNotNull()
+    runCatching { get(pageNum) }
+        .onFailure { Log.e(TAG, "Failed to read additional content cursor page #$pageNum", it) }
+        .getOrNull()
+        ?.asSafeSequence()
+        ?.filterNotNull()
+
+private fun <T> Sequence<T>.asSafeSequence(): Sequence<T> {
+    return if (this is SafeSequence) this else SafeSequence(this)
+}
+
+private class SafeSequence<T>(private val sequence: Sequence<T>) : Sequence<T> {
+    override fun iterator(): Iterator<T> =
+        sequence.iterator().let { if (it is SafeIterator) it else SafeIterator(it) }
+}
+
+private class SafeIterator<T>(private val iterator: Iterator<T>) : Iterator<T> by iterator {
+    override fun hasNext(): Boolean {
+        return runCatching { iterator.hasNext() }
+            .onFailure { Log.e(TAG, "Failed to read cursor", it) }
+            .getOrDefault(false)
+    }
+}
 
 @Qualifier @MustBeDocumented @Retention(AnnotationRetention.RUNTIME) annotation class PageSize
 
diff --git a/tests/unit/src/com/android/intentresolver/contentpreview/payloadtoggle/domain/interactor/CursorPreviewsInteractorTest.kt b/tests/unit/src/com/android/intentresolver/contentpreview/payloadtoggle/domain/interactor/CursorPreviewsInteractorTest.kt
index 48e43190..c4ba8105 100644
--- a/tests/unit/src/com/android/intentresolver/contentpreview/payloadtoggle/domain/interactor/CursorPreviewsInteractorTest.kt
+++ b/tests/unit/src/com/android/intentresolver/contentpreview/payloadtoggle/domain/interactor/CursorPreviewsInteractorTest.kt
@@ -18,10 +18,13 @@
 
 package com.android.intentresolver.contentpreview.payloadtoggle.domain.interactor
 
+import android.database.Cursor
 import android.database.MatrixCursor
 import android.net.Uri
 import android.provider.MediaStore.MediaColumns.HEIGHT
 import android.provider.MediaStore.MediaColumns.WIDTH
+import android.service.chooser.AdditionalContentContract.Columns.URI
+import android.service.chooser.AdditionalContentContract.CursorExtraKeys.POSITION
 import android.util.Size
 import androidx.core.os.bundleOf
 import com.android.intentresolver.contentpreview.FileInfo
@@ -39,6 +42,7 @@ import com.android.intentresolver.util.cursor.CursorView
 import com.android.intentresolver.util.cursor.viewBy
 import com.android.intentresolver.util.runTest
 import com.android.systemui.kosmos.Kosmos
+import com.google.common.truth.Correspondence
 import com.google.common.truth.Truth.assertThat
 import kotlinx.coroutines.ExperimentalCoroutinesApi
 import kotlinx.coroutines.launch
@@ -93,9 +97,9 @@ class CursorPreviewsInteractorTest {
         private val cursorSizes: Map<Int, Size>,
     ) {
         val cursor: CursorView<CursorRow?> =
-            MatrixCursor(arrayOf("uri", WIDTH, HEIGHT))
+            MatrixCursor(arrayOf(URI, WIDTH, HEIGHT))
                 .apply {
-                    extras = bundleOf("position" to cursorStartPosition)
+                    extras = bundleOf(POSITION to cursorStartPosition)
                     for (i in cursorRange) {
                         val size = cursorSizes[i]
                         addRow(
@@ -279,22 +283,83 @@ class CursorPreviewsInteractorTest {
         ) { deps ->
             previewSelectionsRepository.selections.value =
                 PreviewModel(
-                    uri = uri(1),
-                    mimeType = "image/png",
-                    order = 0,
-                ).let { mapOf(it.uri to it) }
+                        uri = uri(1),
+                        mimeType = "image/png",
+                        order = 0,
+                    )
+                    .let { mapOf(it.uri to it) }
             backgroundScope.launch {
                 cursorPreviewsInteractor.launch(deps.cursor, deps.initialPreviews)
             }
             runCurrent()
 
-            assertThat(previewSelectionsRepository.selections.value.values).containsExactly(
-                PreviewModel(
-                    uri = uri(1),
-                    mimeType = "image/bitmap",
-                    order = 1,
+            assertThat(previewSelectionsRepository.selections.value.values)
+                .containsExactly(
+                    PreviewModel(
+                        uri = uri(1),
+                        mimeType = "image/bitmap",
+                        order = 1,
+                    )
+                )
+        }
+
+    @Test
+    fun testReadFailedPages() =
+        runTestWithDeps(
+            initialSelection = listOf(4),
+            cursor = emptyList(),
+            cursorStartPosition = 0,
+            pageSize = 2,
+            maxLoadedPages = 5,
+        ) { deps ->
+            val cursor =
+                MatrixCursor(arrayOf(URI)).apply {
+                    extras = bundleOf(POSITION to 4)
+                    for (i in 0 until 10) {
+                        addRow(arrayOf(uri(i)))
+                    }
+                }
+            val failingPositions = setOf(1, 5, 8)
+            val failingCursor =
+                object : Cursor by cursor {
+                        override fun move(offset: Int): Boolean = moveToPosition(position + offset)
+
+                        override fun moveToPosition(position: Int): Boolean {
+                            if (failingPositions.contains(position)) {
+                                throw RuntimeException(
+                                    "A test exception when moving the cursor to position $position"
+                                )
+                            }
+                            return cursor.moveToPosition(position)
+                        }
+
+                        override fun moveToFirst(): Boolean = moveToPosition(0)
+
+                        override fun moveToLast(): Boolean = moveToPosition(count - 1)
+
+                        override fun moveToNext(): Boolean = move(1)
+
+                        override fun moveToPrevious(): Boolean = move(-1)
+                    }
+                    .viewBy {
+                        getString(0)?.let { uriStr ->
+                            CursorRow(Uri.parse(uriStr), readSize(), position)
+                        }
+                    }
+            backgroundScope.launch {
+                cursorPreviewsInteractor.launch(failingCursor, deps.initialPreviews)
+            }
+            runCurrent()
+
+            assertThat(cursorPreviewsRepository.previewsModel.value).isNotNull()
+            assertThat(cursorPreviewsRepository.previewsModel.value!!.previewModels)
+                .comparingElementsUsing<PreviewModel, Uri>(
+                    Correspondence.transforming({ it.uri }, "has a Uri of")
+                )
+                .containsExactlyElementsIn(
+                    (0..7).filterNot { failingPositions.contains(it) }.map { uri(it) }
                 )
-            )
+                .inOrder()
         }
 }