On Tue, 4 Oct 2022 14:06:59 GMT, Jeanette Winzenburg <faste...@openjdk.org>
wrote:
>>> Perhaps the test is too artificial, something is not being done correctly
>>> or exactly as in the real application? Using StageLoader or showControl()
>>> hooks up the missing dependencies.
>>
>> one last time: there is _no_ such thing as a "too artificial" test - a class
>> must _always_ fulfil its contract in whatever valid context. It's not enough
>> to do so for some (or even the majority) of use-cases. Plus: logically, any
>> assumption (like: there are no memory leaks) is invalidated by a single
>> counter-example (like the valid test).
>>
>> Have a nice weekend, I'm off now :)
>
>> Thanks again, @kleopatra With your permission, I'll add tests with and
>> without scene property set. Or do we want to keep the original set?
>
> SkinMemoryLeakTest already has both methods, that is testing the replacement
> of a skin with/out residing in a scene .. no need for adding anything ;)
> In fact, it's so easy that most built-in skins do it (and these were created
> by the developers of JavaFX itself, so obviously it's a major problem if even
> those people get it wrong so often).
>
well, just guessing games here but: mine is that they didn't put too many
thoughts into the possibility of replacing a skin and if they did, they
actively prevented it - as seen in the implementation of TextAreaSkin.dispose
when we started the current cleanup round:
/** {@inheritDoc} */
@Override public void dispose() {
super.dispose();
if (behavior != null) {
behavior.dispose();
}
// TODO Unregister listeners on text editor, paragraph list
throw new UnsupportedOperationException();
}
-------------
PR: https://git.openjdk.org/jfx/pull/906
