Re: RFR: JDK-8297413: Remove easy warnings in javafx.graphics [v2]

2022-11-28 Thread John Hendrikx
> - Remove unsupported/unnecessary SuppressWarning annotations > - Remove reduntant type specifications (use diamond operator) > - Remove unused or duplicate imports > - Remove unnecessary casts (type is already correct type or can be autoboxed) > - Remove unnecessary semi-colons (at end of class d

Re: RFR: 8290040: Provide simplified deterministic way to manage listeners [v8]

2022-11-28 Thread John Hendrikx
On Wed, 26 Oct 2022 07:56:52 GMT, John Hendrikx wrote: >> This PR adds a new (lazy*) property on `Node` which provides a boolean which >> indicates whether or not the `Node` is currently part of a `Scene`, which in >> turn is part of a currently showing `Window`. >> >> It also adds a new fluen

Re: RFR: 8265828: [TestBug] Save and restore the default Locale in javafx.base unit test LocalDateTimeStringConverterTest

2022-11-28 Thread Kevin Rushforth
On Fri, 25 Nov 2022 15:04:38 GMT, Lukasz Kostyra wrote: > After implementing CR fixes it turned out that these tests started to fail at > random. Upon more investigation it turned out that the order in which JUnit > calls test methods matters a lot. Thank you for getting to the bottom of this.

Re: RFR: 8265828: [TestBug] Save and restore the default Locale in javafx.base unit test LocalDateTimeStringConverterTest [v3]

2022-11-28 Thread Lukasz Kostyra
> The change moves Locale setting in the test to `@BeforeClass` and > `@AfterClass` calls. `@BeforeClass` method call stores current default VM > locale and applies Locale.US, while `@AfterClass` method restores old VM > locale after all tests are completed. > > I tested it both on Mac and Wind

Re: RFR: 8265828: [TestBug] Save and restore the default Locale in javafx.base unit test LocalDateTimeStringConverterTest [v4]

2022-11-28 Thread Lukasz Kostyra
> The change moves Locale setting in the test to `@BeforeClass` and > `@AfterClass` calls. `@BeforeClass` method call stores current default VM > locale and applies Locale.US, while `@AfterClass` method restores old VM > locale after all tests are completed. > > I tested it both on Mac and Wind

Re: RFR: 8265828: [TestBug] Save and restore the default Locale in javafx.base unit test LocalDateTimeStringConverterTest [v4]

2022-11-28 Thread Kevin Rushforth
On Mon, 28 Nov 2022 14:56:49 GMT, Lukasz Kostyra wrote: >> The change moves Locale setting in the test to `@BeforeClass` and >> `@AfterClass` calls. `@BeforeClass` method call stores current default VM >> locale and applies Locale.US, while `@AfterClass` method restores old VM >> locale after

Re: RFR: 8265828: [TestBug] Save and restore the default Locale in javafx.base unit test LocalDateTimeStringConverterTest [v3]

2022-11-28 Thread Kevin Rushforth
On Mon, 28 Nov 2022 14:44:17 GMT, Lukasz Kostyra wrote: >> The change moves Locale setting in the test to `@BeforeClass` and >> `@AfterClass` calls. `@BeforeClass` method call stores current default VM >> locale and applies Locale.US, while `@AfterClass` method restores old VM >> locale after

Re: RFR: 8265828: [TestBug] Save and restore the default Locale in javafx.base unit test LocalDateTimeStringConverterTest [v3]

2022-11-28 Thread Andy Goryachev
On Mon, 28 Nov 2022 14:47:16 GMT, Kevin Rushforth wrote: >> Lukasz Kostyra has updated the pull request with a new target base due to a >> merge or a rebase. The pull request now contains four commits: >> >> - Merge branch 'master' of https://git.openjdk.org/jfx into >> JDK-8265828-locale >>

Re: RFR: 8265828: [TestBug] Save and restore the default Locale in javafx.base unit test LocalDateTimeStringConverterTest [v4]

2022-11-28 Thread Andy Goryachev
On Mon, 28 Nov 2022 14:56:49 GMT, Lukasz Kostyra wrote: >> The change moves Locale setting in the test to `@BeforeClass` and >> `@AfterClass` calls. `@BeforeClass` method call stores current default VM >> locale and applies Locale.US, while `@AfterClass` method restores old VM >> locale after

Integrated: 8294809: ListenerHelper for managing and disconnecting listeners

2022-11-28 Thread Andy Goryachev
On Fri, 7 Oct 2022 20:50:55 GMT, Andy Goryachev wrote: > Introduction > > There is a number of places where various listeners (strong as well as weak) > are added, to be later disconnected in one go. For example, Skin > implementations use dispose() method to clean up the listeners installed i

Re: RFR: JDK-8297412: Remove easy warnings in javafx.fxml, javafx.media, javafx.swing, javafx.swt and javafx.web [v2]

2022-11-28 Thread John Hendrikx
On Tue, 22 Nov 2022 19:10:27 GMT, Kevin Rushforth wrote: >> modules/javafx.fxml/src/main/java/javafx/fxml/FXMLLoader.java line 1446: >> >>> 1444: } >>> 1445: } >>> 1446: >> >> this suggests a specific pattern, even though there is probably no >> development expected in thi

