On Mon, 9 Sep 2024 06:27:26 GMT, Sergey Bylokhov <[email protected]> wrote:
>> Several tests modified by https://github.com/openjdk/jdk/pull/19339 have >> been tweaked, see inline comments. >> >> Notes: >> * We have a few XXXRepaint.java tests and in this patch, I updated all of >> them to follow the change added to the ListRepaint.java >> >> @azvegint @aivanov-jdk please take a look. > > test/jdk/java/awt/Frame/MiscUndecorated/ActiveAWTWindowTest.java line 42: > >> 40: private volatile Frame frame, frame2; >> 41: private volatile Button button, button2; >> 42: private volatile TextField textField, textField2; > > probably unnecessary but some of these fields might be used on different > threads, so just in case. Likely, it's unnecessary. I don't mind though. I'd rather refactor the test to simplify it. Using `CountDownLatch` or other synchronisation primitives instead of wait/notify. I submitted [JDK-8339791](https://bugs.openjdk.org/browse/JDK-8339791). Feel free to re-assign if you like. > test/jdk/java/awt/List/KeyEventsTest/KeyEventsTest.java line 68: > >> 66: KeyEventsTest app = new KeyEventsTest(); >> 67: try { >> 68: app.initAndShowGui(); > > The test was changed > [here](https://github.com/openjdk/jdk/pull/19339/files#diff-054074499ab6611e764a315fb13c51af4ad063b07f481e775a2262196077d285R69), > the root cause of the failure discussed in > https://github.com/openjdk/jdk/pull/19339/files#r1609189736 is a product bug: > https://bugs.openjdk.org/browse/JDK-8201307. Not sure if it's a product bug… AWT components do not explicitly state whether they're thread-safe or not. It's more or less assumed they should be. At the same time, I don't see code which ensures all the cases are thread-safe. As far as I know, clientlibs group has been treating AWT components just like Swing ones: for safety create, access and destroy AWT components on EDT only. I haven't looked at the test code thoroughly. It looks it still needs more investigation. At the same time, I find it weird that `repaint` paints directly in its code and then posts paint event. https://github.com/openjdk/jdk/blob/77468c284c068f921da543edd28333911e915b61/src/java.desktop/unix/classes/sun/awt/X11/XListPeer.java#L389-L390 Overall, it needs more investigation. It could be that the fix for [JDK-6471693](https://bugs.openjdk.org/browse/JDK-6471693) needs revising. > test/jdk/java/awt/Paint/ListRepaint.java line 43: > >> 41: for (int i = 0; i < 10; ++i) { >> 42: try { >> 43: EventQueue.invokeLater(ListRepaint::createAndShowGUI); > > The purpose of the test is to run some methods of the AWT component on the > main thread and check if it will be refreshed on the EDT. It was changed > [here](https://github.com/openjdk/jdk/pull/19339/files#r1609190355). It is a > product bug: https://bugs.openjdk.org/browse/JDK-8201307. Yep, the previous update has changed the logic of the test so that it no longer reproduces the original problem for which it was written, namely [JDK-7090424](https://bugs.openjdk.org/browse/JDK-7090424). ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/20861#discussion_r1750768459 PR Review Comment: https://git.openjdk.org/jdk/pull/20861#discussion_r1750821284 PR Review Comment: https://git.openjdk.org/jdk/pull/20861#discussion_r1750839501
