Comment for JDK-8147823 - ListView's selected items list contains null elements although no items are null

[Kober-Sotzek, Alice]

Hi,

could someone please add the following lines as comment to the issue JDK-8147823 (https://bugs.openjdk.java.net/browse/JDK-8147823)? Thank you very much.

This bug was very likely introduced by the fix for JDK-8093204 (https://bugs.openjdk.java.net/browse/JDK-8093204) which is contained in changeset http://hg.openjdk.java.net/openjfx/8u-dev/rt/rev/81af9caad4e2. The following test code, which tries to simulate the issue, should illustrate the difference. It contains the implementation of MultipleSelectionModelBase.createListFromBitSet(..).get(..) from 8u60 (currently active) and the one from 8u40 (commented out).

When the version of this method of 8u40 is used, the output is:
Set all bits
Selected indices list: [0, 1, 2]
Cleared 0.
Selected indices list: [1, 2]
Cleared 1.
Selected indices list: [2]
Selected indices list (second call without changes): [2]

However, when the version of this method of 8u60 is used, a -1 shows up as index (which results in the null elements the bug report mentions).
Set all bits
Selected indices list: [0, 1, 2]
Cleared 0.
Selected indices list: [1, 2]
Cleared 1.
Selected indices list: [-1]
Selected indices list (second call without changes): [2]

Test code:

import java.util.BitSet;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.function.IntSupplier;

import com.sun.javafx.scene.control.ReadOnlyUnbackedObservableList;

public class CreateListFromBitsetTester {

    public static void main(String[] args) {
        BitSet selectedIndices = new BitSet();
        AtomicInteger itemCounter = new AtomicInteger(0);
        ReadOnlyUnbackedObservableList<Integer> selectedIndicesSeq = createListFromBitSet(selectedIndices, itemCounter::get);

        selectedIndices.set(0);
        selectedIndices.set(1);
        selectedIndices.set(2);
        itemCounter.set(3);

        System.out.println("Set all bits");
        System.out.println("Selected indices list: " + selectedIndicesSeq);

        selectedIndices.clear(0);
        System.out.println("Cleared 0.");
        itemCounter.set(2);
        System.out.println("Selected indices list: " + selectedIndicesSeq);

        selectedIndices.clear(1);
        System.out.println("Cleared 1.");
        itemCounter.set(1);
        System.out.println("Selected indices list: " + selectedIndicesSeq);
        System.out.println("Selected indices list (second call without changes): " + selectedIndicesSeq);
    }

    // This code is normally contained in MultipleSelectionModelBase.
    private static ReadOnlyUnbackedObservableList<Integer> createListFromBitSet(final BitSet bitset, IntSupplier itemCounter) {
        return new ReadOnlyUnbackedObservableList<Integer>() {
            private int lastGetIndex = -1;
            private int lastGetValue = -1;

            // This code is from 8u60.
            @Override public Integer get(int index) {
                final int itemCount = itemCounter.getAsInt();
                if (index < 0 || index >= itemCount) return -1;

                if (index == (lastGetIndex + 1) && lastGetValue < itemCount) {
                    // we're iterating forward in order, short circuit for
                    // performance reasons (RT-39776)
                    lastGetIndex++;
                    lastGetValue = bitset.nextSetBit(lastGetValue + 1);
                    return lastGetValue;
                } else if (index == (lastGetIndex - 1) && lastGetValue > 0) {
                    // we're iterating backward in order, short circuit for
                    // performance reasons (RT-39776)
                    lastGetIndex--;
                    lastGetValue = bitset.previousSetBit(lastGetValue - 1);
                    return lastGetValue;
                } else {
                    for (lastGetIndex = 0, lastGetValue = bitset.nextSetBit(0);
                         lastGetValue >= 0 || lastGetIndex == index;
                         lastGetIndex++, lastGetValue = bitset.nextSetBit(lastGetValue + 1)) {
                        if (lastGetIndex == index) {
                            return lastGetValue;
                        }
                    }
                }

                return -1;
            }

            // This code is from 8u40.
//            @Override public Integer get(int index) {
//                if (index < 0 || index >= itemCounter.getAsInt()) return -1;
//
//                for (int pos = 0, val = bitset.nextSetBit(0);
//                     val >= 0 || pos == index;
//                     pos++, val = bitset.nextSetBit(val+1)) {
//                    if (pos == index) return val;
//                }
//
//                return -1;
//            }

            @Override public int size() {
                return bitset.cardinality();
            }

            @Override public boolean contains(Object o) {
                if (o instanceof Number) {
                    Number n = (Number) o;
                    int index = n.intValue();

                    return index >= 0 && index < bitset.length() &&
                           bitset.get(index);
                }

                return false;
            }
        };
    }

}

Thanks,
Alice