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

[Jose Pereda]

On Tue, 27 Feb 2024 09:30:14 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
>
> Johan Vos has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 11 additional commits since the last revision:
> 
>  - Merge branch 'master' into 8319779-systemmenu
>  - Add additional test for IOOBE detection.
>    This test comes from JDK-8323787
>  - Revert some of the conditional bindings.
>    Clear menu construction when an menuitem that is a menu needs to be removed
>    Add a test for the latter
>  - Merge remote-tracking branch 'upstream/master' into 8319779-systemmenu
>  - Cleanup test
>  - Add shim class so that we can access the references to com.sun.glass.ui.Menu instances.
>    Add a test to make sure those references are gone.
>  - Revert WeakInvalidationListeners and use new listener resource management approach.
>  - Fix more memoryleaks due to listeners never being unregistered.
>  - These changes are related to JBS-8318841 so we want to have that code in
>    as well.
>    
>    Merge branch 'master' into 8319779-systemmenu
>  - process reviewers comments
>  - ... and 1 more: https://git.openjdk.org/jfx/compare/a68c0a68...ec7308df

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

> 224:         mb.textProperty().when(active).addListener(valueModel -> glassMenu.setTitle(parseText(mb)));
> 225:         mb.disableProperty().when(active).addListener(valueModel -> glassMenu.setEnabled(!mb.isDisable()));
> 226:         mb.mnemonicParsingProperty().when(active).addListener(valueModel -> glassMenu.setTitle(parseText(mb)));

Since you are not calling `get()` from the `when` resulting observable (to allow for invalidation), these need to use a `ChangeListener` instead of an `InvalidationListener`, otherwise, while `active` doesn't change, any change in those menuBase properties will trigger just one notification. 

Alternative, just replace `addListener` with  `subscribe` (as it internally uses a `ChangeListener`).

@hjohn does this make sense to you?

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

PR Review Comment: https://git.openjdk.org/jfx/pull/1283#discussion_r1511687632