On Fri, 22 Jul 2022 20:31:44 GMT, Phil Race <[email protected]> wrote:
>> 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 .. The interleaving does seem to make the code more complex. And at test-level, it causes some confusion as few method need to called within EDT and few out of EDT. I wanted to check if there was a simpler, much clearer approach and evaluate possible options before proceeding. I might have gone with invokeLater, since we are calling waitForIdle() later and it waits until the queue is empty. But I think invokeAndWait would be a better option here. ------------- PR: https://git.openjdk.org/jdk/pull/9525
