RFR: 8252935: Add treeShowing listener only when needed [v4]

[John Hendrikx]

On Tue, 9 Feb 2021 10:31:57 GMT, Jeanette Winzenburg <fastegal at openjdk.org> wrote:

>> My concern is about having a similar way of doing something. It would keep the code uniform. We have been following the earlier pattern under a cleanup task [JDK-8241364](https://bugs.openjdk.java.net/browse/JDK-8241364). Several bugs under this task are being fixed in earlier way.
>> May be we can discuss the new way of handling properties under a separate issue and plan to modify all such instances at once. Does that sound ok ?
>
> hmm ... might appear convenient (in very controlled contexts) but looks like a precondition violation: the sender of the change must not be null (concededly not explicitly spec'ed but logically implied, IMO)
> 
> so would tend to _not_ see this as blueprint for a general pattern fx code base

So, what you are suggesting is to replace the above single line to this:

        Scene scene = node.getScene();
        
        if (scene != null) {
            Window window = scene.getWindow();
            
            scene.addListener(nodeSceneChangedListener);
            
            if (window != null) {
                window.showingProperty().addListener(windowShowingChangedListener);
            }
        }

        updateTreeShowing();

And something similarly wordy in dispose again -- and donot forget that I would then need to also duplicate the testing code to make sure all these branches are covered in both the constructor and in dispose again.

Are you sure?   I could extract the methods instead if you donot like me passing in `null` for the `ObservableValue`, it would then look like:

        sceneChanged(null, node.getScene());  // register chain

        sceneChanged(node.getScene(), null);  // unregister chain

No further changes in testing required as it is all covered and proven to work correctly.

Also I should mention that this is not Skin code in case it was missed.

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

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