[Rev 04] RFR: 8236259: MemoryLeak in ProgressIndicator

[Jeanette Winzenburg]

On Wed, 11 Mar 2020 12:51:12 GMT, Kevin Rushforth <kcr at openjdk.org> wrote:

>> modules/javafx.controls/src/main/java/javafx/scene/control/skin/ProgressIndicatorSkin.java line 550:
>> 
>>> 549:         }
>>> 550:
>>> 551:         private void setFillOverride(Paint fillOverride) {
>> 
>> The issue can be alternatively fixed using below change at line#510
>>             text.fontProperty().addListener(new WeakChangeListener<>((observable, oldValue, newValue) -> {
>>                 doneTextWidth = Utils.computeTextWidth(text.getFont(), DONE, 0);
>>                 doneTextHeight = Utils.computeTextHeight(text.getFont(), DONE, 0, TextBoundsType.LOGICAL_VERTICAL_CENTER);
>>             }));
>> 
>> I am not sure which one is better.
>> As there are no objections, I am good with the any.
>
> Nope. This won't work, since the listener will be garbage collected and stop being called. It could be done by keeping
> the listener is an instance variable, but I recommend the original fix.

@arapte the (mine :) general rule to follow is to use the highest abstraction available - here that would be to favour
skin's api to un/register listeners to properties (which ideally is thoroughly tested) over ad-hoc manual code (which
might not work as expected <g>)

sry if I confused anybody with my earlier comment ...

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

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