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