On Fri, 17 Jan 2020 23:40:18 GMT, Kevin Rushforth <k...@openjdk.org> wrote:

>>> 
>>> 
>>> @arapte - This bug looks like a good candidate for JavaFX 14. Can you 
>>> retarget this PR to the jfx14 branch?
>> 
>> Thanks for guiding Kevin, PR is now targeted to jfx14.
> 
> The fix looks good. I'll take a closer look at the unit test later.
> 
> Speaking of tests...since the addition of the `TabPane` reordering logic was 
> a victim of the already-existing leak in the `viewOrderChildren` list in 
> `Parent`, it should be possible to write a test case using a Group node and a 
> few Shape nodes, using setViewOrder directly on the Group node (this would be 
> in addition to the system test you wrote). Can you take a look at adding one? 
> It might even be possible to do it as a `javafx.graphics` module unit test 
> rather than a system test, although you would need to see if the bug 
> reproduced there (I suspect it will).

> 
> 
> The fix looks good. I'll take a closer look at the unit test later.
> 
> Speaking of tests...since the addition of the `TabPane` reordering logic was 
> a victim of the already-existing leak in the `viewOrderChildren` list in 
> `Parent`, it should be possible to write a test case using a Group node and a 
> few Shape nodes, using setViewOrder directly on the Group node (this would be 
> in addition to the system test you wrote). Can you take a look at adding one? 
> It might even be possible to do it as a `javafx.graphics` module unit test 
> rather than a system test, although you would need to see if the bug 
> reproduced there (I suspect it will).

Hello Kevin,
The bug can be reproduced with system test written using `Group` and `Shape` 
which is very similar to `TabPaneHeaderLeakTest` test. but it seems the bug is 
not reproducible with unit test. I tried a unit test very similar to the newly 
added system test `ShapeViewOrderLeakTest`, but looks like 
`Parent.viewOrderChildren` list does not get populated and so the issue does 
not occur.

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

PR: https://git.openjdk.java.net/jfx/pull/79

Reply via email to