[Rev 01] RFR: 8236259: MemoryLeak in ProgressIndicator

[Jeanette Winzenburg]

On Thu, 19 Dec 2019 13:35:27 GMT, Florian Kirmaier <fkirmaier at openjdk.org> wrote:

>> modules/javafx.controls/src/main/java/javafx/scene/control/skin/ProgressIndicatorSkin.java line 521:
>> 
>>> 520:             registerChangeListener(text.fontProperty(), (Consumer<ObservableValue<?>>) fontListener);
>>> 521: 
>>> 522:             // The circular background for the progress pie piece
>> 
>> wondering why you add a (strong) field reference here? 
>> 
>> Thought that registerChangeListener(observableValue, consumer) takes care of handling storage/cleanup - provided its counter-method unregisterChangeListener(observableValue) is called, which is missing, so seems to be the only part that has to be added. Or not?
> 
> Yes, that doesn't make a lot of sense. It's probably an artifact because I expected a different API do unregister the listener. I'm gonna change it back!

just thinking aloud: actually, it's rather whacky that the indicator (mis-)uses the skin's multiplePropertyListener - it's api isn't meant for frequent register/unregisters (the unregisters is quite a heavy measure). Maybe it should have its own? Or the other way round: skin listening to the property and indicator having a method to update based on font which is invoked by skin's listener?

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

PR: https://git.openjdk.java.net/jfx/pull/71