ProgressIndicator indeterminate transition bugs

[Cirujano Cuesta, Diego]

Hi all,

We found two important bugs in ProgressIndicator that are related with the following tickets:

https://bugs.openjdk.java.net/browse/JDK-8094829
https://bugs.openjdk.java.net/browse/JDK-8094078

Now are quite critical because in a 4K monitor may cause OutOfMemoryException.

Using the following example:
"
public class JFXMain extends Application{

    @Override
    public void start(Stage primaryStage) throws Exception {
        HBox root = new HBox();
        ToggleButton toggleButton = new ToggleButton();
        ProgressIndicator progressIndicator = new ProgressIndicator(ProgressIndicator.INDETERMINATE_PROGRESS);
        StackPane stackPane = new StackPane(progressIndicator);
        stackPane.visibleProperty().bind(toggleButton.selectedProperty());
        root.getChildren().addAll(toggleButton, stackPane);
        primaryStage.setScene(new Scene(root));
        primaryStage.show();
    }
}
"

** First bug **

Starting the Progress Indicator with indeterminate progress will trigger: rebuildTimeline by ProgressIndicatorSkin and in line 599 start the animation even is not shown already: indeterminateTransition.playFromStart();

** Second bug **

With the last commits in ProgressIndicator, as commented in JDK-8094829, the listeners do not care about the real visibility of the node(before it was used impl_treeVisibleProperty()). The consequence is that the ProgressIndicator in the example won´t be stopped.

I can imagine that impl_treeVisibleProperty() should not be used and Jonathan G. said: " but I can't reliably fix that without an API such as what I expect the treeVisible API should do." But we did not find such alternative property.

The solution we though is the usage of internal tree visible property like this:

1. Modify method:
" 
	protected void updateAnimation(boolean isTreeVisible) {
"
2. Remove current calls to method in 
"
	@Override protected void handleControlPropertyChanged(String p) {
        super.handleControlPropertyChanged(p);

        if ("INDETERMINATE".equals(p)) {
            initialize();
        } else if ("PROGRESS".equals(p)) {
            updateProgress();
        }
    	}
"
3. Add listener at the end of the IndeterminateSpinner contructor the visibility listener:
"
        private IndeterminateSpinner(boolean spinEnabled, Paint fillOverride) {
		[...]
            impl_treeVisibleProperty().addListener((obs, oldVal, newVal) ->{
                updateAnimation(newVal);
            });
	  }
"

What do you think?

Additional note: I would like to add one more thing. I think that could be very good a property ReadOnlyBooleanProperty treeVisibleProperty() available in all Nodes.

Please let me know if we can do something else.

Diego