8256397: MultipleSelectionModel throws IndexOutOfBoundException

Reviewed-by: aghaisas, mstrauss

modules/javafx.controls/src/main/java/javafx/scene/control/MultipleSelectionModelBase.java

@@ -683,8 +683,9 @@ void stopAtomic() {
             if (index < 0 || index >= itemCount)  {
                 throw new IndexOutOfBoundsException(index + " >= " + itemCount);
             }
-
-            if (index == (lastGetIndex + 1) && lastGetValue < itemCount) {
+            if (lastGetIndex == index) {
+                return lastGetValue;
+            } else if (index == (lastGetIndex + 1) && lastGetValue < itemCount) {
                 // we're iterating forward in order, short circuit for
                 // performance reasons (RT-39776)
                 lastGetIndex++;
@@ -717,6 +718,7 @@ public void set(int index) {
             _beginChange();
             size = -1;
             bitset.set(index);
+            if (index <= lastGetValue) reset();
             int indicesIndex = indexOf(index);
             _nextAdd(indicesIndex, indicesIndex + 1);
             _endChange();
@@ -739,12 +741,14 @@ public void set(int index, int end, boolean isSet) {
             size = -1;
             if (isSet) {
                 bitset.set(index, end, isSet);
+                if (index <= lastGetValue) reset();
                 int indicesIndex = indexOf(index);
                 int span = end - index;
                 _nextAdd(indicesIndex, indicesIndex + span);
             } else {
                 // TODO handle remove
                 bitset.set(index, end, isSet);
+                if (index <= lastGetValue) reset();
             }
             _endChange();
         }
@@ -779,30 +783,22 @@ public void set(int index, int... indices) {
                     _endChange();
                 } else {
                     _beginChange();
-                    int pos = 0;
-                    int start = 0;
-                    int end = 0;
-
-                    // starting from pos, we keep going until the value is
-                    // not the next value
-                    int startValue = sortedNewIndices.get(pos++);
-                    start = indexOf(startValue);
-                    end = start + 1;
-                    int endValue = startValue;
-                    while (pos < size) {
-                        int previousEndValue = endValue;
-                        endValue = sortedNewIndices.get(pos++);
-                        ++end;
-                        if (previousEndValue != (endValue - 1)) {
-                            _nextAdd(start, end);
-                            start = end;
-                            continue;
-                        }
 
-                        // special case for when we get to the point where the loop is about to end
-                        // and we have uncommitted changes to fire.
-                        if (pos == size) {
-                            _nextAdd(start, start + pos);
+                    int startIndex = indexOf(sortedNewIndices.get(0));
+                    int endIndex = startIndex + 1;
+
+                    for (int i = 1; i < sortedNewIndices.size(); ++i) {
+                        int currentValue = get(endIndex);
+                        int currentNewValue = sortedNewIndices.get(i);
+                        if (currentValue != currentNewValue) {
+                            _nextAdd(startIndex, endIndex);
+                            while (get(endIndex) != currentNewValue) ++endIndex;
+                            startIndex = endIndex++;
+                        } else {
+                            ++endIndex;
+                        }
+                        if (i == sortedNewIndices.size() - 1) {
+                            _nextAdd(startIndex, endIndex);
                         }
                     }
 
@@ -816,6 +812,7 @@ public void clear() {
             List<Integer> removed = bitset.stream().boxed().collect(Collectors.toList());
             size = 0;
             bitset.clear();
+            reset();
             _nextRemove(0, removed);
             _endChange();
         }
@@ -827,6 +824,7 @@ public void clear(int index) {
             _beginChange();
             size = -1;
             bitset.clear(index);
+            if (index <= lastGetValue) reset();
             _nextRemove(indicesIndex, index);
             _endChange();
         }

modules/javafx.controls/src/test/java/test/javafx/scene/control/MultipleSelectionModelImplTest.java

@@ -34,6 +34,7 @@
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertSame;
 import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
 
 import java.util.*;
 import java.util.concurrent.atomic.AtomicInteger;
@@ -1445,4 +1446,67 @@ public void test_jdk_8088896() {
 
         assertEquals(2, model.getSelectedIndices().size());
     }
+
+
+    @Test
+    public void testSelectedIndicesChangeEvents() throws InterruptedException {
+
+        testSelectedIndicesChangeEventsFactory(0, new int[]{2,3}, 1, new int[]{5,7},
+                new int[][]{new int[]{1}, new int[]{5,7}});
+
+        testSelectedIndicesChangeEventsFactory(1, new int[]{3,4}, 0, new int[]{5,7},
+                new int[][]{new int[]{0}, new int[]{5,7}});
+
+        testSelectedIndicesChangeEventsFactory(0, new int[]{1, 3,4, 5, 7}, 6, new int[]{8,9},
+                new int[][]{new int[]{6}, new int[]{8,9}});
+
+        testSelectedIndicesChangeEventsFactory(5, new int[]{6, 7}, 2, new int[]{1,0},
+                new int[][]{new int[]{0,1,2}});
+
+        testSelectedIndicesChangeEventsFactory(2, new int[]{3, 6,7}, 0, new int[]{1,4,5,8,9},
+                new int[][]{new int[]{0,1},new int[]{4,5},new int[]{8,9}});
+    }
+
+    public ListView createListViewWithMultipleSelection() {
+        ListView<String> listView = new ListView<>();
+        listView.getItems().addAll(
+                "item-0",
+                "item-1",
+                "item-2",
+                "item-3",
+                "item-4",
+                "item-5",
+                "item-6",
+                "item-7",
+                "item-8",
+                "item-9",
+                "item-10"
+        );
+        listView.getSelectionModel().setSelectionMode(SelectionMode.MULTIPLE);
+        return listView;
+    }
+
+    public void testSelectedIndicesChangeEventsFactory(int initialSelected, int[] initialSelectedI, int selected, int[] selectedI, int[][] expected) throws InterruptedException {
+        ListView<String> listView = createListViewWithMultipleSelection();
+        listView.getSelectionModel().selectIndices(initialSelected,initialSelectedI);
+        testSelectedIndicesChangeHelper(listView, selected, selectedI, expected);
+    }
+    public void testSelectedIndicesChangeHelper(ListView listView, int selected, int[] selectedI, int[][] expected) {
+        List<int[]> expectedEntries = new LinkedList<>(Arrays.asList(expected));
+        MultipleSelectionModel<String> selectionModel = listView.getSelectionModel();
+        selectionModel.getSelectedIndices().addListener((ListChangeListener<? super Integer>) c -> {
+            while (c.next()) {
+                try {
+                    assertEquals(Arrays.stream(expectedEntries.get(0)).boxed().collect(Collectors.toList()), c.getAddedSubList());
+                    expectedEntries.remove(0);
+                } catch (IndexOutOfBoundsException e) {
+                    fail(e.getMessage());
+                }
+            }
+        });
+
+        selectionModel.selectIndices(selected, selectedI);
+
+        assertTrue("A ListEvent was missing: " + Arrays.deepToString(expectedEntries.toArray()), expectedEntries.isEmpty());
+    }
 }