RFR: JDK-8290469: Add new positioning options to PassFailJFrame test framework [v3]

[Phil Race]

On Thu, 21 Jul 2022 21:31:08 GMT, Harshitha Onkar <honkar at openjdk.org> wrote:

>> In the recent update the following points are addressed:
>> 
>> - Screen insets are used to account of taskbar position and the placement of the instruction frame at top-left corner
>> - Position option description update
>> 
>> Changes regarding pushing the latest position to window manager are still being evaluated and in progress.
>
> Following is a proposed solution to push the latest position changes to the window manager.
> 
> Robot.waitForIdle() are added between setLocation() and the subsequent getLocation() calls. This ensures any pending location updates are pushed to the window manager before the new location of the frame is in turn used to position the testWindow.
> 
> In multiple test window cases ONLY the test instruction frame and the main/root test window are positioned using `positionTestWindow()`, the rest of the windows are positioned within EDT at test-level.
> 
> **Impact of the proposed fix:** 
> With this change `positionTestWindow()` can no longer be called on EDT at test-level, hence any existing manual tests calling this method on EDT requires changes.
> 
> For example: In this particular test, the following line `PassFailJFrame.positionTestWindow(frame,PassFailJFrame.Position.HORIZONTAL)` should be called after `createAndShowGUI()`
> 
> https://github.com/openjdk/jdk/blob/d333df8ff59c6242171da1a6d02d05e6a8e65c9d/test/jdk/javax/swing/JComboBox/JComboBoxActionEvent.java#L75
>  
> 
> public static void positionTestWindow(Window testWindow, Position position) throws AWTException {
> 
>         if (isEventDispatchThread()) {
>             throw new Exception("positionTestWindow() should not be called on EDT");
>         }
> 
>         robot = new Robot();
>         Dimension screenSize = Toolkit.getDefaultToolkit().getScreenSize();
> 
>         if (position.equals(Position.HORIZONTAL)) {
>             SwingUtilities.invokeLater(() -> {
>                 int newX = ((screenSize.width / 2) - frame.getWidth());
>                 frame.setLocation(newX, frame.getY());
>            });
>             // waits until the queue is flushed
>             robot.waitForIdle();
>             robot.delay(300);
> 
>             SwingUtilities.invokeLater(() -> testWindow.setLocation((frame.getX() +
>                                              frame.getWidth() + 5), frame.getY()));
> 
>         } else if (position.equals(Position.VERTICAL)) {
>             SwingUtilities.invokeLater(() -> {
>                 int newY = ((screenSize.height / 2) - frame.getHeight());
>                 frame.setLocation(frame.getX(), newY);
>             });
> 
>             robot.waitForIdle();
>             robot.delay(300);
> 
>             SwingUtilities.invokeLater(() -> testWindow.setLocation(frame.getX(),
>                                             (frame.getY() + frame.getHeight() + 5)));
> 
>         } else if (position.equals(Position.TOP_LEFT_CORNER)) {
>             SwingUtilities.invokeLater(() -> {
>                 GraphicsConfiguration gc = GraphicsEnvironment.getLocalGraphicsEnvironment()
>                         .getDefaultScreenDevice().getDefaultConfiguration();
>                 Insets screenInsets = Toolkit.getDefaultToolkit().getScreenInsets(gc);
>                 frame.setLocation(screenInsets.left, screenInsets.top);
>             });
> 
>             robot.waitForIdle();
>             robot.delay(300);
> 
>             SwingUtilities.invokeLater(() -> testWindow.setLocation((frame.getX() +
>                    frame.getWidth() + 5), frame.getY()));
>         }
>     }

We should discuss that impact. 
This method is useful to simplify the logic of individual tests but is it now becoming awkward to use ?
Or is it something that we should have required from the beginning  ?
And why invokeLater and not invokeAndWait ? I'd have expected Later to not work for this case ..

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

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