RFR: 8288415: java/awt/PopupMenu/PopupMenuLocation.java is unstable in MacOS machines [v2]

[Alexey Ivanov]

On Fri, 6 Jan 2023 07:21:12 GMT, Manukumar V S <mvs at openjdk.org> wrote:

>> java/awt/PopupMenu/PopupMenuLocation.java seems to be unstable in MacOS machines, especially in MacOSX 12 & 13 machines. It seems to be a testbug as adding some stability improvements fixes the issue. It intermittently fails in CI causing some noise. This test was already problem listed in windows due to an umbrella bug JDK-8238720. This fix doesn't cover the Windows issue, so the problem listing in windows will remain same.
>> 
>> Fix:
>> Some stability improvements have been done and the test has been run 100 times per platform in mach5 and got full PASS.
>> Also updated the screen capture code, made frame undecorated, added some prints for better debugging.
>
> Manukumar V S has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Review comments fixed: Replaced one robot.waitForIdle with robot.delay(), formatting corrections, taken the insets into account for initial frame position

Changes requested by aivanov (Reviewer).

test/jdk/java/awt/PopupMenu/PopupMenuLocation.java line 77:

> 75:                 while (point.x <
> 76:                        screenBounds.x + screenBounds.width - insets.right -
> 77:                        SIZE) {

Suggestion:

            final int yBound = screenBounds.y + screenBounds.height
                               - insets.bottom - SIZE;
            final int xBound = screenBounds.x + screenBounds.width
                               - insets.right - SIZE;
            while (point.y < yBound) {
                while (point.x < xBound) {


I think this looks clearer and more readable than the wrapped condition.

test/jdk/java/awt/PopupMenu/PopupMenuLocation.java line 87:

> 85:     }
> 86: 
> 87:     private static void test(final Point tmp) {

Suggestion:

    private static void test(final Point loc) {

Let's give a better name to the parameter.

test/jdk/java/awt/PopupMenu/PopupMenuLocation.java line 104:

> 102:             frame.setSize(SIZE, SIZE);
> 103:             frame.setVisible(true);
> 104:             frame.setLocation(tmp.x, tmp.y);

Suggestion:

            frame.setLocation(tmp);
            frame.setVisible(true);

You can pass a `Point` object to `setLocation` directly.

Setting the location before making the frame visible avoids flickering where the frame appears at 0, 0 and then moves to another location.

-------------

PR: https://git.openjdk.org/jdk/pull/10655