RFR: 8089398: [ChoiceBox, ComboBox] throws NPE on setting value on null selectionModel [v5]

[Jeanette Winzenburg]

On Thu, 22 Jul 2021 19:49:44 GMT, Marius Hanl <mhanl at openjdk.org> wrote:

>> This PR fixes multiple NPEs in Choice-and ComboBox, when the selection model is null.
>> 
>> ChoiceBox: 
>> - Null check in **valueProperty()** listener
>> 
>> ComboBox:
>> - Null check in **editableProperty()** listener
>> - Null check in **valueProperty()** listener
>> - Null check in **ComboBoxListViewSkin#updateValue()**
>> - Null check in **ComboBoxListViewSkin#layoutChildren()**
>> - Null check in **ComboBoxListViewSkin#createListView()**
>> 
>> ~~The tests checks, that no NPE is printed to the console.
>> Some checks, that the set value is still displayed (either in the ComboBox button cell or the ChoiceBox display label)~~
>
> Marius Hanl has updated the pull request incrementally with two additional commits since the last revision:
> 
>  - removed unneeded assert and added comment
>  - removed unneeded asserts and added another NPE fix.

there were two NPEs in createLists - one is fixed, the other not yet :) See the failing test in my comment

copying the comment, don't know if you could find it otherwise (any way to un-resolve a review conversation?)

>````
>  // another test, exposing one of the NPEs in createList
>  ComboBox<String> comboBox = new ComboBox<>(items);
>  comboBox.setSelectionModel(null);
>  installDefaultSkin(comboBox);
> 

> the difference is that setup already installs a skin - so at the time of init the selectionModel still is available ;)

still failing 

> 
> The other access is in the listener to listView's selectedIndex .. it might be a bit tricky to expose in a test, I would go for a combo in a stage/loader, access the list and change its selectedIndex (maybe needs the popup open and/or firing a key onto it)

good fix and test :)

-------------

Changes requested by fastegal (Reviewer).

PR: https://git.openjdk.java.net/jfx/pull/557