RFR: 8290040: Provide simplified deterministic way to manage listeners [v10]

[Michael Strauß]

On Fri, 2 Dec 2022 09:56:42 GMT, John Hendrikx <jhendrikx at openjdk.org> wrote:

>> This PR adds a new (lazy*) property on `Node` which provides a boolean which indicates whether or not the `Node` is currently part of a `Scene`, which in turn is part of a currently showing `Window`.
>> 
>> It also adds a new fluent binding method on `ObservableValue` dubbed `when` (open for discussion, originally I had `conditionOn` here).
>> 
>> Both of these together means it becomes much easier to break strong references that prevent garbage collection between a long lived property and one that should be shorter lived. A good example is when a `Label` is bound to a long lived property:
>> 
>>      label.textProperty().bind(longLivedProperty.when(label::isShowingProperty));
>> 
>> The above basically ties the life cycle of the label to the long lived property **only** when the label is currently showing.  When it is not showing, the label can be eligible for GC as the listener on `longLivedProperty` is removed when the condition provided by `label::isShowingProperty` is `false`.  A big advantage is that these listeners stop observing the long lived property **immediately** when the label is no longer showing, in contrast to weak bindings which may keep observing the long lived property (and updating the label, and triggering its listeners in turn) until the next GC comes along.
>> 
>> The issue in JBS also describes making the `Subscription` API public, but I think that might best be a separate PR.
>> 
>> Note that this PR contains a bugfix in `ObjectBinding` for which there is another open PR: https://github.com/openjdk/jfx/pull/829 -- this is because the tests for the newly added method would fail otherwise; once it has been integrated to jfx19 and then to master, I can take the fix out.
>> 
>> (*) Lazy means here that the property won't be creating any listeners unless observed itself, to avoid problems creating too many listeners on Scene/Window.
>
> John Hendrikx has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Improve wording in javadoc and comments

I think that `ObservableValue.when` is good to go, but the case for `Node.shown` seems less clear.

Applications might want to bind the active scope of an `ObservableValue` to any number of conditions, for example:
* whether a node is connected to a `Scene`
* ... that is connected to a window
* ... that is currently showing
* whether a node is currently `visible`
* ... and it is also not hidden by other controls or outside the viewport
* all of the above combined

The proposed `shown` property picks one of these semantics and promotes it to a first-class API on `Node`. But it doesn't add any functionality that can't be provided by libraries or applications, since it's essentially just a convenience method for

sceneProperty()
    .flatMap(Scene::windowProperty)
    .flatMap(Window::showingProperty)


The bar for adding convenience methods to `Node` should be high, and I'm not sure that `shown` clears the bar. My main objection would be that it's quite easy to add this (and other useful convenience methods) in a way that's not clearly worse, for example using a collection of static methods:

public static class NodeUtil {
    public static ObservableValue<Boolean> shown(Node node) {
        ObservableValue<Boolean> shown = (ObservableValue<Boolean>)node.getProperties().get("shown");
        if (shown == null) {
            shown = node.sceneProperty()
                    .flatMap(Scene::windowProperty)
                    .flatMap(Window::showingProperty);
            node.getProperties().put("shown", shown);
        }
        return shown;
    }

    public static ObservableValue<Boolean> visiblyShown(Node node) {
        ...
    }

    public static ObservableValue<Boolean> visiblyShownOnScreen(Node node) {
        ...
    }   
}


When put into use, it's doesn't look terrible compared to a `Node.shown` property:

label.textProperty().bind(longLivedProperty.when(label::shownProperty));
label.textProperty().bind(longLivedProperty.when(NodeUtil.shown(label));


This suggests to me that we should discuss the `Node` additions separately from `ObservableValue.when`.

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

PR: https://git.openjdk.org/jfx/pull/830