On Thu, 15 May 2025 17:21:37 GMT, Sergey Bylokhov <s...@openjdk.org> wrote:
>> Jeremy Wood has updated the pull request incrementally with two additional >> commits since the last revision: >> >> - 8356061: restructuring to move things to EDT >> >> This was requested here: >> https://github.com/openjdk/jdk/pull/25244#discussion_r2091656355 >> >> mrserb asked: "Also please move creation and access_to all Swing >> components to EDT" >> >> (I'm not sure this will help much? My understanding was *creation* of >> Swing components could happen off the EDT as long as they were made >> displayable on the EDT.) >> >> Now we still call jc.getLocationOnScreen off the EDT. If that posed a >> thread-based problem it'd probably manifest as a >> IllegalComponentStateException, which is not mentioned in 8356061. >> - 8356061: adding 1000ms pause after window construction >> >> This was requested here: >> https://github.com/openjdk/jdk/pull/25244#discussion_r2091656355 >> >> So now we'll pause at least 1.1s before the first call to >> `jc.getLocationOnScreen`, and at least 2.1s before the first call to >> `robot.getPixelColor(x, y)`. >> >> (getLocationOnScreen has never failed with an IllegalStateException.) > > test/jdk/com/apple/laf/RootPane/RootPaneDefaultButtonTest.java line 131: > >> 129: robot.mouseRelease(InputEvent.BUTTON1_DOWN_MASK); >> 130: >> 131: robot.delay(1000); > > it will be useful to add the same delay before the first call to test, right > after invokeAndWait.....setVisible(...) > Also please move creation and access_to all Swing components to EDT(button, > radioButton1) I added an additional delay. I'm not sure exactly what refactor you're looking for, but I rewrote the test to put more in the EDT. (I double-checked that the test still passes, but that doesn't prove much since I've never reproduced a failure.) I did try at one point blocking the testing thread until the EDT had a chance to complete an invokeLater runnable, but that didn't seem to make a difference so I didn't commit it. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/25244#discussion_r2091836908