Re: RFR: JDK-8297412: Remove easy warnings in javafx.fxml, javafx.media, javafx.swing, javafx.swt and javafx.web [v5]

2022-11-28 Thread John Hendrikx
> - Remove unsupported/unnecessary SuppressWarning annotations > - Remove reduntant type specifications (use diamond operator) > - Remove unused or duplicate imports > - Remove unnecessary casts (type is already correct type or can be autoboxed) > - Remove unnecessary semi-colons (at end of class d

RFR: 8295754: PaginationSkin: memory leak when changing skin

2022-11-28 Thread Andy Goryachev
Fixes memory leaks as determined by SkinMemoryLeakTest (remove line 171) and a leak tester https://github.com/andy-goryachev-oracle/Test/blob/main/src/goryachev/apps/LeakTest.java Make sure to configure the current test in LeakTest: protected final Type WE_ARE_TESTING = Type.PAGINATION; Found an

Re: RFR: 8295754: PaginationSkin: memory leak when changing skin

2022-11-28 Thread Andy Goryachev
On Thu, 20 Oct 2022 20:48:07 GMT, Andy Goryachev wrote: > Fixes memory leaks as determined by SkinMemoryLeakTest (remove line 171) and > a leak tester > https://github.com/andy-goryachev-oracle/Test/blob/main/src/goryachev/apps/LeakTest.java > > Make sure to configure the current test in LeakTe

Re: RFR: 8295754: PaginationSkin: memory leak when changing skin

2022-11-28 Thread Kevin Rushforth
On Thu, 20 Oct 2022 20:48:07 GMT, Andy Goryachev wrote: > Fixes memory leaks as determined by SkinMemoryLeakTest (remove line 171) and > a leak tester > https://github.com/andy-goryachev-oracle/Test/blob/main/src/goryachev/apps/LeakTest.java > > Make sure to configure the current test in LeakTe

Re: RFR: 8295754: PaginationSkin: memory leak when changing skin

2022-11-28 Thread Andy Goryachev
On Wed, 23 Nov 2022 00:13:07 GMT, Kevin Rushforth wrote: >> Fixes memory leaks as determined by SkinMemoryLeakTest (remove line 171) and >> a leak tester >> https://github.com/andy-goryachev-oracle/Test/blob/main/src/goryachev/apps/LeakTest.java >> >> Make sure to configure the current test in

Re: RFR: 8295754: PaginationSkin: memory leak when changing skin

2022-11-28 Thread Kevin Rushforth
On Fri, 28 Oct 2022 20:08:33 GMT, Andy Goryachev wrote: >> Fixes memory leaks as determined by SkinMemoryLeakTest (remove line 171) and >> a leak tester >> https://github.com/andy-goryachev-oracle/Test/blob/main/src/goryachev/apps/LeakTest.java >> >> Make sure to configure the current test in L

Re: RFR: 8295754: PaginationSkin: memory leak when changing skin

2022-11-28 Thread Andy Goryachev
On Thu, 3 Nov 2022 18:46:36 GMT, Kevin Rushforth wrote: >> modules/javafx.controls/src/main/java/javafx/scene/control/skin/PaginationSkin.java >> line 917: >> >>> 915: }); >>> 916: >>> 917: >>> ListenerHelper.get(PaginationSkin.this).addChangeListener(getSkinnable().cu

RFR: 8295796: ScrollPaneSkin: memory leak when changing skin

2022-11-28 Thread Andy Goryachev
as determined by SkinMemoryLeakTest (remove line 174) and a leak tester https://github.com/andy-goryachev-oracle/Test/blob/main/src/goryachev/apps/LeakTest.java caused by: - adding and not removing listeners - adding and not removing event handlers/filters NOTE: this fix requires JDK-8295242 scr

RFR: 8295500: AccordionSkin: memory leak when changing skin

2022-11-28 Thread Andy Goryachev
as determined by SkinMemoryLeakTest (remove line 164) and a leak tester https://github.com/andy-goryachev-oracle/Test/blob/main/src/goryachev/apps/LeakTest.java Make sure to configure the current test in LeakTest: protected final Type WE_ARE_TESTING = Type.ACCORDION; caused by: - adding and not

Re: RFR: 8294589: MenuBarSkin: memory leak when changing skin [v11]

2022-11-28 Thread Andy Goryachev
> Fixed memory leak by using weak listeners and making sure to undo everything > that has been done to MenuBar/Skin in dispose(). > > This PR depends on a new internal class ListenerHelper, a replacement for > LambdaMultiplePropertyChangeListenerHandler. > See https://github.com/openjdk/jfx/pull

RFR: 8295175: SplitPaneSkin: memory leak when changing skin

2022-11-28 Thread Andy Goryachev
Fixed memory leak by removing all the listeners in dispose(); This PR depends on a new internal class ListenerHelper, a replacement for LambdaMultiplePropertyChangeListenerHandler. See https://github.com/openjdk/jfx/pull/908 - Commit messages: - Merge remote-tracking branch 'origin

RFR: 8295531: ComboBoxBaseSkin: memory leak when changing skin

2022-11-28 Thread Andy Goryachev
as determined by SkinMemoryLeakTest (remove line 166) and a leak tester https://github.com/andy-goryachev-oracle/Test/blob/main/src/goryachev/apps/LeakTest.java Affected skins: - ColorPicker - DatePicker - ComboBox caused by: - out-of-order modification of the control property (skin.install) - ad

RFR: 8295426: MenuButtonSkin: memory leak when changing skin

2022-11-28 Thread Andy Goryachev
as determined by SkinMemoryLeakTest (remove line 170) and a leak tester https://github.com/andy-goryachev-oracle/Test/blob/main/src/goryachev/apps/LeakTest.java Also applies to SplitMenuButton, since they share the same base class MenuButtonSkinBase. Make sure to configure the current test in Le

Re: RFR: 8295796: ScrollPaneSkin: memory leak when changing skin

2022-11-28 Thread Andy Goryachev
On Fri, 21 Oct 2022 19:01:54 GMT, Andy Goryachev wrote: > as determined by SkinMemoryLeakTest (remove line 174) and a leak tester > https://github.com/andy-goryachev-oracle/Test/blob/main/src/goryachev/apps/LeakTest.java > > caused by: > - adding and not removing listeners > - adding and not re

RFR: 8295242: ScrollBarSkin: memory leak when changing skin

2022-11-28 Thread Andy Goryachev
Fixed memory leak when changing skin in ScrollBarSkin (was adding an event handler without removing it in dispose()). This change depends on ListenerHelper PR https://github.com/openjdk/jfx/pull/908 - Commit messages: - 8295242: cleanup - Merge remote-tracking branch 'origin/maste

RFR: 8295506: ButtonBarSkin: memory leak when changing skin

2022-11-28 Thread Andy Goryachev
as determined by SkinMemoryLeakTest (remove line 165) and a leak tester https://github.com/andy-goryachev-oracle/Test/blob/main/src/goryachev/apps/LeakTest.java Make sure to configure the current test in LeakTest: protected final Type WE_ARE_TESTING = Type.BUTTON_BAR; caused by: - adding and not

RFR: 8245145: Spinner: throws IllegalArgumentException when replacing skin

2022-11-28 Thread Andy Goryachev
Fixed SpinnerSkin initialization using new Skin.install(). Also in this PR - fixing memory leaks in SpinnerSkin by properly removing all listeners and event filters added in the constructor, as well as any child Nodes. NOTE: the fix requires both ListenerHelper [JDK-8294809](https://bugs.openj

RFR: 8295806: TableViewSkin: memory leak when changing skin

2022-11-28 Thread Andy Goryachev
as determined by SkinMemoryLeakTest (remove line 179) and a leak tester https://github.com/andy-goryachev-oracle/Test/blob/main/src/goryachev/apps/LeakTest.java caused by: - adding and not removing listeners - adding and not removing event handlers/filters NOTE: this fix requires [JDK-8294809](ht

Re: RFR: 8294589: MenuBarSkin: memory leak when changing skin [v12]

2022-11-28 Thread Andy Goryachev
> Fixed memory leak by using weak listeners and making sure to undo everything > that has been done to MenuBar/Skin in dispose(). > > This PR depends on a new internal class ListenerHelper, a replacement for > LambdaMultiplePropertyChangeListenerHandler. > See https://github.com/openjdk/jfx/pull

Re: RFR: 8293119: Additional constrained resize policies for Tree/TableView [v4]

2022-11-28 Thread Andy Goryachev
On Wed, 23 Nov 2022 22:02:08 GMT, Kevin Rushforth wrote: > 1. The divider being dragged doesn't track the cursor. All policies other > than CONSTRAINED_RESIZE_POLICY_ALL_COLUMNS should be able to track (the Swing > implementation does). Could you describe the exact scenario please? It does tr

Re: RFR: 8295796: ScrollPaneSkin: memory leak when changing skin

2022-11-28 Thread Kevin Rushforth
On Fri, 21 Oct 2022 19:01:54 GMT, Andy Goryachev wrote: > as determined by SkinMemoryLeakTest (remove line 174) and a leak tester > https://github.com/andy-goryachev-oracle/Test/blob/main/src/goryachev/apps/LeakTest.java > > caused by: > - adding and not removing listeners > - adding and not re

Re: RFR: 8293119: Additional constrained resize policies for Tree/TableView [v7]

2022-11-28 Thread Andy Goryachev
> The current CONSTRAINED_RESIZE_POLICY has a number of issues as explained in > [JDK-8292810](https://bugs.openjdk.org/browse/JDK-8292810). > > We propose to address all these issues by replacing the old column resize > algorithm with a different one, which not only honors all the constraints

Withdrawn: 8295242: ScrollBarSkin: memory leak when changing skin

2022-11-28 Thread Andy Goryachev
On Wed, 12 Oct 2022 23:18:46 GMT, Andy Goryachev wrote: > Fixed memory leak when changing skin in ScrollBarSkin (was adding an event > handler without removing it in dispose()). > > This change depends on ListenerHelper PR > https://github.com/openjdk/jfx/pull/908 This pull request has been cl

Re: RFR: 8295242: ScrollBarSkin: memory leak when changing skin

2022-11-28 Thread Andy Goryachev
On Wed, 12 Oct 2022 23:18:46 GMT, Andy Goryachev wrote: > Fixed memory leak when changing skin in ScrollBarSkin (was adding an event > handler without removing it in dispose()). > > This change depends on ListenerHelper PR > https://github.com/openjdk/jfx/pull/908 changes will be reviewed as a

Re: RFR: JDK-8297414: Remove easy warnings in javafx.controls [v5]

2022-11-28 Thread John Hendrikx
> Note: I ran into a `javac` compiler bug while replacing types with diamond > operators (ecj has no issues). I had two options, add a > `SuppressWarnings("unused")` or to use a lambda instead of a method reference > to make `javac` happy. I choose the later and added a comment so it can be >

Re: RFR: JDK-8297414: Remove easy warnings in javafx.controls [v3]

2022-11-28 Thread John Hendrikx
On Wed, 23 Nov 2022 23:38:59 GMT, John Hendrikx wrote: >> Yes, but this is now getting to be a bit out of scope for this PR. Even >> adding the `assertTrue` requires running all the tests (which thankfully, >> GHA will do). >> >> Perhaps a better approach is to not make any more changes here,

Re: RFR: JDK-8297414: Remove easy warnings in javafx.controls [v6]

2022-11-28 Thread John Hendrikx
> Note: I ran into a `javac` compiler bug while replacing types with diamond > operators (ecj has no issues). I had two options, add a > `SuppressWarnings("unused")` or to use a lambda instead of a method reference > to make `javac` happy. I choose the later and added a comment so it can be >

Re: RFR: 8293119: Additional constrained resize policies for Tree/TableView [v4]

2022-11-28 Thread Kevin Rushforth
On Mon, 28 Nov 2022 20:33:07 GMT, Andy Goryachev wrote: > > 1. The divider being dragged doesn't track the cursor. All policies other > > than CONSTRAINED_RESIZE_POLICY_ALL_COLUMNS should be able to track (the > > Swing implementation does). > > Could you describe the exact scenario please? It

Re: RFR: JDK-8297414: Remove easy warnings in javafx.controls [v6]

2022-11-28 Thread Andy Goryachev
On Mon, 28 Nov 2022 21:11:33 GMT, John Hendrikx wrote: >> Note: I ran into a `javac` compiler bug while replacing types with diamond >> operators (ecj has no issues). I had two options, add a >> `SuppressWarnings("unused")` or to use a lambda instead of a method >> reference to make `javac` h

Re: RFR: JDK-8297414: Remove easy warnings in javafx.controls [v7]

2022-11-28 Thread John Hendrikx
> Note: I ran into a `javac` compiler bug while replacing types with diamond > operators (ecj has no issues). I had two options, add a > `SuppressWarnings("unused")` or to use a lambda instead of a method reference > to make `javac` happy. I choose the later and added a comment so it can be >