8246202: ChoiceBoxSkin: misbehavior on switching skin, part 2

Reviewed-by: kcr, aghaisas

Author: Jeanette Winzenburg

modules/javafx.controls/src/main/java/javafx/scene/control/skin/ChoiceBoxSkin.java

@@ -147,7 +147,7 @@ public ChoiceBoxSkin(ChoiceBox<T> control) {
         initialize();
 
         itemsObserver = observable -> updateChoiceBoxItems();
-        control.itemsProperty().addListener(new WeakInvalidationListener(itemsObserver));
+        control.itemsProperty().addListener(itemsObserver);
 
         control.requestLayout();
         registerChangeListener(control.selectionModelProperty(), e -> updateSelectionModel());
@@ -206,6 +206,9 @@ public ChoiceBoxSkin(ChoiceBox<T> control) {
 
     /** {@inheritDoc} */
     @Override public void dispose() {
+        if (getSkinnable() == null) return;
+        // removing itemsObserver fixes NP on setting items
+        getSkinnable().itemsProperty().removeListener(itemsObserver);
          // removing the content listener fixes NPE from listener
         if (choiceBoxItems != null) {
             choiceBoxItems.removeListener(weakChoiceBoxItemsListener);

modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/SkinCleanupTest.java

@@ -111,10 +111,9 @@ public void testListViewSetItems() {
 //-------- choiceBox, toolBar
 
     /**
-     * FIXME: Left-over from ChoiceBox fix.
      * NPE on sequence setItems -> modify items after skin is replaced.
      */
-    @Test @Ignore("8246202")
+    @Test
     public void testChoiceBoxSetItems() {
         ChoiceBox<String> box = new ChoiceBox<>();
         installDefaultSkin(box);