RFR: 8319779: SystemMenu: memory leak due to listener never being removed

[John Hendrikx]

On Fri, 10 Nov 2023 10:34:08 GMT, Johan Vos <jvos at openjdk.org> wrote:

> A listener was added but never removed.
> This patch removes the listener when the menu it links to is cleared. Fix for https://bugs.openjdk.org/browse/JDK-8319779

Should there be an updated test for this?

I see a lot of raw type use (`ListChangeListener`, `ObservableList`), any chance those can be avoided?

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<>();

modules/javafx.graphics/src/main/java/com/sun/javafx/tk/quantum/GlassSystemMenu.java line 171:

> 169:             if (item instanceof MenuBase) {
> 170:                 // submenu
> 171:                 addMenu(glassMenu, (MenuBase)item);

Suggestion:

            if (item instanceof MenuBase baseItem) {
                // submenu
                addMenu(glassMenu, baseItem);

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)

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

PR Review: https://git.openjdk.org/jfx/pull/1283#pullrequestreview-1724683737
PR Review Comment: https://git.openjdk.org/jfx/pull/1283#discussion_r1389343964
PR Review Comment: https://git.openjdk.org/jfx/pull/1283#discussion_r1389338458
PR Review Comment: https://git.openjdk.org/jfx/pull/1283#discussion_r1389340165