On Mon, 22 Mar 2021 14:30:58 GMT, Florian Kirmaier <fkirma...@openjdk.org>
wrote:
>> Fixing a memory leak.
>> A node hard references its old parent after CSS layout and getting removed.
>> This shouldn't be the case, this is very counterintuitive.
>>
>> The fix uses a WeakReference in CSSStyleHelper for firstStyleableAncestor.
>> This should be fine because the CSS should only depend on it if it's still
>> the real parent.
>> In that case, it doesn't get collected.
>
> Florian Kirmaier has updated the pull request incrementally with one
> additional commit since the last revision:
>
> 8263402
> Rewrote the style memoryleak test
Provided few comments on test.
tests/system/src/test/java/test/javafx/scene/StyleMemoryLeakTest.java line 48:
> 46: import static org.junit.Assert.fail;
> 47: import static org.junit.Assert.assertTrue;
> 48:
The packages Application, Node and Parent are not used.
tests/system/src/test/java/test/javafx/scene/StyleMemoryLeakTest.java line 59:
> 57: startupLatch.countDown();
> 58: });
> 59: assertTrue("Timeout waiting for FX runtime to start",
> startupLatch.await(15, TimeUnit.SECONDS));
The runnable passed to `Platform.startup()` is executed on JavaFX Application
thread once it is started. `Platform.setImplicitExit(false);` can be called
from any thread. So we do not need to pass the runnable and do not need
CountDownLatch.
This block can be replaced by,
Platform.startup(null);
Platform.setImplicitExit(false);
tests/system/src/test/java/test/javafx/scene/StyleMemoryLeakTest.java line 100:
> 98: Platform.runLater(() -> {
> 99: Platform.exit();
> 100: });
`Platform.exit()` can be called from any thread. So `Platform.runLater` is not
required.
tests/system/src/test/java/test/javafx/scene/StyleMemoryLeakTest.java line 102:
> 100: });
> 101: }
> 102: }
Include a new line at end of file.
-------------
Changes requested by arapte (Reviewer).
PR: https://git.openjdk.java.net/jfx/pull/424
