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

[Michael Strauß]

On Tue, 28 Nov 2023 23:34:30 GMT, Andy Goryachev <angorya at openjdk.org> wrote:

>> Provide a public utility method for use by custom and core Nodes to simplify boilerplate around styleable properties as well as to save some memory.
>> 
>> 
>>     /**
>>      * Utility method which combines {@code CssMetaData} items in one immutable list.
>>      * <p>
>>      * The intended usage is to combine the parent and the child CSS meta data for
>>      * the purposes of {@code getClassCssMetaData()} method, see for example {@link Node#getClassCssMetaData()}.
>>      * <p>
>>      * Example:
>>      * <pre>
>>      * private static final List<CssMetaData<? extends Styleable, ?>> STYLEABLES = CssMetaData.combine(
>>      *      <Parent>.getClassCssMetaData(),
>>      *      STYLEABLE1,
>>      *      STYLEABLE2
>>      *  );
>>      * </pre>
>>      * This method returns an instance of {@link java.util.RandomAccess} interface.
>>      *
>>      * @param list the css metadata items, usually from the parent, must not be null
>>      * @param items the additional items
>>      * @return the immutable list containing all of the items
>>      *
>>      * @since 22
>>      */
>>     public static List<CssMetaData<? extends Styleable, ?>> combine(
>>         List<CssMetaData<? extends Styleable, ?>> list,
>>         CssMetaData<? extends Styleable, ?>... items)
>> 
>> 
>> Problem(s):
>> 
>> - the same less-than-optimal implementation is duplicated throughout the javafx code base which combines the parent and child styleable properties, using ArrayList wrapped in a  Collections.unmodifiableList()
>> - the capacity of underlying ArrayList might exceed the number of items, wasting memory
>> - a potential exists for the custom component developer to inadvertently create a sub-standard implementation (i.e. return a List which does not implement RandomAccess interface), or forget to include parent's styleables.
>> 
>> Non-Goals:
>> 
>> - not redesigning the lazy initialization of STYLEABLES list
>> - not changing the way styleables are enumerated in Nodes and Controls
>> - not adding any new methods to JDK (collections)
>> 
>> To Be Discussed:
>> 
>> - it is not clear whether the order of styleables returned by <N extends Node>.getClassCssMetaData() is of any importance
>> - the current spec for Node.getCssMetaData() specifies the return value as "The CssMetaData associated with this node, which may include the CssMetaData of its superclasses.", implying that it may or may not include.  It is unclear whether it must include the parent's styleables (well, except the Node whose superclass is Obje...
>
> Andy Goryachev has updated the pull request incrementally with one additional commit since the last revision:
> 
>   review comments

The downside of John's proposal is that the metadata list would now be duplicated in every node, while currently these lists are only duplicated in `Control` nodes. The size of these lists is typically around 20-40 elements.

In any case, expanding on what I wrote earlier, here is how the performance requirements could be specified for `getClassCssMetaData`, if we go for "option 1":

/**
 * Gets an unmodifiable {@code CssMetaData} list for the styleable properties of this class.
 * <p>
 * If a derived class declares new styleable properties, it must redeclare this method to hide the
 * base method. From the redeclared method, it must return the concatenation of the base metadata
 * and the metadata of the newly-added styleable properties.
 * <p>
 * Implementations of this method must satisfy the following performance requirements to prevent
 * severe performance degradation in the CSS system:
 * <ol>
 *     <li>The returned list must support random access.
 *     <li>The method must not allocate a new list on each invocation.
 * </ol>
 * It is strongly advised to use {@link CssMetaData#combine} to create the concatenated list and
 * store it in a static field:
 * <pre>{@code
 *     private static final List<CssMetaData<? extends Styleable, ?>> CSS_METADATA =
 *         CssMetaData.combine(
 *             <Parent>.getClassCssMetaData(),
 *             CUSTOM_CSS_METADATA_1,
 *             CUSTOM_CSS_METADATA_2,
 *             CUSTOM_CSS_METADATA_3
 *         );
 *
 *     public static List<CssMetaData<? extends Styleable, ?>> getClassCssMetaData() {
 *         return CSS_METADATA;
 *     }
 * }</pre>
 * Note that in this example, {@code <Parent>} should be substituted with the name of the direct base
 * class, even if the direct base class does not have its own {@code getClassCssMetaData()} method.
 * This ensures that the code remains correct when such a method is added to the direct base class.
 *
 * @return the {@code CssMetaData} list for the styleable properties of this class
 * @see CssMetaData#combine
 * @since JavaFX 8.0
 */
public static List<CssMetaData<? extends Styleable, ?>> getClassCssMetaData()

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

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