On Thu, 1 Dec 2022 19:47:52 GMT, Andy Goryachev <ango...@openjdk.org> wrote:
>>> but instead the whole component or Pane might be removed from the scene and
>>> discarded.
>>
>> If that happens, weak reference won't make a difference there either.
>> Removing an entire branch from the Scene and not referring to it anymore
>> will do correct clean-up even without a call to dispose and without the use
>> of weak references.
>>
>> The only use case I see is that we still don't trust the Skin lifecycle to
>> be adhered to and "just in case" are using weak references in case somehow
>> dispose is not called. I would much prefer to see an actual reason to use
>> it, and then adding a comment as to why this weak reference is needed so
>> that in 6 months time we're not scratching our heads as to why a weak
>> reference is needed here.
>
> just to be sure, which weak listener are you referring to?
I didn't mean a weak listener, but a weak reference. This line creates one:
sceneListenerHelper = new ListenerHelper(MenuBarSkin.this);
I think a plain `new ListenerHelper()` would work correct here. If you don't
think so, I think it may be good to add a comment why the weak reference is
needed to make the clean up work correctly.
-------------
PR: https://git.openjdk.org/jfx/pull/906
