Merge "Fix IndexOutOfBoundException in position lookup" into androidx-master-dev
diff --git a/paging/runtime/src/androidTest/java/androidx/paging/PagedStorageDiffHelperTest.kt b/paging/runtime/src/androidTest/java/androidx/paging/PagedStorageDiffHelperTest.kt
index e9b0eac..afddc32 100644
--- a/paging/runtime/src/androidTest/java/androidx/paging/PagedStorageDiffHelperTest.kt
+++ b/paging/runtime/src/androidTest/java/androidx/paging/PagedStorageDiffHelperTest.kt
@@ -205,6 +205,28 @@
}
}
+ @Test
+ fun transformAnchorIndex_loadingSnapshot() {
+ val oldList = PagedStorage(10, listOf("a"), 10)
+ val newList = PagedStorage(10, listOf("a"), 10)
+
+ oldList.allocatePlaceholders(10, 5, 1,
+ /* ignored */ mock(PagedStorage.Callback::class.java))
+
+ assertEquals(5, oldList.leadingNullCount)
+ assertEquals(10, oldList.computeLeadingNulls())
+
+ validateTwoListDiffTransform(
+ oldList,
+ newList) { transformAnchorIndex ->
+ // previously, this would cause a crash where we tried to use storage space
+ // (getLeadingNullCount..size-getTrailingNulls) incorrectly, instead of diff space
+ // (computeLeadingNulls..size-computeTrailingNulls). Diff space greedily excludes the
+ // nulls that represent partially loaded pages to minimize diff computation cost.
+ assertEquals(15, transformAnchorIndex(15))
+ }
+ }
+
companion object {
private val DIFF_CALLBACK = object : DiffUtil.ItemCallback<String>() {
override fun areItemsTheSame(oldItem: String, newItem: String): Boolean {
diff --git a/paging/runtime/src/main/java/androidx/paging/AsyncPagedListDiffer.java b/paging/runtime/src/main/java/androidx/paging/AsyncPagedListDiffer.java
index b3fa608..27e6c01 100644
--- a/paging/runtime/src/main/java/androidx/paging/AsyncPagedListDiffer.java
+++ b/paging/runtime/src/main/java/androidx/paging/AsyncPagedListDiffer.java
@@ -374,9 +374,12 @@
// Transform the last loadAround() index from the old list to the new list by passing it
// through the DiffResult. This ensures the lastKey of a positional PagedList is carried
- // to new list even if no in-viewport item changes (AsyncPagedListDiffer#get not called)
+ // to new list even if no in-viewport item changes (AsyncPagedListDiffer#get not called).
+ // Note: we don't take into account loads between new list snapshot and new list, but this
+ // is only a problem in rare cases when placeholders are disabled, and a load starts (for
+ // some reason) and finishes before diff completes.
int newPosition = PagedStorageDiffHelper.transformAnchorIndex(
- diffResult, previousSnapshot.mStorage, newList.mStorage, lastAccessIndex);
+ diffResult, previousSnapshot.mStorage, diffSnapshot.mStorage, lastAccessIndex);
// copy lastLoad position, clamped to list bounds
mPagedList.mLastLoad = Math.max(0, Math.min(mPagedList.size(), newPosition));
diff --git a/paging/runtime/src/main/java/androidx/paging/PagedStorageDiffHelper.java b/paging/runtime/src/main/java/androidx/paging/PagedStorageDiffHelper.java
index e98cdd1..a89baf9 100644
--- a/paging/runtime/src/main/java/androidx/paging/PagedStorageDiffHelper.java
+++ b/paging/runtime/src/main/java/androidx/paging/PagedStorageDiffHelper.java
@@ -21,6 +21,21 @@
import androidx.recyclerview.widget.DiffUtil;
import androidx.recyclerview.widget.ListUpdateCallback;
+/**
+ * Methods for computing and applying DiffResults between PagedLists.
+ *
+ * To minimize the amount of diffing caused by placeholders, we only execute DiffUtil in a reduced
+ * 'diff space' - in the range (computeLeadingNulls..size-computeTrailingNulls).
+ *
+ * This allows the diff of a PagedList, e.g.:
+ * 100 nulls, placeholder page, (empty page) x 5, page, 100 nulls
+ *
+ * To only inform DiffUtil about single loaded page in this case, by pruning all other nulls from
+ * consideration.
+ *
+ * @see PagedStorage#computeLeadingNulls()
+ * @see PagedStorage#computeTrailingNulls()
+ */
class PagedStorageDiffHelper {
private PagedStorageDiffHelper() {
}
@@ -178,12 +193,16 @@
*/
static int transformAnchorIndex(@NonNull DiffUtil.DiffResult diffResult,
@NonNull PagedStorage oldList, @NonNull PagedStorage newList, final int oldPosition) {
+ final int oldOffset = oldList.computeLeadingNulls();
+
// diffResult's indices starting after nulls, need to transform to diffutil indices
// (see also dispatchDiff(), which adds this offset when dispatching)
- int diffIndex = oldPosition - oldList.getLeadingNullCount();
+ int diffIndex = oldPosition - oldOffset;
+
+ final int oldSize = oldList.size() - oldOffset - oldList.computeTrailingNulls();
// if our anchor is non-null, use it or close item's position in new list
- if (diffIndex >= 0 && diffIndex < oldList.getStorageCount()) {
+ if (diffIndex >= 0 && diffIndex < oldSize) {
// search outward from old position for position that maps
for (int i = 0; i < 30; i++) {
int positionToTry = diffIndex + (i / 2 * (i % 2 == 1 ? -1 : 1));
