On Tue, 9 Jan 2024 13:19:53 GMT, Johan Vos <j...@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 incrementally with two additional
> commits since the last revision:
>
> - 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.
modules/javafx.graphics/src/main/java/com/sun/javafx/tk/quantum/GlassSystemMenu.java
line 98:
> 96: active.set(false);
> 97: }
> 98: active = new SimpleBooleanProperty(true);
should this be
if(active == null) {
active = new ...
} else {
active.set(false);
}
instead?
modules/javafx.graphics/src/main/java/com/sun/javafx/tk/quantum/GlassSystemMenu.java
line 224:
> 222: mb.textProperty().when(active).addListener(valueModel ->
> glassMenu.setTitle(parseText(mb)));
> 223: mb.disableProperty().when(active).addListener(valueModel ->
> glassMenu.setEnabled(!mb.isDisable()));
> 224: mb.mnemonicParsingProperty().when(active).addListener(valueModel
> -> glassMenu.setTitle(parseText(mb)));
is `active` guaranteed to be non-null at this point (and also on L279)?
assuming the code on L98 is fixed and does not clobber the old pointer.
tests/system/src/test/java/test/com/sun/javafx/tk/quantum/SystemMenuBarTest.java
line 227:
> 225: if (!JMemoryBuddy.checkCollectable(wr)) {
> 226: strongCount++;
> 227: assertTrue("Too much refs", strongCount < 2);
"too many" ?
tests/system/src/test/java/test/com/sun/javafx/tk/quantum/SystemMenuBarTest.java
line 254:
> 252: return menuBar;
> 253: }
> 254:
extra newline?
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1283#discussion_r1446313415
PR Review Comment: https://git.openjdk.org/jfx/pull/1283#discussion_r1446323094
PR Review Comment: https://git.openjdk.org/jfx/pull/1283#discussion_r1446325052
PR Review Comment: https://git.openjdk.org/jfx/pull/1283#discussion_r1446327877
