JDK-8265063 : SkinBase: add api to un-/register invalidation-/listChange listeners
  • Type: CSR
  • Component: javafx
  • Sub-Component: controls
  • Priority: P4
  • Status: Closed
  • Resolution: Approved
  • Fix Versions: openjfx17
  • Submitted: 2021-04-12
  • Updated: 2021-04-17
  • Resolved: 2021-04-17
Related Reports
CSR :  
Description
Summary
-------

Add support for safe un-/registration of listeners to invalidation and list change events.

Problem
-------

Concrete skin implementations listen to observable state changes of their associated control. Those listeners must be removed when no longer needed to prevent memory leaks and side-effects, particularly after disposal of the skin. SkinBase supports safe un/registration and automatic removal for change events, it is missing analogous support for invalidation and list change events.

Solution
--------

Add methods to un/register listeners for invalidation and list change events, similar to the methods for change events.

Specification
-------------

Add the following methods to `javafx.scene.control.SkinBase`

    /**
     * Registers an operation to perform when the given {@code observable} sends an invalidation event.
     * Does nothing if either {@code observable} or {@code operation} are {@code null}.
     * If multiple operations are registered on the same observable, they will be performed in the
     * order in which they were registered.
     *
     * @param observable the observable to observe for invalidation events, may be {@code null}
     * @param operation the operation to perform when the observable sends an invalidation event,
     *  may be {@code null}
     * @since 17
     */
    protected final void registerInvalidationListener(Observable observable, Consumer<Observable> operation) {

    /**
     * Unregisters all operations that have been registered using
     * {@link #registerInvalidationListener(Observable, Consumer)}
     * for the given {@code observable}. Does nothing if {@code observable} is {@code null}.
     *
     * @param observable the observable for which the registered operations should be removed,
     *  may be {@code null}
     * @return a composed consumer representing all previously registered operations, or
     *  {@code null} if none have been registered or the observable is {@code null}
     * @since 17
     */
    protected final Consumer<Observable> unregisterInvalidationListeners(Observable observable) {

    /**
     * Registers an operation to perform when the given {@code observableList} sends a list change event.
     * Does nothing if either {@code observableList} or {@code operation} are {@code null}.
     * If multiple operations are registered on the same observable list, they will be performed in the
     * order in which they were registered.
     *
     * @param observableList the observableList to observe for list change events, may be {@code null}
     * @param operation the operation to perform when the observableList sends a list change event,
     *  may be {@code null}
     * @since 17
     */
    protected final void registerListChangeListener(ObservableList<?> observableList, Consumer<Change<?>> operation) {

    /**
     * Unregisters all operations that have been registered using
     * {@link #registerListChangeListener(ObservableList, Consumer)}
     * for the given {@code observableList}. Does nothing if {@code observableList} is {@code null}.
     *
     * @param observableList the observableList for which the registered operations should be removed,
     *  may be {@code null}
     * @return a composed consumer representing all previously registered operations, or
     *  {@code null} if none have been registered or the observableList is {@code null}
     * @since 17
     */
    protected final Consumer<Change<?>> unregisterListChangeListeners(ObservableList<?> observableList) {

Changed javadoc of existing methods to be consistent with that of added methods:

         /**
    -     * Subclasses can invoke this method to register that they want to listen to
    -     * property change events for the given property. Registered {@link Consumer} instances
    -     * will be executed in the order in which they are registered.
    -     * @param property the property
    -     * @param consumer the consumer
    +     * Registers an operation to perform when the given {@code observable} sends a change event.
    +     * Does nothing if either {@code observable} or {@code operation} are {@code null}.
    +     * If multiple operations are registered on the same observable, they will be performed in the
    +     * order in which they were registered.
    +     *
    +     * @param observable the observable to observe for change events, may be {@code null}
    +     * @param operation the operation to perform when the observable sends a change event,
    +     *  may be {@code null}
          */
    -    protected final void registerChangeListener(ObservableValue<?> property, Consumer<ObservableValue<?>> consumer) {
    +    protected final void registerChangeListener(ObservableValue<?> observable, Consumer<ObservableValue<?>> operation) {

         /**
    -     * Unregisters all change listeners that have been registered using {@link #registerChangeListener(ObservableValue, Consumer)}
    -     * for the given property. The end result is that the given property is no longer observed by any of the change
    -     * listeners, but it may still have additional listeners registered on it through means outside of
    -     * {@link #registerChangeListener(ObservableValue, Consumer)}.
    +     * Unregisters all operations that have been registered using
    +     * {@link #registerChangeListener(ObservableValue, Consumer)}
    +     * for the given {@code observable}. Does nothing if {@code observable} is {@code null}.
          *
    -     * @param property The property for which all listeners should be removed.
    -     * @return A single chained {@link Consumer} consisting of all {@link Consumer consumers} registered through
    -     *      {@link #registerChangeListener(ObservableValue, Consumer)}. If no consumers have been registered on this
    -     *      property, null will be returned.
    +     * @param observable the observable for which the registered operations should be removed,
    +     *  may be {@code null}
    +     * @return a composed consumer representing all previously registered operations, or
    +     *  {@code null} if none have been registered or the observable is {@code null}
          * @since 9
          */
    -    protected final Consumer<ObservableValue<?>> unregisterChangeListeners(ObservableValue<?> property) {
    +    protected final Consumer<ObservableValue<?>> unregisterChangeListeners(ObservableValue<?> observable) {


Comments
Moving to Approved.
17-04-2021

Looks good.
16-04-2021

Moving to Provisional.
14-04-2021

This looks ready to move to the Proposed state. I'll Review the CSR after a more careful look at the javadoc-generated API docs, in connection with the code review.
12-04-2021