RFR: 8294589: MenuBarSkin: memory leak when changing skin [v15]

Andy Goryachev angorya at openjdk.org
Wed Nov 30 23:16:30 UTC 2022


On Wed, 30 Nov 2022 19:08:31 GMT, John Hendrikx <jhendrikx at openjdk.org> wrote:

>> Andy Goryachev has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   8294589: cleanup
>
> modules/javafx.controls/src/main/java/javafx/scene/control/skin/MenuBarSkin.java line 286:
> 
>> 284:         ParentHelper.setTraversalEngine(getSkinnable(), engine);
>> 285: 
>> 286:         lh.addChangeListener(control.sceneProperty(), true, (scene) -> {
> 
> minor: generally, the parenthesis are omitted for lambda's with one parameter (multiple occurences)

I like consistency:

`() ->` use parentheses
`a ->` don't use, why?
`(a,b) ->` use parentheses

> modules/javafx.controls/src/main/java/javafx/scene/control/skin/MenuBarSkin.java line 1180:
> 
>> 1178: 
>> 1179:     private static final CssMetaData<MenuBar,Pos> ALIGNMENT =
>> 1180:             new CssMetaData<>("-fx-alignment", new EnumConverter<Pos>(Pos.class), Pos.TOP_LEFT ) {
> 
> minor: You can use the diamond operator here, probably came from the merge conflict
> 
> Suggestion:
> 
>             new CssMetaData<>("-fx-alignment", new EnumConverter<>(Pos.class), Pos.TOP_LEFT) {

indeed, thanks!

> modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/SkinMemoryLeakTest.java line 227:
> 
>> 225:                 ComboBox.class,
>> 226:                 DatePicker.class,
>> 227:                 //MenuBar.class,
> 
> minor: commented out code

intentionally, to avoid merge conflicts.

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

PR: https://git.openjdk.org/jfx/pull/906


More information about the openjfx-dev mailing list