WIP: Moved treeShowingProperty into its own class

Author: hjohn

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

@@ -26,6 +26,7 @@
 package javafx.scene.control.skin;
 
 import com.sun.javafx.scene.NodeHelper;
+import com.sun.javafx.scene.TreeShowingExpression;
 import com.sun.javafx.scene.control.skin.Utils;
 import java.util.ArrayList;
 import java.util.Collections;
@@ -104,6 +105,7 @@ public class ProgressIndicatorSkin extends SkinBase<ProgressIndicator> {
     private IndeterminateSpinner spinner;
     private DeterminateIndicator determinateIndicator;
     private ProgressIndicator control;
+    private TreeShowingExpression treeShowingExpression;
 
     Animation indeterminateTransition;
 
@@ -129,9 +131,11 @@ public ProgressIndicatorSkin(ProgressIndicator control) {
         // register listeners
         registerChangeListener(control.indeterminateProperty(), e -> initialize());
         registerChangeListener(control.progressProperty(), e -> updateProgress());
-        registerChangeListener(NodeHelper.treeShowingProperty(control), e -> updateAnimation());
         registerChangeListener(control.sceneProperty(), e->updateAnimation());
 
+        treeShowingExpression = new TreeShowingExpression(control);
+        treeShowingExpression.addListener((obs, old, current) -> updateAnimation());
+
         initialize();
         updateAnimation();
     }
@@ -232,6 +236,8 @@ Paint getProgressColor() {
     @Override public void dispose() {
         super.dispose();
 
+        treeShowingExpression.dispose();
+
         if (indeterminateTransition != null) {
             indeterminateTransition.stop();
             indeterminateTransition = null;

modules/javafx.graphics/src/main/java/com/sun/javafx/scene/NodeHelper.java

@@ -294,10 +294,6 @@ public static boolean isTreeShowing(Node node) {
         return nodeAccessor.isTreeShowing(node);
     }
 
-    public static BooleanExpression treeShowingProperty(Node node) {
-        return nodeAccessor.treeShowingProperty(node);
-    }
-
     public static List<Style> getMatchingStyles(CssMetaData cssMetaData, Styleable styleable) {
         return nodeAccessor.getMatchingStyles(cssMetaData, styleable);
     }
@@ -361,7 +357,6 @@ boolean doComputeIntersects(Node node, PickRay pickRay,
         boolean isTreeVisible(Node node);
         BooleanExpression treeVisibleProperty(Node node);
         boolean isTreeShowing(Node node);
-        BooleanExpression treeShowingProperty(Node node);
         List<Style> getMatchingStyles(CssMetaData cssMetaData, Styleable styleable);
         Map<StyleableProperty<?>,List<Style>> findStyles(Node node,
                 Map<StyleableProperty<?>,List<Style>> styleMap);

modules/javafx.graphics/src/main/java/com/sun/javafx/scene/TreeShowingExpression.java

@@ -0,0 +1,147 @@
+/*
+ * Copyright (c) 2013, 2020, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.  Oracle designates this
+ * particular file as subject to the "Classpath" exception as provided
+ * by Oracle in the LICENSE file that accompanied this code.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+package com.sun.javafx.scene;
+
+import com.sun.javafx.binding.ExpressionHelper;
+import javafx.beans.InvalidationListener;
+import javafx.beans.binding.BooleanExpression;
+import javafx.beans.value.ChangeListener;
+import javafx.scene.Node;
+import javafx.scene.Scene;
+import javafx.stage.Window;
+
+/**
+ * Used to observe changes in tree showing status for a {@link Node}.  For a Node's tree to be showing
+ * it must be visible, its ancestors must be visible, the node must be part of a {@link Scene} and
+ * the scene must have a {@link Window} which is currently showing.<p>
+ *
+ * This class provides the exact same functionality as {@link NodeHelper#isTreeShowing(Node)} in
+ * an observable form.
+ */
+public class TreeShowingExpression extends BooleanExpression {
+    private final ChangeListener<Boolean> windowShowingChangedListener = (obs, oldVal, newVal) -> updateTreeShowing();
+
+    private final ChangeListener<Window> sceneWindowChangedListener = (scene, oldWindow, newWindow) -> {
+        if (oldWindow != null) {
+            oldWindow.showingProperty().removeListener(windowShowingChangedListener);
+        }
+        if (newWindow != null) {
+            newWindow.showingProperty().addListener(windowShowingChangedListener);
+        }
+        updateTreeShowing();
+    };
+
+    private final ChangeListener<Scene> nodeSceneChangedListener = (node, oldScene, newScene) -> {
+        if (oldScene != null) {
+            oldScene.windowProperty().removeListener(sceneWindowChangedListener);
+        }
+        if (newScene != null) {
+            newScene.windowProperty().addListener(sceneWindowChangedListener);
+        }
+
+        sceneWindowChangedListener.changed(
+            null,
+            oldScene == null ? null : oldScene.getWindow(),
+            newScene == null ? null : newScene.getWindow()
+        );
+    };
+
+    private final Node node;
+
+    private ExpressionHelper<Boolean> helper;
+    private boolean valid;
+    private boolean treeShowing;
+
+    /**
+     * Constructs a new instance.
+     *
+     * @param node a {@link Node} for which the tree showing status should be observed, cannot be null
+     */
+    public TreeShowingExpression(Node node) {
+        this.node = node;
+        this.node.sceneProperty().addListener(nodeSceneChangedListener);
+
+        NodeHelper.treeVisibleProperty(node).addListener(windowShowingChangedListener);
+
+        nodeSceneChangedListener.changed(null, null, node.getScene());
+    }
+
+    /**
+     * Cleans up any listeners that this class may have registered on the {@link Node}
+     * that was supplied at construction.
+     */
+    public void dispose() {
+        node.sceneProperty().removeListener(nodeSceneChangedListener);
+
+        NodeHelper.treeVisibleProperty(node).removeListener(windowShowingChangedListener);
+    }
+
+    private void updateTreeShowing() {
+        boolean newValue = NodeHelper.isTreeShowing(node);
+
+        if (newValue != treeShowing) {
+            treeShowing = newValue;
+            invalidate();
+        }
+    }
+
+    @Override
+    public void addListener(InvalidationListener listener) {
+        helper = ExpressionHelper.addListener(helper, this, listener);
+    }
+
+    @Override
+    public void removeListener(InvalidationListener listener) {
+        helper = ExpressionHelper.removeListener(helper, listener);
+    }
+
+    @Override
+    public void addListener(ChangeListener<? super Boolean> listener) {
+        helper = ExpressionHelper.addListener(helper, this, listener);
+    }
+
+    @Override
+    public void removeListener(ChangeListener<? super Boolean> listener) {
+        helper = ExpressionHelper.removeListener(helper, listener);
+    }
+
+    protected void invalidate() {
+        if (valid) {
+            valid = false;
+            ExpressionHelper.fireValueChangedEvent(helper);
+        }
+    }
+
+    @Override
+    public boolean get() {
+        if (!valid) {
+            updateTreeShowing();
+            valid = true;
+        }
+
+        return treeShowing;
+    }
+}

modules/javafx.graphics/src/main/java/javafx/scene/Node.java

@@ -603,11 +603,6 @@ public boolean isTreeShowing(Node node) {
                 return node.isTreeShowing();
             }
 
-            @Override
-            public BooleanExpression treeShowingProperty(Node node) {
-                return node.treeShowingProperty();
-            }
-
             @Override
             public List<Style> getMatchingStyles(CssMetaData cssMetaData,
                     Styleable styleable) {
@@ -1006,20 +1001,6 @@ public String getName() {
 
     private final InvalidationListener parentTreeVisibleChangedListener = valueModel -> updateTreeVisible(true);
 
-    private final ChangeListener<Boolean> windowShowingChangedListener
-            = (win, oldVal, newVal) -> updateTreeShowing();
-
-    private final ChangeListener<Window> sceneWindowChangedListener = (scene, oldWindow, newWindow) -> {
-        // Replace the windowShowingListener and call updateTreeShowing()
-        if (oldWindow != null) {
-            oldWindow.showingProperty().removeListener(windowShowingChangedListener);
-        }
-        if (newWindow != null) {
-            newWindow.showingProperty().addListener(windowShowingChangedListener);
-        }
-        updateTreeShowing();
-    };
-
     private SubScene subScene = null;
 
     /**
@@ -1081,26 +1062,6 @@ private void invalidatedScenes(Scene oldScene, SubScene oldSubScene) {
         }
         scenesChanged(newScene, newSubScene, oldScene, oldSubScene);
 
-        // isTreeShowing needs to take into account of Window's showing
-        if (oldScene != null) {
-            oldScene.windowProperty().removeListener(sceneWindowChangedListener);
-
-            Window window = oldScene.windowProperty().get();
-            if (window != null) {
-                window.showingProperty().removeListener(windowShowingChangedListener);
-            }
-        }
-        if (newScene != null) {
-            newScene.windowProperty().addListener(sceneWindowChangedListener);
-
-            Window window = newScene.windowProperty().get();
-            if (window != null) {
-                window.showingProperty().addListener(windowShowingChangedListener);
-            }
-
-        }
-        updateTreeShowing();
-
         if (sceneChanged) reapplyCSS();
 
         if (sceneChanged && !isDirtyEmpty()) {
@@ -8423,69 +8384,8 @@ private boolean isWindowShowing() {
         return w != null && w.isShowing();
     }
 
-    private void updateTreeShowing() {
-        setTreeShowing(isTreeVisible() && isWindowShowing());
-    }
-
-    private boolean treeShowing;
-    private TreeShowingPropertyReadOnly treeShowingRO;
-
-    final void setTreeShowing(boolean value) {
-        if (treeShowing != value) {
-            treeShowing = value;
-            ((TreeShowingPropertyReadOnly) treeShowingProperty()).invalidate();
-        }
-    }
-
     final boolean isTreeShowing() {
-        return treeShowingProperty().get();
-    }
-
-    final BooleanExpression treeShowingProperty() {
-        if (treeShowingRO == null) {
-            treeShowingRO = new TreeShowingPropertyReadOnly();
-        }
-        return treeShowingRO;
-    }
-
-    class TreeShowingPropertyReadOnly extends BooleanExpression {
-
-        private ExpressionHelper<Boolean> helper;
-        private boolean valid;
-
-        @Override
-        public void addListener(InvalidationListener listener) {
-            helper = ExpressionHelper.addListener(helper, this, listener);
-        }
-
-        @Override
-        public void removeListener(InvalidationListener listener) {
-            helper = ExpressionHelper.removeListener(helper, listener);
-        }
-
-        @Override
-        public void addListener(ChangeListener<? super Boolean> listener) {
-            helper = ExpressionHelper.addListener(helper, this, listener);
-        }
-
-        @Override
-        public void removeListener(ChangeListener<? super Boolean> listener) {
-            helper = ExpressionHelper.removeListener(helper, listener);
-        }
-
-        protected void invalidate() {
-            if (valid) {
-                valid = false;
-                ExpressionHelper.fireValueChangedEvent(helper);
-            }
-        }
-
-        @Override
-        public boolean get() {
-            valid = true;
-            return Node.this.treeShowing;
-        }
-
+        return isTreeVisible() && isWindowShowing();
     }
 
     private void updateTreeVisible(boolean parentChanged) {
@@ -8504,8 +8404,6 @@ && isDirty(DirtyBits.NODE_VISIBLE)) {
             addToSceneDirtyList();
         }
         setTreeVisible(isTreeVisible);
-
-        updateTreeShowing();
     }
 
     private boolean treeVisible;

modules/javafx.graphics/src/main/java/javafx/scene/SubScene.java

@@ -306,10 +306,9 @@ protected void invalidated() {
                                 "is already set as root of another scene or subScene");
                     }
 
-                    // disabled, isTreeVisible and isTreeShowing properties are inherited
+                    // disabled and isTreeVisible properties are inherited
                     _value.setTreeVisible(isTreeVisible());
                     _value.setDisabled(isDisabled());
-                    _value.setTreeShowing(isTreeShowing());
 
                     if (oldRoot != null) {
                         StyleManager.getInstance().forget(SubScene.this);