From 0a36077b10a731491b6bd19feb179a074dd981de Mon Sep 17 00:00:00 2001 From: Andrey Kulikov Date: Fri, 10 Jun 2022 15:02:24 +0100 Subject: Fix for a crash in IntervalList It was happening when Lazy list was used with Int.MAX_VALUE size, it is a popular pattern in Pager in order to achieve infinity scrolling pager. The issue was introduced when we recently introduced public MutableIntervalList API and the forEach logic there was incorrectly calculating indexes. Fixes: 235495226 Test: new tests in LazyGridTest, LazyListTest, MutableIntervalListTest Change-Id: I11a89de2023700d51382505bcaea6e74d95a9273 (cherry picked from commit 69af04a9ff80cba3987b9c4d0baf2b00d9e2baf7) Merged-In: I11a89de2023700d51382505bcaea6e74d95a9273 --- .../compose/foundation/lazy/grid/LazyGridTest.kt | 20 +++++++++++++++++ .../compose/foundation/lazy/list/LazyListTest.kt | 22 ++++++++++++++++++ .../compose/foundation/lazy/layout/IntervalList.kt | 2 +- .../foundation/lazy/MutableIntervalListTest.kt | 26 ++++++++++++++++++++++ 4 files changed, 69 insertions(+), 1 deletion(-) diff --git a/compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/lazy/grid/LazyGridTest.kt b/compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/lazy/grid/LazyGridTest.kt index 7b5e760aad2..396ec8dbb82 100644 --- a/compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/lazy/grid/LazyGridTest.kt +++ b/compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/lazy/grid/LazyGridTest.kt @@ -720,6 +720,26 @@ class LazyGridTest( rule.onNodeWithTag("0").assertDoesNotExist() } + @Test + fun maxIntElements_withKey_startInMiddle() { + val itemSize = with(rule.density) { 15.toDp() } + + rule.setContent { + LazyGrid( + cells = 1, + modifier = Modifier.size(itemSize), + state = LazyGridState(firstVisibleItemIndex = Int.MAX_VALUE / 2) + ) { + items(Int.MAX_VALUE, key = { it }) { + Box(Modifier.size(itemSize).testTag("$it")) + } + } + } + + rule.onNodeWithTag("${Int.MAX_VALUE / 2}") + .assertMainAxisStartPositionInRootIsEqualTo(0.dp) + } + @Test fun pointerInputScrollingIsAllowedWhenUserScrollingIsEnabled() { val itemSize = with(rule.density) { 30.toDp() } diff --git a/compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/lazy/list/LazyListTest.kt b/compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/lazy/list/LazyListTest.kt index e38956c49e4..48a7d4bc94e 100644 --- a/compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/lazy/list/LazyListTest.kt +++ b/compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/lazy/list/LazyListTest.kt @@ -1507,6 +1507,28 @@ class LazyListTest(orientation: Orientation) : BaseLazyListTestWithOrientation(o rule.onNodeWithTag("0").assertDoesNotExist() } + @Test + fun maxIntElements_withKey_startInMiddle() { + val itemSize = with(rule.density) { 15.toDp() } + + rule.setContent { + LazyColumnOrRow( + modifier = Modifier.requiredSize(itemSize), + state = LazyListState(firstVisibleItemIndex = Int.MAX_VALUE / 2) + ) { + items(Int.MAX_VALUE, key = { it }) { + Box( + Modifier + .size(itemSize) + .testTag("$it")) + } + } + } + + rule.onNodeWithTag("${Int.MAX_VALUE / 2}") + .assertStartPositionInRootIsEqualTo(0.dp) + } + @Test fun scrollingByExactlyTheItemSize_switchesTheFirstVisibleItem() { val itemSize = with(rule.density) { 30.toDp() } diff --git a/compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/lazy/layout/IntervalList.kt b/compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/lazy/layout/IntervalList.kt index 19c8d19e586..f855bea50b5 100644 --- a/compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/lazy/layout/IntervalList.kt +++ b/compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/lazy/layout/IntervalList.kt @@ -147,7 +147,7 @@ class MutableIntervalList : IntervalList { } var intervalIndex = intervals.binarySearch(fromIndex) - var itemIndex = fromIndex + var itemIndex = intervals[intervalIndex].startIndex while (itemIndex <= toIndex) { val interval = intervals[intervalIndex] block(interval) diff --git a/compose/foundation/foundation/src/test/kotlin/androidx/compose/foundation/lazy/MutableIntervalListTest.kt b/compose/foundation/foundation/src/test/kotlin/androidx/compose/foundation/lazy/MutableIntervalListTest.kt index 9ec910c8a65..78b57260a32 100644 --- a/compose/foundation/foundation/src/test/kotlin/androidx/compose/foundation/lazy/MutableIntervalListTest.kt +++ b/compose/foundation/foundation/src/test/kotlin/androidx/compose/foundation/lazy/MutableIntervalListTest.kt @@ -210,6 +210,20 @@ class MutableIntervalListTest { assertThat(intervals.map { it.startIndex }).isEqualTo(listOf(2)) } + @Test + fun forEach_fromIndexIsInTheMiddleOfInterval() { + intervalList.addInterval(10, 0) // startIndex = 0 + intervalList.addInterval(1, 1) // startIndex = 10 + intervalList.addInterval(1, 2) // startIndex = 11 + val intervals = mutableListOf>() + + intervalList.forEach(fromIndex = 5, toIndex = 10) { + intervals.add(it) + } + + assertThat(intervals.map { it.startIndex }).isEqualTo(listOf(0, 10)) + } + @Test fun forEach_startLargerThanEndThrows() { addFiveSingleIntervals() @@ -261,6 +275,18 @@ class MutableIntervalListTest { assertThat(wasException4).isTrue() } + @Test + fun forEach_MAX_VALUE_size_andFromIndexIsInTheMiddle() { + intervalList.addInterval(Int.MAX_VALUE, 0) + val intervals = mutableListOf>() + + intervalList.forEach(fromIndex = Int.MAX_VALUE / 2, toIndex = Int.MAX_VALUE - 1) { + intervals.add(it) + } + + assertThat(intervals.map { it.startIndex }).isEqualTo(listOf(0)) + } + private fun addFiveSingleIntervals() { intervalList.addInterval(1, 10) intervalList.addInterval(1, 20) -- cgit v1.2.3