RFR: 8294809: ListenerHelper for managing and disconnecting listeners [v11]

Andy Goryachev angorya at openjdk.org
Mon Nov 21 23:53:37 UTC 2022


On Tue, 15 Nov 2022 18:30:33 GMT, Andy Goryachev <angorya at openjdk.org> wrote:

>> Introduction
>> 
>> There is a number of places where various listeners (strong as well as weak) are added, to be later disconnected in one go. For example, Skin implementations use dispose() method to clean up the listeners installed in the corresponding Control (sometimes using LambdaMultiplePropertyChangeListenerHandler class).
>> 
>> This pattern is also used by app developers, but there is no public utility class to do that, so every one must invent their own, see for instance
>> https://github.com/andy-goryachev/FxTextEditor/blob/master/src/goryachev/fx/FxDisconnector.java
>> 
>> Proposal
>> 
>> It might be beneficial to create a class (ListenerHelper, feel free to suggest a better name) which:
>> 
>> - provides convenient methods like addChangeListener, addInvalidationListener, addWeakChangeListener, etc.
>> - keeps track of the listeners and the corresponding ObservableValues
>> - provides a single disconnect() method to remove all the listeners in one go.
>> - optionally, it should be possible to add a lambda (Runnable) to a group of properties
>> - optionally, there should be a boolean flag to fire the lambda immediately
>> - strongly suggest implementing an IDisconnectable functional interface with a single disconnect() method
>> 
>> Make it Public Later
>> 
>> Initially, I think we could place the new class in package com.sun.javafx.scene.control; to be made publicly accessible later.
>> 
>> Where It Will Be Useful
>> 
>> [JDK-8294589](https://bugs.openjdk.org/browse/JDK-8294589) "MenuBarSkin: memory leak when changing skin"
>> and other skins, as a replacement for LambdaMultiplePropertyChangeListenerHandler.
>> 
>> https://github.com/openjdk/jfx/pull/908#:~:text=19%20hours%20ago-,8295175%3A%20SplitPaneSkinSkin%3A%20memory%20leak%20when%20changing%20skin%20%23911,-Draft
>> 
>> https://github.com/openjdk/jfx/pull/914
>
> Andy Goryachev has updated the pull request incrementally with one additional commit since the last revision:
> 
>   8294809: review comments

Kevin:

my initial intent was to label register*() methods as @Deprecated and switch completely to `ListenerHelper` (LH).  Since there is general reluctance to alter public APIs (quite understandable) the next bright idea is to keep the methods, but switch implementation to use LH, [JDK-8296076](https://bugs.openjdk.org/browse/JDK-8296076).

At the end, I'd like to have 0 callers of register*() inside javafx, keeping these methods for any external skins that might use it.  The main reason is orthogonality - LH has a richer set of methods, which register*() support just a few, and are themselves delegates.

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

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


More information about the openjfx-dev mailing list