Uploaded image for project: 'JDK'
  1. JDK
  2. JDK-8276056

Control.skin.setSkin(Skin) fails to call dispose() on discarded Skin

    XMLWordPrintable

    Details

    • Subcomponent:
    • CPU:
      generic
    • OS:
      generic

      Description

      ADDITIONAL SYSTEM INFORMATION :
      JavaFX 17.0.0.1 and 18ea+4

      A DESCRIPTION OF THE PROBLEM :
      Control.skin.setSkin(Skin) discards a new Skin of the same class as the existing Skin, but fails to call Skin.dispose(). That fails to perform the cleanup the discarded Skin's dispose() method should have done, such as removing Behavior InputMappings that the discarded Skin's constructor added.

      STEPS TO FOLLOW TO REPRODUCE THE PROBLEM :
      Run the attached application. Open the ComboBox popup. Use the down arrow key to navigate past the bottom of the items in the viewport. Observe that the ListView fails to scroll to keep the selected item in the viewport. You can set a breakpoint in ListViewSkin.onSelectNextCell() to see that the skinnable's Skin is a different instance than the instance being called, because the Control's skin property discarded the instance but failed to dispose() it.

      EXPECTED VERSUS ACTUAL BEHAVIOR :
      EXPECTED -
      The ListView should scroll to follow the selected item during keyboard arrow navigation. The discarded ListViewSkin instance's ListViewBehavior should not have InputMap KeyMappings installed on the ListView.
      ACTUAL -
      The ListView fails to scroll to follow the selected item during keyboard arrow navigation. The discarded ListViewSkin instance's ListViewBehavior's InputMap KeyMappings are installed on the ListView and consuming the events.

      ---------- BEGIN SOURCE ----------
      import javafx.application.Application;
      import javafx.collections.FXCollections;
      import javafx.scene.Scene;
      import javafx.scene.control.ComboBox;
      import javafx.scene.control.Skin;
      import javafx.scene.layout.StackPane;
      import javafx.stage.Stage;

      import java.util.stream.Collectors;
      import java.util.stream.IntStream;

      public class ControlDiscardsSkinOfSameClassWithoutDisposing extends Application {

         public static void main( final String[] args ) {
            launch( args );
         }

         @Override
         public void start( final Stage primaryStage ) throws Exception {
            final ComboBox<Integer> comboBox = new ComboBox<>(
                  FXCollections.observableList( IntStream.rangeClosed( 0, 20 )
                                                         .boxed().collect( Collectors.toList() ) )
            );
            comboBox.setEditable( true );
            final StackPane root = new StackPane( comboBox );
            root.setStyle( "-fx-min-width: 30em; -fx-min-height: 30em;" );
            final Scene scene = new Scene( root );
            scene.getStylesheets().add( getClass()
                  .getResource( "ControlDiscardsSkinOfSameClassWithoutDisposing.css" )
                  .toExternalForm() );
            primaryStage.setScene( scene );
            primaryStage.sizeToScene();
            primaryStage.centerOnScreen();
            primaryStage.show();
            // Now, the ComboBox popup ListView has an initial ListViewSkin installed on it,
            // but Control.skin.setSkin(Skin) discarded a second ListViewSkin and failed to call dispose() on it.
            // If you open the ComboBox popup and use the down arrow key to navigate past the bottom item in the viewport,
            // then you will see that the ListView fails to scroll to follow the selected item.
            // That is because the second (discarded) ListViewSkin's ListViewBehavior's InputMap KeyMappings are still
            // observing the ListView and consuming events to make calls on the second (discarded) ListViewSkin's
            // VirtualFlow, preventing the first (kept) ListViewSkin's ListViewBehavior's InputMap KeyMappings from
            // receiving the events and making calls on the VirtualFlow that is actually displayed.
         }
      }

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

      ControlDiscardsSkinOfSameClassWithoutDisposing.css:

      .list-view {
         -fx-skin: "javafx.scene.control.skin.ListViewSkin";
      }

      .combo-box {
         -fx-skin: "javafx.scene.control.skin.ComboBoxListViewSkin";
      }

      ---------- END SOURCE ----------

      CUSTOMER SUBMITTED WORKAROUND :
      I could not find a place to hook to implement a general purpose workaround.
      In the case of ListView, I implemented a custom ListViewSkin that overrides createVirtualFlow() to return an instance of a derived class of VirtualFlow. That derived class overrides VirtualFlow.scrollTo(int) to check if the skinnable's Skin is not equal to the current Skin. And, just to be sure, it checks if "ListViewBehavior" is in one of the class names in the current Thread's stack trace. If so, it assumes that Control.skin.setSkin(Skin) discarded this Skin instance, and then calls dispose() on itself. That disposes the ListViewBehavior and prevents it from consuming future events. Of course, the single event that triggered that call to scrollTo(int) gets consumed, preventing the active VirtualFlow from getting the desired call.

      For the actual fix, I suggest that Control.skin.setSkin(Skin) not discard a new Skin merely because it is of the same class as the existing Skin.
      If there is a good reason to continue discarding same-class new Skins, then call Skin.dispose() on the discarded Skin, and document the behavior in the javadoc of Control.skinProperty().

      FREQUENCY : always


        Attachments

          Issue Links

            Activity

              People

              Assignee:
              aghaisas Ajit Ghaisas
              Reporter:
              webbuggrp Webbug Group
              Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

                Dates

                Created:
                Updated: