RFR: 8254605: repaint on Android broken
Kevin Rushforth
kcr at openjdk.java.net
Fri Oct 16 23:10:15 UTC 2020
On Fri, 16 Oct 2020 12:31:31 GMT, Johan Vos <jvos at openjdk.org> wrote:
>> Don't check on windows visiblity when requesting focus.
>> For a new window, the call to requestFocus is done before the visibility is set to true.
>> Fix for JDK-8254605
>
> @arapte and @kevinrushforth Since you reviewed https://github.com/openjdk/jfx/pull/153 can you review this?
> I think the change in #153 does not check on "closed stages" but on "windows that are not visible yet" which is not the
> same. Therefore, this PR would revert #153 as it introduces regression.
Reverting the fix for [JDK-8241840](https://bugs.openjdk.java.net/browse/JDK-8241840) does seem the best way to address
this, since the current failure is much more serious that the memory leak was.
Two comments:
1. Can you file a new bug to redo the fix for [JDK-8241840](https://bugs.openjdk.java.net/browse/JDK-8241840) with a
summary like:
[REDO] Memoryleak: Closed focused Stages are not collected with Monocle
Indicate that the original fix is being reverted, so will need to be redone. Link the new bug to both this bug and to
[JDK-8241840](https://bugs.openjdk.java.net/browse/JDK-8241840).
2. Unsurprisingly, this backout fix causes one of the newly added unit tests to fail:
$ gradle ...
test.javafx.stage.FocusedWindowMonocleTest > testClosedFocusedStageLeak FAILED
junit.framework.AssertionFailedError: Expected: <null> but was: javafx.stage.Stage at 22ff86f5
at junit.framework.Assert.fail(Assert.java:47)
at junit.framework.Assert.assertTrue(Assert.java:20)
at junit.framework.Assert.assertNull(Assert.java:233)
at junit.framework.Assert.assertNull(Assert.java:226)
at test.javafx.stage.FocusedWindowTestBase.assertCollectable(FocusedWindowTestBase.java:101)
at test.javafx.stage.FocusedWindowTestBase.testClosedFocusedStageLeakBase(FocusedWindowTestBase.java:85)
at test.javafx.stage.FocusedWindowMonocleTest.testClosedFocusedStageLeak(FocusedWindowMonocleTest.java:47)
Can you update your PR to ignore this test with `@Ignore("JDK-nnnnnnn")` using the newly filed "REDO" bug as the bug ID?
-------------
PR: https://git.openjdk.java.net/jfx/pull/322
More information about the openjfx-dev
mailing list