The webrev link:
http://cr.openjdk.java.net/~alexsch/robin.stevens/8158325/webrev.00/
The fix looks good to me.
Thanks,
Alexandr.
On 6/23/2016 7:08 PM, Robin Stevens wrote:
Hello all,
attached is a webrev for issue JDK-8158325 Memory leak in
com.apple.laf.ScreenMenu: removed JMenuItems are still referenced.
Patch contains a test case which reveals the bug, and a fix.
There were a few issues with the ScreenMenu class:
- The ContainerListener was attached to the JMenu and not to the
JMenu#getPopupMenu. The JMenu itself does not fire any
ContainerEvents, but
the popup does. As a result, the cleanup code in ScreenMenu was never
triggered. The patch fixes this by attaching the ContainerListener to
the popup menu.
Note that the ScreenMenu class also attaches a ComponentListener to
the JMenu. I had no idea whether that one must be attached to the
popup menu as well, so I did not change it.
- The cleanup code was not triggered when removeAll() was called from
the updateItems method. I fixed this by overriding the remove(int)
method, and
putting the cleanup code in that method. An alternative here would be
to not override the remove(int) method, but instead call
fItems.clear() after calling removeAll() . However, overriding the
remove(int) method sounded more robust to me.
- The cleanup code was incorrect. It tried to remove an item from
fItems (a map) by calling remove with the value instead of the key.
Now the remove is called with the key. Because the cleanup code has
been moved, this required me to loop over the map as I have no direct
access to the key in the
remove(int) method
- The test can be run on all platforms, although it was written for an
OS X specific bug. As it can run on all platforms, I did not disable
it on non OS X platforms. Let me know if I need to adjust this.
Kind regards,
Robin
