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

[John Hendrikx]

On Tue, 9 Feb 2021 20:43:08 GMT, John Hendrikx <jhendrikx at openjdk.org> wrote:

>> 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.

I could also split the register/unregister part into separate methods for Scene and Window (as you can see, I have a great aversion against duplicating code, mostly because it would then need to be tested multiple times as well).

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

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