RFR: 8320796: CssMetaData.combine() [v6]

[John Hendrikx]

On Wed, 29 Nov 2023 02:44:31 GMT, Nir Lisker <nlisker at openjdk.org> wrote:

> What each class brings with itself is a list of its own metadata properties. That's all it should declare. This list should automatically be added to the one of its parent, recursively, forming the final list. This part shouldn't be done by users as it's just another place to make a mistake in.

I definitely always found the implementation for gathering CSS properties rather ugly. The problem however is in part that you can't define a method that only provides the CSS properties of the current class and have it be part of `Node` or some interface for the CSS subsystem to call.  Such a method implementation would have to call `super` itself, as an external caller can't call super methods to gather the full list.

Lacking that it would have to be a static method with a specific signature that needs to be discovered via reflection, and then go up the hierarchy to find all properties.  A static method however won't work for Skin based CSS properties.

Still, there are IMHO better ways to do this; IMHO `Node` should just have a dedicated list of all CSS properties (exposed via `getCssMetaData`) stored in a `CopyOnWriteArrayList` to which the subclasses contribute additional properties at construction time.   The only writes to this list occur at construction time and when Skin changes happen, so this would be an almost ideal usecase for `CopyOnWriteArrayList`.  The `getCssMetaData` method could then have been `final`:

Node:

         private final List<CssMetaData> cssProps = new CopyOnWriteArrayList<>();
         private final List<CssMetaData> cssPropsUnmodifiable = Collections.unmodifiableList(cssProps);
         
         protected final int contributeProperties(List<CssMetaData> props) { 
               cssProps.addAll(props); 
               return cssProps.size(); 
         }

         protected final void replaceProperties(int fromIndex, List<CssMetaData> props) { 
               cssProps.sublist(fromIndex, cssProps.size()).clear(); 
               cssProps.addAll(props); 
         }

         public final List<CssMetaData> getCssMetaData() { return cssPropsUnmodifiable; }
         
Subclasses simply call `contributeProperties` and they're part of the "immutable" list automatically.  `Control` subclasses are a bit more advanced as they first call `contributeProperties` with their fixed properties, store the returned index, and then call `replaceProperties` each time their `Skin` changes.  The `Skin` properties are always at the end, so clearing and re-adding them is easy.

No messing around with concatenations, static methods, overrides that are not calling `super`, etc, and just as performant as there are no object allocations involved when calling `getCssMetaData`.

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

PR Comment: https://git.openjdk.org/jfx/pull/1296#issuecomment-1831367054