RFR: 8350917: Allow parent nodes to provide CSS styleable properties for child nodes

[Michael Strauß]

On Sun, 16 Mar 2025 21:29:31 GMT, John Hendrikx <jhendrikx at openjdk.org> wrote:

>> modules/javafx.graphics/src/main/java/javafx/css/Styleable.java line 101:
>> 
>>> 99:      * only for its direct children!
>>> 100:      *
>>> 101:      * @return an immutable list of CSS meta data, never {@code null}
>> 
>> Since we don't specify that `getCssMetaData()` should never return null, we should consider also not specifying it here. Alternatively, maybe we should add the non-null requirement to the existing `getCssMetaData()`.
>
> That might be a good idea, as there are several places in `CssStyleHelper` where `getCssMetaData` is assumed to be non-null already (and also a few locations where it is assumed it can be `null`).  In other words, if the function would return `null`, FX would break currently.
> 
> For example, `recalculateRelativeSizeProperties` will do:
> 
>         final List<CssMetaData<? extends Styleable,  ?>> styleables = node.getCssMetaData();
>         final int numStyleables = styleables.size();
>         
> Which would be an NPE.

I think then you can do this as part of this PR, since you’re already touching the `Styleable` interface.

>> modules/javafx.graphics/src/main/java/javafx/scene/CssStyleHelper.java line 465:
>> 
>>> 463:     }
>>> 464: 
>>> 465:     private record StylingContext(Node node, CalculatedValue font, StyleMap styleMap, Set<PseudoClass> pseudoClasses) {}
>> 
>> It might not be a good trade-off to create lots of transient objects on a hot path just to save a few arguments in the calling convention. This would be a nice improvement once we have value types in Java.
>
> I checked this before hand, there is quite a bit more going on in creating the cache keys (see `getTransitionStates`), and I think this extra object will therefore be lost in the noise.

I still think that it doesn’t carry its weight (bundling up private method arguments in the same translation unit is not a big win). It will increase churn on the GC, even if slightly, and many of these decisions taken together may cause it to run more often or earlier.

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

PR Review Comment: https://git.openjdk.org/jfx/pull/1714#discussion_r1998018887
PR Review Comment: https://git.openjdk.org/jfx/pull/1714#discussion_r1998016829