[Rev 01] RFR: 8241840: Memoryleak: Closed focused Stages are not collected with Monocle.

[Florian Kirmaier]

On Tue, 7 Apr 2020 09:51:18 GMT, Ambarish Rapte <arapte at openjdk.org> wrote:

>> Florian Kirmaier has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   JDK-8241840
>>   Some code cleanups
>
> tests/system/src/test/java/test/javafx/stage/FocusedWindowTest.java line 133:
> 
>> 132:
>> 133:         Assert.assertNull(weakReference.get());
>> 134:     }
> 
> This assert check call should be added to the @Test method.
> I would recommend to refer [this
> method](https://github.com/openjdk/jfx/blob/364c64a2e55b561df4ca1afc85c91054b339eea8/modules/javafx.controls/src/test/java/test/javafx/scene/control/ListViewTest.java#L1998)
> from ListViewTest.

Unifying memory-tests is something I discussed in my last PR. This code is based on my previous pull request:
https://github.com/openjdk/jfx/pull/71 I would like to keep this version which I've used there. But it really has to be
a library/utility-class which is used across the project instead of reinventing it on every test.

> tests/system/src/test/java/test/javafx/stage/FocusedWindowTest.java line 84:
> 
>> 83:             counter += 1;
>> 84:             testLeakOnce();
>> 85:         }
> 
> If the test has constant behavior on every run, then only one run should be done.

done

> modules/javafx.graphics/src/main/java/com/sun/glass/ui/Window.java line 354:
> 
>> 353:         }
>> 354:         if(Window.focusedWindow == this) {
>> 355:             Window.focusedWindow = null;
> 
> Please correct format by adding space after `if`.

done

> tests/system/src/test/java/test/javafx/stage/FocusedWindowTest.java line 113:
> 
>> 112:         });
>> 113:
>> 114:         Assert.assertTrue("Timeout, waiting for runLater. ", leakLatch.await(15, TimeUnit.SECONDS));
> 
> Is it really required to nest the `Platform.runLater()` calls ? Can you please check if this code can be simplified by
> making the calls sequential, Consider using `Util.runAndWait()`

done

> tests/system/src/test/java/test/javafx/stage/FocusedWindowTest.java line 53:
> 
>> 52:         System.setProperty("monocle.platform","Headless");
>> 53:     }
>> 54:
> 
> The test will always run with Monocle. I see that if this static block is removed then the test fails on Windows 10.
> Can you please verify all the platforms and change the test such that it runs on all platforms/ combinations.

I wasn't able to reproduce the Problem on Window10 with VirtualBox.

How should I do it? Add a second test class without the static code? Do you have a good recommendation on how to add it?

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

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