On Tue, 16 Jan 2024 10:16:08 GMT, Florian Kirmaier <fkirma...@openjdk.org> wrote:
>> A few things about the latest commit: >> >> 1. The usage of the `active` property caused regression: the memoryLeak test >> that was introduced in the fix for JDK-8318841 now failed. A number of >> listeners were hard referenced from the `active` property. >> The complexity is that there are different entry points by which a listener >> should be removed. What we try to do in this PR, is making sure we remove >> listeners after a defocus/focus operation that would otherwise stay >> referenced. A focus operation will result in GlassSystemMenu.setMenus() >> being called. >> I reverted some of the dependencies on the `active` property, and kept those >> that were directly created as a consequence of the setMenus call. >> >> 2. I added a test that demonstrates the issue when not removing menu's >> completely inside the listener, as reported by @jperedadnr and this issue is >> fixed as well now. >> >> 3. All tests now pass, but I noticed that in some cases, the systemtests do >> not correctly work with the application lifecycle management (see >> https://mail.openjdk.org/pipermail/openjfx-dev/2024-January/044516.html). >> For now, I consider this anomaly to be independent from JDK-8319779 > > @johanvos > **ticket** > We found a bug, which is also fixed by this PR. This is the ticket: > https://bugs.openjdk.org/browse/JDK-8323787 > It's an IndexOutOfBounds exception, related to the visible flag/listeners. > > **unittest** > We wrote a unit test, which goes green with this PR. > Can you do us a favor, and add the test from the ticket to your PR? > Then we can be sure, that this issue will be solved and never reappears. @FlorianKirmaier Thanks for the additional test. There was already a test in the SystemMenuBarTest, but it's really good to have an additional test for this -- I added it. ------------- PR Comment: https://git.openjdk.org/jfx/pull/1283#issuecomment-1902259510
