RFR: 8319779: SystemMenu: memory leak due to listener never being removed [v2]
Johan Vos
jvos at openjdk.org
Mon Nov 20 08:00:59 UTC 2023
On Fri, 10 Nov 2023 12:37:13 GMT, John Hendrikx <jhendrikx at openjdk.org> wrote:
>> Johan Vos has updated the pull request incrementally with one additional commit since the last revision:
>>
>> process reviewers comments
>
> modules/javafx.graphics/src/main/java/com/sun/javafx/tk/quantum/GlassSystemMenu.java line 63:
>
>> 61: private MenuBar glassSystemMenuBar = null;
>> 62: private final Map <Menu, ListChangeListener> menuListeners = new HashMap<>();
>> 63: private final Map <ListChangeListener, ObservableList> listenerItems = new HashMap<>();
>
> minor:
> Suggestion:
>
> private final Map<Menu, ListChangeListener> menuListeners = new HashMap<>();
> private final Map<ListChangeListener, ObservableList> listenerItems = new HashMap<>();
done
> modules/javafx.graphics/src/main/java/com/sun/javafx/tk/quantum/GlassSystemMenu.java line 189:
>
>> 187:
>> 188: private ListChangeListener createListener(final Menu glassMenu) {
>> 189: ListChangeListener<MenuItemBase> answer = ((ListChangeListener.Change<? extends MenuItemBase> change) -> {
>
> minor: I see extra parenthesis around the whole, and I think you can just `return` this immediately (no need for `answer` local)
I typically add a local variable for this (which is removed by javac) to make it easier to add debugging, but I'll remove it.
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1283#discussion_r1398775195
PR Review Comment: https://git.openjdk.org/jfx/pull/1283#discussion_r1398774978
More information about the openjfx-dev
mailing list