Hi all,
This pull request contains a backport of commit 25ac6fed from the openjdk/jfx
repository.
The commit being backported was authored by Johan Vos on 25 Jul 2024 and was
reviewed by John Hendrikx, Ambarish Rapte and Jose Pereda.
Thanks!
-
Commit messages:
- Backport 25ac6fe
On Thu, 1 Aug 2024 10:04:12 GMT, Johan Vos wrote:
> almost clean backport of 8319779: SystemMenu: memory leak due to listener
> never being removed
> Didn't apply clean because the imports where shuffled.
Marked as reviewed by jpereda (Reviewer).
-
PR Review: https://git.openjdk.o
almost clean backport of 8319779: SystemMenu: memory leak due to listener never
being removed
Didn't apply clean because the imports where shuffled.
-
Commit messages:
- Backport 25ac6fed22d0f49d01c831aaa48049c34899fe96
Changes: https://git.openjdk.org/jfx21u/pull/66/files
Webrev
On Thu, 1 Aug 2024 08:41:59 GMT, Johan Vos wrote:
> Backport of 8323787: Mac System MenuBar throws IOB exception
>
> Required manual changes as the location of the test has changed.
closed in favor of #66
-
PR Comment: https://git.openjdk.org/jfx21u/pull/63#issuecomment-226265826
Backport of 8323787: Mac System MenuBar throws IOB exception
Required manual changes as the location of the test has changed.
-
Commit messages:
- 8319779: SystemMenu: memory leak due to listener never being removed
Changes: https://git.openjdk.org/jfx21u/pull/63/files
Webrev: h
On Tue, 9 Jul 2024 09:06:58 GMT, Johan Vos wrote:
>> A listener was added but never removed.
>> This patch removes the listener when the menu it links to is cleared. Fix
>> for https://bugs.openjdk.org/browse/JDK-8319779
>
> Johan Vos has updated the pull request incrementally with one additiona
On Tue, 9 Jul 2024 09:06:58 GMT, Johan Vos wrote:
>> A listener was added but never removed.
>> This patch removes the listener when the menu it links to is cleared. Fix
>> for https://bugs.openjdk.org/browse/JDK-8319779
>
> Johan Vos has updated the pull request incrementally with one additiona
On Tue, 9 Jul 2024 09:06:58 GMT, Johan Vos wrote:
>> A listener was added but never removed.
>> This patch removes the listener when the menu it links to is cleared. Fix
>> for https://bugs.openjdk.org/browse/JDK-8319779
>
> Johan Vos has updated the pull request incrementally with one additiona
On Tue, 9 Jul 2024 14:48:30 GMT, Kevin Rushforth wrote:
>> I manually reverted the add/remove part, and replaced it with `git mv`. I
>> assume/hope that by squashing the commits, the add/remove will not be part
>> of the change.
>
> git doesn't actually track renames and copies, so there is ult
On Tue, 9 Jul 2024 14:50:08 GMT, Kevin Rushforth wrote:
>> tests/system/src/test/java/test/com/sun/javafx/tk/quantum/SystemMenuBarTest.java
>> line 321:
>>
>>> 319: Stage stage = new Stage();
>>> 320: stage.setScene(new Scene(root));
>>> 321: stage.show();
>>
On Wed, 19 Jun 2024 14:25:52 GMT, Ambarish Rapte wrote:
>> Johan Vos has updated the pull request incrementally with two additional
>> commits since the last revision:
>>
>> - process more reviewer comments
>> - Process reviewer comments
>
> tests/system/src/test/java/test/com/sun/javafx/tk/q
On Tue, 9 Jul 2024 09:13:13 GMT, Johan Vos wrote:
>> tests/system/src/test/java/test/com/sun/javafx/tk/quantum/SystemMenuBarTest.java
>> line 1:
>>
>>> 1: /*
>>
>> This test file is moved from a different location, could do `git mv` instead
>> removing and adding.
>
> I manually reverted the
On Tue, 9 Jul 2024 09:09:30 GMT, Johan Vos wrote:
>> tests/system/src/test/java/test/com/sun/javafx/tk/quantum/SystemMenuBarTest.java
>> line 287:
>>
>>> 285:
>>> 286: @Test
>>> 287: public void testJDK8309935() {
>>
>> minor: Not sure what our policies are, but I find test names like
On Tue, 9 Jul 2024 09:06:58 GMT, Johan Vos wrote:
>> A listener was added but never removed.
>> This patch removes the listener when the menu it links to is cleared. Fix
>> for https://bugs.openjdk.org/browse/JDK-8319779
>
> Johan Vos has updated the pull request incrementally with one additiona
On Wed, 19 Jun 2024 14:25:52 GMT, Ambarish Rapte wrote:
>> Johan Vos has updated the pull request incrementally with two additional
>> commits since the last revision:
>>
>> - process more reviewer comments
>> - Process reviewer comments
>
> tests/system/src/test/java/test/com/sun/javafx/tk/q
On Wed, 19 Jun 2024 12:39:11 GMT, Ambarish Rapte wrote:
>> Johan Vos has updated the pull request incrementally with two additional
>> commits since the last revision:
>>
>> - process more reviewer comments
>> - Process reviewer comments
>
> tests/system/src/test/java/test/com/sun/javafx/tk/q
On Thu, 13 Jun 2024 07:17:01 GMT, John Hendrikx wrote:
>> Johan Vos has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Add more type info
>
> tests/system/src/test/java/test/com/sun/javafx/tk/quantum/SystemMenuBarTest.java
> line 287:
>
>
> A listener was added but never removed.
> This patch removes the listener when the menu it links to is cleared. Fix for
> https://bugs.openjdk.org/browse/JDK-8319779
Johan Vos has updated the pull request incrementally with one additional commit
since the last revision:
Allow scheduled runn
> A listener was added but never removed.
> This patch removes the listener when the menu it links to is cleared. Fix for
> https://bugs.openjdk.org/browse/JDK-8319779
Johan Vos has updated the pull request incrementally with three additional
commits since the last revision:
- Add changes back
> A listener was added but never removed.
> This patch removes the listener when the menu it links to is cleared. Fix for
> https://bugs.openjdk.org/browse/JDK-8319779
Johan Vos has updated the pull request with a new target base due to a merge or
a rebase. The pull request now contains 21 commi
On Thu, 13 Jun 2024 20:07:32 GMT, Johan Vos wrote:
>> A listener was added but never removed.
>> This patch removes the listener when the menu it links to is cleared. Fix
>> for https://bugs.openjdk.org/browse/JDK-8319779
>
> Johan Vos has updated the pull request incrementally with two addition
On Thu, 13 Jun 2024 20:07:32 GMT, Johan Vos wrote:
>> A listener was added but never removed.
>> This patch removes the listener when the menu it links to is cleared. Fix
>> for https://bugs.openjdk.org/browse/JDK-8319779
>
> Johan Vos has updated the pull request incrementally with two addition
On Thu, 13 Jun 2024 10:25:15 GMT, Jose Pereda wrote:
>> Johan Vos has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Add more type info
>
> modules/javafx.graphics/src/shims/java/com/sun/javafx/tk/quantum/GlassSystemMenuShim.java
> line 37
On Tue, 12 Dec 2023 07:05:49 GMT, Ambarish Rapte wrote:
>> Johan Vos has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> process reviewers comments
>
> Looks all good to me.
> Is it possible to add an automated test ?
@arapte if possible, c
> A listener was added but never removed.
> This patch removes the listener when the menu it links to is cleared. Fix for
> https://bugs.openjdk.org/browse/JDK-8319779
Johan Vos has updated the pull request incrementally with two additional
commits since the last revision:
- process more revie
> A listener was added but never removed.
> This patch removes the listener when the menu it links to is cleared. Fix for
> https://bugs.openjdk.org/browse/JDK-8319779
Johan Vos has updated the pull request with a new target base due to a merge or
a rebase. The pull request now contains 18 commi
On Thu, 13 Jun 2024 07:21:47 GMT, John Hendrikx wrote:
> The new test passed with both the old and the new version (I reverted the
> code, aside from leaving `setMenuBindings` `protected`). Is the test testing
> the right thing?
>
> (If relevant, this was done on a Windows platform)
That's no
On Thu, 9 May 2024 19:48:19 GMT, Johan Vos wrote:
>> A listener was added but never removed.
>> This patch removes the listener when the menu it links to is cleared. Fix
>> for https://bugs.openjdk.org/browse/JDK-8319779
>
> Johan Vos has updated the pull request incrementally with one additiona
On Thu, 9 May 2024 19:48:19 GMT, Johan Vos wrote:
>> A listener was added but never removed.
>> This patch removes the listener when the menu it links to is cleared. Fix
>> for https://bugs.openjdk.org/browse/JDK-8319779
>
> Johan Vos has updated the pull request incrementally with one additiona
On Wed, 13 Dec 2023 09:20:12 GMT, Jose Pereda wrote:
>> Johan Vos has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> process reviewers comments
>
> About adding an automated test, the leak that PR tries to fix happens in
> `com.sun.javafx.
On Thu, 9 May 2024 19:48:19 GMT, Johan Vos wrote:
>> A listener was added but never removed.
>> This patch removes the listener when the menu it links to is cleared. Fix
>> for https://bugs.openjdk.org/browse/JDK-8319779
>
> Johan Vos has updated the pull request incrementally with one additiona
> A listener was added but never removed.
> This patch removes the listener when the menu it links to is cleared. Fix for
> https://bugs.openjdk.org/browse/JDK-8319779
Johan Vos has updated the pull request incrementally with one additional commit
since the last revision:
Add more type info
> A listener was added but never removed.
> This patch removes the listener when the menu it links to is cleared. Fix for
> https://bugs.openjdk.org/browse/JDK-8319779
Johan Vos has updated the pull request incrementally with one additional commit
since the last revision:
Add type info
Fix
> A listener was added but never removed.
> This patch removes the listener when the menu it links to is cleared. Fix for
> https://bugs.openjdk.org/browse/JDK-8319779
Johan Vos has updated the pull request with a new target base due to a merge or
a rebase. The pull request now contains 15 commi
On Tue, 5 Mar 2024 08:55:41 GMT, Jose Pereda wrote:
>> This could be a problem normally, but I think in this case you won't be able
>> to get this to produce incorrect results.
>>
>> `parseText` is accessing the property (and it calls `isMnemonicParsing`),
>> but only if the text is not empty.
> A listener was added but never removed.
> This patch removes the listener when the menu it links to is cleared. Fix for
> https://bugs.openjdk.org/browse/JDK-8319779
Johan Vos has updated the pull request incrementally with one additional commit
since the last revision:
Remove whiteline and
On Wed, 6 Mar 2024 07:59:10 GMT, Johan Vos wrote:
>> A listener was added but never removed.
>> This patch removes the listener when the menu it links to is cleared. Fix
>> for https://bugs.openjdk.org/browse/JDK-8319779
>
> Johan Vos has updated the pull request incrementally with one additiona
> A listener was added but never removed.
> This patch removes the listener when the menu it links to is cleared. Fix for
> https://bugs.openjdk.org/browse/JDK-8319779
Johan Vos has updated the pull request incrementally with one additional commit
since the last revision:
Ignore test in case
On Tue, 5 Mar 2024 08:20:03 GMT, Johan Vos wrote:
>> A listener was added but never removed.
>> This patch removes the listener when the menu it links to is cleared. Fix
>> for https://bugs.openjdk.org/browse/JDK-8319779
>
> Johan Vos has updated the pull request incrementally with one additiona
On Fri, 1 Mar 2024 14:55:58 GMT, Johan Vos wrote:
> The failure on Linux has an interesting cause. The GlassSystemMenu (in
> com.sun.javafx.tk.quantum) has this code:
>
> ```
> /*
> * Leave the Apple menu in place
> */
> for (int index = existin
On Tue, 5 Mar 2024 08:21:55 GMT, Johan Vos wrote:
>> tests/system/src/test/java/test/com/sun/javafx/tk/quantum/SystemMenuBarTest.java
>> line 254:
>>
>>> 252: return menuBar;
>>> 253: }
>>> 254:
>>
>> extra newline?
>
> Not sure I understand where you want to add or remove a newli
On Tue, 5 Mar 2024 07:34:32 GMT, John Hendrikx wrote:
>> modules/javafx.graphics/src/main/java/com/sun/javafx/tk/quantum/GlassSystemMenu.java
>> line 226:
>>
>>> 224: mb.textProperty().when(active).addListener(valueModel ->
>>> glassMenu.setTitle(parseText(mb)));
>>> 225: mb.di
On Tue, 9 Jan 2024 16:32:51 GMT, Andy Goryachev wrote:
>> Johan Vos has updated the pull request incrementally with two additional
>> commits since the last revision:
>>
>> - Cleanup test
>> - Add shim class so that we can access the references to
>> com.sun.glass.ui.Menu instances.
>>Ad
> A listener was added but never removed.
> This patch removes the listener when the menu it links to is cleared. Fix for
> https://bugs.openjdk.org/browse/JDK-8319779
Johan Vos has updated the pull request incrementally with one additional commit
since the last revision:
Update
modules/java
On Thu, 11 Jan 2024 00:20:19 GMT, John Hendrikx wrote:
>> No, the explicit goal of this construction is to set active to false (in
>> case it exists) so that existing listeners can be released; followed by
>> creating a new active property that is by default set to true until
>> `setMenus` is
On Mon, 4 Mar 2024 19:26:37 GMT, Jose Pereda wrote:
>> Johan Vos has updated the pull request with a new target base due to a merge
>> or a rebase. The incremental webrev excludes the unrelated changes brought
>> in by the merge/rebase. The pull request contains 11 additional commits
>> since
On Tue, 27 Feb 2024 09:30:14 GMT, Johan Vos wrote:
>> A listener was added but never removed.
>> This patch removes the listener when the menu it links to is cleared. Fix
>> for https://bugs.openjdk.org/browse/JDK-8319779
>
> Johan Vos has updated the pull request with a new target base due to a
On Tue, 27 Feb 2024 09:30:14 GMT, Johan Vos wrote:
>> A listener was added but never removed.
>> This patch removes the listener when the menu it links to is cleared. Fix
>> for https://bugs.openjdk.org/browse/JDK-8319779
>
> Johan Vos has updated the pull request with a new target base due to a
> A listener was added but never removed.
> This patch removes the listener when the menu it links to is cleared. Fix for
> https://bugs.openjdk.org/browse/JDK-8319779
Johan Vos has updated the pull request with a new target base due to a merge or
a rebase. The incremental webrev excludes the un
On Mon, 26 Feb 2024 15:10:33 GMT, Kevin Rushforth wrote:
>> Johan Vos has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Add additional test for IOOBE detection.
>> This test comes from JDK-8323787
>
> Getting back to this I ran the t
On Sat, 20 Jan 2024 20:34:50 GMT, Johan Vos wrote:
>> A listener was added but never removed.
>> This patch removes the listener when the menu it links to is cleared. Fix
>> for https://bugs.openjdk.org/browse/JDK-8319779
>
> Johan Vos has updated the pull request incrementally with one addition
On Sat, 20 Jan 2024 20:35:15 GMT, Johan Vos wrote:
> 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.
@johanvos [8323787](https://bugs.openjdk.org/browse/JDK-8323787) seems like a
dis
On Tue, 16 Jan 2024 10:16:08 GMT, Florian Kirmaier
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 fro
> A listener was added but never removed.
> This patch removes the listener when the menu it links to is cleared. Fix for
> https://bugs.openjdk.org/browse/JDK-8319779
Johan Vos has updated the pull request incrementally with one additional commit
since the last revision:
Add additional test
On Thu, 11 Jan 2024 14:04:36 GMT, Johan Vos wrote:
>> Johan Vos has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Revert some of the conditional bindings.
>> Clear menu construction when an menuitem that is a menu needs to be removed
>>
On Thu, 11 Jan 2024 11:45:02 GMT, Johan Vos wrote:
>> A listener was added but never removed.
>> This patch removes the listener when the menu it links to is cleared. Fix
>> for https://bugs.openjdk.org/browse/JDK-8319779
>
> Johan Vos has updated the pull request incrementally with one addition
> A listener was added but never removed.
> This patch removes the listener when the menu it links to is cleared. Fix for
> https://bugs.openjdk.org/browse/JDK-8319779
Johan Vos has updated the pull request incrementally with one additional commit
since the last revision:
Revert some of the c
> A listener was added but never removed.
> This patch removes the listener when the menu it links to is cleared. Fix for
> https://bugs.openjdk.org/browse/JDK-8319779
Johan Vos has updated the pull request with a new target base due to a merge or
a rebase. The incremental webrev excludes the un
On Wed, 10 Jan 2024 19:55:35 GMT, Johan Vos wrote:
>> modules/javafx.graphics/src/main/java/com/sun/javafx/tk/quantum/GlassSystemMenu.java
>> line 98:
>>
>>> 96: active.set(false);
>>> 97: }
>>> 98: active = new SimpleBooleanProperty(true);
>>
>> should this be
>>
On Wed, 10 Jan 2024 23:20:23 GMT, John Hendrikx wrote:
>> thank you for clarification!
>>
>> perhaps we ought to add a comment explaining why: it's one thing to create a
>> single chain of conditions linked to a property, but here we keep creating
>> new property instance, so the side effect
On Wed, 10 Jan 2024 20:05:16 GMT, Andy Goryachev wrote:
>> No, the explicit goal of this construction is to set active to false (in
>> case it exists) so that existing listeners can be released; followed by
>> creating a new active property that is by default set to true until
>> `setMenus` is
On Wed, 10 Jan 2024 19:55:35 GMT, Johan Vos wrote:
>> modules/javafx.graphics/src/main/java/com/sun/javafx/tk/quantum/GlassSystemMenu.java
>> line 98:
>>
>>> 96: active.set(false);
>>> 97: }
>>> 98: active = new SimpleBooleanProperty(true);
>>
>> should this be
>>
On Tue, 9 Jan 2024 16:21:39 GMT, Andy Goryachev wrote:
>> Johan Vos has updated the pull request incrementally with two additional
>> commits since the last revision:
>>
>> - Cleanup test
>> - Add shim class so that we can access the references to
>> com.sun.glass.ui.Menu instances.
>>Ad
On Tue, 9 Jan 2024 13:19:53 GMT, Johan Vos wrote:
>> A listener was added but never removed.
>> This patch removes the listener when the menu it links to is cleared. Fix
>> for https://bugs.openjdk.org/browse/JDK-8319779
>
> Johan Vos has updated the pull request incrementally with two additiona
> A listener was added but never removed.
> This patch removes the listener when the menu it links to is cleared. Fix for
> https://bugs.openjdk.org/browse/JDK-8319779
Johan Vos has updated the pull request incrementally with two additional
commits since the last revision:
- Cleanup test
- Ad
On Tue, 9 Jan 2024 13:19:53 GMT, Johan Vos wrote:
>> A listener was added but never removed.
>> This patch removes the listener when the menu it links to is cleared. Fix
>> for https://bugs.openjdk.org/browse/JDK-8319779
>
> Johan Vos has updated the pull request incrementally with two additiona
On Tue, 26 Dec 2023 10:24:42 GMT, Johan Vos wrote:
> I'm still looking into adding a test. The challenge is that the (previously)
> uncollected items are of type `com.sun.glass.ui.MenuItem`, and afaik we can't
> use JMemoryBuddy to check the collectable state of instances on the heap that
> we
On Tue, 19 Dec 2023 13:25:38 GMT, John Hendrikx wrote:
>> As for the memory leak issue: there were several. The first commit in this
>> PR fixed a clear memory leak, but the one that is still left is not
>> described in the issue.
>> It occurs because whenever the SystemMenuBar is shown after
> A listener was added but never removed.
> This patch removes the listener when the menu it links to is cleared. Fix for
> https://bugs.openjdk.org/browse/JDK-8319779
Johan Vos has updated the pull request incrementally with one additional commit
since the last revision:
Fix more memoryleaks
> A listener was added but never removed.
> This patch removes the listener when the menu it links to is cleared. Fix for
> https://bugs.openjdk.org/browse/JDK-8319779
Johan Vos has updated the pull request with a new target base due to a merge or
a rebase. The incremental webrev excludes the un
On Mon, 18 Dec 2023 13:18:02 GMT, Johan Vos wrote:
>> A listener was added but never removed.
>> This patch removes the listener when the menu it links to is cleared. Fix
>> for https://bugs.openjdk.org/browse/JDK-8319779
>
> Johan Vos has updated the pull request incrementally with one addition
On Mon, 20 Nov 2023 08:00:58 GMT, Johan Vos wrote:
>> A listener was added but never removed.
>> This patch removes the listener when the menu it links to is cleared. Fix
>> for https://bugs.openjdk.org/browse/JDK-8319779
>
> Johan Vos has updated the pull request incrementally with one addition
On Mon, 20 Nov 2023 08:00:58 GMT, Johan Vos wrote:
>> A listener was added but never removed.
>> This patch removes the listener when the menu it links to is cleared. Fix
>> for https://bugs.openjdk.org/browse/JDK-8319779
>
> Johan Vos has updated the pull request incrementally with one addition
On Mon, 20 Nov 2023 08:00:58 GMT, Johan Vos wrote:
>> A listener was added but never removed.
>> This patch removes the listener when the menu it links to is cleared. Fix
>> for https://bugs.openjdk.org/browse/JDK-8319779
>
> Johan Vos has updated the pull request incrementally with one addition
On Fri, 10 Nov 2023 12:37:13 GMT, John Hendrikx wrote:
>> Johan Vos has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> process reviewers comments
>
> modules/javafx.graphics/src/main/java/com/sun/javafx/tk/quantum/GlassSystemMenu.java
> li
> A listener was added but never removed.
> This patch removes the listener when the menu it links to is cleared. Fix for
> https://bugs.openjdk.org/browse/JDK-8319779
Johan Vos has updated the pull request incrementally with one additional commit
since the last revision:
process reviewers co
On Fri, 10 Nov 2023 10:34:08 GMT, Johan Vos wrote:
> A listener was added but never removed.
> This patch removes the listener when the menu it links to is cleared. Fix for
> https://bugs.openjdk.org/browse/JDK-8319779
@arapte can you be the primary reviewer for this?
-
PR Comment
On Fri, 10 Nov 2023 10:34:08 GMT, Johan Vos wrote:
> A listener was added but never removed.
> This patch removes the listener when the menu it links to is cleared. Fix for
> https://bugs.openjdk.org/browse/JDK-8319779
Should there be an updated test for this?
I see a lot of raw type use (`Lis
A listener was added but never removed.
This patch removes the listener when the menu it links to is cleared. Fix for
https://bugs.openjdk.org/browse/JDK-8319779
-
Commit messages:
- A listener was added but never removed.
Changes: https://git.openjdk.org/jfx/pull/1283/files
Webre
79 matches
Mail list logo
