[StaggeredGridLayoutManager] Avoid OutOfBounds exception during mutations if SpanLookup data structure has not yet been lengthened
**Background**
A use case within our app ran into this issue frequently as a result of inserting items between a set of full span items. We applied numerous band-aids (clearing of the span cache, filler items, etc.), but those had a bunch of unintended side-effects.
- Within the code, my first approach was to limit the array fill to `MIN(length, position)`, but that really didn't feel like the right fix.
- Digging deeper, I found that the position being extended to with `ensureSize()` did not factor in the maximum extent of items in `mData` or `mFullSpanItems` (which do not necessarily have the same "cap" in terms of position / length).
- A fix that I tried relating to this was to always `ensureSize()` for mData's length, but that results in expontential growth because of the fact that mData's length ≠ number of items.
- To keep it simple, I realized the easiest thing to do is just ensure that mData is large enough for the `item count` we're supposed to be displaying.
- Through discussion in review, we ended up reverting to the simpler version using `MIN()`.
Note that the test case does something which I *think* is pretty uncommon in vanilla adapters, but is the case for us when using it paired with Litho. That was the easiest repro case for me to arrive at, but I'm sure there are others.
Bug:122303625
Bug:74877618
Bug:160193663
Bug:37086625
Test: New test case in StaggeredGridLayoutManagerTest validates that the `Arrays.fill()` invocation does not lead to a crash.
Change-Id: Iab0a1220b4eae8f2b184822d518c6d696c278b19
diff --git a/recyclerview/recyclerview/src/androidTest/java/androidx/recyclerview/widget/StaggeredGridLayoutManagerTest.java b/recyclerview/recyclerview/src/androidTest/java/androidx/recyclerview/widget/StaggeredGridLayoutManagerTest.java
index d298a91..ae80cc9 100644
--- a/recyclerview/recyclerview/src/androidTest/java/androidx/recyclerview/widget/StaggeredGridLayoutManagerTest.java
+++ b/recyclerview/recyclerview/src/androidTest/java/androidx/recyclerview/widget/StaggeredGridLayoutManagerTest.java
@@ -1046,6 +1046,36 @@
}
@Test
+ public void testBatchInsertionsBetweenTailFullSpanItems() throws Throwable {
+ // Magic numbers here aren't super specific to repro, but were the example test case that
+ // led to the isolation of this bug.
+ setupByConfig(new Config().spanCount(2).itemCount(22));
+
+ // Last few items are full spans. Create a variable to reference later, even though it's
+ // basically just a few repeated calls.
+ mAdapter.mFullSpanItems.add(18);
+ mAdapter.mFullSpanItems.add(19);
+ mAdapter.mFullSpanItems.add(20);
+ mAdapter.mFullSpanItems.add(21);
+
+ waitFirstLayout();
+
+ // Scroll to the end to populate full span items.
+ smoothScrollToPosition(mAdapter.mItems.size() - 1);
+
+ // Incrementally add a handful of items, mimicking some adapter usages.
+ final int numberOfItemsToAdd = 12;
+ final int fullSpanItemIndexToInsertFrom = 18 + 1;
+ for (int i = 0; i < numberOfItemsToAdd; i++) {
+ final int insertAt = fullSpanItemIndexToInsertFrom + i;
+ mAdapter.addAndNotify(insertAt, 1);
+ }
+
+ requestLayoutOnUIThread(mRecyclerView);
+ mLayoutManager.waitForLayout(3);
+ }
+
+ @Test
public void temporaryGapHandling() throws Throwable {
int fullSpanIndex = 100;
setupByConfig(new Config()
diff --git a/recyclerview/recyclerview/src/main/java/androidx/recyclerview/widget/StaggeredGridLayoutManager.java b/recyclerview/recyclerview/src/main/java/androidx/recyclerview/widget/StaggeredGridLayoutManager.java
index c78a39a..0387e96 100644
--- a/recyclerview/recyclerview/src/main/java/androidx/recyclerview/widget/StaggeredGridLayoutManager.java
+++ b/recyclerview/recyclerview/src/main/java/androidx/recyclerview/widget/StaggeredGridLayoutManager.java
@@ -2846,9 +2846,11 @@
Arrays.fill(mData, position, mData.length, LayoutParams.INVALID_SPAN_ID);
return mData.length;
} else {
- // just invalidate items in between
- Arrays.fill(mData, position, endPosition + 1, LayoutParams.INVALID_SPAN_ID);
- return endPosition + 1;
+ // Just invalidate items in between `position` and the next full span item, or the
+ // end of the tracked spans in mData if it's not been lengthened yet.
+ final int invalidateToIndex = Math.min(endPosition + 1, mData.length);
+ Arrays.fill(mData, position, invalidateToIndex, LayoutParams.INVALID_SPAN_ID);
+ return invalidateToIndex;
}
}
